This is the mail archive of the cygwin-patches@cygwin.com 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]

Name aliasing in security.cc


Corinna,

1) Currently if a domain user and a local user have the same name
(a common situation) then setuid() to the local user will create 
a token with the groups of the domain user. 
This is due to always looking up group names on the primary domain 
server first, indexing with the user name, and then possibly on the
local host.

The fix uses the same method as mkpasswd: it looks up the user name
on the server responsible for the user domain.

As added benefits, no network access is required to setuid()
to a local user, and more than one domain (+ local) can be 
handled.

This patch does not address the following existing issue. User A
setuid()'s and exec()'s user B, who is privileged. User B setuid()
itself with a new gid. In this weird case, the weird behavior of
LookupAccountSid() causes the token to inherit the groups of user A.

2) The current code uses LookupAccountName() to obtain the SIDs of
the groups returned by Windows. LookupAccountName() searches 
for unqualified names everywhere in a forest of domains, returning
as soon as it finds a match.
It can thus  a) return the wrong info if it first finds a matching 
name in an unexpected domain and a) be very slow (particularly if
a name does not exist nearby).

The patch improves the situation by exploiting the fact that 
LookupAccountName() accepts qualified names of the form domain\user 
and we know that:
a) a global group has the same domain as the user
b) a local group returned by NetLocalGroupEnum has domain either 
BUILTIN or the localhost.

3) I noticed a function lookup_name() in sec_helper.cc. It also uses
LookupAccountName() but it is very careful to add domains. 
It is called only from alloc_sd() and seems to date from the time 
when the passwd and group files didn't contain the SIDs.
So it can probably be deleted.

Alternatively its use could be widened, by calling it directly 
from getfrompw() and getfromgr() whenever a SID is missing from an
entry in the passwd/group files.

It has the pecularity that the search rule depends on the user
making the call. Thus two users looking up the same name at the same
time from the same passwd/group files could obtain different answers.
This is probably undesirable.
Do you want to
a) keep lookup_name() as it is?
b) remove it entirely?
c) call it whenever a SID is missing from a passwd/group entry, using
user independent search rules (except if a user looks up itself)? 

 
The changes have been tested on a standalone PC and in two single
domain networks, with WinNT and Win2000 PCs. 
It has not been tested e.g. on a PC that is itself a domain controller, 
or in networks with several domains. If members of this list can test
setuid() (creating internal tokens) in such environments, feedback 
would be welcome.

Pierre

2002-05-30  Pierre Humblet <pierre.humblet@ieee.org>

	* security.cc (lsa2wchar): Suppressed.
	(get_lsa_srv_inf): Suppressed.
	(get_logon_server_and_user_domain): Suppressed.
	(get_logon_server): Essentially new.
	(get_user_groups): Add "domain" argument. Only lookup the
	designated server and use "domain" in LookupAccountName.
	(is_group_member): Simplify the arguments.
	(get_user_local_groups): Simplify the arguments. Do only a
	local lookup. Use "BUILTIN" and local domain in LookupAccountName.
	(get_user_primary_group). Only lookup the designated server.
	(get_group_sidlist): Remove logonserver argument. Do not lookup
	any server for the SYSTEM account.
	(create_token): Delete logonserver and call to get_logon_server.
	Adjust arguments of get_group_sidlist, see above.
	* security.h: Delete declaration of get_logon_server_and_user_domain
	and add declaration of get_logon_server.
	* uinfo.cc (internal_get_login): Call get_logon_server instead of
	get_logon_server_and_user_domain.
	

Attachment: uinfo.cc.diff
Description: Text document

Attachment: security.h.diff
Description: Text document

Attachment: security.cc.diff
Description: Text document


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