Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm List-Subscribe: List-Archive: List-Post: List-Help: , Sender: cygwin-owner AT cygwin DOT com Delivered-To: mailing list cygwin AT cygwin DOT com Date: Tue, 12 Feb 2002 16:57:19 +0100 From: Corinna Vinschen To: "Pierre A. Humblet" Cc: cygwin Subject: Re: More security issues Message-ID: <20020212165719.X14241@cygbert.vinschen.de> Mail-Followup-To: "Pierre A. Humblet" , cygwin References: <3 DOT 0 DOT 5 DOT 32 DOT 20020210143455 DOT 007f2100 AT pop DOT ne DOT mediaone DOT net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3.0.5.32.20020210143455.007f2100@pop.ne.mediaone.net> User-Agent: Mutt/1.3.22.1i On Sun, Feb 10, 2002 at 02:34:55PM -0500, Pierre A. Humblet wrote: > I wonder what the sa in CreateProcess > really does... The only thing that has an effect is the Inherit flag. MSDN documents the SD in the lpProcessAttributes/lpThreadAttributes argument being used as the SD of the called process/main thread. The SD of the process seems not to correspond with the default DACL in the token. However, the sec_user() isn't w/o effect. You can easily check that by changing the function to create a wrong DACL. > In the course of debugging I also noticed that the sid2 passed > to sec_user() from just before CreateProcessAsUser() is useless. > It is actually equal to the sid that sec_user() gets from > cygheap->user.sid () [cygheap->user is set in seteuid()] Does the following patch help? Index: spawn.cc =================================================================== RCS file: /cvs/src/src/winsup/cygwin/spawn.cc,v retrieving revision 1.97 diff -u -p -r1.97 spawn.cc --- spawn.cc 2002/02/10 13:38:49 1.97 +++ spawn.cc 2002/02/12 15:54:53 @@ -647,6 +647,11 @@ spawn_guts (HANDLE hToken, const char * } else { + /* Remove impersonation */ + if (cygheap->user.impersonated + && cygheap->user.token != INVALID_HANDLE_VALUE) + RevertToSelf (); + cygsid sid; DWORD ret_len; if (!GetTokenInformation (hToken, TokenUser, &sid, sizeof sid, &ret_len)) @@ -659,11 +664,6 @@ spawn_guts (HANDLE hToken, const char * PSECURITY_ATTRIBUTES sec_attribs = allow_ntsec && sid ? sec_user (sa_buf, sid) : &sec_all_nih; - - /* Remove impersonation */ - if (cygheap->user.impersonated - && cygheap->user.token != INVALID_HANDLE_VALUE) - RevertToSelf (); static BOOL first_time = TRUE; if (first_time) > All of this effort was motivated by weird access issues to the > impersonation token. I can fix that by opening the thread token > security descriptor after ImpersonateLoggedOnUser() in seteuid() > and changing the ACL (using the ACL from sec_user(), that works!). > Unfortunately the work must be redone each time the sequence > RevertToSelf(), ..., ImpersonateLoggedOnUser() occurs. That can't be the way to go. Somehow we should try to figure out to do it correctly. > Back to setegid(), another safe way would be to > RevertToSelf(),..,Impersonate..() if currently impersonated. > That's because there is also a RevertToSelf() before CreateProcessAsUser() > Why is there one, by the way? Microsoft seems to suggest working in the > security context of the new user. It says it's useful if the executable > is only executable by the new user. Did you try if that works reliable? Nobody keeps you from patching it ;-) Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Developer mailto:cygwin AT cygwin DOT com 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/