Browse Source

fix: read the item for Set-NTFSSecurityDescriptor -PassThru outside the write retry

The cmdlet read the item again for -PassThru inside the try block that
retries a denied write as the owner (R5 of the review of #113). A read
that was denied after a successful write therefore started another
attempt of the write and ended in a WriteSdError, and a write that
needed the ownership retry wrote no object at all.

The read now follows the write and its retry, and a failed read is a
ReadSecurityError. The page also says what happens when setting the
previous owner back fails.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
ai/release-5.0.0-rc6
Raimund Andree 4 days ago
parent
commit
75b0099190
  1. 4
      CHANGELOG.md
  2. 4
      Docs/Cmdlets/Set-NTFSSecurityDescriptor.md
  3. 20
      NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs
  4. 4
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  5. 44
      Tests/SecurityDescriptor.Tests.ps1

4
CHANGELOG.md

@ -300,5 +300,9 @@ The format is based on
a file, and `New-NTFSHardLink -PassThru`, which stopped with a
terminating error on a share after it had created the link; both now
write a non-terminating `GetHardLinkError`
- Fix `-PassThru` of `Set-NTFSSecurityDescriptor`, which returned nothing
for a descriptor that the cmdlet wrote as the owner, and which turned a
failed read after a successful write into another attempt of the write
and a `WriteSdError`; it now writes a `ReadSecurityError` for that read
[Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD

4
Docs/Cmdlets/Set-NTFSSecurityDescriptor.md

@ -23,9 +23,9 @@ The `Set-NTFSSecurityDescriptor` cmdlet writes a `Security2.FileSystemSecurity2`
Each descriptor remembers the item it was read from, and the cmdlet writes it back to exactly that item. There is no parameter that redirects the write to a different path. The cmdlet writes only the sections of the descriptor that changed since it was read or last written, such as the DACL after `Add-NTFSAccess`, and leaves the other sections of the item as they are, so a descriptor that you did not change writes nothing. With `-Verbose`, the cmdlet names the sections that it writes, or says that it writes nothing. Before 5.0.0, the cmdlet wrote every section that it had read, also an unchanged owner, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.
The cmdlet produces no output unless you use `-PassThru`, which reads the item again after the write and returns a new `FileSystemSecurity2` object that reflects what is now stored on disk. Descriptors can be passed as an array or through the pipeline, and each one is processed on its own.
The cmdlet produces no output unless you use `-PassThru`, which reads the item again after the write and returns a new `FileSystemSecurity2` object that reflects what is now stored on disk. If that read fails, for example because the written descriptor denies the account the right to read it, the cmdlet writes a non-terminating `ReadSecurityError`; the descriptor is written all the same. Descriptors can be passed as an array or through the pipeline, and each one is processed on its own.
When the write fails because access is denied, the cmdlet takes ownership of the item with the account of the current session, writes the descriptor, and restores the previous owner, also when that write fails. A descriptor that sets a new owner keeps it; before 5.0.0, the cmdlet set the previous owner back over it. If the write fails as well, the cmdlet writes a non-terminating error and continues with the next descriptor. Windows checks each section separately: changing the access control list requires the Change Permissions right on the item, changing the owner requires the Take Ownership right or the Take Ownership privilege, assigning ownership to another account requires the Restore privilege, and writing audit entries requires the Security privilege.
When the write fails because access is denied, the cmdlet takes ownership of the item with the account of the current session, writes the descriptor, and restores the previous owner, also when that write fails. A descriptor that sets a new owner keeps it; before 5.0.0, the cmdlet set the previous owner back over it. If setting the previous owner back fails, the cmdlet writes a non-terminating `RestoreOwnerError`, and the account of the session stays the owner of the item. If the write fails as well, the cmdlet writes a non-terminating error and continues with the next descriptor. Windows checks each section separately: changing the access control list requires the Change Permissions right on the item, changing the owner requires the Take Ownership right or the Take Ownership privilege, assigning ownership to another account requires the Restore privilege, and writing audit entries requires the Security privilege. Before 5.0.0, `-PassThru` returned nothing for a descriptor that the cmdlet wrote as the owner, and a failed read for `-PassThru` started another attempt of the write and ended in a `WriteSdError`, although the write had succeeded.
## EXAMPLES

20
NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs

@ -52,11 +52,6 @@ namespace NTFSSecurity
}
sd.WriteChanges();
if (passThru)
{
WriteObject(new FileSystemSecurity2(sd.Item));
}
}
catch (UnauthorizedAccessException)
{
@ -73,6 +68,21 @@ namespace NTFSSecurity
catch (Exception ex)
{
WriteError(new ErrorRecord(ex, "WriteSdError", ErrorCategory.WriteError, sd.Item));
continue;
}
// After the write and outside its retry: before 5.0.0-rc6, a write that needed ownership wrote no
// object, and a denied read started a retry of the write and ended in a WriteSdError.
if (passThru)
{
try
{
WriteObject(new FileSystemSecurity2(sd.Item));
}
catch (Exception ex)
{
WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.ReadError, sd.Item));
}
}
}
}

4
NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml

@ -9886,8 +9886,8 @@ PS C:\&gt; Set-NTFSSecurityDescriptor -SecurityDescriptor $sd</dev:code>
<maml:description>
<maml:para>The `Set-NTFSSecurityDescriptor` cmdlet writes a `Security2.FileSystemSecurity2` object to the file system. It is the final step of the security descriptor workflow: `Get-NTFSSecurityDescriptor` reads a descriptor into memory, cmdlets such as `Add-NTFSAccess`, `Remove-NTFSAccess`, `Set-NTFSOwner`, and `Disable-NTFSAccessInheritance` change that copy through their `-SecurityDescriptor` parameter, and this cmdlet applies all of those changes in a single write.</maml:para>
<maml:para>Each descriptor remembers the item it was read from, and the cmdlet writes it back to exactly that item. There is no parameter that redirects the write to a different path. The cmdlet writes only the sections of the descriptor that changed since it was read or last written, such as the DACL after `Add-NTFSAccess`, and leaves the other sections of the item as they are, so a descriptor that you did not change writes nothing. With `-Verbose`, the cmdlet names the sections that it writes, or says that it writes nothing. Before 5.0.0, the cmdlet wrote every section that it had read, also an unchanged owner, which failed with error 1307, "This security ID may not be assigned as the owner of this object", when the account may not assign that owner, such as on some file servers.</maml:para>
<maml:para>The cmdlet produces no output unless you use `-PassThru`, which reads the item again after the write and returns a new `FileSystemSecurity2` object that reflects what is now stored on disk. Descriptors can be passed as an array or through the pipeline, and each one is processed on its own.</maml:para>
<maml:para>When the write fails because access is denied, the cmdlet takes ownership of the item with the account of the current session, writes the descriptor, and restores the previous owner, also when that write fails. A descriptor that sets a new owner keeps it; before 5.0.0, the cmdlet set the previous owner back over it. If the write fails as well, the cmdlet writes a non-terminating error and continues with the next descriptor. Windows checks each section separately: changing the access control list requires the Change Permissions right on the item, changing the owner requires the Take Ownership right or the Take Ownership privilege, assigning ownership to another account requires the Restore privilege, and writing audit entries requires the Security privilege.</maml:para>
<maml:para>The cmdlet produces no output unless you use `-PassThru`, which reads the item again after the write and returns a new `FileSystemSecurity2` object that reflects what is now stored on disk. If that read fails, for example because the written descriptor denies the account the right to read it, the cmdlet writes a non-terminating `ReadSecurityError`; the descriptor is written all the same. Descriptors can be passed as an array or through the pipeline, and each one is processed on its own.</maml:para>
<maml:para>When the write fails because access is denied, the cmdlet takes ownership of the item with the account of the current session, writes the descriptor, and restores the previous owner, also when that write fails. A descriptor that sets a new owner keeps it; before 5.0.0, the cmdlet set the previous owner back over it. If setting the previous owner back fails, the cmdlet writes a non-terminating `RestoreOwnerError`, and the account of the session stays the owner of the item. If the write fails as well, the cmdlet writes a non-terminating error and continues with the next descriptor. Windows checks each section separately: changing the access control list requires the Change Permissions right on the item, changing the owner requires the Take Ownership right or the Take Ownership privilege, assigning ownership to another account requires the Restore privilege, and writing audit entries requires the Security privilege. Before 5.0.0, `-PassThru` returned nothing for a descriptor that the cmdlet wrote as the owner, and a failed read for `-PassThru` started another attempt of the write and ended in a `WriteSdError`, although the write had succeeded.</maml:para>
</maml:description>
<command:syntax>
<command:syntaxItem>

44
Tests/SecurityDescriptor.Tests.ps1

@ -208,6 +208,50 @@ Describe 'Set-NTFSSecurityDescriptor' {
$setErrors | Should -BeNullOrEmpty
(Get-Acl -LiteralPath $file).GetOwner($sidType).Value | Should -Be 'S-1-5-32-544'
}
# Before 5.0.0-rc6, the cmdlet wrote no object with -PassThru when it had to take ownership for the write.
It 'Should return the written descriptor with -PassThru also when it took ownership for the write' -Skip:(-not $canAssignAnyOwner) {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'RetryPassThru'
Add-TestDenyRule -Sandbox $sandbox -Path $file -Rights @{ $currentUser = 'ChangePermissions' }
Set-TestOwner -Sandbox $sandbox -Path $file -Sid $trustedInstaller
$sd = Get-NTFSSecurityDescriptor -Path $file
Add-NTFSAccess -SecurityDescriptor $sd -Account 'Everyone' -AccessRights ReadData
$sd.SecurityDescriptor.SetOwner((New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-5-32-544'))
$result = @(Set-NTFSSecurityDescriptor -SecurityDescriptor $sd -PassThru -ErrorVariable setErrors -ErrorAction SilentlyContinue)
$setErrors | Should -BeNullOrEmpty
$result | Should -HaveCount 1
$result[0].FullName | Should -Be $file
$result[0].SecurityDescriptor.GetOwner($sidType).Value | Should -Be 'S-1-5-32-544'
}
}
Context 'When the written descriptor denies reading it again' {
BeforeAll {
$privateData['EnablePrivileges'] = $false
}
AfterAll {
$privateData['EnablePrivileges'] = $enablePrivileges
}
# Before 5.0.0-rc6, the cmdlet read the item again for -PassThru inside the block that retries a denied write,
# so that a denied read started an ownership retry and ended in a WriteSdError, although the write succeeded.
It 'Should write the descriptor and report a read error for -PassThru, not a write error' {
$file = New-TestSandboxItem -Sandbox $sandbox -Name 'PassThruDenied'
$sd = Get-NTFSSecurityDescriptor -Path $file
# A deny entry for OWNER RIGHTS replaces the right of the owner to read the security descriptor.
Add-NTFSAccess -SecurityDescriptor $sd -Account 'S-1-3-4' -AccessRights ReadPermissions -AccessType Deny -AppliesTo ThisFolderOnly
$result = @(Set-NTFSSecurityDescriptor -SecurityDescriptor $sd -PassThru -ErrorVariable setErrors -ErrorAction SilentlyContinue)
$result | Should -BeNullOrEmpty
$setErrors | Should -HaveCount 1
$setErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*'
# Get-Acl of an elevated Windows PowerShell still reads the item, so .NET checks that the entry was written.
{ [System.IO.File]::GetAccessControl($file) } | Should -Throw
}
}
}

Loading…
Cancel
Save