From 16b33c0b6d16a17027c3f78c023ba2a8fd192df5 Mon Sep 17 00:00:00 2001 From: sammiller Date: Sun, 25 Oct 2026 16:36:56 +0000 Subject: [PATCH 1/2] [MR-12] Verify named pipe owners against the token user SID Use explicit user ownership for parent-child listeners, keep foreign and group owners denied, and exercise both listener modes with real local pipes. Co-Authored-By: OpenAI Codex --- .../LocalRpcClientChannel.cs | 29 +++++- .../ArcForges.LocalRpc/README.md | 8 +- .../ArcForges.LocalRpc/StreamAcceptSources.cs | 32 +++++-- .../Tests/LocalRpcOsStreamChecks.cs | 4 +- .../Tests/LocalRpcPipeOwnershipTests.cs | 93 +++++++++++++++++++ .../Tests/LocalRpcServiceOsChecks.cs | 2 +- .../Tests/WrongUserHelperProcess.cs | 2 +- .../Tests/wrong-user-procedure.md | 9 +- 8 files changed, 158 insertions(+), 21 deletions(-) create mode 100644 src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcPipeOwnershipTests.cs diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/LocalRpcClientChannel.cs b/src/BuildingBlocks/ArcForges.LocalRpc/LocalRpcClientChannel.cs index b3625471..785a5399 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/LocalRpcClientChannel.cs +++ b/src/BuildingBlocks/ArcForges.LocalRpc/LocalRpcClientChannel.cs @@ -2,6 +2,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO.Pipes; using System.Net.Sockets; +using System.Runtime.Versioning; using System.Security.Principal; using Grpc.Core; using Grpc.Core.Interceptors; @@ -112,24 +113,31 @@ private static async ValueTask ConnectAsync(LocalRpcEndpoint endpoint, T bounded.CancelAfter(timeout); return endpoint.Transport switch { - LocalRpcTransport.NamedPipe => await ConnectNamedPipeAsync(endpoint.Address, bounded.Token).ConfigureAwait(false), + LocalRpcTransport.NamedPipe when OperatingSystem.IsWindows() => await ConnectNamedPipeAsync(endpoint.Address, bounded.Token).ConfigureAwait(false), LocalRpcTransport.UnixDomainSocket => await ConnectUnixSocketAsync(endpoint.Address, bounded.Token).ConfigureAwait(false), _ => throw new InvalidOperationException("A local RPC endpoint names exactly one private stream transport."), }; } - private static async ValueTask ConnectNamedPipeAsync(string name, CancellationToken cancellationToken) + [SupportedOSPlatform("windows")] + internal static async ValueTask ConnectNamedPipeAsync(string name, CancellationToken cancellationToken) { - // CurrentUserOnly makes the runtime verify that the server end is owned by this user after connecting. + // CurrentUserOnly compares the token's default owner, which can be BUILTIN\Administrators when elevated. + // Compare the connected server end with the token's user SID instead, before handing any stream to HTTP/2. var pipe = new NamedPipeClientStream( ".", name, PipeDirection.InOut, - PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly, + PipeOptions.Asynchronous, TokenImpersonationLevel.Anonymous); try { await pipe.ConnectAsync(cancellationToken).ConfigureAwait(false); + if (!IsOwnedByCurrentUser(pipe)) + { + throw new UnauthorizedAccessException("The server pipe is not owned by this token's user SID."); + } + return pipe; } catch @@ -139,6 +147,19 @@ private static async ValueTask ConnectNamedPipeAsync(string name, Cancel } } + [SupportedOSPlatform("windows")] + private static bool IsOwnedByCurrentUser(PipeStream pipe) + { + var owner = pipe.GetAccessControl().GetOwner(typeof(SecurityIdentifier)) as SecurityIdentifier; + using var identity = WindowsIdentity.GetCurrent(); + return OwnerIsUser(owner, identity.User); + } + + /// A group owner, a different user or a missing SID never proves ownership by this user. + [SupportedOSPlatform("windows")] + internal static bool OwnerIsUser(SecurityIdentifier? owner, SecurityIdentifier? user) => + owner is not null && user is not null && owner == user; + [SuppressMessage("Reliability", "CA2000", Justification = "Ownership of the connected socket passes to the returned stream; failures dispose it.")] private static async ValueTask ConnectUnixSocketAsync(string path, CancellationToken cancellationToken) { diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/README.md b/src/BuildingBlocks/ArcForges.LocalRpc/README.md index 108f83ea..6c0f98ec 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/README.md +++ b/src/BuildingBlocks/ArcForges.LocalRpc/README.md @@ -450,9 +450,11 @@ minutes and needs `ARCFORGES_LOCALRPC_LONG_STREAM=1` as well. `tests/LocalRpcSer ## OS user boundary -A Named Pipe is created with `CurrentUserOnly` (a DACL granting only the current user); the first instance also -uses `FirstPipeInstance`, so binding fails when the name already has an instance, and the client requires the -server end to be owned by the current user. The pipe is not created with the reject-remote-clients flag and a +A parent-child Named Pipe is created with an explicit owner and protected DACL for the token's user SID; the first instance also +uses `FirstPipeInstance`, so binding fails when the name already has an instance. The client checks the connected server end's owner +against its token's user SID before sending any bytes, including when the token's default owner is BUILTIN\Administrators. Another +user, a group owner or a missing SID is refused. This does not grant a service-mode server access to an otherwise unidentifiable peer. +The parent-child pipe is not created with the reject-remote-clients flag and a client reaches it through `\\127.0.0.1\pipe\` as well; it is limited to the current account by its DACL and is not claimed to be network-isolated. A Unix socket is bound only in a directory with no group or other permission bits, is never created over an existing file, is set to mode 0600 and is removed when the listener is diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/StreamAcceptSources.cs b/src/BuildingBlocks/ArcForges.LocalRpc/StreamAcceptSources.cs index c01c2cef..4a16c0f9 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/StreamAcceptSources.cs +++ b/src/BuildingBlocks/ArcForges.LocalRpc/StreamAcceptSources.cs @@ -2,6 +2,8 @@ using System.Diagnostics.CodeAnalysis; using System.IO.Pipes; using System.Net.Sockets; +using System.Security.AccessControl; +using System.Security.Principal; namespace ArcForges.LocalRpc; @@ -141,12 +143,30 @@ private NamedPipeServerStream CreateInstance(bool first) => _service && Operatin ? LocalRpcWindowsNative.CreateServicePipe(_name, first) : CreatePlainInstance(first); - private NamedPipeServerStream CreatePlainInstance(bool first) => new( - _name, - PipeDirection.InOut, - NamedPipeServerStream.MaxAllowedServerInstances, - PipeTransmissionMode.Byte, - PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly | (first ? PipeOptions.FirstPipeInstance : PipeOptions.None)); + private NamedPipeServerStream CreatePlainInstance(bool first) + { + if (!OperatingSystem.IsWindows()) + { + throw new PlatformNotSupportedException("Named Pipes are the Windows transport."); + } + + // Parent-child listeners use the same user-SID owner rule as service listeners and the client, even in an elevated token. + using var identity = WindowsIdentity.GetCurrent(); + var user = identity.User ?? throw new InvalidOperationException("The current token has no user SID."); + var security = new PipeSecurity(); + security.SetOwner(user); + security.SetAccessRuleProtection(isProtected: true, preserveInheritance: false); + security.AddAccessRule(new PipeAccessRule(user, PipeAccessRights.FullControl, AccessControlType.Allow)); + return NamedPipeServerStreamAcl.Create( + _name, + PipeDirection.InOut, + NamedPipeServerStream.MaxAllowedServerInstances, + PipeTransmissionMode.Byte, + PipeOptions.Asynchronous | (first ? PipeOptions.FirstPipeInstance : PipeOptions.None), + 0, + 0, + security); + } } /// diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcOsStreamChecks.cs b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcOsStreamChecks.cs index 669bb426..01936f20 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcOsStreamChecks.cs +++ b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcOsStreamChecks.cs @@ -94,7 +94,7 @@ public async Task EveryNamedPipeInstanceGrantsOnlyTheCurrentUser() var name = "af-os-acl-" + Guid.NewGuid().ToString("N"); await using var source = NamedPipeAcceptSource.Bind(name); var accepted = source.AcceptAsync(ct).AsTask(); - await using var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly, TokenImpersonationLevel.Anonymous); + await using var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, PipeOptions.Asynchronous, TokenImpersonationLevel.Anonymous); await client.ConnectAsync(ct); await using var stream = await accepted.WaitAsync(Patience, ct); @@ -343,7 +343,7 @@ private static async Task OpenRawAsync(LocalRpcEndpoint endpoint, Cancel { if (endpoint.Transport == LocalRpcTransport.NamedPipe) { - var pipe = new NamedPipeClientStream(".", endpoint.Address, PipeDirection.InOut, PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly, TokenImpersonationLevel.Anonymous); + var pipe = new NamedPipeClientStream(".", endpoint.Address, PipeDirection.InOut, PipeOptions.Asynchronous, TokenImpersonationLevel.Anonymous); await pipe.ConnectAsync(ct); return pipe; } diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcPipeOwnershipTests.cs b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcPipeOwnershipTests.cs new file mode 100644 index 00000000..e91bc395 --- /dev/null +++ b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcPipeOwnershipTests.cs @@ -0,0 +1,93 @@ +// SPDX-License-Identifier: AGPL-3.0-only +using System.IO.Pipes; +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; +using Xunit; + +namespace ArcForges.LocalRpc.Tests; + +/// User-SID ownership, separate from service-mode peer resolution. Real pipe checks are local opt-in, never hosted CI. +[Collection(LocalRpcCollection.Name)] +[SupportedOSPlatform("windows")] +public sealed class LocalRpcPipeOwnershipTests +{ + private const string WindowsOnly = "Windows security identifiers."; + private const string OptIn = "Explicit local OS pipe ownership checks only (ARCFORGES_LOCALRPC_OS_STREAMS=1)."; + + public static bool IsWindows => OperatingSystem.IsWindows(); + + public static bool WindowsEnabled => LocalRpcServiceOsChecks.WindowsEnabled; + + [Fact(Skip = WindowsOnly, SkipUnless = nameof(IsWindows))] + public void OnlyTheTokenUserSidOwnsTheServerEnd() + { + var user = new SecurityIdentifier("S-1-5-21-1000000001-1000000002-1000000003-1001"); + var other = new SecurityIdentifier("S-1-5-21-1000000001-1000000002-1000000003-1002"); + var administrators = new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null); + var system = new SecurityIdentifier(WellKnownSidType.LocalSystemSid, null); + + Assert.True(LocalRpcClientChannel.OwnerIsUser(user, user)); + Assert.False(LocalRpcClientChannel.OwnerIsUser(other, user)); + Assert.False(LocalRpcClientChannel.OwnerIsUser(administrators, user)); + Assert.False(LocalRpcClientChannel.OwnerIsUser(system, user)); + Assert.False(LocalRpcClientChannel.OwnerIsUser(null, user)); + Assert.False(LocalRpcClientChannel.OwnerIsUser(user, null)); + } + + [Theory(Skip = OptIn, SkipUnless = nameof(WindowsEnabled), Timeout = 60_000)] + [InlineData(false)] + [InlineData(true)] + public async Task EveryListenerInstanceUsesTheUserSidAndExchangesBytes(bool service) + { + var ct = TestContext.Current.CancellationToken; + using var identity = WindowsIdentity.GetCurrent(); + ReportSession(identity); + var name = "af-owner-" + Guid.NewGuid().ToString("N"); + await using var source = service ? NamedPipeAcceptSource.BindService(name) : NamedPipeAcceptSource.Bind(name); + + for (var instance = 0; instance < 3; instance++) + { + var accepted = source.AcceptAsync(ct).AsTask(); + await using var client = await LocalRpcClientChannel.ConnectNamedPipeAsync(name, ct); + await using var stream = await accepted.WaitAsync(ServiceHarness.Patience, ct); + var server = Assert.IsType(stream); + Assert.Equal(identity.User, server.GetAccessControl().GetOwner(typeof(SecurityIdentifier))); + var bytes = new byte[1]; + var read = server.ReadAsync(bytes, ct).AsTask(); + await client.WriteAsync(new byte[] { 42 }, ct); + Assert.Equal(1, await read); + Assert.Equal(42, bytes[0]); + } + } + + [Fact(Skip = OptIn, SkipUnless = nameof(WindowsEnabled), Timeout = 60_000)] + public async Task TheTokensDefaultOwnerIsAcceptedOnlyWhenItIsTheUser() + { + var ct = TestContext.Current.CancellationToken; + using var identity = WindowsIdentity.GetCurrent(); + ReportSession(identity); + var name = "af-owner-default-" + Guid.NewGuid().ToString("N"); + await using var server = new NamedPipeServerStream(name, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous); + Assert.Equal(identity.Owner, server.GetAccessControl().GetOwner(typeof(SecurityIdentifier))); + var accepted = server.WaitForConnectionAsync(ct); + + if (identity.Owner != identity.User) + { + _ = await Assert.ThrowsAsync(async () => await LocalRpcClientChannel.ConnectNamedPipeAsync(name, ct)); + await accepted; + var bytes = new byte[1]; + Assert.Equal(0, await server.ReadAsync(bytes, ct)); + } + else + { + await using var client = await LocalRpcClientChannel.ConnectNamedPipeAsync(name, ct); + await accepted; + } + } + + private static void ReportSession(WindowsIdentity identity) => + TestContext.Current.TestOutputHelper?.WriteLine(identity.Owner == identity.User + ? "Token default owner equals user SID; this run does not prove cross-integrity behavior." + : "Token default owner differs from user SID; explicit user-owned listeners must still connect."); +} diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcServiceOsChecks.cs b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcServiceOsChecks.cs index d4746e7c..0a35162f 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcServiceOsChecks.cs +++ b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcServiceOsChecks.cs @@ -51,7 +51,7 @@ public async Task EveryServicePipeInstanceIsOwnedByTheUserGrantsOnlyTheUserAndSy for (var instance = 0; instance < 3; instance++) { var accepted = source.AcceptAsync(ct).AsTask(); - await using var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, PipeOptions.Asynchronous | PipeOptions.CurrentUserOnly, TokenImpersonationLevel.Anonymous); + await using var client = new NamedPipeClientStream(".", name, PipeDirection.InOut, PipeOptions.Asynchronous, TokenImpersonationLevel.Anonymous); await client.ConnectAsync(ct); await using var stream = await accepted.WaitAsync(ServiceHarness.Patience, ct); var security = Assert.IsType(stream).GetAccessControl(); diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/WrongUserHelperProcess.cs b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/WrongUserHelperProcess.cs index c230b7a1..182cebbc 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/WrongUserHelperProcess.cs +++ b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/WrongUserHelperProcess.cs @@ -420,7 +420,7 @@ internal static int ExitCode(WrongUserOutcome outcome, WrongUserExpectation expe /// /// Classifies a failed library-client call. It is a refusal exactly when the chain holds the library's connect failure wrapping - /// an (the runtime's CurrentUserOnly ownership check, or the endpoint's access list); + /// an (the library's user-SID ownership check, or the endpoint's access list); /// any other connect failure proves nothing; a failure after the stream opened means the ownership check passed. /// internal static WrongUserOutcome ClassifyClientFailure(Exception exception) diff --git a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/wrong-user-procedure.md b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/wrong-user-procedure.md index de66daa0..dca27225 100644 --- a/src/BuildingBlocks/ArcForges.LocalRpc/Tests/wrong-user-procedure.md +++ b/src/BuildingBlocks/ArcForges.LocalRpc/Tests/wrong-user-procedure.md @@ -264,9 +264,10 @@ Cleanup: step 5 removes the pipe. Record: S5, section W1. ## W2. PLT.09 foreign-pipe client ownership -The library client opens a pipe with `CurrentUserOnly`. The runtime then requires the server end to be owned by the -current user. Here B owns a pipe that grants A access. The raw connect is admitted, so only the ownership check can -refuse the client. +The library client checks the connected server end's owner against the current token's user SID before sending bytes. It does not +use `CurrentUserOnly`, whose runtime check compares the token's default owner (potentially BUILTIN\Administrators when elevated). +A different user or group owner is refused. Here B owns a pipe that grants A access. The raw connect is admitted, so only the +ownership check can refuse the client. Set-up and staging: S1 to S3 (Windows). Consoles A1, B1 and B2. @@ -326,7 +327,7 @@ Cleanup: B1 runs `Remove-Item -Recurse C:\ArcForgesWrongUser\roots\rt-w3`, and A ## W4. PLT.10 launch endpoint denial (Named Pipe; Unix socket) The serving helper creates a launch authority with a root under A's temporary folder, issues one launch, and serves its -endpoint with a launch-authorized server. On Windows, the Named Pipe is `afl-<32 hex>`, created with `CurrentUserOnly`. +endpoint with a launch-authorized server. On Windows, the Named Pipe is `afl-<32 hex>`, created with an explicit user-SID owner and protected user-only DACL. The Unix socket is `\afwu-*\rt\<12 hex>\s`, inside the library's protected owner-only launch directories. Run the steps once with `` = `named-pipe` and once with `unix-socket`. From 66ffbc7c6ec38dfa52e0f909df48f9122ffe1668 Mon Sep 17 00:00:00 2001 From: sammiller Date: Sun, 25 Oct 2026 16:39:13 +0000 Subject: [PATCH 2/2] [MR-12] Register the authored pipe ownership tests in provenance Co-Authored-By: OpenAI Codex --- eng/provenance/files.json | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/provenance/files.json b/eng/provenance/files.json index bded692d..dd38cfb2 100644 --- a/eng/provenance/files.json +++ b/eng/provenance/files.json @@ -1002,6 +1002,7 @@ "src/BuildingBlocks/ArcForges.LocalRpc/LocalRpcWindowsNative.cs", "src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcServiceModeTests.cs", "src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcServiceOsChecks.cs", + "src/BuildingBlocks/ArcForges.LocalRpc/Tests/LocalRpcPipeOwnershipTests.cs", "src/BuildingBlocks/ArcForges.LocalRpc/Tests/ServiceModeFixtures.cs", "tests/LocalRpcServiceAotTests/LocalRpcServiceAotTests.csproj", "tests/LocalRpcServiceAotTests/Program.cs",