This is the mail archive of the mailing list for the Cygwin project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

More security issues

Hi Corinna,

I have some free time and easy access to an NT
so I came back to security issues.

As you recall, in setegid(), setting the PrimaryGroup
in the process token isn't reliable and was #if'ed out.
Consequently non-cygwin subprocesses may create objects
with the wrong primary group.

I tried to fix that by setting the primary group based on  getegid()
in the security descriptor created in sec_user(). To my surprise
that didn't have any effect. In fact sec_user() doesn't seem to
have much effect at all! It creates an ACL with 4 or 5 ACE's, but
my token printing program only shows two ACE's in the process
tokens: admins and system. I wonder what the sa in CreateProcess
really does... The only thing that has an effect is the Inherit flag.

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

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. 
It would be much better if we could get the sd to have an effect
in DuplicateTokenEx() [in create_token(),].
That may be related to what I observed above.
Any ideas?

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.
P.S.: please cc me directly. 

Unsubscribe info:
Bug reporting:

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]