diff --git a/.github/workflows/windows-recovery.yml b/.github/workflows/windows-recovery.yml index 0530553541..c8243d2519 100644 --- a/.github/workflows/windows-recovery.yml +++ b/.github/workflows/windows-recovery.yml @@ -35,7 +35,9 @@ jobs: - name: Verify Runtime Host Local IPC trust boundary shell: pwsh - run: ./scripts/windows-runtime-host-local-ipc-trust.ps1 + run: | + node.exe --test packages/runtime-host/dist/__tests__/control-endpoint.test.js + ./scripts/windows-runtime-host-local-ipc-trust.ps1 - name: Verify SQLite crash recovery shell: pwsh diff --git a/packages/runtime-host/src/__tests__/control-endpoint.test.ts b/packages/runtime-host/src/__tests__/control-endpoint.test.ts index 1fa79d75df..1ffc074fa2 100644 --- a/packages/runtime-host/src/__tests__/control-endpoint.test.ts +++ b/packages/runtime-host/src/__tests__/control-endpoint.test.ts @@ -22,10 +22,9 @@ function legacyPrefix(): string { } describe('runtime host Windows named-pipe endpoint', { skip: process.platform !== 'win32' }, () => { - test('derives a stable pipe name and has idempotent lifecycle hooks', async () => { + test('derives a stable pipe name and has idempotent cleanup', async () => { const endpoint = await prepareRuntimeHostEndpoint({ rootId: ROOT_ID, hostEpoch: 'epoch-1' }); assert.equal(endpoint.path, `\\\\.\\pipe\\maka-runtime-host-${ROOT_ID.slice(0, 16)}-epoch-1`); - await endpoint.prepareAfterListen(); await endpoint.cleanup(); await endpoint.cleanup(); }); diff --git a/packages/runtime-host/src/__tests__/fixtures/windows-local-ipc-trust-host.ts b/packages/runtime-host/src/__tests__/fixtures/windows-local-ipc-trust-host.ts index 19e2cdfbbe..460201ab05 100644 --- a/packages/runtime-host/src/__tests__/fixtures/windows-local-ipc-trust-host.ts +++ b/packages/runtime-host/src/__tests__/fixtures/windows-local-ipc-trust-host.ts @@ -1,21 +1,24 @@ import { once } from 'node:events'; import { startLocalIpcRuntimeHostListener } from '../../server/local-ipc-listener.js'; +import type { RuntimeHostMessageTransport } from '../../transport/message-transport.js'; if (process.platform !== 'win32') { throw new Error('Windows Local IPC trust fixture must run on Windows'); } +const transports = new Set(); const listener = await startLocalIpcRuntimeHostListener({ rootId: 'ab'.repeat(32), hostEpoch: `trust-${process.pid}`, accept(connection) { + transports.add(connection.transport); + void connection.transport.closed.then(() => transports.delete(connection.transport)); process.stdout.write( `${JSON.stringify({ type: 'accepted', principalKind: connection.authority.principalKind, })}\n`, ); - connection.transport.abort(); }, }); @@ -23,5 +26,11 @@ process.stdout.write(`${JSON.stringify({ type: 'ready', endpoint: listener.endpo process.stdin.resume(); await once(process.stdin, 'data'); process.stdin.pause(); +await Promise.all( + [...transports].map((transport) => { + transport.abort(); + return transport.closed; + }), +); await listener.closeAdmission(); await listener.cleanup(); diff --git a/packages/runtime-host/src/control/endpoint.ts b/packages/runtime-host/src/control/endpoint.ts index a22e31531c..6df70e8b71 100644 --- a/packages/runtime-host/src/control/endpoint.ts +++ b/packages/runtime-host/src/control/endpoint.ts @@ -1,3 +1,4 @@ +import { execFile } from 'node:child_process'; import { chmod, lstat, mkdtemp, readdir, rm, rmdir, unlink } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -6,6 +7,20 @@ const FALLBACK_ENDPOINT_ROOT = '/tmp'; const PORTABLE_UNIX_SOCKET_PATH_LIMIT = 100; const ENDPOINT_SOCKET_NAME = 'h.sock'; const MKDTEMP_SUFFIX_LENGTH = 6; +const WINDOWS_PIPE_PATH_ENV = 'MAKA_RUNTIME_HOST_PIPE_PATH'; +const WINDOWS_PIPE_ACL_SCRIPT = String.raw` +$ErrorActionPreference = 'Stop' +$identity = [System.Security.Principal.WindowsIdentity]::GetCurrent() +$security = [System.Security.AccessControl.FileSecurity]::new() +$security.SetAccessRuleProtection($true, $false) +$rights = [System.Security.AccessControl.FileSystemRights]::FullControl +$allow = [System.Security.AccessControl.AccessControlType]::Allow +$security.AddAccessRule([System.Security.AccessControl.FileSystemAccessRule]::new($identity.User, $rights, $allow)) +$system = [System.Security.Principal.SecurityIdentifier]::new('S-1-5-18') +$security.AddAccessRule([System.Security.AccessControl.FileSystemAccessRule]::new($system, $rights, $allow)) +$pipe = Get-Item -LiteralPath $env:${WINDOWS_PIPE_PATH_ENV} +$pipe.SetAccessControl($security) +`; export interface RuntimeHostEndpointInput { rootId: string; @@ -37,8 +52,7 @@ export async function prepareRuntimeHostEndpoint( return { path, async prepareAfterListen() { - // Node creates the pipe with the process token's default DACL. The - // blocking cross-user CI pins that a foreign user cannot open it duplex. + await secureWindowsNamedPipe(path); }, async cleanup() {}, }; @@ -89,6 +103,42 @@ export async function prepareRuntimeHostEndpoint( } } +function secureWindowsNamedPipe(path: string): Promise { + const systemRoot = process.env.SystemRoot; + if (!systemRoot) { + return Promise.reject( + new RuntimeHostEndpointError( + 'insecure_endpoint_directory', + 'Runtime Host cannot locate Windows PowerShell to secure its Local IPC endpoint', + ), + ); + } + const powershell = join(systemRoot, 'System32', 'WindowsPowerShell', 'v1.0', 'powershell.exe'); + return new Promise((resolve, reject) => { + execFile( + powershell, + ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', WINDOWS_PIPE_ACL_SCRIPT], + { + env: { ...process.env, [WINDOWS_PIPE_PATH_ENV]: path }, + timeout: 10_000, + windowsHide: true, + }, + (error) => { + if (!error) { + resolve(); + return; + } + reject( + new RuntimeHostEndpointError( + 'insecure_endpoint_directory', + 'Runtime Host could not restrict its Windows Local IPC endpoint to the current user', + ), + ); + }, + ); + }); +} + // Honor TMPDIR via os.tmpdir(), but never at the cost of a socket path over // the portable sun_path budget: macOS per-user temp roots and the parallel // test runner's nested TMPDIR produce bases long enough that the full diff --git a/scripts/windows-runtime-host-local-ipc-trust.ps1 b/scripts/windows-runtime-host-local-ipc-trust.ps1 index 1ef56d80bd..17a6bbd8c4 100644 --- a/scripts/windows-runtime-host-local-ipc-trust.ps1 +++ b/scripts/windows-runtime-host-local-ipc-trust.ps1 @@ -34,7 +34,8 @@ public static class MakaWindowsPipeTrustProbe string username, string domain, string password, - string pipeName) + string pipeName, + PipeDirection direction) { SafeAccessTokenHandle token; if (!LogonUser(username, domain, password, 2, 0, out token)) @@ -44,18 +45,18 @@ public static class MakaWindowsPipeTrustProbe using (token) { - return WindowsIdentity.RunImpersonated(token, () => TryOpen(pipeName)); + return WindowsIdentity.RunImpersonated(token, () => TryOpen(pipeName, direction)); } } - private static string TryOpen(string pipeName) + private static string TryOpen(string pipeName, PipeDirection direction) { try { using (var client = new NamedPipeClientStream( ".", pipeName, - PipeDirection.InOut, + direction, PipeOptions.None, TokenImpersonationLevel.Identification)) { @@ -104,6 +105,7 @@ $userName = "MakaIpc$(([Guid]::NewGuid().ToString('N')).Substring(0, 8))" $password = "Maka-Ipc!$([Guid]::NewGuid().ToString('N'))aA1" $securePassword = ConvertTo-SecureString $password -AsPlainText -Force $fixture = $null +$currentUserClients = @() $createdUser = $false try { @@ -124,11 +126,17 @@ try { $startInfo.RedirectStandardError = $true $startInfo.CreateNoWindow = $true - $fixture = [System.Diagnostics.Process]::new() - $fixture.StartInfo = $startInfo - if (-not $fixture.Start()) { - throw 'Unable to start Runtime Host trust fixture' + $candidateFixture = [System.Diagnostics.Process]::new() + $candidateFixture.StartInfo = $startInfo + try { + if (-not $candidateFixture.Start()) { + throw 'Unable to start Runtime Host trust fixture' + } + } catch { + $candidateFixture.Dispose() + throw } + $fixture = $candidateFixture $ready = Read-ProcessLine -Process $fixture -TimeoutMilliseconds 10000 | ConvertFrom-Json if ($ready.type -ne 'ready' -or $ready.endpoint -notmatch '^\\\\\.\\pipe\\(.+)$') { @@ -136,53 +144,79 @@ try { } $pipeName = $Matches[1] - $currentUserClient = [System.IO.Pipes.NamedPipeClientStream]::new( - '.', - $pipeName, - [System.IO.Pipes.PipeDirection]::InOut, - [System.IO.Pipes.PipeOptions]::None, - [System.Security.Principal.TokenImpersonationLevel]::Identification - ) - try { - $currentUserClient.Connect(5000) - } finally { - $currentUserClient.Dispose() - } + # Keep enough owner connections open to consume libuv's initial pending pipe + # instances and force replacement instances before probing the foreign user. + for ($index = 0; $index -lt 8; $index += 1) { + $currentUserClient = [System.IO.Pipes.NamedPipeClientStream]::new( + '.', + $pipeName, + [System.IO.Pipes.PipeDirection]::InOut, + [System.IO.Pipes.PipeOptions]::None, + [System.Security.Principal.TokenImpersonationLevel]::Identification + ) + try { + $currentUserClient.Connect(5000) + $currentUserClients += $currentUserClient + } catch { + $currentUserClient.Dispose() + throw + } - $accepted = Read-ProcessLine -Process $fixture -TimeoutMilliseconds 5000 | ConvertFrom-Json - if ($accepted.type -ne 'accepted' -or $accepted.principalKind -ne 'local_owner') { - throw "Current-user connection did not receive Local Owner authority" + $accepted = Read-ProcessLine -Process $fixture -TimeoutMilliseconds 5000 | ConvertFrom-Json + if ($accepted.type -ne 'accepted' -or $accepted.principalKind -ne 'local_owner') { + throw "Current-user connection did not receive Local Owner authority" + } } - $foreignResult = [MakaWindowsPipeTrustProbe]::TryOpenAsUser( - $userName, - $env:COMPUTERNAME, - $password, - $pipeName - ) - if ($foreignResult -ne 'access_denied') { - throw "Foreign Windows user unexpectedly reached the Local IPC listener: $foreignResult" + foreach ($direction in @( + [System.IO.Pipes.PipeDirection]::In, + [System.IO.Pipes.PipeDirection]::Out, + [System.IO.Pipes.PipeDirection]::InOut + )) { + $foreignResult = [MakaWindowsPipeTrustProbe]::TryOpenAsUser( + $userName, + $env:COMPUTERNAME, + $password, + $pipeName, + $direction + ) + if ($foreignResult -ne 'access_denied') { + throw "Foreign Windows user unexpectedly opened the Local IPC listener ($direction): $foreignResult" + } } Write-Output 'Runtime Host Windows Local IPC admitted the current user and denied a foreign user.' } finally { - if ($null -ne $fixture) { - if (-not $fixture.HasExited) { - $fixture.StandardInput.WriteLine('close') - $fixture.StandardInput.Flush() - if (-not $fixture.WaitForExit(5000)) { - $fixture.Kill($true) - $fixture.WaitForExit() + try { + try { + foreach ($client in $currentUserClients) { + $client.Dispose() + } + } finally { + if ($null -ne $fixture) { + if (-not $fixture.HasExited) { + $fixture.StandardInput.WriteLine('close') + $fixture.StandardInput.Flush() + if (-not $fixture.WaitForExit(5000)) { + $fixture.Kill($true) + $fixture.WaitForExit() + } + } + if ($fixture.ExitCode -ne 0) { + $stderr = $fixture.StandardError.ReadToEnd() + throw "Runtime Host trust fixture exited with $($fixture.ExitCode): $stderr" + } } } - if ($fixture.ExitCode -ne 0) { - $stderr = $fixture.StandardError.ReadToEnd() - Write-Warning "Runtime Host trust fixture exited with $($fixture.ExitCode): $stderr" + } finally { + try { + if ($null -ne $fixture) { + $fixture.Dispose() + } + } finally { + if ($createdUser) { + Remove-LocalUser -Name $userName -ErrorAction Stop + } } - $fixture.Dispose() - } - if ($createdUser) { - Remove-LocalUser -Name $userName -ErrorAction SilentlyContinue } } -