with most recent git master smbd fails to start in AD DC mode

[Jeremy Allison]

On Sun, Mar 23, 2014 at 07:58:53PM +1300, Andrew Bartlett wrote:
> 
> I'm not convinced this is any more correct either.  The issue is that in
> the AD DC, 'guest account' as a parameter is ignored.  It is defined
> only in the sam as the holder of the guest account SID (-513).
> Additionally, unix groups are not relevant in the AD DC.

Well they are *massively* relevent in smbd :-).

So either the AD-DC needs it's own version
of these functions split out from the ones
in source3/, or it can ignore the extra
S-1-22-XX groups being added here (which
I think *is* the correct thing to do).

> Even in the general case, we should not call getpwuid() if the SID is in
> our local domain - we won't find any more groups anyway, and as the
> original patch says, it is inefficient. 

No, that's not true. It's only inefficient in the
case where this has already been done, and that's
the case where there is no AD-DC entry for the unix
user (and it's ended up as a S-1-22-XX sid already).

We *certainly* will find more groups in the local
domain, as these groups are coming from /etc/group
which we were currently ignoring when creating a
token (which we were before my initial patch).

Remember, anyone can add a LOCAL-DOMAIN\user
user into /etc/groups, which is what triggered
the 

> While I understand the performance issues that created the
> pre-calculation here, this continues to reinforce to me how delicate it
> is, and I do wish we could avoid or dramatically improve it. 

It's not performance at all. It's correctness. 

We can't avoid it without passing the UNIX uid
into these functions, which I don't really want
to do (I did consider it). IMHO they're cleaner
just working with SIDs.

> I appreciate you working with Günter on this.  As you have found out,
> this is a sensitive area, please don't merge a patch to further fix this
> without my review.

Let's have a call tomorrow (Monday) on this.
I think the patch is good and I'd appreciate your
review once you and I have gone through it
carefully.

The only mistake I made in the original
merged patch is that I made a bad assumption
that token->sid[0] once being mappable
to uid always allows getpwuid() to succeed, which
in the guest case isn't true.

Jeremy.