diff --git a/.memory-bank/progress.md b/.memory-bank/progress.md index 51331ec..a6f4ec4 100644 --- a/.memory-bank/progress.md +++ b/.memory-bank/progress.md @@ -184,7 +184,8 @@ Numbered as agreed with the maintainer; each is documented on its page. - Fixed: #3 (braces in a path), #86 (`$PWD` shadowed, also for the default location of nine cmdlets, found by the review), #88 (an object passed by - position); `Docs/FAQ.md` answers the recurring questions. + position), #17 (an entry with `GenericAll`); `Docs/FAQ.md` answers the + recurring questions. - Open bugs: #5 (`-Attributes` matches all, `Get-ChildItem` any; needs a - decision), #17 (`GenericAll`), #34 and #67 (writes owner and group), - #41 (drive root), #82 (`Size` type data), #90 (trailing space). + decision), #34 and #67 (writes owner and group), #41 (drive root), #82 + (`Size` type data), #90 (trailing space). diff --git a/CHANGELOG.md b/CHANGELOG.md index ab9003a..06aeafb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -181,5 +181,9 @@ The format is based on `Get-NTFSOwner $folder`, which Windows PowerShell bound as the name of the item, so the cmdlets looked for it in the current location ([#88](https://github.com/raandree/NTFSSecurity/issues/88)) +- Fix `Remove-NTFSAccess` for an entry with a generic right such as + `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)) [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Remove-NTFSAccess.md b/Docs/Cmdlets/Remove-NTFSAccess.md index e2b4c18..56090b6 100644 --- a/Docs/Cmdlets/Remove-NTFSAccess.md +++ b/Docs/Cmdlets/Remove-NTFSAccess.md @@ -304,6 +304,8 @@ If the ACL of an item cannot be written because access is denied, the cmdlet tri Removing rights from an entry that does not exist is not an error; the cmdlet leaves the ACL unchanged. +An entry with a generic right, such as `GenericAll`, can be removed, for example by piping it from `Get-NTFSAccess`. Windows keeps generic rights in the inherit-only entries of folders. Before 5.0.0, the cmdlet failed for such an entry with the error "The value '269484032' is not valid for this usage of the type FileSystemRights". + Before 5.0.0, the `-RemoveSpecific` switch was missing, although version 4.1 had introduced it. A path that does not exist produces the non-terminating error `ReadFileError`, and the cmdlet continues with the next path. Before 5.0.0, the cmdlet also wrote a misleading `RemoveAceError` for that path, and with `-PassThru` it stopped with a `NullReferenceException`. diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 9b8a512..888514c 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -8416,6 +8416,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor When the module setting `EnablePrivileges` is `$true` (the default in the `PrivateData` section of NTFSSecurity.psd1), this cmdlet tries to enable the Backup, Restore, Take Ownership, and Security privileges while it runs and disables the privileges it enabled when it finishes. These privileges are only available in an elevated session of an account that holds them, such as a member of the local Administrators group. If a privilege cannot be enabled, the cmdlet continues without it and writes a debug message. If the ACL of an item cannot be written because access is denied, the cmdlet tries once more after making the current account the owner of the item, and restores the previous owner afterwards. Changing the owner of an item requires the Take Ownership and Restore privileges, so this fallback only succeeds in an elevated session of an account that holds them. Removing rights from an entry that does not exist is not an error; the cmdlet leaves the ACL unchanged. + An entry with a generic right, such as `GenericAll`, can be removed, for example by piping it from `Get-NTFSAccess`. Windows keeps generic rights in the inherit-only entries of folders. Before 5.0.0, the cmdlet failed for such an entry with the error "The value '269484032' is not valid for this usage of the type FileSystemRights". Before 5.0.0, the `-RemoveSpecific` switch was missing, although version 4.1 had introduced it. A path that does not exist produces the non-terminating error `ReadFileError`, and the cmdlet continues with the next path. Before 5.0.0, the cmdlet also wrote a misleading `RemoveAceError` for that path, and with `-PassThru` it stopped with a `NullReferenceException`. diff --git a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs index ae33da2..f217ed6 100644 --- a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs +++ b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs @@ -6,9 +6,34 @@ namespace Security2 { public partial class FileSystemAccessRule2 { + // Generic rights, such as GENERIC_ALL, appear in inherit-only entries. FileSystemSecurity.RemoveAccessRule + // rebuilds a rule that doesn't match exactly and rejects generic rights then, so such a rule is removed + // through ModifyAccessRule, which works on the access mask (#17). + private static bool HasGenericRights(FileSystemRights2 rights) + { + return ((long)rights & 0xF0000000L) != 0; + } + + private static void RemoveRule(FileSystemSecurity sd, FileSystemAccessRule ace, bool removeSpecific) + { + if (HasGenericRights((FileSystemRights2)(int)ace.FileSystemRights)) + { + bool modified; + sd.ModifyAccessRule(removeSpecific ? AccessControlModification.RemoveSpecific : AccessControlModification.Remove, ace, out modified); + } + else if (removeSpecific) + { + sd.RemoveAccessRuleSpecific(ace); + } + else + { + sd.RemoveAccessRule(ace); + } + } + public static void RemoveFileSystemAccessRule(FileSystemInfo item, IdentityReference2 account, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) { - if (type == AccessControlType.Allow) + if (type == AccessControlType.Allow && !HasGenericRights(rights)) rights = rights | FileSystemRights2.Synchronize; FileSystemAccessRule ace = null; @@ -19,10 +44,7 @@ namespace Security2 var sd = file.GetAccessControl(AccessControlSections.Access); ace = (FileSystemAccessRule)sd.AccessRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); - if (removeSpecific) - sd.RemoveAccessRuleSpecific(ace); - else - sd.RemoveAccessRule(ace); + RemoveRule(sd, ace, removeSpecific); file.SetAccessControl(sd); } @@ -33,10 +55,7 @@ namespace Security2 var sd = directory.GetAccessControl(AccessControlSections.Access); ace = (FileSystemAccessRule)sd.AccessRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); - if (removeSpecific) - sd.RemoveAccessRuleSpecific(ace); - else - sd.RemoveAccessRule(ace); + RemoveRule(sd, ace, removeSpecific); directory.SetAccessControl(sd); } @@ -85,10 +104,7 @@ namespace Security2 var file = (FileInfo)item; var sd = file.GetAccessControl(AccessControlSections.Access); - if (removeSpecific) - sd.RemoveAccessRuleSpecific(ace); - else - sd.RemoveAccessRule(ace); + RemoveRule(sd, ace, removeSpecific); file.SetAccessControl(sd); } @@ -98,10 +114,7 @@ namespace Security2 var sd = directory.GetAccessControl(AccessControlSections.Access); - if (removeSpecific) - sd.RemoveAccessRuleSpecific(ace); - else - sd.RemoveAccessRule(ace); + RemoveRule(sd, ace, removeSpecific); directory.SetAccessControl(sd); } @@ -109,23 +122,17 @@ namespace Security2 public static FileSystemAccessRule2 RemoveFileSystemAccessRule(FileSystemSecurity2 sd, IdentityReference2 account, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) { - if (type == AccessControlType.Allow) + if (type == AccessControlType.Allow && !HasGenericRights(rights)) rights = rights | FileSystemRights2.Synchronize; var ace = (FileSystemAccessRule)sd.SecurityDescriptor.AccessRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); if (sd.IsFile) { - if (removeSpecific) - ((FileSecurity)sd.SecurityDescriptor).RemoveAccessRuleSpecific(ace); - else - ((FileSecurity)sd.SecurityDescriptor).RemoveAccessRule(ace); + RemoveRule(((FileSecurity)sd.SecurityDescriptor), ace, removeSpecific); } else { - if (removeSpecific) - ((DirectorySecurity)sd.SecurityDescriptor).RemoveAccessRuleSpecific(ace); - else - ((DirectorySecurity)sd.SecurityDescriptor).RemoveAccessRule(ace); + RemoveRule(((DirectorySecurity)sd.SecurityDescriptor), ace, removeSpecific); } return ace; diff --git a/Tests/Access.Tests.ps1 b/Tests/Access.Tests.ps1 index 1008201..a3e1f62 100644 --- a/Tests/Access.Tests.ps1 +++ b/Tests/Access.Tests.ps1 @@ -259,6 +259,25 @@ Describe 'Add-NTFSAccess' { } } +Describe 'Remove-NTFSAccess with generic rights' { + # Before 5.0.0, removing an entry with a generic right such as GENERIC_ALL failed with "The value '269484032' is not + # valid", because .NET rebuilds the rule and rejects generic rights (#17). Windows keeps generic rights in + # inherit-only entries of folders. + It 'Should remove an inherit-only GenericAll entry that Get-NTFSAccess returned' { + $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'Generic' -Directory + Assert-TestSandboxPath -Sandbox $sandbox -Path $folder + $acl = Get-Acl -LiteralPath $folder + $acl.SetSecurityDescriptorSddlForm(($acl.Sddl -replace 'D:(?[A-Z]*)', 'D:${flags}(A;OICIIO;GA;;;S-1-5-32-546)')) + Set-Acl -LiteralPath $folder -AclObject $acl + (Get-Acl -LiteralPath $folder).Sddl | Should -Match '\(A;OICIIO;GA;;;BG\)' + + Get-NTFSAccess -Path $folder -Account 'S-1-5-32-546' -ExcludeInherited | + Remove-NTFSAccess -ErrorVariable removeErrors -ErrorAction SilentlyContinue + + $removeErrors | Should -BeNullOrEmpty + (Get-Acl -LiteralPath $folder).Sddl | Should -Not -Match ';;;BG\)' + } +} Describe 'Security descriptor parameter sets' { BeforeAll { $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ParameterSets' -Directory