diff --git a/windows/Sources/FabricSandbox/FabricSandbox.swift b/windows/Sources/FabricSandbox/FabricSandbox.swift index c2757d7..cf6cdfc 100644 --- a/windows/Sources/FabricSandbox/FabricSandbox.swift +++ b/windows/Sources/FabricSandbox/FabricSandbox.swift @@ -106,42 +106,42 @@ class FabricSandbox { // Grant full access to the mounted disk try grantAccess( - sandboxRoot, appContainer: container, + sandboxRoot, trustee: container, accessPermissions: [.genericAll]) if let assetsDir = commandLine.getAssetsDir(), isDevEnv { // Grant read access to the assets dir in dev try grantAccess( - assetsDir, appContainer: container, + assetsDir, trustee: container, accessPermissions: [.genericRead]) } if let log4jConfig = commandLine.getJvmProp("log4j.configurationFile") { // Grant read access to the log4j configuration file try grantAccess( - File(log4jConfig), appContainer: container, + File(log4jConfig), trustee: container, accessPermissions: [.genericRead]) } } else { logger.debug("Working directory is not root, granting access") // Grant read and execute to .minecraft try grantAccess( - sandboxRoot, appContainer: container, + sandboxRoot, trustee: container, accessPermissions: [.genericRead, .genericExecute]) // Grant full access to the working directory try grantAccess( - sandboxWorkingDirectory, appContainer: container, + sandboxWorkingDirectory, trustee: container, accessPermissions: [.genericAll]) try grantAccess( - tempDir, appContainer: container, + tempDir, trustee: container, accessPermissions: [.genericAll]) } // Grant read and execute to Java home try grantAccess( - javaDirectory, appContainer: container, + javaDirectory, trustee: container, accessPermissions: [.genericRead, .genericExecute]) // Create a named pipe server for IPC with the sandboxed process @@ -149,8 +149,8 @@ class FabricSandbox { pipeName: "\\\\.\\pipe\\FabricSandbox" + randomString(length: 10)) // Grant access to the named pipe - try grantNamedPipeAccess( - pipe: namedPipeServer, appContainer: container, + try grantAccess( + namedPipeServer, trustee: container, accessPermissions: [.genericRead, .genericWrite]) let args = try commandLine.getSandboxArgs( diff --git a/windows/Sources/Sandbox/Acl.swift b/windows/Sources/Sandbox/Acl.swift deleted file mode 100644 index 2d1fa4b..0000000 --- a/windows/Sources/Sandbox/Acl.swift +++ /dev/null @@ -1,239 +0,0 @@ -import WinSDK -import WinSDKExtras -// Reads the existing ACL for the given path and adds an entry to it that grants the app container the specified access permissions. -import WindowsUtils - -public func grantAccess( - _ file: File, appContainer: AppContainer, accessPermissions: [AccessPermissions] -) - throws -{ - return try setAccess(file, appContainer: appContainer, accessMode: .grant, accessPermissions: accessPermissions) -} - -public func denyAccess( - _ file: File, appContainer: AppContainer, accessPermissions: [AccessPermissions] -) - throws -{ - return try setAccess(file, appContainer: appContainer, accessMode: .deny, accessPermissions: accessPermissions) -} - -public func setAccess( - _ file: File, appContainer: AppContainer, accessMode: AccessMode, accessPermissions: [AccessPermissions] -) - throws -{ - let path = file.path() - - // Check that the path exists - guard GetFileAttributesW(path.wide) != INVALID_FILE_ATTRIBUTES else { - throw SandboxError("Path does not exist: '\(path)'") - } - - // Read the existing ACL - var acl: PACL? = nil - var result = GetNamedSecurityInfoW( - path.wide, SE_FILE_OBJECT, SECURITY_INFORMATION(DACL_SECURITY_INFORMATION), nil, nil, &acl, nil, - nil) - guard result == ERROR_SUCCESS, let acl = acl else { - throw Win32Error("GetNamedSecurityInfoW") - } - - var explicitAccess: EXPLICIT_ACCESS_W = EXPLICIT_ACCESS_W( - grfAccessPermissions: accessPermissions.reduce(0) { $0 | $1.rawValue }, - grfAccessMode: accessMode.accessMode, - grfInheritance: accessMode.inheritanceFlags, - Trustee: TRUSTEE_W( - pMultipleTrustee: nil, - MultipleTrusteeOperation: NO_MULTIPLE_TRUSTEE, - TrusteeForm: TRUSTEE_IS_SID, - TrusteeType: TRUSTEE_IS_WELL_KNOWN_GROUP, - ptstrName: _CASTSID(appContainer.sid.value) - ) - ) - - // Add an entry to the ACL that grants the app container the specified access permissions - var newAcl: PACL? = nil - result = SetEntriesInAclW(1, &explicitAccess, acl, &newAcl) - guard result == ERROR_SUCCESS, var newAcl = newAcl else { - throw Win32Error("SetEntriesInAclW") - } - defer { LocalFree(newAcl) } - - if accessMode == .deny { - let _ = try removeFirstAceIf(&newAcl) { - switch $0 { - case .AccessAllowed(let sid): - // Remove any existing access allowed ACEs for the app container - // This likely comes from the parent directory, but we can remove it since inheritance is disabled - return EqualSid(sid, appContainer.sid.value) - default: - return false - } - } - } - - // Set the new ACL on the file - result = path.withCString(encodedAs: UTF16.self) { path in - SetNamedSecurityInfoW( - // I dont think this actually mutates the string, at least I hope not - UnsafeMutablePointer(mutating: path), - SE_FILE_OBJECT, - accessMode.securityInformation, - nil, - nil, - newAcl, - nil - ) - } - guard result == ERROR_SUCCESS else { - throw Win32Error("SetNamedSecurityInfoW '\(path)'", errorCode: result) - } -} - -private func removeFirstAceIf( - _ acl: inout PACL, predicate: (Ace) -> Bool -) throws -> Bool { - var aclSize: ACL_SIZE_INFORMATION = ACL_SIZE_INFORMATION() - let success = GetAclInformation(acl, &aclSize, DWORD(MemoryLayout.size), AclSizeInformation) - guard success else { - throw Win32Error("GetAclInformation") - } - - var toRemove: DWORD? = nil - - outer: for i: DWORD in 0.. String { + let acl = try object.getACL() + + var securityDescriptor: SECURITY_DESCRIPTOR? = nil + guard InitializeSecurityDescriptor(&securityDescriptor, DWORD(SECURITY_DESCRIPTOR_REVISION)) else { + throw Win32Error("InitializeSecurityDescriptor") + } + + guard SetSecurityDescriptorDacl(&securityDescriptor, true, acl, false) else { + throw Win32Error("SetSecurityDescriptorDacl") + } + + var stringSecurityDescriptor: LPWSTR? = nil + let result = ConvertSecurityDescriptorToStringSecurityDescriptorW( + &securityDescriptor, DWORD(SDDL_REVISION_1), SECURITY_INFORMATION(DACL_SECURITY_INFORMATION), &stringSecurityDescriptor, nil) + guard result, let stringSecurityDescriptor = stringSecurityDescriptor else { + throw Win32Error("ConvertSecurityDescriptorToStringSecurityDescriptorW") + } + + return String(decodingCString: stringSecurityDescriptor, as: UTF16.self) +} + +// Remove the first ACE that matches the predicate, returning whether an ACE was removed +private func removeFirstAceIf( + _ acl: inout PACL, predicate: (Ace) -> Bool +) throws -> Bool { + var aclSize = ACL_SIZE_INFORMATION() + let success = GetAclInformation(acl, &aclSize, DWORD(MemoryLayout.size), AclSizeInformation) + guard success else { + throw Win32Error("GetAclInformation") + } + + var toRemove: DWORD? = nil + + outer: for i: DWORD in 0.. PACL + func setACL(acl: PACL, accessMode: AccessMode) throws +} + +extension File: SecurityObject { + public func getACL() throws -> PACL { + let path = self.path() + guard GetFileAttributesW(path.wide) != INVALID_FILE_ATTRIBUTES else { + throw Win32Error("Path does not exist: '\(path)'", errorCode: DWORD(ERROR_FILE_NOT_FOUND)) + } + + var acl: PACL? = nil + let result = GetNamedSecurityInfoW( + path.wide, SE_FILE_OBJECT, SECURITY_INFORMATION(DACL_SECURITY_INFORMATION), nil, nil, &acl, nil, nil) + + guard result == ERROR_SUCCESS, let acl = acl else { + throw Win32Error("GetNamedSecurityInfoW", errorCode: result) + } + return acl + } + + public func setACL(acl: PACL, accessMode: AccessMode) throws { + let result = self.path().withCString(encodedAs: UTF16.self) { + SetNamedSecurityInfoW( + UnsafeMutablePointer(mutating: $0), SE_FILE_OBJECT, accessMode.securityInformation, nil, nil, acl, nil) + } + + guard result == ERROR_SUCCESS else { + throw Win32Error("SetNamedSecurityInfoW", errorCode: result) + } + } +} + +extension NamedPipeServer: SecurityObject { + public func getACL() throws -> PACL { + var acl: PACL? = nil + let result = GetSecurityInfo( + self.handle, SE_KERNEL_OBJECT, SECURITY_INFORMATION(DACL_SECURITY_INFORMATION), nil, nil, &acl, nil, nil) + + guard result == ERROR_SUCCESS, let acl = acl else { + throw Win32Error("GetSecurityInfo", errorCode: result) + } + return acl + } + + public func setACL(acl: PACL, accessMode: AccessMode) throws { + let result = SetSecurityInfo( + self.handle, SE_KERNEL_OBJECT, SECURITY_INFORMATION(DACL_SECURITY_INFORMATION), nil, nil, acl, nil) + + guard result == ERROR_SUCCESS else { + throw Win32Error("SetSecurityInfo", errorCode: result) + } + } +} \ No newline at end of file diff --git a/windows/Sources/Sandbox/Sid.swift b/windows/Sources/WindowsUtils/Sid.swift similarity index 81% rename from windows/Sources/Sandbox/Sid.swift rename to windows/Sources/WindowsUtils/Sid.swift index 9d970c4..43a7dc9 100644 --- a/windows/Sources/Sandbox/Sid.swift +++ b/windows/Sources/WindowsUtils/Sid.swift @@ -1,15 +1,23 @@ import WinSDK import WinSDKExtras -import WindowsUtils public class Sid: CustomStringConvertible { - var value: PSID + public var value: PSID - init(_ sid: PSID) { + public init(_ sid: PSID) { self.value = sid } - static func createWellKnown(_ type: WELL_KNOWN_SID_TYPE) throws -> Sid { + public init(_ str: String) throws { + var sid: PSID? = nil + guard ConvertStringSidToSidW(str.wide, &sid), let sid = sid else { + throw Win32Error("ConvertStringSidToSidW") + } + + self.value = sid + } + + public static func createWellKnown(_ type: WELL_KNOWN_SID_TYPE) throws -> Sid { var size = DWORD(_SECURITY_MAX_SID_SIZE()) let sid: PSID = HeapAlloc(GetProcessHeap(), DWORD(HEAP_ZERO_MEMORY), SIZE_T(size))! var result = CreateWellKnownSid(type, nil, sid, &size) @@ -76,16 +84,16 @@ public class Sid: CustomStringConvertible { } } -class SidAndAttributes { - let sid: Sid - let sidAttributes: SID_AND_ATTRIBUTES +public class SidAndAttributes { + public let sid: Sid + public let sidAttributes: SID_AND_ATTRIBUTES - init(sid: Sid) { + public init(sid: Sid) { self.sid = sid self.sidAttributes = SID_AND_ATTRIBUTES(Sid: sid.value, Attributes: DWORD(SE_GROUP_ENABLED)) } - static func createWithCapability(type: SidCapability) throws -> SidAndAttributes { + public static func createWithCapability(type: SidCapability) throws -> SidAndAttributes { let sid = switch type { case .wellKnown(let wellKnownType): diff --git a/windows/Tests/AclTests.swift b/windows/Tests/AclTests.swift new file mode 100644 index 0000000..00a0ff6 --- /dev/null +++ b/windows/Tests/AclTests.swift @@ -0,0 +1,38 @@ +import Testing +import WinSDK +import WindowsUtils + +struct AclTests { + @Test func testAcl() throws { + let tempDir = try createTempDir() + defer { + try! tempDir.delete() + } + + let trustee = try WellKnownTrustee(sid: "S-1-5-21-3456789012-2345678901-1234567890-1234") + let sid = trustee.sid.description + + var sddl = try getStringSecurityDescriptor(tempDir) + #expect(!sddl.contains(";\(sid)")) + + try grantAccess(tempDir, trustee: trustee, accessPermissions: [.genericAll]) + sddl = try getStringSecurityDescriptor(tempDir) + // Allow full access + #expect(sddl.contains("A;;FA;;;\(sid)")) + + try clearAccess(tempDir, trustee: trustee) + sddl = try getStringSecurityDescriptor(tempDir) + #expect(!sddl.contains(";\(sid)")) + + try denyAccess(tempDir, trustee: trustee, accessPermissions: [.genericExecute]) + sddl = try getStringSecurityDescriptor(tempDir) + // Deny execute + #expect(sddl.contains("D;;FX;;;\(sid)")) + + // Test that we can remove mutlipe ACEs + try grantAccess(tempDir, trustee: trustee, accessPermissions: [.genericRead]) + try clearAccess(tempDir, trustee: trustee) + sddl = try getStringSecurityDescriptor(tempDir) + #expect(!sddl.contains(";\(sid)")) + } +} \ No newline at end of file diff --git a/windows/Tests/AppContainerTests.swift b/windows/Tests/AppContainerTests.swift index 8e3290a..594388a 100644 --- a/windows/Tests/AppContainerTests.swift +++ b/windows/Tests/AppContainerTests.swift @@ -1,6 +1,7 @@ import Sandbox import Testing import WinSDK +import WindowsUtils @testable import FabricSandbox @@ -43,6 +44,6 @@ import WinSDK name: "TestContainer", description: "Test Container", capabilities: []) print("SID: '\(container.sid)'") - try grantAccess(tempDir, appContainer: container, accessPermissions: [.genericAll]) + try grantAccess(tempDir, trustee: container, accessPermissions: [.genericAll]) } } diff --git a/windows/Tests/IntergrationTests.swift b/windows/Tests/IntergrationTests.swift index 0fb37d4..bfc568c 100644 --- a/windows/Tests/IntergrationTests.swift +++ b/windows/Tests/IntergrationTests.swift @@ -168,7 +168,7 @@ func runIntergration( for filePermission in filePermissions { try setAccess( - filePermission.path, appContainer: container, + filePermission.path, trustee: container, accessMode: filePermission.accessMode, accessPermissions: filePermission.accessPermissions) } @@ -186,23 +186,23 @@ func runIntergration( // Grant access to the test executable, hook dll and swift binaries try grantAccess( - testExecutable, appContainer: container, + testExecutable, trustee: container, accessPermissions: [.genericRead, .genericExecute]) try grantAccess( - hookDll, appContainer: container, + hookDll, trustee: container, accessPermissions: [.genericRead, .genericExecute]) for dll in swiftDllPaths { try grantAccess( - dll, appContainer: container, + dll, trustee: container, accessPermissions: [.genericRead, .genericExecute]) } var commandLine = [testExecutable.path()] + args if let namedPipe = namedPipe { - try grantNamedPipeAccess( - pipe: namedPipe, appContainer: container, accessPermissions: [.genericRead, .genericWrite]) + try grantAccess( + namedPipe, trustee: container, accessPermissions: [.genericRead, .genericWrite]) if namedPipe is SandboxNamedPipeServer { commandLine.append("-Dsandbox.namedPipe=\(namedPipe.path)") } diff --git a/windows/Tests/JavaTests.swift b/windows/Tests/JavaTests.swift index 6181c98..010c57e 100644 --- a/windows/Tests/JavaTests.swift +++ b/windows/Tests/JavaTests.swift @@ -96,14 +96,14 @@ private func runJava(_ source: String) throws -> String { } try grantAccess( - javaHome, appContainer: container, accessPermissions: [.genericRead, .genericExecute]) + javaHome, trustee: container, accessPermissions: [.genericRead, .genericExecute]) try grantAccess( - File(mountedDisk.drivePath), appContainer: container, accessPermissions: [.genericAll]) + File(mountedDisk.drivePath), trustee: container, accessPermissions: [.genericAll]) try grantAccess( - try getModuleFileName().parent()!, appContainer: container, + try getModuleFileName().parent()!, trustee: container, accessPermissions: [.genericRead, .genericExecute]) try grantAccess( - try findSwiftRuntimeDirectory(), appContainer: container, + try findSwiftRuntimeDirectory(), trustee: container, accessPermissions: [.genericRead, .genericExecute]) let outputConsumer = TestOutputConsumer()