fix: change the audit inheritance of items without audit entries

Enable-NTFSAuditInheritance, Disable-NTFSAuditInheritance, and
Set-NTFSInheritance -AuditInheritanceEnabled failed with "(5) Access is
denied" for a file or folder without a SACL, also elevated with the
Security privilege. The cmdlets read only the audit section and changed
the flag that disables or enables audit inheritance. AlphaFS writes that
flag only together with a SACL, so it wrote no section at all, which
Windows denies; the retry as owner repeated the same write. An empty SACL
is now added first, but only to a descriptor that was read with its SACL.

The elevated CI runs of #100 to #106 showed this through the test of an
omitted -AccessInheritanceEnabled. From #104 on, the test that keeps the
inherited entries of a security descriptor failed as well: elevated,
Get-NTFSSecurityDescriptor reads the SACL, and Windows then returns a DACL
that isn't in the auto-inherit format, such as that of a file in the temp
folder of the user, without its inherited flags. The test now reads the
access section only and checks that the descriptor has inherited entries.

Tests: five cases for items without audit entries, red with "Access is
denied" before the fix; the test that keeps the explicit audit entries now
checks the errors and the inheritance state of both calls; a test checks
that a descriptor read without its audit entries gets no SACL, which would
replace the audit entries of the item when it is written.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>

CHANGELOG.md

@@ -201,5 +201,9 @@ The format is based on
   `GenericAll`, which Windows keeps in the inherit-only entries of folders;
   it failed with "The value '269484032' is not valid"
   ([#17](https://github.com/raandree/NTFSSecurity/issues/17))
+- Fix `Enable-NTFSAuditInheritance`, `Disable-NTFSAuditInheritance`, and
+  `Set-NTFSInheritance -AuditInheritanceEnabled`, which failed with "Access
+  is denied" for a file or folder without audit entries, also in an elevated
+  session with the Security privilege
 
 [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD

Security2/FileSystem/FileSystemInheritanceInfo.cs

@@ -106,8 +106,37 @@ namespace Security2
         #endregion GetFileSystemInheritanceInfo
 
         #region Enable / DisableInheritance internal
+        // An item without audit entries can have no SACL at all. AlphaFS writes the flag that disables or enables
+        // audit inheritance only together with a SACL, and a write without any section is denied: (5) Access is
+        // denied. The empty SACL is added only to a descriptor that was read with its SACL, so that writing it can't
+        // remove audit entries.
+        private static void AddMissingSystemAcl(FileSystemSecurity2 sd)
+        {
+            if (!sd.HasAuditSection)
+            {
+                return;
+            }
+
+            var rawDescriptor = new RawSecurityDescriptor(sd.SecurityDescriptor.GetSecurityDescriptorBinaryForm(), 0);
+            if (rawDescriptor.SystemAcl != null)
+            {
+                return;
+            }
+
+            rawDescriptor.SystemAcl = new RawAcl(GenericAcl.AclRevision, 0);
+            rawDescriptor.SetFlags(rawDescriptor.ControlFlags | ControlFlags.SystemAclPresent);
+            var binaryForm = new byte[rawDescriptor.BinaryLength];
+            rawDescriptor.GetBinaryForm(binaryForm, 0);
+            sd.SecurityDescriptor.SetSecurityDescriptorBinaryForm(binaryForm, AccessControlSections.Audit);
+        }
+
         private static void EnableInheritance(FileSystemSecurity2 sd, bool removeExplicitAccessRules, InheritanceScope scope)
         {
+            if (scope == InheritanceScope.Audit)
+            {
+                AddMissingSystemAcl(sd);
+            }
+
             if (sd.IsFile)
             {
                 if (scope == InheritanceScope.Access)
@@ -174,6 +203,11 @@ namespace Security2
 
         private static void DisableInheritance(FileSystemSecurity2 sd, bool removeInheritedAccessRules, InheritanceScope scope)
         {
+            if (scope == InheritanceScope.Audit)
+            {
+                AddMissingSystemAcl(sd);
+            }
+
             if (sd.IsFile)
             {
                 if (scope == InheritanceScope.Access)

Tests/Inheritance.Tests.ps1

@@ -124,12 +124,18 @@ Describe 'Set-NTFSInheritance' {
         }
 
         # In memory, the kept entries stay marked as inherited; Windows stores them as explicit ones on write.
+        # The descriptor holds only the access entries. Windows marks the inherited entries of a DACL that isn't in
+        # the auto-inherit format, such as that of a file in the temp folder of the user, only when the SACL isn't
+        # read with it, and Get-NTFSSecurityDescriptor reads the SACL with the Security privilege.
         It 'Should keep the inherited access entries of a security descriptor' {
             $file = New-TestSandboxItem -Sandbox $sandbox -Name 'KeepDescriptor'
             Assert-TestSandboxPath -Sandbox $sandbox -Path $file
-            $sd = Get-NTFSSecurityDescriptor -Path $file
+            $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList (
+                (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access
+            )
             $sidType = [System.Security.Principal.SecurityIdentifier]
             $inheritedCount = @($sd.SecurityDescriptor.GetAccessRules($false, $true, $sidType)).Count
+            $inheritedCount | Should -BeGreaterThan 0
 
             Set-NTFSInheritance -SecurityDescriptor $sd -AccessInheritanceEnabled $false
 
@@ -141,10 +147,14 @@ Describe 'Set-NTFSInheritance' {
             $file = New-TestSandboxItem -Sandbox $sandbox -Name 'KeepAudit'
             Assert-TestSandboxPath -Sandbox $sandbox -Path $file
             Add-NTFSAudit -Path $file -Account 'Everyone' -AccessRights Delete -AuditFlags Failure
-            Disable-NTFSAuditInheritance -Path $file
+            Disable-NTFSAuditInheritance -Path $file -ErrorVariable disableErrors -ErrorAction SilentlyContinue
+            $disableErrors | Should -BeNullOrEmpty
+            (Get-NTFSInheritance -Path $file).AuditInheritanceEnabled | Should -BeFalse
 
-            Set-NTFSInheritance -Path $file -AuditInheritanceEnabled $true
+            Set-NTFSInheritance -Path $file -AuditInheritanceEnabled $true -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue
 
+            $inheritanceErrors | Should -BeNullOrEmpty
+            (Get-NTFSInheritance -Path $file).AuditInheritanceEnabled | Should -BeTrue
             @(Get-NTFSAudit -Path $file -ExcludeInherited) | Should -HaveCount 1
         }
     }
@@ -190,6 +200,43 @@ Describe 'Set-NTFSInheritance' {
     }
 }
 
+Describe 'Audit inheritance of an item without audit entries' {
+    # A new item has no SACL. Before 5.0.0, the cmdlets changed only the flag that disables or enables audit
+    # inheritance, which is written only together with a SACL, so they wrote no section at all: (5) Access is denied.
+    It '<Command> should set the audit inheritance of a <Type> and keep its access entries' -Skip:(-not $canChangeAudit) -ForEach @(
+        @{ Command = 'Disable-NTFSAuditInheritance'; Parameters = @{}; Type = 'file'; Expected = $false }
+        @{ Command = 'Disable-NTFSAuditInheritance'; Parameters = @{}; Type = 'folder'; Expected = $false }
+        @{ Command = 'Enable-NTFSAuditInheritance'; Parameters = @{}; Type = 'file'; Expected = $true }
+        @{ Command = 'Enable-NTFSAuditInheritance'; Parameters = @{}; Type = 'folder'; Expected = $true }
+        @{ Command = 'Set-NTFSInheritance'; Parameters = @{ AuditInheritanceEnabled = $false }; Type = 'folder'; Expected = $false }
+    ) {
+        $path = New-TestSandboxItem -Sandbox $sandbox -Name 'NoAudit' -Directory:($Type -eq 'folder')
+        Assert-TestSandboxPath -Sandbox $sandbox -Path $path
+        @((Get-Acl -LiteralPath $path -Audit).Audit) | Should -BeNullOrEmpty
+        $accessEntries = (Get-Acl -LiteralPath $path).GetSecurityDescriptorSddlForm('Access')
+
+        & $Command -Path $path @Parameters -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue
+
+        $inheritanceErrors | Should -BeNullOrEmpty
+        (Get-NTFSInheritance -Path $path).AuditInheritanceEnabled | Should -Be $Expected
+        (Get-Acl -LiteralPath $path).GetSecurityDescriptorSddlForm('Access') | Should -Be $accessEntries
+    }
+
+    # Written later, an added empty SACL would replace the audit entries of the item.
+    It 'Should add no SACL to a security descriptor that was read without its audit entries' {
+        $file = New-TestSandboxItem -Sandbox $sandbox -Name 'NoAuditSection'
+        $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList (
+            (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access
+        )
+
+        Disable-NTFSAuditInheritance -SecurityDescriptor $sd
+
+        $binaryForm = $sd.SecurityDescriptor.GetSecurityDescriptorBinaryForm()
+        $descriptor = New-Object -TypeName 'System.Security.AccessControl.RawSecurityDescriptor' -ArgumentList $binaryForm, 0
+        $null -eq $descriptor.SystemAcl | Should -BeTrue
+    }
+}
+
 Describe 'Audit inheritance switches' {
     # Before 5.0.0, the switches were named after access entries, although they remove audit entries.
     It '<Command> should take -<Name> with the alias -<Alias>' -ForEach @(