diff --git a/CHANGELOG.md b/CHANGELOG.md index 085bcb2..e75f892 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,5 +52,8 @@ The format is based on - Remove `Show-NTFSSimpleAccess`, which no longer exists, and duplicate entries from the cmdlets that the module manifest exports and the PowerShell Gallery lists +- Fix `Set-NTFSInheritance`, which failed with "Nullable object must have a + value" when `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` was + omitted; an omitted parameter now leaves its section unchanged [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Set-NTFSInheritance.md b/Docs/Cmdlets/Set-NTFSInheritance.md index 4297cf6..6eb05e6 100644 --- a/Docs/Cmdlets/Set-NTFSInheritance.md +++ b/Docs/Cmdlets/Set-NTFSInheritance.md @@ -31,7 +31,7 @@ The `Set-NTFSInheritance` cmdlet turns the inheritance of access rules and audit The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches and it does not use their defaults. `-AccessInheritanceEnabled $false` discards the inherited access rules instead of copying them into the item's own DACL, `-AccessInheritanceEnabled $true` keeps the explicit access rules, `-AuditInheritanceEnabled $false` copies the inherited audit rules into the item's own SACL, and `-AuditInheritanceEnabled $true` removes the explicit audit rules. Use the individual Enable and Disable cmdlets when you need the opposite behavior. -Specify both `-AccessInheritanceEnabled` and `-AuditInheritanceEnabled`. The cmdlet compares the current state against the parameter value even when the parameter was not supplied, and when such a comparison reports a difference it fails with the non-terminating error "Nullable object must have a value". Omitting `-AccessInheritanceEnabled` always triggers that error, and omitting `-AuditInheritanceEnabled` triggers it in a session that can read the audit section. Changing the audit section requires the Security privilege and therefore an elevated session. +Omit `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` to leave that section unchanged. Changing the audit section requires the Security privilege and therefore an elevated session. In the `Path` parameter set the cmdlet writes each changed section back to disk immediately. In the `SecurityDescriptor` parameter set it changes the `Security2.FileSystemSecurity2` object in memory only; nothing reaches the file system until you pass that object to `Set-NTFSSecurityDescriptor`. `-Path`, `-AccessInheritanceEnabled`, and `-AuditInheritanceEnabled` all accept pipeline input by property name, so a `Security2.FileSystemInheritanceInfo` object from `Get-NTFSInheritance` binds to all three at once. @@ -192,6 +192,8 @@ If the descriptor cannot be opened because the account has no permission to the A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. +Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value". + ## RELATED LINKS [Get-NTFSInheritance](Get-NTFSInheritance.md) diff --git a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs index 109a2e2..a8effe7 100644 --- a/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs +++ b/NTFSSecurity/InheritanceCmdlets/SetInheritance.cs @@ -84,41 +84,7 @@ namespace NTFSSecurity try { - var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); - - if (currentState.AccessInheritanceEnabled != accessInheritanceEnabled) - { - WriteVerbose("AccessInheritanceEnabled not equal"); - if (accessInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAccessInheritance"); - FileSystemInheritanceInfo.EnableAccessInheritance(item, false); - } - else - { - WriteVerbose("Calling DisableAccessInheritance"); - FileSystemInheritanceInfo.DisableAccessInheritance(item, true); - } - } - else - WriteVerbose("AccessInheritanceEnabled is equal - no change was done"); - - if (currentState.AuditInheritanceEnabled != auditInheritanceEnabled) - { - WriteVerbose("AuditInheritanceEnabled not equal"); - if (auditInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAuditInheritance"); - FileSystemInheritanceInfo.EnableAuditInheritance(item, true); - } - else - { - WriteVerbose("Calling DisableAuditInheritance"); - FileSystemInheritanceInfo.DisableAuditInheritance(item, false); - } - } - else - WriteVerbose("AuditInheritanceEnabled is equal - no change was done"); + SetInheritanceState(item); } catch (UnauthorizedAccessException) { @@ -129,41 +95,7 @@ namespace NTFSSecurity FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); - - if (currentState.AccessInheritanceEnabled != accessInheritanceEnabled) - { - WriteVerbose("AccessInheritanceEnabled not equal"); - if (accessInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAccessInheritance"); - FileSystemInheritanceInfo.EnableAccessInheritance(item, false); - } - else - { - WriteVerbose("Calling DisableAccessInheritance"); - FileSystemInheritanceInfo.DisableAccessInheritance(item, true); - } - } - else - WriteVerbose("AccessInheritanceEnabled is equal - no change was done"); - - if (currentState.AuditInheritanceEnabled != auditInheritanceEnabled) - { - WriteVerbose("AuditInheritanceEnabled not equal"); - if (auditInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAuditInheritance"); - FileSystemInheritanceInfo.EnableAuditInheritance(item, true); - } - else - { - WriteVerbose("Calling DisableAuditInheritance"); - FileSystemInheritanceInfo.DisableAuditInheritance(item, false); - } - } - else - WriteVerbose("AuditInheritanceEnabled is equal - no change was done"); + SetInheritanceState(item); FileSystemOwner.SetOwner(item, previousOwner); } @@ -191,41 +123,7 @@ namespace NTFSSecurity { foreach (var sd in securityDescriptors) { - var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(sd); - - if (currentState.AccessInheritanceEnabled != accessInheritanceEnabled) - { - WriteVerbose("AccessInheritanceEnabled not equal"); - if (accessInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAccessInheritance"); - FileSystemInheritanceInfo.EnableAccessInheritance(sd, false); - } - else - { - WriteVerbose("Calling DisableAccessInheritance"); - FileSystemInheritanceInfo.DisableAccessInheritance(sd, true); - } - } - else - WriteVerbose("AccessInheritanceEnabled is equal - no change was done"); - - if (currentState.AuditInheritanceEnabled != auditInheritanceEnabled) - { - WriteVerbose("AuditInheritanceEnabled not equal"); - if (auditInheritanceEnabled.Value) - { - WriteVerbose("Calling EnableAuditInheritance"); - FileSystemInheritanceInfo.EnableAuditInheritance(sd, true); - } - else - { - WriteVerbose("Calling DisableAuditInheritance"); - FileSystemInheritanceInfo.DisableAuditInheritance(sd, false); - } - } - else - WriteVerbose("AuditInheritanceEnabled is equal - no change was done"); + SetInheritanceState(sd); if (passThru) { @@ -234,5 +132,90 @@ namespace NTFSSecurity } } } + + private void SetInheritanceState(FileSystemInfo item) + { + var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(item); + + if (IsChangeRequested("AccessInheritanceEnabled", accessInheritanceEnabled, currentState.AccessInheritanceEnabled)) + { + if (accessInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAccessInheritance"); + FileSystemInheritanceInfo.EnableAccessInheritance(item, false); + } + else + { + WriteVerbose("Calling DisableAccessInheritance"); + FileSystemInheritanceInfo.DisableAccessInheritance(item, true); + } + } + + if (IsChangeRequested("AuditInheritanceEnabled", auditInheritanceEnabled, currentState.AuditInheritanceEnabled)) + { + if (auditInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAuditInheritance"); + FileSystemInheritanceInfo.EnableAuditInheritance(item, true); + } + else + { + WriteVerbose("Calling DisableAuditInheritance"); + FileSystemInheritanceInfo.DisableAuditInheritance(item, false); + } + } + } + + private void SetInheritanceState(FileSystemSecurity2 sd) + { + var currentState = FileSystemInheritanceInfo.GetFileSystemInheritanceInfo(sd); + + if (IsChangeRequested("AccessInheritanceEnabled", accessInheritanceEnabled, currentState.AccessInheritanceEnabled)) + { + if (accessInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAccessInheritance"); + FileSystemInheritanceInfo.EnableAccessInheritance(sd, false); + } + else + { + WriteVerbose("Calling DisableAccessInheritance"); + FileSystemInheritanceInfo.DisableAccessInheritance(sd, true); + } + } + + if (IsChangeRequested("AuditInheritanceEnabled", auditInheritanceEnabled, currentState.AuditInheritanceEnabled)) + { + if (auditInheritanceEnabled.Value) + { + WriteVerbose("Calling EnableAuditInheritance"); + FileSystemInheritanceInfo.EnableAuditInheritance(sd, true); + } + else + { + WriteVerbose("Calling DisableAuditInheritance"); + FileSystemInheritanceInfo.DisableAuditInheritance(sd, false); + } + } + } + + // An omitted parameter leaves its section unchanged. + private bool IsChangeRequested(string parameterName, bool? requestedState, bool? currentState) + { + if (!requestedState.HasValue) + { + WriteVerbose(string.Format("{0} not specified - no change was done", parameterName)); + return false; + } + + if (currentState == requestedState) + { + WriteVerbose(string.Format("{0} is equal - no change was done", parameterName)); + return false; + } + + WriteVerbose(string.Format("{0} not equal", parameterName)); + return true; + } } } \ No newline at end of file diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 039bfe6..e59d3a7 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -9169,7 +9169,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The `Set-NTFSInheritance` cmdlet turns the inheritance of access rules and audit rules on or off in a single call. It reads the current state of the item first and changes a section only when the requested value differs from the current one, which makes the cmdlet suitable for repeatedly applying a desired state to a folder tree. The cmdlet performs the same operations as `Enable-NTFSAccessInheritance`, `Disable-NTFSAccessInheritance`, `Enable-NTFSAuditInheritance`, and `Disable-NTFSAuditInheritance`, but it does not expose their switches and it does not use their defaults. `-AccessInheritanceEnabled $false` discards the inherited access rules instead of copying them into the item's own DACL, `-AccessInheritanceEnabled $true` keeps the explicit access rules, `-AuditInheritanceEnabled $false` copies the inherited audit rules into the item's own SACL, and `-AuditInheritanceEnabled $true` removes the explicit audit rules. Use the individual Enable and Disable cmdlets when you need the opposite behavior. - Specify both `-AccessInheritanceEnabled` and `-AuditInheritanceEnabled`. The cmdlet compares the current state against the parameter value even when the parameter was not supplied, and when such a comparison reports a difference it fails with the non-terminating error "Nullable object must have a value". Omitting `-AccessInheritanceEnabled` always triggers that error, and omitting `-AuditInheritanceEnabled` triggers it in a session that can read the audit section. Changing the audit section requires the Security privilege and therefore an elevated session. + Omit `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` to leave that section unchanged. Changing the audit section requires the Security privilege and therefore an elevated session. In the `Path` parameter set the cmdlet writes each changed section back to disk immediately. In the `SecurityDescriptor` parameter set it changes the `Security2.FileSystemSecurity2` object in memory only; nothing reaches the file system until you pass that object to `Set-NTFSSecurityDescriptor`. `-Path`, `-AccessInheritanceEnabled`, and `-AuditInheritanceEnabled` all accept pipeline input by property name, so a `Security2.FileSystemInheritanceInfo` object from `Get-NTFSInheritance` binds to all three at once. @@ -9382,6 +9382,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The audit section of a security descriptor can only be read and written with the Security privilege (`SeSecurityPrivilege`), which an account can only use in an elevated session. Without it, a requested change of `-AuditInheritanceEnabled` produces a non-terminating error that reports Windows error 1314, "A required privilege is not held by the client". The access section is processed first, so a change of `-AccessInheritanceEnabled` in the same command is applied even when the audit change fails. If the descriptor cannot be opened because the account has no permission to the item, the cmdlet takes ownership of the item, applies the changes, and sets the previous owner back. That fallback only succeeds when the account can take ownership of the item and restore the original owner; otherwise the cmdlet writes an error and continues with the next item. A path that does not exist produces a non-terminating error and the cmdlet continues with the remaining paths. + Before 5.0.0, omitting `-AccessInheritanceEnabled` or `-AuditInheritanceEnabled` could fail with the error "Nullable object must have a value". diff --git a/Tests/Inheritance.Tests.ps1 b/Tests/Inheritance.Tests.ps1 new file mode 100644 index 0000000..f21b891 --- /dev/null +++ b/Tests/Inheritance.Tests.ps1 @@ -0,0 +1,77 @@ +<# + Tests the inheritance cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. + Tests that change the audit section need the Security privilege and skip without it; CI runs them elevated. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeDiscovery { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $canChangeAudit = Test-PrivilegeHeld -Name 'SeSecurityPrivilege' +} + +BeforeAll { + Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $sandbox = New-TestSandbox -Name 'Inheritance' + Push-Location -LiteralPath $sandbox + + function New-SandboxFile { + $path = Join-Path -Path $sandbox -ChildPath ('File-{0}.txt' -f [guid]::NewGuid().ToString('N').Substring(0, 8)) + Assert-TestSandboxPath -Sandbox $sandbox -Path $path + Set-Content -LiteralPath $path -Value 'Inheritance test' + $path + } +} + +AfterAll { + Pop-Location + Remove-TestSandbox -Sandbox $sandbox + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +Describe 'Set-NTFSInheritance' { + Context 'When -AccessInheritanceEnabled or -AuditInheritanceEnabled is omitted' { + BeforeEach { + $file = New-SandboxFile + Assert-TestSandboxPath -Sandbox $sandbox -Path $file + } + + It 'Should change nothing and write no error when both are omitted' { + Set-NTFSInheritance -Path $file -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue + + $inheritanceErrors | Should -BeNullOrEmpty + (Get-NTFSInheritance -Path $file).AccessInheritanceEnabled | Should -BeTrue + } + + It 'Should leave a security descriptor unchanged when both are omitted' { + $sd = Get-NTFSSecurityDescriptor -Path $file + + { Set-NTFSInheritance -SecurityDescriptor $sd -ErrorAction Stop } | Should -Not -Throw + $sd.SecurityDescriptor.AreAccessRulesProtected | Should -BeFalse + } + + It 'Should change only the access inheritance when -AuditInheritanceEnabled is omitted' { + $before = Get-NTFSInheritance -Path $file + + Set-NTFSInheritance -Path $file -AccessInheritanceEnabled $false -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue + + $inheritanceErrors | Should -BeNullOrEmpty + $after = Get-NTFSInheritance -Path $file + $after.AccessInheritanceEnabled | Should -BeFalse + $after.AuditInheritanceEnabled | Should -Be $before.AuditInheritanceEnabled + } + + It 'Should change only the audit inheritance when -AccessInheritanceEnabled is omitted' -Skip:(-not $canChangeAudit) { + Set-NTFSInheritance -Path $file -AuditInheritanceEnabled $false -ErrorVariable inheritanceErrors -ErrorAction SilentlyContinue + + $inheritanceErrors | Should -BeNullOrEmpty + $after = Get-NTFSInheritance -Path $file + $after.AccessInheritanceEnabled | Should -BeTrue + $after.AuditInheritanceEnabled | Should -BeFalse + } + } +}