diff --git a/src/Engine/LoginGate.cs b/src/Engine/LoginGate.cs index 86882a5..abd2acf 100644 --- a/src/Engine/LoginGate.cs +++ b/src/Engine/LoginGate.cs @@ -39,21 +39,42 @@ public sealed class LoginGate /// Claims the desktop sign-in for the login batch. Returns null when the caller /// should run the batch, or the reason it should not. /// + /// + /// The account is matched by SID when both sides have one. Names are not a reliable + /// match: for an Entra ID sign-in the event names the account by its UPN + /// (user@domain) while the session reports the short name, so comparing names + /// rejected every Entra sign-in as another account logging on inside the session. + /// Names are compared only when a SID is missing, with the domain prefix and the + /// UPN suffix both removed. + /// /// TargetUserName from the event, or null for the start-up catch-up. /// The desktop session. /// The user signed in to that session now. /// When that user signed in to the session. - public string? Claim(string? logonUser, int sessionId, string? sessionUser, DateTime? sessionLogonTime) + /// TargetUserSid from the event, when known. + /// The SID of the user signed in to the session, when known. + public string? Claim( + string? logonUser, + int sessionId, + string? sessionUser, + DateTime? sessionLogonTime, + string? logonUserSid = null, + string? sessionUserSid = null) { if (string.IsNullOrWhiteSpace(sessionUser)) return $"nobody is signed in to session {sessionId}"; - if (logonUser != null && !string.Equals(StripDomain(logonUser), StripDomain(sessionUser), StringComparison.OrdinalIgnoreCase)) + if (!IsSameAccount(logonUser, logonUserSid, sessionUser, sessionUserSid)) return $"session {sessionId} belongs to {sessionUser}, so this was another account logging on inside it (runas or an elevation prompt)"; lock (_lock) { - var signIn = (StripDomain(sessionUser).ToUpperInvariant(), sessionLogonTime); + // Keyed on the session's identity, never the event's, so the start-up + // catch-up (which has no event) and a later event for the same sign-in agree. + var who = HasSid(sessionUserSid) + ? sessionUserSid!.Trim().ToUpperInvariant() + : AccountName(sessionUser).ToUpperInvariant(); + var signIn = (who, sessionLogonTime); if (_handled.TryGetValue(sessionId, out var handled) && handled == signIn) return $"the login payloads already ran for {sessionUser}'s sign-in to session {sessionId}"; @@ -62,9 +83,30 @@ public sealed class LoginGate } } - private static string StripDomain(string name) + private static bool IsSameAccount(string? logonUser, string? logonUserSid, string sessionUser, string? sessionUserSid) { + if (HasSid(logonUserSid) && HasSid(sessionUserSid)) + return string.Equals(logonUserSid!.Trim(), sessionUserSid!.Trim(), StringComparison.OrdinalIgnoreCase); + + // The start-up catch-up has no event, so nothing to compare. + if (logonUser == null) + return true; + + return string.Equals(AccountName(logonUser), AccountName(sessionUser), StringComparison.OrdinalIgnoreCase); + } + + private static bool HasSid(string? sid) => !string.IsNullOrWhiteSpace(sid); + + /// + /// The bare account name: DOMAIN\user, AzureAD\user and user@domain all become user. + /// + internal static string AccountName(string name) + { + name = name.Trim(); var slash = name.LastIndexOf('\\'); - return slash >= 0 ? name[(slash + 1)..] : name; + if (slash >= 0) name = name[(slash + 1)..]; + var at = name.IndexOf('@'); + if (at > 0) name = name[..at]; + return name; } } diff --git a/src/Engine/Native/ShellReadiness.cs b/src/Engine/Native/ShellReadiness.cs index c5567d1..625aa6d 100644 --- a/src/Engine/Native/ShellReadiness.cs +++ b/src/Engine/Native/ShellReadiness.cs @@ -174,8 +174,46 @@ public static int GetActiveConsoleSessionId() } } + /// + /// The SID of the user signed in to , or null when it + /// cannot be read. + /// + /// + /// The session's user name is not enough to tell whether a logon event belongs to + /// the person at the desktop: an Entra ID sign-in is named by its UPN in the event + /// and by its short name in the session. The SID is the same on both sides. + /// + /// WTSQueryUserToken needs SeTcbPrivilege, which the service has as SYSTEM. Anywhere + /// else -- a test run, the CLI -- this returns null and the caller falls back to + /// comparing names. + /// + public static string? GetSessionUserSid(int sessionId) + { + if (sessionId <= 0) + return null; + + try + { + if (!WTSQueryUserToken(sessionId, out var token)) + return null; + + using (token) + using (var identity = new System.Security.Principal.WindowsIdentity(token.DangerousGetHandle())) + { + return identity.User?.Value; + } + } + catch + { + return null; + } + } + private const int WtsUserName = 5; + [System.Runtime.InteropServices.DllImport("wtsapi32.dll", SetLastError = true)] + private static extern bool WTSQueryUserToken(int sessionId, out Microsoft.Win32.SafeHandles.SafeAccessTokenHandle token); + [System.Runtime.InteropServices.DllImport("kernel32.dll")] private static extern uint WTSGetActiveConsoleSessionId(); diff --git a/src/Service/Workers/LogonEventWorker.cs b/src/Service/Workers/LogonEventWorker.cs index 58bfb76..6ffe96d 100644 --- a/src/Service/Workers/LogonEventWorker.cs +++ b/src/Service/Workers/LogonEventWorker.cs @@ -96,6 +96,7 @@ private async void OnLogonEvent(object? sender, EventRecordWrittenEventArgs e) var record = e.EventRecord; var username = GetEventDataValue(record, "TargetUserName"); var domain = GetEventDataValue(record, "TargetDomainName"); + var userSid = GetEventDataValue(record, "TargetUserSid"); var logonType = GetEventDataValue(record, "LogonType"); // Skip system accounts @@ -122,7 +123,7 @@ private async void OnLogonEvent(object? sender, EventRecordWrittenEventArgs e) Volatile.Write(ref _observedLogon, 1); var sessionId = logonSessionId >= 0 ? logonSessionId : GetActiveConsoleSession(); - await RunLoginPayloadsAsync(username, username, sessionId, CancellationToken.None); + await RunLoginPayloadsAsync(username, username, userSid, sessionId, CancellationToken.None); } catch (Exception ex) { @@ -141,7 +142,8 @@ private async void OnLogonEvent(object? sender, EventRecordWrittenEventArgs e) /// one path and miss the other. /// /// The account the logon event names, or null for the catch-up. - private async Task RunLoginPayloadsAsync(string? username, string? logonUser, int sessionId, CancellationToken stoppingToken) + /// The SID the logon event names, or null for the catch-up. + private async Task RunLoginPayloadsAsync(string? username, string? logonUser, string? logonUserSid, int sessionId, CancellationToken stoppingToken) { try { @@ -176,7 +178,13 @@ await ShellReadiness.WaitForDesktopAsync( // Checked after the desktop wait: until then the session may not show who // signed in to it yet. var sessionUser = ShellReadiness.GetSessionUserName(sessionId); - var notASignIn = _loginGate.Claim(logonUser, sessionId, sessionUser, LogonSessions.GetSessionLogonTime(sessionId)); + var notASignIn = _loginGate.Claim( + logonUser, + sessionId, + sessionUser, + LogonSessions.GetSessionLogonTime(sessionId), + logonUserSid, + ShellReadiness.GetSessionUserSid(sessionId)); if (notASignIn != null) { StartSetLogger.Debug("Not running login payloads for {User}: {Reason}", logonUser ?? username ?? "Unknown", notASignIn); @@ -316,7 +324,7 @@ private async Task RunForAlreadySignedInUserAsync(CancellationToken stoppingToke Volatile.Write(ref _observedLogon, 1); - await RunLoginPayloadsAsync(username, null, sessionId, stoppingToken); + await RunLoginPayloadsAsync(username, null, null, sessionId, stoppingToken); } catch (OperationCanceledException) { @@ -377,6 +385,7 @@ private static int GetActiveConsoleSession() // TargetUserSid, TargetUserName, TargetDomainName, TargetLogonId, LogonType... return name switch { + "TargetUserSid" => properties.Count > 4 ? properties[4].Value?.ToString() : null, "TargetUserName" => properties.Count > 5 ? properties[5].Value?.ToString() : null, "TargetDomainName" => properties.Count > 6 ? properties[6].Value?.ToString() : null, "TargetLogonId" => properties.Count > 7 ? properties[7].Value?.ToString() : null, diff --git a/tests/StartSet.Tests/Engine/LoginGateTests.cs b/tests/StartSet.Tests/Engine/LoginGateTests.cs index ea24323..c2d9881 100644 --- a/tests/StartSet.Tests/Engine/LoginGateTests.cs +++ b/tests/StartSet.Tests/Engine/LoginGateTests.cs @@ -90,6 +90,63 @@ public void AnotherAccountLoggingOnInsideTheSessionIsNotASignIn() Assert.Null(gate.Claim("alice", 1, "alice", SignedIn)); } + private const string AliceSid = "S-1-12-1-1111111111-2222222222-3333333333-4444444444"; + private const string HelpdeskSid = "S-1-12-1-5555555555-6666666666-7777777777-8888888888"; + + [Fact] + public void AnEntraSignInNamedByUpnMatchesTheSessionBySid() + { + var gate = new LoginGate(); + + Assert.Null(gate.Claim("alice@contoso.example", 1, "alice", SignedIn, AliceSid, AliceSid)); + } + + [Fact] + public void ADifferentSidIsAnotherAccountEvenWhenTheNamesMatch() + { + var gate = new LoginGate(); + + Assert.NotNull(gate.Claim("alice", 1, "alice", SignedIn, HelpdeskSid, AliceSid)); + Assert.NotNull(gate.Claim("helpdesk@contoso.example", 1, "alice", SignedIn, HelpdeskSid, AliceSid)); + Assert.Null(gate.Claim("alice@contoso.example", 1, "alice", SignedIn, AliceSid, AliceSid)); + } + + [Theory] + [InlineData("alice@contoso.example")] + [InlineData(@"AzureAD\alice")] + [InlineData(@"AzureAD\alice@contoso.example")] + [InlineData(@"CONTOSO\Alice")] + public void WithoutASidTheNamesAreComparedWithoutDomainOrUpnSuffix(string logonUser) + { + Assert.Null(new LoginGate().Claim(logonUser, 1, "alice", SignedIn, null, AliceSid)); + Assert.Null(new LoginGate().Claim(logonUser, 1, @"AzureAD\alice", SignedIn, AliceSid, null)); + } + + [Fact] + public void WithoutASidAnotherNameIsStillAnotherAccount() + { + Assert.NotNull(new LoginGate().Claim("helpdesk@contoso.example", 1, "alice", SignedIn, HelpdeskSid, null)); + } + + [Fact] + public void TheCatchUpAndALaterEventForTheSameSignInRunTheBatchOnce() + { + var gate = new LoginGate(); + + Assert.Null(gate.Claim(null, 1, "alice", SignedIn, null, AliceSid)); + Assert.NotNull(gate.Claim("alice@contoso.example", 1, "alice", SignedIn, AliceSid, AliceSid)); + } + + [Fact] + public void TheSessionSidNeedsSystemToRead() + { + if (new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null))) + return; + + Assert.Null(ShellReadiness.GetSessionUserSid(Process.GetCurrentProcess().SessionId)); + Assert.Null(ShellReadiness.GetSessionUserSid(0)); + } + [Fact] public void ASessionNobodyIsSignedInToIsNotASignIn() {