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]