AW: nested groups patch

[Matthias Dieter Wallnöfer]

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