From 9edcd2584b7db1be9136b509cb8c83e70a4bcd64 Mon Sep 17 00:00:00 2001 From: Rod Christiansen Date: Thu, 22 Oct 2026 12:06:37 -0700 Subject: [PATCH] Match the signed-in account by SID so Entra ID sign-ins run the login payloads The login gate compared the 4624 TargetUserName with the session's user name. For an Entra ID sign-in the event names the account by its UPN while the session reports the short name, so every Entra sign-in was rejected as another account logging on inside the session, and login-every and login-privileged-every never ran. The worker now reads TargetUserSid from the event and the session user's SID from its token, and the gate compares SIDs when both are present. The name comparison remains as a fallback and now strips both the domain prefix and the UPN suffix. The once-per-sign-in key uses the session SID when it is known. --- src/Engine/LoginGate.cs | 52 +++++++++++++++-- src/Engine/Native/ShellReadiness.cs | 38 +++++++++++++ src/Service/Workers/LogonEventWorker.cs | 17 ++++-- tests/StartSet.Tests/Engine/LoginGateTests.cs | 57 +++++++++++++++++++ 4 files changed, 155 insertions(+), 9 deletions(-) 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() {