From 35557679720dfcbd0535d0d7c630520bc924cb1e Mon Sep 17 00:00:00 2001 From: vibesoftwarecoder Date: Fri, 30 Oct 2026 10:58:29 -0500 Subject: [PATCH] Fix: the console owner was refused from the host USB panel ConsoleCallerCheck counted every account AccountManager stores as a seat account. That includes accounts that were only linked (IsManaged false), and the person at the console is often one of them. On the reference host roelb is linked, so GET /api/host/usb answered 403 NotConsoleUser to the console user themself. The seat-account set is now the accounts MultiSeat created, plus the account of every live seat and of every saved seat preset. A linked account that hosts a seat still counts. The rule is a pure function (SeatSidsFrom) so it can be tested; the new test fails on the old rule. Co-Authored-By: Claude Sonnet 5.5 --- docs/security-posture.md | 4 +- .../Api/ConsoleCallerCheck.cs | 38 ++++++++++--- .../HostUsb/ConsoleCallerCheckTests.cs | 54 +++++++++++++++++++ 3 files changed, 89 insertions(+), 7 deletions(-) diff --git a/docs/security-posture.md b/docs/security-posture.md index fea8626..40bbcee 100644 --- a/docs/security-posture.md +++ b/docs/security-posture.md @@ -139,7 +139,9 @@ change made here lasts only until that Apollo restarts. - The process must run in the **active console session**, and its token user must be **the user logged on there**, not SYSTEM or another service account, and **not one of MultiSeat's seat accounts**. A seat user who moves their own session to the console with `tscon` is the console user but a seat account, so is still - refused. A seat account MultiSeat cannot list is not excluded by that rule; it is still refused by the + refused. A "seat account" is one MultiSeat created, or one that hosts a live seat or has a saved seat preset. + An ordinary Windows account that was only linked to MultiSeat, such as the console owner's, is not one. + A seat account MultiSeat cannot list is not excluded by that rule; it is still refused by the session and user comparisons unless it is the console user. - A request that is not loopback, whose caller cannot be found, or when nobody is at the console, is refused. Another program of the console user can still ask; that is the trust the console user already has over the diff --git a/src/MultiSeat.Service/Api/ConsoleCallerCheck.cs b/src/MultiSeat.Service/Api/ConsoleCallerCheck.cs index 5955ab3..5965f55 100644 --- a/src/MultiSeat.Service/Api/ConsoleCallerCheck.cs +++ b/src/MultiSeat.Service/Api/ConsoleCallerCheck.cs @@ -1,6 +1,8 @@ using System.Net; using MultiSeat.Service.Accounts; +using MultiSeat.Service.Configuration; using MultiSeat.Service.Interop; +using MultiSeat.Service.Sessions; namespace MultiSeat.Service.Api; @@ -54,13 +56,13 @@ public sealed class ConsoleCallerCheck private readonly Func _consoleUserSid; private readonly Func> _seatSids; - public ConsoleCallerCheck(AccountManager accounts) + public ConsoleCallerCheck(AccountManager accounts, SeatManager seats, SeatPresetStore presets) : this( (clientPort, serverPort) => TcpTable.ClientOwner(TcpTable.ReadConnectionsWithTimes(), clientPort, serverPort), OwnerProcess.Open, Kernel32.WTSGetActiveConsoleSessionId, ConsoleUser.SidOf, - () => SeatSidsOf(accounts)) + () => SeatSidsOf(accounts, seats, presets)) { } @@ -75,15 +77,39 @@ internal ConsoleCallerCheck( _seatSids = seatSids; } - /// The SIDs of the seat accounts MultiSeat manages. A name that cannot be resolved is skipped. - private static IReadOnlyCollection SeatSidsOf(AccountManager accounts) + /// + /// The SIDs of the accounts that are, or can become, a seat: accounts MultiSeat created, the account of + /// every live seat, and the account of every saved seat preset. NOT every account MultiSeat has stored: + /// AccountManager also holds ordinary accounts that were only LINKED (IsManaged false), and + /// the person at the console is often one of them. Counting those refused the console user from the + /// panel meant for them. A linked account that does host a seat still counts, through its seat or preset. + /// A name that cannot be resolved is skipped. + /// + internal static IReadOnlyCollection SeatSidsOf(AccountManager accounts, SeatManager seats, SeatPresetStore presets) => + SeatSidsFrom( + accounts.ListManagedAccounts().Select(a => (a.Sid, a.Username, a.IsManaged)), + seats.GetAllSeats().Select(s => s.AccountName), + presets.GetAll().Select(p => p.AccountName), + name => AccountManager.TryResolveSid(name, Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance)); + + /// The rule behind , with its inputs passed in so a test can drive it. + internal static IReadOnlyCollection SeatSidsFrom( + IEnumerable<(string? Sid, string Username, bool IsManaged)> stored, + IEnumerable liveSeatAccounts, IEnumerable presetAccounts, Func resolve) { var sids = new HashSet(StringComparer.OrdinalIgnoreCase); - foreach (var account in accounts.ListManagedAccounts()) + + void Add(string? knownSid, string username) { - var sid = account.Sid ?? AccountManager.TryResolveSid(account.Username, Microsoft.Extensions.Logging.Abstractions.NullLogger.Instance); + var sid = !string.IsNullOrEmpty(knownSid) ? knownSid + : string.IsNullOrWhiteSpace(username) ? null : resolve(username); if (!string.IsNullOrEmpty(sid)) sids.Add(sid); } + + foreach (var (sid, username, isManaged) in stored) + if (isManaged) Add(sid, username); + foreach (var name in liveSeatAccounts) Add(null, name); + foreach (var name in presetAccounts) Add(null, name); return sids; } diff --git a/src/MultiSeat.Tests/HostUsb/ConsoleCallerCheckTests.cs b/src/MultiSeat.Tests/HostUsb/ConsoleCallerCheckTests.cs index 0671db6..4e1ba21 100644 --- a/src/MultiSeat.Tests/HostUsb/ConsoleCallerCheckTests.cs +++ b/src/MultiSeat.Tests/HostUsb/ConsoleCallerCheckTests.cs @@ -168,6 +168,60 @@ public void IfTheSeatAccountsCannotBeListed_NothingExtraIsRefused() Assert.Equal(CallerVerdict.Allowed, check.Check(IPAddress.Loopback, 50000, 9550)); } + // ── which accounts count as seat accounts ───────────────────────── + + private const string ConsoleOwnerSid = "S-1-5-21-1-1-1-1001"; + private const string SeatOwnerSid = "S-1-5-21-1-1-1-1010"; + + private static string? ResolveFrom(string name) => name.ToLowerInvariant() switch + { + "roelb" => ConsoleOwnerSid, + "gaming" => SeatOwnerSid, + _ => null, + }; + + [Fact] + public void ALinkedAccountThatHostsNoSeat_IsNotASeatAccount() + { + // The person at the console is often linked to MultiSeat as an ordinary account (IsManaged false). + // Listing every stored account made the check refuse them from their own panel (0.6.24). + var sids = ConsoleCallerCheck.SeatSidsFrom( + [(null, "roelb", false), (null, "Gaming", true)], ["Gaming"], ["Gaming"], ResolveFrom); + Assert.Equal([SeatOwnerSid], sids); + + var s = new Setup { ConsoleUserSid = ConsoleOwnerSid, Owner = new FakeOwner { User = ConsoleOwnerSid }, SeatSids = sids.ToArray() }; + Assert.Equal(CallerVerdict.Allowed, Run(s)); + } + + [Fact] + public void ALinkedAccountThatHostsALiveSeat_IsASeatAccount() + { + var sids = ConsoleCallerCheck.SeatSidsFrom([(null, "roelb", false)], ["roelb"], [], ResolveFrom); + Assert.Contains(ConsoleOwnerSid, sids); + var s = new Setup { ConsoleUserSid = ConsoleOwnerSid, Owner = new FakeOwner { User = ConsoleOwnerSid }, SeatSids = sids.ToArray() }; + Assert.Equal(CallerVerdict.NotConsoleUser, Run(s)); + } + + [Fact] + public void ALinkedAccountWithASavedPreset_IsASeatAccount() + { + var sids = ConsoleCallerCheck.SeatSidsFrom([(null, "roelb", false)], [], ["Roelb"], ResolveFrom); + Assert.Contains(ConsoleOwnerSid, sids); + } + + [Fact] + public void AnAccountMultiSeatCreated_IsASeatAccount_EvenWithNoSeatRunning() + { + Assert.Contains(SeatOwnerSid, ConsoleCallerCheck.SeatSidsFrom([(null, "Gaming", true)], [], [], ResolveFrom)); + Assert.Contains("S-1-5-21-9-9-9-5", ConsoleCallerCheck.SeatSidsFrom([("S-1-5-21-9-9-9-5", "x", true)], [], [], _ => null)); + } + + [Fact] + public void AnUnresolvableName_IsSkipped_NotAnError() + { + Assert.Empty(ConsoleCallerCheck.SeatSidsFrom([(null, "ghost", true)], ["ghost"], ["ghost"], _ => null)); + } + // ── a process id can be given to another process ────────────────── [Fact]