[Patch] Allow to disable root privileges with CYGWIN=noroot
Christian Franke
Christian.Franke@t-online.de
Sun Oct 11 20:45:00 GMT 2009
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>
More information about the Cygwin-patches
mailing list