PATCH: login under privileged user != SYSTEM

[Charles Wilson]

Corinna Vinschen wrote:
> On Apr 17 01:48, Charles Wilson wrote:

>> With these changes, I can now get passwordless rlogin when inetd is running 
>> under a privileged user, and not SYSTEM.
>>
>> Most of the code was adapted from editrights/main.c...
> 
> Cool, thanks!  Would you mind to take over login maintainance, too?  It
> was always just the wagging tail of inetutils anyway...

Sure.

> Other than that, I'd like to suggest a few minor changes to the patch:
> 
> - The SeServiceLogonRight doesn't have to be tested, IMHO.  It doesn't
>   have anything to do with user account switching.

Ack.

> - I don't understand why NT4 is handled specially by only checking for the
>   uid while 2K and XP get the additional account check if necessary.  None
>   of the functions are new with 2K, they all exists since NT 3.51.

I initially thought the program flow would be rather awkward, to make NT 
act like 2k/xp -- and I just didn't care much for NT.  But after I 
finished coding I should have noticed that it would really be quite 
simple to do that.  But I didn't.

Fixed now.

> - I wouldn't do the automatic yes for uid 18 anymore.  Even for NT/2K/XP
>   it would be more correct to check if the current account running the
                                              ^^^^^^^^^^^^^^^^^^^^^^^
>   process is the one with SID S-1-5-18.  

But that's not exactly what you want, here. Sometimes, login.c does
   isROOTUID(getuid())
which could be replaced as you suggest. But *most* of the time, login.c does
   isROOTUID(pw->pw_uid)
before it has actually switched to that user.

And saying that isROOTUID(uid) ==
   {
     setuid(pw->pw_uid);
     isCurrentProcessRunningAsROOT();
     setuid(saved_uid);
   }
is overkill -- especially as I want "isROOTUID(uid)" to work even if the 
current user does NOT have the privileges needed for setuid() to work.

>   Given that there's already
>   so much code for Windows specific privilege checking, I don't think
>   it hurts a lot to add something along the lines of
> 
>     AllocateAndInitializeSid (SECURITY_NT_AUTHORITY, 1, 18, ..., &system_sid);
>     token = OpenProcessToken (GetCurrentProcess ());
>     user_sid = GetTokenInformation(token, TOKEN_USER);
>     if (EqualSid (user_sid, system_sid))
>       yes
>     else
>       check_privileges

I implemented something along these lines. After creating several 
building block functions, I wrapped it all up into:

extern int currentUserIsLocalSystem();
extern int currentUserIsMemberOfLocalAdministrators();

These two use OpenProcessToken/GetTokenInformation as you suggested. 
However, the one I really use is:

extern int uidIsLocalSystem(uid_t uid);

==== aside:
And I wish I could have figured out how to make 
uidIsMemberOfLocalAdmin(uid_t uid), but if uid != current user it's 
really hard to get the either (a) the list of groups a particular user 
is a member of, or (b) the list of users that are members of a 
particular group.  Since I already have a make-SID-from-uid method, if I 
had (a) I could iterate that list trying to match the local 
Administrators SID, or if I had (b) I could iterate through the list and 
compare to my SID-from-uid.

I know there is NetUserGetLocalGroups, but what if the user is a member 
of a global group, and the local security policy makes that global group 
a member of the (local) Administrators group? With the multi-level 
inclusion of groups, it's almost easier to go the other way: get the 
local administrator group, and use (recursively) NetLocalGroupGetMembers 
and NetGroupGetUsers to build a list of all users that are (directly or 
by inclusion) members of the (local) Administrators group -- and THEN 
iterate that to see if any of them match SID-from-uid.

But neither is easy.
==== end aside

So, I'm still not checking that the uid specified is a member of the 
local Administrators group.

I did discover one awkward thing: in my make-SID-from-uid function, I do 
the following

1. get struct passwd* for uid
2. cygwin_internal(CW_EXTRACT_DOMAIN_AND_USER, pw, domain, name);
3. get the servername for the domain by using either
    DsGetDcName or NetGetDCName
4. use NetUserGetInfo to get a PUSER_INFO_3 structure
    (if domain user, and call fails, try again locally...)
5. use LookupAccountName to get the SID
    (if basic call fails and returned account type is SidTypeDomain,
    try again after adding domain spec to username)

However, if uid = 18 it turns out that NetUserGetInfo(...., 
toUnicode("LocalSystem"),...) always fails. I even tested that 
proposition in a quick test app. It just doesn't work.

Which means that my uidIsLocalSystem(uid_t uid) function is broken in 
this case. It's algorithm is:

1) create SID for LocalSystem manually. call this "desiredSID"
2) get SID corresponding to supplied uid. call this "userSID"
3) compare desiredSID == userSID

And for LocalSystem, step #2 fails.  So, I had to modify 
make-SID-from-uid, by adding step 1a:

1a. if pw->pw_name == "LocalSystem" (after canonicalizing from "SYSTEM")
     manually create SID for SECURITY_LOCAL_SYSTEM_RID and return

But when you boil it all down, that just means that I'm now comparing...

      18 == 18

to see if uid is LocalSystem. I'm doing a lot more work to get there, 
though. <g>

I'll upload a test package tomorrow, with all these changes. It'll need 
lots of testing, though, in various environments: I have no domain to 
test it on.

--
Chuck

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/