[PATCH 1/4] Make sure groups[0] is the effective gid on FreeBSD.

[James Peach]

On Jun 8, 2007, at 9:48 AM, Jeremy Allison wrote:

> On Fri, Jun 08, 2007 at 09:01:44AM -0700, James Peach wrote:
>>
>> There's 2 related issues in this patch set. The first is the BSD- 
>> style
>> changes for setgroups, the second is the Darwin-specific changes to
>> the order of credential operations.
>
> If it's Darwin specific then I want it split into a Darwin-specific
> change.
>
>> The current code intertwines these two parts in two places,
>> set_sec_ctx() and pop_sec_ctx(). I felt that splitting the credential
>> switch into a separate function made this a lot clearer. There is
>> exactly one place where the credential is switched and the credential
>> switching code is not mixed with the security context stack  
>> management
>> code. The result is credential switching code that is easier to read,
>> audit and log.
>
> Sez you. The existing code works, and works well on all
> but one class of broken systems (*BSD).

Yes sez me :)

I think that it's a mistake to separate the set of system calls used  
to alter your credentials, because switching to a credential is a  
logically single operation that should pass or fail as a whole. In  
SAMBA_4_0, the credential switch is done in a single function,  
set_unix_security(). This seems to me to be the right way to do it.

> Please do not modify existing working code, just fix
> the specific problem. Don't get creative here.
>
>> Yep, I have a tree with a similar patch, but the Darwin initgroups
>> wrapper needs to be passed the UID you are switching to.  
>> Additionally,
>> for Darwin the order of operations in the credential switch is
>> important. This means that I can't hide all of this behind
>> sys_setgroups().
>
> That's a Darwin bug (IMHO). Their credentials system is now
> non-POSIX and now non-standard.
>
> When are they planning to fix that ?
>
>> I would be very happy to split apply_unix_token into a separate  
>> Darwin
>> implementation, if you would prefer that better. Perhaps it is poorly
>> named, maybe it should be:
>>
>> 	BOOL switch_to_credential(const UNIX_USER_TOKEN *ut)
>
> No, I want the change I asked for - a modified
> sys_setgroups that works for *BSD.

I originally had this, but I didn't like how this interacted with  
HAVE_SETGROUPS and HAVE_BROKEN_SETGROUPS. I thought it best to  
maintain the existing convention that the sys_* wrappers should call  
the thing they wrap as precisely as possibly.

I'll work up a patch containing just the modified sys_setgroups and  
post that before committing anything.

--
James Peach | jpeach at samba.org