AW: nested groups patch

[Matthias Dieter Wallnöfer]

I pushed a fix to "master" which should prevent the failure (tested with 
"smbclient"). Sorry for the inconvenience.

Matthias

Matthias Dieter Wallnöfer schrieb:
> Hi Andrew(s)!
>  
> It's very unpleasant to see that the patch has an error case. But at the moment I'm a bit busy and can't fix it - only later this day. If you can wait for tomorrow - I think I'd be able to do it.
> Otherwise please revert - there are two commits: one of metze and the other is mine - and I'll try to fix it with calm.
>  
> Matthias
>
> --- tridge at samba.org <tridge at samba.org> schrieb am Di, 4.8.2009:
>
>
> Von: tridge at samba.org <tridge at samba.org>
> Betreff: nested groups patch
> An: mdw at samba.org, abartlet at samba.org
> CC: samba-technical at samba.org
> Datum: Dienstag, 4. August 2009, 7:37
>
>
> Hi Matthias and Andrew,
>
> The patch 71b013f4deb79f66a28545dc3be910815b123f7c "s4: Patch to
> implement nested group and privileges" causes all file operations to
> fail with NT_STATUS_INVALID_SID. A simple 'dir' in smbclient shows the
> error.
>
> The problem seems to be that the code in
> authsam_expand_nested_groups() always puts the account_sid in the list
> returned in res_sids[]. That function is called from
> authsam_make_server_info() where it is used to fill in the groupSIDs
> array.
>
> The result is that the first element of groupSIDs becomes the users
> account SID. This ends up in the users token, as a one of the token
> groups.
>
> Then the code in nt_token_to_unix_security() fires at vfs_unixuid.c
> line 220, which returns NT_STATUS_INVALID_SID.
>
> I think you didn't notice this bug in your testing as the 'make test'
> environment doesn't load the unixuid VFS module, as that module
> requires root privileges in order to be able to change uid and the
> test environment doesn't have root privileges.
>
> Could you please revert the nested groups code, or fix it to not
> include any user SIDs in the list of group SIDs? 
>
> It is critical for patches that change the security token that they be
> very well tested and reviewed before committing.
>
> We should also see if we can find a way to include the unixuid module
> in our testing. Perhaps we need fake versions of seteuid(), setegid()
> and setgroups() for the test environment, similar to our socket
> wrapper code?
>
> Cheers, Tridge
>