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

[James Peach]

On Jun 7, 2007, at 9:49 PM, Jeremy Allison wrote:

> On Thu, Jun 07, 2007 at 09:39:39PM -0700, Jeremy Allison wrote:
>>
>> What I'd like to see is a parallel implementation
>> of the functions you want to change in the security
>> context code, that only work for *BSD.
>
> One more thing (sorry for going on about this but
> I'm really paranoid about this :-).

No worries. This is exactly why I posted the patches for review :)

> I dislike the way you've added the new
> apply_unix_token() call - I feel this is
> the wrong way to do things.

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.

There's really 2 parts to the security context switching. The first  
part is managing the stack data structure, the second part is actually  
switching to the desired credential.

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.

> What I'd like to see is a *BSD specific
> version of sys_setgroups() that re-arranges
> the groups as *BSD requires. I'm not
> averse to changing the function interface
> from it's current :
>
>
> int sys_setgroups(int setlen, gid_t *gidset);
>
> to be :
>
> int sys_setgroups(gid_t primary_gid, int setlen, gid_t *gidset);
>
> as I think on *BSD you need to know the primary gid
> for setgroups to work correctly.

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().

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)

--
James Peach | jpeach at samba.org