[Patch] Allow to disable root privileges with CYGWIN=noroot

[Christian Franke]

Corinna Vinschen wrote:
> Thanks for the patch.  You did check that the normal setuid/seteuid
> cases still work, didn't you?
>
>   

Yes.

>> I would suggest to add another cygwin_internal() call to check if current 
>> process is considered 'equivalent root'. This could be used e.g. by shells 
>> to set the root prompt properly.
>> http://sourceware.org/ml/cygwin/2009-09/msg00138.html
>>     
>
> What's wrong with:
>
>   for i in $(id -G);
>   do
>     [ $i -eq 544 ] && PS1='# '
>   done
>
>   

Is OK, except if admin group is mapped to other gid (0?) in /etc/group.


A function that checks the token instead would IMO be helpful, e.g.:

#ifdef __CYGWIN__
  if (cygwin_internal (CW_IS_ROOT_EQUIV, flags?))
#else
  if (geteuid () == 0)
#endif



> The patch looks pretty good.  I have a few remarks, though.
>
>   

Thanks. New patch below.


> I would like it better to have the actual functionality concentrated in
> a function in sec_auth.cc.  A function `set_imp_token (HANDLE, int)'
> which is called from cygwin_internal and from cygwin_set_impersonation_token,
> for instance.  The debug output in cygwin_set_impersonation_token should
> be moved into set_imp_token, too.  What do you think?
>
>   

Good idea. Done.

>> -  cygheap->user.reimpersonate ();
>> +
>> +  if (!cygheap->user.reimpersonate ())
>> +    /* User possibly set an external token without HANDLE_FLAG_INHERIT.  */
>> +    api_fatal ("reimpersonate after fork failed (%d)", (int)GetLastError());
>>     
>
> This bugs me.  Wouldn't it be better if we call DuplicateHandleEx
> on the external token to make sure it's inheritable?  This would also
> require to call CloseHandle on an existing external token if
> a process calls cygwin_set_impersonation_token (INVALID_HANDLE_VALUE) or
> INVALID_HANDLE_VALUE (NULL), but that's not really much extra code to
> worry about.  Dropping any chance that fork or exec fail sounds worth
> it, IMHO.  I'm still blushing that this never occured to me before.
>
>   

I removed the error check and set HANDLE_FLAG_INHERIT in seteuid32().


>> @@ -2835,7 +2877,11 @@ setuid32 (__uid32_t uid)
>>  {
>>    int ret = seteuid32 (uid);
>>    if (!ret)
>> -    cygheap->user.real_uid = myself->uid;
>> +    {
>> +      cygheap->user.real_uid = myself->uid;
>> +      /* If restricted token, forget original privileges on exec ().  */
>> +      cygheap->user.setuid_to_restricted = cygheap->user.curr_token_is_restricted;
>> +    }
>>     
>
> Do I miss something or is the setuid_to_restricted flag equivalent to
> the curr_token_is_restricted flag anyway, and as such redundant?  If so,
> it would look nice to make setuid_to_restricted an inline method instead:
>
>   bool setuid_to_restricted () const { return curr_token_is_restricted; }
>
>   

setuid_to_restricted is only set in setuid32, not in seteuid32. If 
seteuid(geteuid()) is called, the behaviour is similar to the ruid != 
euid case: The exec()ed process can revert to the original token.

Christian

2009-10-11  Christian Franke  <franke@computer.org>
            Corinna Vinschen  <corinna@vinschen.de>

	* include/sys/cygwin.h: Add new cygwin_getinfo_type
	CW_SET_EXTERNAL_TOKEN.
	Add new enum CW_TOKEN_IMPERSONATION, CW_TOKEN_RESTRICTED.
	* cygheap.h (cyguser): New flags ext_token_is_restricted,
	curr_token_is_restricted and setuid_to_restricted.
	* external.cc (cygwin_internal): Add CW_SET_EXTERNAL_TOKEN.
	* sec_auth.cc (set_imp_token): New function.
	(cygwin_set_impersonation_token): Call set_imp_token ().
	* security.h (set_imp_token): New prototype.
	* spawn.cc (spawn_guts): Use CreateProcessAsUserW if
	restricted token was enabled by setuid ().
	Do not create new window station in this case.
	* syscalls.cc (seteuid32): Add handling of restricted
	external tokens. Set HANDLE_FLAG_INHERIT for primary token.
	(setuid32): Set setuid_to_restricted flag.
	* uinfo.cc (uinfo_init): Do not reimpersonate if
	restricted token was enabled by setuid ().
	Initialize user.*_restricted flags.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: cygwin-1.7-restricted-token-2.patch
Type: text/x-diff
Size: 10568 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20091011/6261cef5/attachment.bin>