Harden named pipe security (#22)

Only grant access to the current user and the sandbox

windows/Sources/FabricSandbox/FabricSandbox.swift

@@ -146,7 +146,9 @@ class FabricSandbox {
 
     // Create a named pipe server for IPC with the sandboxed process
     let namedPipeServer = try SandboxNamedPipeServer(
-      pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10))
+      pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10),
+      allowedTrustees: [container]
+    )
 
     logger.debug("Named pipe: \(namedPipeServer.path)")
 

windows/Sources/Sandbox/AppContainer.swift

@@ -24,7 +24,7 @@ public class AppContainer: Trustee {
       MultipleTrusteeOperation: NO_MULTIPLE_TRUSTEE,
       TrusteeForm: TRUSTEE_IS_SID,
       TrusteeType: TRUSTEE_IS_WELL_KNOWN_GROUP,
-      ptstrName: _CASTSID(sid.value)
+      ptstrName: sid.ptstrName
     )
   }
 

windows/Sources/Sandbox/SandboxNamedPipeServer.swift

@@ -10,8 +10,8 @@ import CxxStdlib
 public class SandboxNamedPipeServer: NamedPipeServer {
   private var speech: Speech? = nil
 
-  public override init(pipeName: String) throws {
-    try super.init(pipeName: pipeName)
+  public override init(pipeName: String, allowedTrustees: [Trustee]) throws {
+    try super.init(pipeName: pipeName, allowedTrustees: allowedTrustees + [TokenUserTrustee()])
   }
 
   public override func onMessage(_ data: [UInt16]) -> Bool {

windows/Sources/WinSDKExtras/WinSDKExtras.cpp

@@ -64,10 +64,6 @@ DWORD _SECURITY_MAX_SID_SIZE() {
     return SECURITY_MAX_SID_SIZE;
 }
 
-LPWCH _CASTSID(PSID pSid) {
-    return static_cast<LPWCH>(pSid);
-}
-
 LPPROC_THREAD_ATTRIBUTE_LIST allocateAttributeList(size_t size) {
     return (LPPROC_THREAD_ATTRIBUTE_LIST)HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size);
 }

windows/Sources/WinSDKExtras/include/WinSDKExtras.h

@@ -35,9 +35,6 @@ DWORD _MAKELANGID(WORD p, WORD s);
 
 DWORD _SECURITY_MAX_SID_SIZE();
 
-// TODO - This is a hack, need to find a better way to cast PSID to LPWCH in swift
-LPWCH _CASTSID(PSID pSid);
-
 // TODO - again how to do this nicely in swift
 LPPROC_THREAD_ATTRIBUTE_LIST allocateAttributeList(size_t dwAttributeCount);
 

windows/Sources/WindowsUtils/Acl.swift

@@ -121,7 +121,7 @@ public func hasAceEntry(_ object: SecurityObject, trustee: Trustee) throws -> Bo
 public func getStringSecurityDescriptor(_ object: SecurityObject) throws -> String {
   let acl = try object.getACL()
 
-  var securityDescriptor: SECURITY_DESCRIPTOR? = nil
+  var securityDescriptor = SECURITY_DESCRIPTOR()
   guard InitializeSecurityDescriptor(&securityDescriptor, DWORD(SECURITY_DESCRIPTOR_REVISION)) else {
     throw Win32Error("InitializeSecurityDescriptor")
   }
@@ -130,6 +130,10 @@ public func getStringSecurityDescriptor(_ object: SecurityObject) throws -> Stri
     throw Win32Error("SetSecurityDescriptorDacl")
   }
 
+  return try getStringSecurityDescriptor(&securityDescriptor)
+}
+
+public func getStringSecurityDescriptor(_ securityDescriptor: inout SECURITY_DESCRIPTOR) throws -> String {
   var stringSecurityDescriptor: LPWSTR? = nil
   let result = ConvertSecurityDescriptorToStringSecurityDescriptorW(
     &securityDescriptor, DWORD(SDDL_REVISION_1), SECURITY_INFORMATION(DACL_SECURITY_INFORMATION), &stringSecurityDescriptor, nil)
@@ -140,6 +144,45 @@ public func getStringSecurityDescriptor(_ object: SecurityObject) throws -> Stri
   return String(decodingCString: stringSecurityDescriptor, as: UTF16.self)
 }
 
+public func createACLWithTrustees(_ trustees: [Trustee], accessMode: AccessMode = .grant, accessPermissions: [AccessPermissions] = [.genericAll]) throws -> PACL {
+  var explicitAccess = trustees.map { trustee in
+    return EXPLICIT_ACCESS_W(
+      grfAccessPermissions: accessPermissions.reduce(0) { $0 | $1.rawValue },
+      grfAccessMode: accessMode.accessMode,
+      grfInheritance: accessMode.inheritanceFlags,
+      Trustee: trustee.trustee
+    )
+  }
+
+  var acl: PACL? = nil
+  let result = SetEntriesInAclW(ULONG(explicitAccess.count), &explicitAccess, nil, &acl)
+  guard result == ERROR_SUCCESS, let acl = acl else {
+    throw Win32Error("SetEntriesInAclW")
+  }
+
+  return acl
+}
+
+public func createSelfRelativeSecurityDescriptor(_ securityDescriptor: inout SECURITY_DESCRIPTOR) throws -> UnsafeMutablePointer<SECURITY_DESCRIPTOR> {
+  var relativeSize = DWORD(0)
+    guard !MakeSelfRelativeSD(&securityDescriptor, nil, &relativeSize)
+              && GetLastError() == ERROR_INSUFFICIENT_BUFFER else {
+      throw Win32Error("MakeSelfRelativeSD")
+    }
+
+    let relativeDescriptor = UnsafeMutableRawPointer.allocate(
+      byteCount: Int(relativeSize),
+      alignment: MemoryLayout<SECURITY_DESCRIPTOR>.alignment
+    ).assumingMemoryBound(to: SECURITY_DESCRIPTOR.self)
+
+    guard MakeSelfRelativeSD(&securityDescriptor, relativeDescriptor, &relativeSize) else {
+      relativeDescriptor.deallocate()
+      throw Win32Error("MakeSelfRelativeSD")
+    }
+
+    return relativeDescriptor
+}
+
 // Remove the first ACE that matches the predicate, returning whether an ACE was removed
 private func removeFirstAceIf(
   _ acl: inout PACL, predicate: (Ace) -> Bool
@@ -194,6 +237,39 @@ private func removeFirstAceIf(
   return false
 }
 
+fileprivate func getTokenUserSid() throws -> Sid {
+  var processHandle: HANDLE? = nil
+  let result = OpenProcessToken(GetCurrentProcess(), DWORD(TOKEN_QUERY), &processHandle)
+  guard result, processHandle != INVALID_HANDLE_VALUE, let processHandle = processHandle else {
+    throw Win32Error("OpenProcessToken")
+  }
+
+  let tokenUser = try getTokenInformation(of: TOKEN_USER.self, token: processHandle, tokenClass: TokenUser)
+  defer {
+    tokenUser.deallocate()
+  }
+
+  return try Sid(copy: tokenUser.pointee.User.Sid)
+}
+
+// Based on: https://github.com/apple/swift-system/blob/61a2af5e8b55535be68c35efebb3d398de512352/Sources/System/Internals/WindowsSyscallAdapters.swift#L372
+fileprivate func getTokenInformation<T>(of: T.Type, token: HANDLE, tokenClass: TOKEN_INFORMATION_CLASS) throws -> UnsafePointer<T> {
+  var size = DWORD(1024)
+  for _ in 0..<2 {
+    let buffer = UnsafeMutableRawPointer.allocate(
+      byteCount: Int(size),
+      alignment: MemoryLayout<T>.alignment
+    )
+
+    if GetTokenInformation(token, tokenClass, buffer, size, &size) {
+      return UnsafePointer(buffer.assumingMemoryBound(to: T.self))
+    }
+
+    buffer.deallocate()
+  }
+  throw Win32Error("GetTokenInformation")
+}
+
 public enum AccessPermissions: DWORD {
   // https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights
   case genericAll = 0x1000_0000
@@ -249,11 +325,29 @@ public class WellKnownTrustee: Trustee {
             MultipleTrusteeOperation: NO_MULTIPLE_TRUSTEE,
             TrusteeForm: TRUSTEE_IS_SID,
             TrusteeType: TRUSTEE_IS_WELL_KNOWN_GROUP,
-            ptstrName: _CASTSID(self.sid.value)
+            ptstrName: self.sid.ptstrName
         )
     }
 }
 
+// A trustee that represents the current process/user
+public class TokenUserTrustee: Trustee {
+  public let sid: Sid
+  public let trustee: TRUSTEE_W
+
+  public init() throws {
+    let sid = try getTokenUserSid()
+    self.sid = sid
+    self.trustee = TRUSTEE_W(
+      pMultipleTrustee: nil,
+      MultipleTrusteeOperation: NO_MULTIPLE_TRUSTEE,
+      TrusteeForm: TRUSTEE_IS_SID,
+      TrusteeType: TRUSTEE_IS_USER,
+      ptstrName: sid.ptstrName
+    )
+  }
+}
+
 public protocol SecurityObject {
   func getACL() throws -> PACL
   func setACL(acl: PACL, accessMode: AccessMode) throws

windows/Sources/WindowsUtils/NamedPipe.swift

@@ -1,15 +1,6 @@
 import WinSDK
 
-// https://stackoverflow.com/questions/39138674/accessing-named-pipe-servers-from-within-ie-epm-bho
-// Local System - full access
-// Everyone - full access
-// App packages - full access
-
-/// A named pipe server can receive messages from a single client
-
-/// A write only named pipe client
 let bufferSize = DWORD(4096)
-let securityDescriptor = "S:(ML;;NW;;;LW)D:(A;;FA;;;SY)(A;;FA;;;WD)(A;;FA;;;AC)"
 
 protocol NamedPipe {
   var pipe: HANDLE { get }
@@ -20,22 +11,28 @@ open class NamedPipeServer: Thread, NamedPipe {
   public let pipe: HANDLE
   public let path: String
 
-  public init(pipeName: String) throws {
-    // Create a security descriptor
-    var security: PSECURITY_DESCRIPTOR?
-    let result = ConvertStringSecurityDescriptorToSecurityDescriptorW(
-      securityDescriptor.wide,
-      DWORD(SDDL_REVISION_1),
-      &security,
-      nil
-    )
+  public init(pipeName: String, allowedTrustees: [Trustee]) throws {
+    let acl = try createACLWithTrustees(allowedTrustees)
+    defer { LocalFree(acl) }
+
+    var securityDescriptor = SECURITY_DESCRIPTOR()
+
+    var result = InitializeSecurityDescriptor(&securityDescriptor, DWORD(SECURITY_DESCRIPTOR_REVISION))
     guard result else {
-      throw Win32Error("ConvertStringSecurityDescriptorToSecurityDescriptorW")
+      throw Win32Error("InitializeSecurityDescriptor")
     }
 
+    result = SetSecurityDescriptorDacl(&securityDescriptor, true, acl, false)
+    guard result else {
+      throw Win32Error("SetSecurityDescriptorDacl")
+    }
+
+    let relativeDescriptor = try createSelfRelativeSecurityDescriptor(&securityDescriptor)
+    defer { relativeDescriptor.deallocate() }
+
     var securityAttributesValue = SECURITY_ATTRIBUTES(
       nLength: DWORD(MemoryLayout<SECURITY_ATTRIBUTES>.size),
-      lpSecurityDescriptor: security,
+      lpSecurityDescriptor: relativeDescriptor,
       bInheritHandle: false)
 
     let pipe = CreateNamedPipeW(

windows/Sources/WindowsUtils/Sid.swift

@@ -3,11 +3,25 @@ import WinSDKExtras
 
 public class Sid: CustomStringConvertible {
   public var value: PSID
+  public var ptstrName: UnsafeMutablePointer<UInt16> {
+    return value.assumingMemoryBound(to: UInt16.self)
+  }
 
   public init(_ sid: PSID) {
     self.value = sid
   }
 
+  public init (copy: PSID) throws {
+    let size = GetLengthSid(copy)
+    let sid: PSID = HeapAlloc(GetProcessHeap(), DWORD(HEAP_ZERO_MEMORY), SIZE_T(size))!
+    let result = CopySid(size, sid, copy)
+    guard result else {
+      throw Win32Error("CopySid")
+    }
+
+    self.value = sid
+  }
+
   public init(_ str: String) throws {
     var sid: PSID? = nil
     guard ConvertStringSidToSidW(str.wide, &sid), let sid = sid else {

windows/Tests/IntergrationTests.swift

@@ -95,8 +95,17 @@ import WindowsUtils
   }
 
   @Test func namedPipe() throws {
-    let server = try TestNamedPipeServer()
-    let (exitCode, output) = try runIntergration(["namedPipe", server.pipeName], namedPipe: server)
+    let pipeName = "\\\\.\\pipe\\FabricSandboxTest" + randomString(length: 10)
+    var server: TestNamedPipeServer? = nil
+    let (exitCode, output) = try runIntergration(["namedPipe", pipeName]) { container in
+      server = try TestNamedPipeServer(allowedTrustees: [container], pipeName: pipeName)
+      return server!
+    }
+    #expect(server != nil)
+    guard let server = server else {
+      return
+    }
+
     #expect(exitCode == 0)
     #expect(output.contains("Sent message to named pipe"))
 
@@ -139,24 +148,29 @@ import WindowsUtils
   }
 
   @Test func mouseMovements() throws {
-    let server = try SandboxNamedPipeServer(
-      pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10))
-    let (exitCode, _) = try runIntergration(
-      ["mouseMovements", "-Dsandbox.namedPipe=\(server.path)"], namedPipe: server)
+    let (exitCode, _) = try runIntergration(["mouseMovements"]) { container in
+        try SandboxNamedPipeServer(
+          pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10),
+          allowedTrustees: [container]
+        )
+      }
     #expect(exitCode == 0)
   }
 
   @Test func testSpeech() throws {
-    let server = try SandboxNamedPipeServer(
-      pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10))
-    let (exitCode, output) = try runIntergration(["speech"], namedPipe: server)
+    let (exitCode, output) = try runIntergration(["speech"]) { container in
+      try SandboxNamedPipeServer(
+        pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10),
+        allowedTrustees: [container]
+      )
+    }
     #expect(exitCode == 0)
     #expect(output == "Spoke")
   }
 }
 func runIntergration(
   _ args: [String], capabilities: [SidCapability] = [], filePermissions: [FilePermission] = [],
-  lpac: Bool = false, namedPipe: NamedPipeServer? = nil
+  lpac: Bool = false, namedPipeFactory: ((AppContainer) throws -> NamedPipeServer)? = nil
 ) throws -> (Int, String) {
   let workingDirectory = try getWorkingDirectory()
   let moduleDir = try getModuleFileName().parent()!
@@ -200,6 +214,8 @@ func runIntergration(
 
   var commandLine = [testExecutable.path()] + args
 
+  let namedPipe = try namedPipeFactory?(container)
+
   if let namedPipe = namedPipe {
     if namedPipe is SandboxNamedPipeServer {
       commandLine.append("-Dsandbox.namedPipe=\(namedPipe.path)")

windows/Tests/NamedPipeTests.swift

@@ -24,10 +24,10 @@ class TestNamedPipeServer: NamedPipeServer {
   var onMessageEvent: HANDLE
   let pipeName: String
 
-  init() throws {
+  init(allowedTrustees: [Trustee] = [], pipeName: String? = nil) throws {
     onMessageEvent = CreateEventW(nil, false, false, nil)
-    self.pipeName = "\\\\.\\pipe\\FabricSandboxTest" + randomString(length: 10)
-    try super.init(pipeName: self.pipeName)
+    self.pipeName = pipeName ?? "\\\\.\\pipe\\FabricSandboxTest" + randomString(length: 10)
+    try super.init(pipeName: self.pipeName, allowedTrustees: allowedTrustees + [TokenUserTrustee()])
   }
 
   override func onMessage(_ data: [UInt16]) -> Bool {