From 2cbe2190fb20b6448d642781071f22995cf8e4af Mon Sep 17 00:00:00 2001
From: modmuss <modmuss50@gmail.com>
Date: Tue, 4 Jun 2024 20:03:54 +0100
Subject: [PATCH] Add support for denying access to certain files, without
 inheritance. (#14)

---
 windows/Sources/Sandbox/Acl.swift             | 127 ++++++++++++++++--
 windows/Sources/Sandbox/Sid.swift             |   4 +-
 windows/Sources/WinSDKExtras/WinSDKExtras.cpp |   8 ++
 .../WinSDKExtras/include/WinSDKExtras.h       |   6 +-
 windows/Tests/IntergrationTests.swift         |  29 +++-
 5 files changed, 162 insertions(+), 12 deletions(-)

diff --git a/windows/Sources/Sandbox/Acl.swift b/windows/Sources/Sandbox/Acl.swift
index 8784274..2d1fa4b 100644
--- a/windows/Sources/Sandbox/Acl.swift
+++ b/windows/Sources/Sandbox/Acl.swift
@@ -7,6 +7,22 @@ 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()
 
@@ -23,12 +39,11 @@ public func grantAccess(
   guard result == ERROR_SUCCESS, let acl = acl else {
     throw Win32Error("GetNamedSecurityInfoW")
   }
-  //defer { LocalFree(acl) } // TODO is this needed? seems to crash a lot
-
+  
   var explicitAccess: EXPLICIT_ACCESS_W = EXPLICIT_ACCESS_W(
     grfAccessPermissions: accessPermissions.reduce(0) { $0 | $1.rawValue },
-    grfAccessMode: GRANT_ACCESS,
-    grfInheritance: DWORD(OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE),
+    grfAccessMode: accessMode.accessMode,
+    grfInheritance: accessMode.inheritanceFlags,
     Trustee: TRUSTEE_W(
       pMultipleTrustee: nil,
       MultipleTrusteeOperation: NO_MULTIPLE_TRUSTEE,
@@ -41,12 +56,23 @@ public func grantAccess(
   // 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, let newAcl = newAcl else {
+  guard result == ERROR_SUCCESS, var newAcl = newAcl else {
     throw Win32Error("SetEntriesInAclW")
   }
   defer { LocalFree(newAcl) }
 
-  //print("Granting access to '\(path)' for '\(try appContainer.sidString())'")
+  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
@@ -54,7 +80,7 @@ public func grantAccess(
       // I dont think this actually mutates the string, at least I hope not
       UnsafeMutablePointer(mutating: path),
       SE_FILE_OBJECT,
-      SECURITY_INFORMATION(DACL_SECURITY_INFORMATION),
+      accessMode.securityInformation,
       nil,
       nil,
       newAcl,
@@ -65,6 +91,60 @@ public func grantAccess(
     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<ACL_SIZE_INFORMATION>.size), AclSizeInformation)
+  guard success else {
+    throw Win32Error("GetAclInformation")
+  }
+
+  var toRemove: DWORD? = nil
+
+  outer: for i: DWORD in 0..<aclSize.AceCount {
+    var ace: LPVOID? = nil
+    let success = GetAce(acl, DWORD(i), &ace)
+    guard success, let ace = ace else {
+      throw Win32Error("GetAce")
+    }
+
+    let aceHeader = ace.assumingMemoryBound(to: ACE_HEADER.self).pointee
+
+    switch Int32(aceHeader.AceType) {
+    case ACCESS_ALLOWED_ACE_TYPE:
+      let accessAllowedAce = ace.assumingMemoryBound(to: ACCESS_ALLOWED_ACE.self).pointee
+      let sid = SidFromAccessAllowedAce(ace, accessAllowedAce.SidStart)
+
+      if predicate(.AccessAllowed(sid!)) {
+        toRemove = i
+        break outer
+      }
+    case ACCESS_DENIED_ACE_TYPE:
+      let accessDeniedAce = ace.assumingMemoryBound(to: ACCESS_DENIED_ACE.self).pointee
+      let sid = SidFromAccessDeniedAce(ace, accessDeniedAce.SidStart)
+
+      if predicate(.AccessDenied(sid!)) {
+        toRemove = i
+        break outer
+      }
+    default:
+      break
+    }
+  }
+
+  if let toRemove = toRemove {
+    let success = DeleteAce(acl, toRemove)
+    guard success else {
+      throw Win32Error("DeleteAce")
+    }
+    return true
+  }
+
+  return false
+}
+
 public func grantNamedPipeAccess(
   pipe: NamedPipeServer, appContainer: AppContainer, accessPermissions: [AccessPermissions]
 )
@@ -82,7 +162,6 @@ public func grantNamedPipeAccess(
   guard result == ERROR_SUCCESS, let acl = acl else {
     throw Win32Error("GetNamedSecurityInfoW")
   }
-  //defer { LocalFree(acl) } // TODO is this needed? seems to crash a lot
 
   var explicitAccess: EXPLICIT_ACCESS_W = EXPLICIT_ACCESS_W(
     grfAccessPermissions: accessPermissions.reduce(0) { $0 | $1.rawValue },
@@ -119,6 +198,7 @@ public func grantNamedPipeAccess(
     throw Win32Error("SetSecurityInfo", errorCode: result)
   }
 }
+
 public enum AccessPermissions: DWORD {
   // https://learn.microsoft.com/en-us/windows/win32/secauthz/generic-access-rights
   case genericAll = 0x1000_0000
@@ -126,3 +206,34 @@ public enum AccessPermissions: DWORD {
   case genericWrite = 0x4000_0000
   case genericRead = 0x8000_0000
 }
+
+public enum AccessMode {
+  case grant
+  case deny
+
+  var accessMode: _ACCESS_MODE {
+    switch self {
+    case .grant: return GRANT_ACCESS
+    case .deny: return DENY_ACCESS
+    }
+  }
+
+  var inheritanceFlags: DWORD {
+    switch self {
+    case .grant: return DWORD(OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE)
+    case .deny: return DWORD(NO_INHERITANCE)
+    }
+  }
+
+  var securityInformation: SECURITY_INFORMATION {
+    switch self {
+    case .grant: return SECURITY_INFORMATION(DACL_SECURITY_INFORMATION)
+    case .deny: return SECURITY_INFORMATION(UInt32(DACL_SECURITY_INFORMATION) | PROTECTED_DACL_SECURITY_INFORMATION)
+    }
+  }
+}
+
+public enum Ace {
+  case AccessAllowed(PSID)
+  case AccessDenied(PSID)
+}
\ No newline at end of file
diff --git a/windows/Sources/Sandbox/Sid.swift b/windows/Sources/Sandbox/Sid.swift
index 404500f..9d970c4 100644
--- a/windows/Sources/Sandbox/Sid.swift
+++ b/windows/Sources/Sandbox/Sid.swift
@@ -57,10 +57,10 @@ public class Sid: CustomStringConvertible {
   }
 
   public var description: String {
-    return (try? getSidString(value)) ?? "Invalid SID"
+    return (try? Sid.getSidString(value)) ?? "Invalid SID"
   }
 
-  func getSidString(_ sid: PSID) throws -> String {
+  static func getSidString(_ sid: PSID) throws -> String {
     var sidString: LPWSTR? = nil
     let result = ConvertSidToStringSidW(sid, &sidString)
 
diff --git a/windows/Sources/WinSDKExtras/WinSDKExtras.cpp b/windows/Sources/WinSDKExtras/WinSDKExtras.cpp
index dc9c683..d5a76b8 100644
--- a/windows/Sources/WinSDKExtras/WinSDKExtras.cpp
+++ b/windows/Sources/WinSDKExtras/WinSDKExtras.cpp
@@ -86,4 +86,12 @@ DWORD Win32FromHResult(HRESULT hr) {
     }
     // Not a Win32 HRESULT so return a generic error code.
     return ERROR_CAN_NOT_COMPLETE;
+}
+
+PSID SidFromAccessAllowedAce(LPVOID ace, DWORD sidStart) {
+    return &((ACCESS_ALLOWED_ACE*)ace)->SidStart;
+}
+
+PSID SidFromAccessDeniedAce(LPVOID ace, DWORD sidStart) {
+    return &((ACCESS_DENIED_ACE*)ace)->SidStart;
 }
\ No newline at end of file
diff --git a/windows/Sources/WinSDKExtras/include/WinSDKExtras.h b/windows/Sources/WinSDKExtras/include/WinSDKExtras.h
index 5b34cb1..d7f7267 100644
--- a/windows/Sources/WinSDKExtras/include/WinSDKExtras.h
+++ b/windows/Sources/WinSDKExtras/include/WinSDKExtras.h
@@ -43,4 +43,8 @@ LPPROC_THREAD_ATTRIBUTE_LIST allocateAttributeList(size_t dwAttributeCount);
 
 BOOL _IsWindows10OrGreater();
 
-DWORD Win32FromHResult(HRESULT hr);
\ No newline at end of file
+DWORD Win32FromHResult(HRESULT hr);
+
+PSID SidFromAccessAllowedAce(LPVOID ace, DWORD sidStart);
+
+PSID SidFromAccessDeniedAce(LPVOID ace, DWORD sidStart);
\ No newline at end of file
diff --git a/windows/Tests/IntergrationTests.swift b/windows/Tests/IntergrationTests.swift
index bf8316f..350519c 100644
--- a/windows/Tests/IntergrationTests.swift
+++ b/windows/Tests/IntergrationTests.swift
@@ -53,6 +53,31 @@ import WindowsUtils
     #expect(output.contains("Hello, World!"))
   }
 
+  @Test func testReadFileWithDenyAccess() throws {
+    let tempDir = try createTempDir()
+    defer {
+      try! tempDir.delete()
+    }
+
+    let allowedFile = tempDir.child("allow.txt")
+    try allowedFile.writeString("Hello, World!")
+    let deniedFile = tempDir.child("deny.txt")
+    try deniedFile.writeString("Top secret!")
+
+    print(tempDir.path())
+
+    let (exitCode, output) = try runIntergration(
+      ["readFile", deniedFile.path()],
+      filePermissions: [
+        // Grant read access to the directory
+        FilePermission(path: tempDir, accessPermissions: [.genericRead]),
+        // Deny read access to the file
+        FilePermission(path: deniedFile, accessPermissions: [.genericAll], accessMode: .deny)
+      ])
+    #expect(exitCode == 0)
+    #expect(output.contains("Access is denied"))
+  }
+
   @Test func testRegistryCreate() throws {
     let (exitCode, output) = try runIntergration(["registry", "create"])
     #expect(exitCode == 0)
@@ -142,8 +167,9 @@ func runIntergration(
     capabilities: capabilities, lpac: lpac)
 
   for filePermission in filePermissions {
-    try grantAccess(
+    try setAccess(
       filePermission.path, appContainer: container,
+      accessMode: filePermission.accessMode,
       accessPermissions: filePermission.accessPermissions)
   }
 
@@ -192,6 +218,7 @@ func runIntergration(
 struct FilePermission {
   var path: File
   var accessPermissions: [AccessPermissions]
+  var accessMode: AccessMode = .grant
 }
 class TestOutputConsumer: OutputConsumer {
   var output = ""