DCE/RPC service security design [was: Re: usermgr problems]

[Luke Kenneth Casson Leighton]

On Tue, Sep 18, 2001 at 01:20:04PM +0200, Kai Krueger wrote:
> 
> ----- Original Message -----
> From: "Elrond" <elrond at samba-tng.org> Sent: Monday, September 17, 2001 6:36
> PM
> Subject: Re: DCE/RPC service security design [was: Re: usermgr problems]
> 
> 
> > On Mon, Sep 17, 2001 at 05:21:21PM +0200, Luke Kenneth Casson Leighton
> wrote:
> > [...]
> >
> >
> > Hi,
> >
> >
> > IF someone is realy going to invest this stuff.
> 
> I could try and do it, if it isn't to much work.
> 
> > I can merge over the se_access_check from the "other
> > place", they use it heavily now for printing and some other
> > stuff, so it is likely to be correct.
> 
> The one in tng doesn't seem to be correct. From what I saw while, trying
> yesterday, there are some errors in ace_grant() 

> and se_access_check(). It
> therefor would probably be better to try and use it from "the other place"

the logic was worked out by matthew chapman and todd sabin, originally.

> >
> > Their version doesn't any more accept a NET_USER_INFO_3,
> > but instead an NT_USER_TOKEN, which is basicaly a list of
> > SIDs.
> 
> The function itself accepts a curent_user struct that contains the
> NET_USER_INFO_3 struct and is optained directly from
> get_valid_user_struct(). 

yep.

> Therefore it doesn't lool like you need to do
> anything there. se_access_check gets the NT_USER_TOKEN from
> user_struct->nt_user_token as it seems.
 
?

oh dear.  okay.

> > I can write code to generate that from a NET_USER_INFO_3.
> >
> > I'm thinking about a caching get_current_nt_user_token(),
> > which does caching by vuid. (vuid changes --> go recalc).
> >
> >
> > The basic problems with samr are:
> >
> >
> > We don't know the bits precisely!
> 
> can't we use the bits we know allready to start of with and as we get to
> know the others implement a better (or correct) checking later?
 
well, it actually doesn't matter what they are, in some
ways (and in others, it definitely does).

yes, you could get away with knowing what the generic bits
are, but for a per-service basis you ARE going to need to
know what the top 16 service-specific bits are.

for example, in samr, one of those top 16 service-specific
bits represents permission to change own user-password.

for example, in spoolss, one of those top 16 service-specific
bits represents permission to modify print job priorities.

and those _don't_ fall into read, write, execute - obviously.

> >
> > We have (or can get) the usualy Security Descriptors for
> > the various objects.
> >
> > It _will_ be a lot of work.
> 
> IMHO that depends on how exact and correct you wan't to implement it. A
> first hack should be able to be done without _too_ much work. 

v. true.

> This would
> include just a simple SD allowing full access to the Domain Admin group and
> Read access to everybody. 

basically?  yes!

> This shouldn't be a disadvantage to what is
> implemented at the moment and will at least allow all Domain Admins to edit
> the SAM db.
 
yep.

use the rpcclient samquerysec command to list permissions: you
could simply lift them from a SAM db query, store the output
in a binary blob and stuff that into the code!

> >
> > In the first run, we could try and ignore the access_masks
> > on the handles. 

urrr, yukk.

> > This is just an additional layer of
> > access-control.

no it isn't.  permissions get requested like read / write,
and you have to call se_access_check() inside the
handle-opening-function (e.g. _samr_connect) in order to
determine _even then_ if the user has the right to be returned
that handle.

so, let's say that a user has read permission only in the SD
on SAM user.

they request, in _samr_open_user(), SE_READ | SE_WRITE.

this MUST return NT_STATUS_ACCESS_DENIED (via se_access_check()),
as called from inside _samr_open_user().

and, i believe, the permissions from the handle on which the
samr_open_user() - the Domain handle - have to be passed
through to se_access_check() too!  i could be wrong about
this.

[which is why SeAccessCheck has so many function parameters]


otherwise, what you will be doing is granting a user write
permission to an object that they should only have read
permission on.

remember: those permissions from the access_mask are
"attached" to the handle, and any subsequent operations
_with_ that handle are "qualified" by the access_mask.

in this way, a write operation is always denied if the handle
was originally requested with a read-only access_mask.



> > The se_access_check in head anyway only has one
> > desired_access field.
 
well, that's not enough.  examine the MSDN definition of SeAccessCheck
it takes at least five or six parameters.

> > So for the first run, it would probably be easier to just
> > the rights needed in the special case against the sec-desc.
> 
> I'd say that would be a good first step. And if the additional
> access-control is required it can still be added later. The prev_req_acc is
> only used in a single (or very few) line(s), so it shouldn't be difficult to
> add it to the HEAD one if it is merged over to tng.
> 
> >
> > This will allow for some nice things:
> >
> > Even normal users will be able to double click on users in
> > usrmgr.exe. With NT PDC this isn't possible, because
> > usrmgr.exe requests RW when opening the user...
> >
> 
> This isn't directly nice (or secure) but this is allready possible so it
> wouldn't loosen security in comparison to what we have now. 

1) well, certain anonymous information _has_ to be made available.
USRMGR.EXE denies you this information as a client-side implementation
(!!! :) :) but you can get it with rpcclient or any other similar tool.

2) permissions can be modified, at the admin's risk of destroying
the useability of the entire system, to suit the admin's whims and
environment.


> Appart from, if
> usrmgr.exe (client) requests the permission, wouldn't it be possible to
> wright your own client program, run it as a normal user and get full (read)
> information on a NT PDC as well?
 
correct.  see 1) above.

> >
> > Can people please comment?
> 
> As mentioned before I think it is important for all Domain Admins to be able
> to use usrmgr.exe to edit the sam db. This is IMHO especially, when the
> administrators "don't know much about there job" and about LINUX/UNIX in
> particular as often the case in schools, where teachers try and build up a
> netwerk in there spare time.
 

> >
> >
> > I don't want to merge over se_access_check, if nobody is
> > really thinking about using it.
> 
> Is it possible to just exchange the util_seaccess.c or will that come into
> conflict with the includes? As far as I could see, the se_access_check isn't
> used in tng at all atm.
 
the additional parameters are definitely needed.

> The above applies to the ldap code. I neither know how the normal smbpasswd
> reacts nor whether it is needed or possible to implement the NT access check
> there.
> 
> >     Elrond
> 
> Kai
> 
>