MutonUfoAI / pgina

pGina fork: Open Source Windows Authentication
http://mutonufoai.github.io/pgina
BSD 3-Clause "New" or "Revised" License
156 stars 39 forks source link

Cannot unlock a second pGina-created user #136

Open AlanEpHws opened 5 years ago

AlanEpHws commented 5 years ago

version used: 3.9.9.12

Steps to reproduce:

  1. Log into any user account
  2. Lock the account
  3. Log into a pGina-created user account via pGina
  4. Lock the second account
  5. Attempt to log into the second account again RESULT: An error is shown stating that "User is Locked in Session but was not authenticated by pGina..."

I debugged the issue and found out that the problem lies in wrong data being added to m_sessionPropertyCache within Service.cs (line 579) when logging into the local user for the first time (step 3). More specifically, the session data is being added to the cache with the wrong key (session ID); it is getting the session ID of the first session rather than the second one. Therefore, the session data is not being found later when the second login attempt is made, hence the error.

Further digging led me to find that this session ID is being obtained via ProcessIdToSessionId within pGinaMessages.h (line 259). It is my belief that this ID is being read before the new session is created. I also checked if the bug was present in v3.1.8.0, and it turned out that it was, but no check was made for it so it during login requests, so it did not produce any errors on screen.

MutonUfoAI commented 5 years ago

If a process is running in the context of this user you want to login and this process was started by runas than pgina will block that login. You can start a program in context of this user if you use pgina to handle UAC. http://mutonufoai.github.io/pgina/documentation/uac.html

You can't change the userprofile while the user is still logged in and the whole login chain would be called if you log this user in.

AlanEpHws commented 5 years ago

Thank you for your reply, MutonUfoAI.

However, I think there may have been a misunderstanding. I was not running any programs or processes in the context of the second user to trigger this issue, so I don't think UAC has anything to do with it. I am talking about simply unlocking a user from the Windows login screen.

If you take a look at how m_sessionPropertyCache is being populated upon a second user login, you will see that the second user's session details are being added to the first user's session ID. I am using Windows 10 in my tests.

In my solution, I "solved" this by adding code which retroactively fixes the cache entries by relocating session details in the cache to their proper session IDs upon unlocking. I am not happy with this solution but I cannot think of any other option.

MutonUfoAI commented 5 years ago

I can reproduce this bug be so kind and show me your modifications ... I'll dig into it as soon as i can

thank you

AlanEpHws commented 5 years ago

Sure. The following code was made instead of lines 449 - 467 of Service.cs

//the user is still logged in
                        isLoggedIN = true;

                        lockedSessionID = Convert.ToInt32(iUsers.Find(s => s.EndsWith("\\" + sessionDriver.UserInformation.Username, StringComparison.CurrentCultureIgnoreCase)).Split('\\').First());
                        m_logger.DebugFormat("User:{0} is Locked in Session:{1}", sessionDriver.UserInformation.Username, lockedSessionID);
                        bool found = false;
                        foreach (int sessionID in m_sessionPropertyCache.GetAll())
                        {
                            // search for session properties via the username
                            SessionProperties userSessionProperties = null;
                            List<SessionProperties> sessionPropertiesList = m_sessionPropertyCache.Get(sessionID);
                            foreach (SessionProperties sessionProperties in sessionPropertiesList)
                            {
                                UserInformation uInfo = sessionProperties.GetTrackedSingle<UserInformation>();
                                if (uInfo.Username.Equals(sessionDriver.UserInformation.Username, StringComparison.CurrentCultureIgnoreCase))
                                {
                                    userSessionProperties = sessionProperties;
                                    break;
                                }
                            }

                            if (userSessionProperties != null)
                            {
                                if (lockedSessionID != sessionID)
                                {
                                    // remove session properties from their old location
                                    sessionPropertiesList.Remove(userSessionProperties);
                                    if (sessionPropertiesList.Count > 0)
                                        m_sessionPropertyCache.Add(sessionID, sessionPropertiesList);
                                    else
                                        m_sessionPropertyCache.Remove(sessionID);

                                    // add session properties to their proper location
                                    List<SessionProperties> newSessionPropertiesList = m_sessionPropertyCache.Get(lockedSessionID);
                                    newSessionPropertiesList.Add(userSessionProperties);
                                    m_sessionPropertyCache.Add(lockedSessionID, newSessionPropertiesList);
                                }
                                found = true;
                                break;
                            }
                        }
                        if (!found)
                        {
                            m_logger.ErrorFormat("User {0} is Locked in Session {1} but was not authenticated by pGina. Unable to find SessionProperty in m_sessionPropertyCache.Get({1})", sessionDriver.UserInformation.Username, lockedSessionID);
                            //return new LoginResponseMessage() { Result = false, Message = string.Format("User {0} is Locked in Session {1} but was not authenticated by pGina\n\nIt is possible that another Credential Provider was used\nor the pGina service has crashed.\n", sessionDriver.UserInformation.Username, lockedSessionID) };
                            isLoggedIN = false;
                        }
MutonUfoAI commented 5 years ago

This is a bigger problem again in Windows 10. If figured out that this problem only occurs if you lock a user session if you switch to another session everything is working fine.

I than found this https://docs.microsoft.com/en-us/windows/desktop/api/credentialprovider/ne-credentialprovider-credential_provider_usage_scenario

Starting in Windows 10, the CPUS_LOGON and CPUS_UNLOCK_WORKSTATION user scenarios have been combined. This enables the system to support multiple users logging into a machine without creating and switching sessions unnecessarily. Any user on the machine can log into it once it has been locked without needing to back out of a current session and create a new one. Because of this, CPUS_LOGON can be used both for logging onto a system or when a workstation is unlocked. However, CPUS_LOGON cannot be used in all cases. Because of policy restrictions imposed by various systems, sometimes it is necessary for the user scenario to be CPUS_UNLOCK_WORKSTATION. Your credential provider should be robust enough to create the appropriate credential structure based on the scenario given to it. Windows will request the appropriate user scenario based on the situation. Some of the factors that impact whether or not a CPUS_UNLOCK_WORKSTATION scenario must be used include the following. Note that this is just a subset of possibilities.

They simply decided to drop the CPUS_UNLOCK_WORKSTATION event which result in this strange behavior.

Your code is working around the problem and can lead to other problems

I need to make a list what doesn't work in Windows 10 anymore ... and find a way to fix this problem.

AlanEpHws commented 5 years ago

Thank you very much for your detailed reply.

Yes, I know that my code is just a workaround; I needed to get it working asap for a project I'm working on. Of course, I would much rather have a proper fix, which is why I contacted you in the first place.

It is strange that Microsoft recommends using WTSQuerySessionInformation to keep track of the Session ID. I tried using the following code to get the Session ID instead of ProcessIdToSessionId in pGinaMessages.h line 259 :

DWORD sessionID = -1;
LPWSTR buffer = NULL;
DWORD bytes = 0;
if (WTSQuerySessionInformation(WTS_CURRENT_SERVER_HANDLE, WTS_CURRENT_SESSION, WTSSessionId, &buffer, &bytes)) {
    sessionID = (DWORD)*buffer;
}

However I get the same result with the wrong Session ID. Perhaps we are querying it too early? We know for sure that logging into a new account does indeed create a new Session ID; searching for it after its creation proves this; we're just getting the wrong one on login.

I also find it strange that the merging of CPUS_UNLOCK_WORKSTATION is causing this issue, because the actual bug is occurring during the logging in of the second account, not during its unlocking. Of course, the symptoms of the bug are apparent during the unlock, but that's only because erroneous data had been recorded before.

Looking forward to your new fix. Keep up the good work :)