Subject: Re: More security issues
From: Corinna Vinschen <[email-masked]>
To: "Pierre A. Humblet" <[email-masked]>
Cc: cygwin <[email-masked]>
Date: Tue, 5 Mar 2002 09:57:04 +0100

On Sun, Mar 03, 2002 at 10:05:39PM -0500, Pierre A. Humblet wrote:
> At 11:19 PM 2/23/2002 +0100, Corinna Vinschen wrote:
> 3) after a sequence setegid(newg1), seteuid(newuid), 
> seteuid(original), the process has an unused primary token, 
> which can be used again if there is another setegid(newg2), 
> seteuid(newuid). 
> However that token may not be appropriate, depending on newg1 
> and newg2. If the token is internal and newg1 was not in the 
> "natural groups" of newuid, then newg2 must be the same as 
> newg1. Otherwise it is enough that newg2 be in the token groups.
> [what is there now is too strict for internal tokens, but not 
> strict enough for tokens from cygwin_logon_user()] 
> That is checked in a new function verify_token() 
> (in security.cc), called from seteuid().

I don't understand that description.  Could you try to explain
in other words?  What do you mean by "natural group"?  Primary
group as set by Windows (RID 513, "None" or "Domain Users",
typically) or the primary group as set in /etc/passwd or ...?

> 4) the primary group that was used when creating an internal token
> is now saved in the token sd. This allows to set the token default

With what access rights?  The pgrp shouldn't have write access
to the token's sd, isn't it?

> primary group appropriately if the user calls setegid() after
> creating the token, e.g. seteuid(uid1) .... setegid(gid1) ....  
> [can this order be legitimate ?].

That won't work due to the create_token call being only in
seteuid().  This would require a redesign.

> 5) internal_getlogin() is called in seteuid(). Why is it necessary
> to call it again after CreateProcessAsUser()? Won't the environment 
> already be OK?

Not if somebody called spawn()/exec() using an external token
(one that hasn't been created inside of Cygwin).  This is true
for older implementations of login or sshd which used some
flavor of `sexecve(token, ...)'.

> 6) The role of/need for  the last creator/owner ACE in __sec_user() 
> is not clear to me. Are we ever in situations to propagate 
> permissions?  

Uhm, perhaps, it's actually not needed?

> 8) Also in spawn.cc, I believe that the following code
>       static BOOL first_time = TRUE;
>       if (first_time)
> 	{
> 	  set_process_privilege (SE_RESTORE_NAME);
> 	  first_time = FALSE;
> 	}
> isn't robust. The static variable could be FALSE in a forked process
> if the parent had spawned something before. 
> My suggestion is to delete those lines and modify registry.cc as follows:
> 
> --- registry.cc.org     Tue Feb 19 20:39:44 2002
> +++ registry.cc Thu Feb 21 10:56:32 2002
> @@ -235,12 +235,13 @@
>    /* Check if user hive is already loaded. */
>    cygsid csid (psid);
>    csid.string (sid);
> -  if (!RegOpenKeyExA (HKEY_USERS, csid.string (sid), 0, KEY_READ, &hkey))
> +  if (!RegOpenKeyExA (HKEY_USERS, sid, 0, KEY_READ, &hkey))
>      {
>        debug_printf ("User registry hive for %s already exists", sid);
>        RegCloseKey (hkey);
>        return;
>      }
> +  set_process_privilege (SE_RESTORE_NAME);
>    if (get_registry_hive_path (psid, path))
>      {
>        strcat (path, "\\NTUSER.DAT");

Ok with me.
> 
> 9) get_dacl() (in security.cc) gives no access to admins if the
> user is not in the admins group. I don't understand the logic.

It's Windows logic.  This is how a default security descriptor
is created.

> My suggestion is to call instead the new sec_dacl() (see above),
> which always has system, admins, sid1, [sid2] and creator/owner.

Why?  What's the effect you want to get by this?

> 11) Can cygwin_logon_user() be called by a user not in admins?

Yep.  Don't think in groups here, it just depends on the privileges
set for the user in the security policy.  Up to W2K the user needs
the SE_TCB_NAME privilege ("Act as part of the operating system").
The admin group doesn't have that permissions either (by default).
Since XP, this restriction has been dropped, fortunately.  But that
only helps applications which are never intended to run on OSes
prior to XP.

Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:[email-masked]
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/