From e22b56a8b782493a42b1a54c19e471a03532dba6 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Tue, 6 Oct 2026 16:57:36 +0000 Subject: [PATCH] fix: drive roots, audit descriptors, -WhatIf conflicts, and small items - #41: for the root of a drive, the cmdlets read and changed the security descriptor of the drive, a device object. FileSystemSecurity2 now routes drive roots through the path-based AlphaFS methods, which keep the trailing backslash; the removal and inheritance helpers use it too. - #109: Add-, Remove-, and Clear-NTFSAudit report a security descriptor without the audit entries like Get-NTFSAudit, through one helper, and Get-NTFSEffectiveAccess names the cause that Windows reported instead of a missing Security privilege. - #108: Copy-Item2 and Move-Item2 check the destination only for an operation that runs; with -WhatIf, a verbose message names the conflict. - #111: Disable-Privileges skips the privileges that the token doesn't hold, the privilege messages are spelled right, and Get-FileHash2 declares the type name of its objects; 05-Releasing.md documents the release metadata tests. - rc3 review leftovers: Remove-NTFSAudit writes nothing for an item without a SACL, the owner retry of Set-NTFSSecurityDescriptor restores the previous owner in a finally block and keeps an owner that the descriptor sets, and FileSystemSecurity2.Write with another item writes only the sections that were read. Each fix has a test that failed first, in Windows PowerShell 5.1 and PowerShell 7; writing a drive root was checked once on a temporary VHD. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 31 ++++ Docs/Cmdlets/Add-NTFSAudit.md | 2 + Docs/Cmdlets/Clear-NTFSAudit.md | 2 + Docs/Cmdlets/Copy-Item2.md | 2 +- Docs/Cmdlets/Get-FileHash2.md | 4 +- Docs/Cmdlets/Get-NTFSAccess.md | 2 + Docs/Cmdlets/Get-NTFSEffectiveAccess.md | 2 +- Docs/Cmdlets/Move-Item2.md | 2 +- Docs/Cmdlets/Remove-NTFSAudit.md | 4 + Docs/Cmdlets/Set-NTFSSecurityDescriptor.md | 2 +- Docs/Contributing/05-Releasing.md | 7 +- .../AccessCmdlets/GetEffectiveAccess.cs | 7 +- NTFSSecurity/AuditCmdlets/AddAudit.cs | 5 + NTFSSecurity/AuditCmdlets/ClearAudit.cs | 5 + .../AuditCmdlets/Get-OrphanedAudit.cs | 5 +- NTFSSecurity/AuditCmdlets/GetAudit.cs | 5 +- NTFSSecurity/AuditCmdlets/RemoveAudit.cs | 5 + NTFSSecurity/BaseCmdlets.cs | 52 +++--- NTFSSecurity/ItemCmdlets/CopyItem2.cs | 40 ++--- NTFSSecurity/ItemCmdlets/MoveItem2.cs | 35 +++-- NTFSSecurity/MiscCmdlets/GetFileHash2.cs | 2 +- .../SetSecurityDescriptor.cs | 40 ++++- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 17 +- ...AccessRule2.RemoveFileSystemAccessRules.cs | 49 ++---- ...temAuditRule2.RemoveFileSystemAuditRule.cs | 56 ++----- .../FileSystem/FileSystemInheritanceInfo.cs | 39 ++--- Security2/FileSystem/FileSystemSecurity2.cs | 148 +++++++++--------- Tests/Access.Tests.ps1 | 27 ++++ Tests/Audit.Tests.ps1 | 41 +++++ Tests/DriveRoot.Tests.ps1 | 42 +++++ Tests/ItemCmdlets.Tests.ps1 | 22 +++ Tests/OutputTypes.Tests.ps1 | 12 +- Tests/Privileges.Tests.ps1 | 24 +++ Tests/SecurityDescriptor.Tests.ps1 | 53 +++++++ Tests/TestHelpers.Tests.ps1 | 26 +++ Tests/TestHelpers.psm1 | 3 +- 36 files changed, 536 insertions(+), 284 deletions(-) create mode 100644 Tests/DriveRoot.Tests.ps1 diff --git a/CHANGELOG.md b/CHANGELOG.md index dc58999..96e2d60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -234,5 +234,36 @@ The format is based on - Fix `Clear-NTFSAudit`, which finished without an error but changed nothing in a session without the Security privilege; it now writes an error, like the other audit cmdlets +- Fix the cmdlets for the root of a drive, such as `C:\`, which read and + changed the security descriptor of the drive, a device object, instead of + that of its root folder, so that `Get-NTFSAccess` showed other entries than + Explorer ([#41](https://github.com/raandree/NTFSSecurity/issues/41)) +- Fix `Add-NTFSAudit`, `Remove-NTFSAudit`, and `Clear-NTFSAudit` with a + security descriptor that was read without the audit entries; they now + write an error like `Get-NTFSAudit` instead of changing the missing entries + without one ([#109](https://github.com/raandree/NTFSSecurity/issues/109)) +- Fix `Get-NTFSEffectiveAccess`, which blamed a missing Security privilege + for every failure while the privilege wasn't enabled; the error now names + the cause that Windows reported + ([#109](https://github.com/raandree/NTFSSecurity/issues/109)) +- Fix `Copy-Item2` and `Move-Item2`, which wrote an error with `-WhatIf` when + the destination file existed, so that `-WhatIf -ErrorAction Stop` stopped + the preview; they now name the existing file in a verbose message + ([#108](https://github.com/raandree/NTFSSecurity/issues/108)) +- Fix `Remove-NTFSAudit`, which failed with "(5) Access is denied" for a file + or folder without audit entries +- Fix `Set-NTFSSecurityDescriptor`, which set the previous owner back after it + had taken ownership to write a descriptor that sets a new owner, so that + the new owner was lost or the write failed with error 1307 +- Fix the `Write` method of a `Security2.FileSystemSecurity2` object for + another item, which wrote every section of the descriptor, also an owner + that Windows returns with the DACL; it now writes the sections that the + descriptor was read with + ([#34](https://github.com/raandree/NTFSSecurity/issues/34)) +- Fix `Disable-Privileges`, which warned that it couldn't disable the + privileges that the access token doesn't hold, and the declared output type + of `Get-FileHash2`, which named the AlphaFS `FileInfo` instead of the type + name of its objects + ([#111](https://github.com/raandree/NTFSSecurity/issues/111)) [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Add-NTFSAudit.md b/Docs/Cmdlets/Add-NTFSAudit.md index 5973097..f6b3bcd 100644 --- a/Docs/Cmdlets/Add-NTFSAudit.md +++ b/Docs/Cmdlets/Add-NTFSAudit.md @@ -288,6 +288,8 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `AddAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. +A security descriptor that was read without the Security privilege doesn't contain the audit entries. With such a descriptor, the cmdlet writes a `ReadSecurityError` and changes nothing, like `Get-NTFSAudit`; before 5.0.0, it added the entry to the missing audit entries in memory and wrote no error. + If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. In the `Path` parameter sets, the cmdlet reads and writes only the SACL of the item and leaves its owner, its group, and its DACL as they are. Before 5.0.0, it also wrote the owner and the DACL back, 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. In an elevated session, it could also store the inherited access entries of the item as explicit entries. diff --git a/Docs/Cmdlets/Clear-NTFSAudit.md b/Docs/Cmdlets/Clear-NTFSAudit.md index 7af9672..e69e64c 100644 --- a/Docs/Cmdlets/Clear-NTFSAudit.md +++ b/Docs/Cmdlets/Clear-NTFSAudit.md @@ -144,6 +144,8 @@ Reading and writing the SACL requires the Security privilege (`SeSecurityPrivile In the `Path` parameter set, the cmdlet reads and writes only the SACL of the item and leaves its owner, its group, and its DACL as they are; it writes nothing for an item without a SACL. Before 5.0.0, it also wrote the owner back, 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. +A security descriptor that was read without the Security privilege doesn't contain the audit entries. With such a descriptor, the cmdlet writes a `ReadSecurityError` and changes nothing, like `Get-NTFSAudit`; before 5.0.0, it wrote no error. + If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. ## RELATED LINKS diff --git a/Docs/Cmdlets/Copy-Item2.md b/Docs/Cmdlets/Copy-Item2.md index 9e7b0b6..fd7fcaa 100644 --- a/Docs/Cmdlets/Copy-Item2.md +++ b/Docs/Cmdlets/Copy-Item2.md @@ -24,7 +24,7 @@ The `Copy-Item2` cmdlet copies the items in `-Path` to the location in `-Destina How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and copies it into that folder. In every other case the value is the full path of the new item, which lets you copy and rename in one step. `-Destination` is resolved against the current location once, when the cmdlet starts. -Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it. With `-Force`, an existing file is replaced. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported. +Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it. With `-WhatIf`, it names an existing destination file in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, an existing file is replaced. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported. The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`. diff --git a/Docs/Cmdlets/Get-FileHash2.md b/Docs/Cmdlets/Get-FileHash2.md index 7f3e086..c47a81d 100644 --- a/Docs/Cmdlets/Get-FileHash2.md +++ b/Docs/Cmdlets/Get-FileHash2.md @@ -111,9 +111,9 @@ You can supply the `-Algorithm` value through a pipeline object that has an `Alg ## OUTPUTS -### Alphaleonis.Win32.Filesystem.FileInfo +### Alphaleonis.Win32.Filesystem.FileInfo+Hash -For every hashed file, the cmdlet writes the file object of that file, decorated with the type name `Alphaleonis.Win32.Filesystem.FileInfo+Hash` and extended with the `Hash` and `Algorithm` note properties, so all regular file properties such as `FullName`, `Name`, and `Length` remain available. +For every hashed file, the cmdlet writes the file object of that file, decorated with the type name `Alphaleonis.Win32.Filesystem.FileInfo+Hash` and extended with the `Hash` and `Algorithm` note properties, so all regular file properties such as `FullName`, `Name`, and `Length` remain available. Before 5.0.0, the cmdlet declared `Alphaleonis.Win32.Filesystem.FileInfo` as its output type. ## NOTES diff --git a/Docs/Cmdlets/Get-NTFSAccess.md b/Docs/Cmdlets/Get-NTFSAccess.md index b765835..e8a3392 100644 --- a/Docs/Cmdlets/Get-NTFSAccess.md +++ b/Docs/Cmdlets/Get-NTFSAccess.md @@ -186,6 +186,8 @@ Entries whose account cannot be translated into a name are returned with their S Before 5.0.0, after a path whose ACL could not be read, the cmdlet returned the entries of the previous item again. +For the root of a drive, such as `C:\`, the cmdlets of the module read and change the root folder of the volume, like Explorer, `icacls`, and `Get-Acl`. Before 5.0.0, they read and changed the security descriptor of the drive itself, a device object with other entries. + ## RELATED LINKS [Add-NTFSAccess](Add-NTFSAccess.md) diff --git a/Docs/Cmdlets/Get-NTFSEffectiveAccess.md b/Docs/Cmdlets/Get-NTFSEffectiveAccess.md index 0385c01..9a24f8a 100644 --- a/Docs/Cmdlets/Get-NTFSEffectiveAccess.md +++ b/Docs/Cmdlets/Get-NTFSEffectiveAccess.md @@ -180,7 +180,7 @@ One object per item, with the calculated rights in `AccessRights` and the accoun 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. -Reading effective access needs the Security privilege. In a session that does not hold it, the cmdlet warns before it starts and the calculation may fail with an error. Use `Enable-Privileges` in an elevated session to enable the privilege, and `Get-Privileges` to see which privileges the session holds. +Reading effective access needs the Security privilege. In a session that does not hold it, the cmdlet warns before it starts and the calculation may fail with an error. Use `Enable-Privileges` in an elevated session to enable the privilege, and `Get-Privileges` to see which privileges the session holds. When the calculation fails, the error names the cause that Windows reported, such as a security descriptor without an owner; before 5.0.0, it blamed a missing Security privilege whenever the privilege wasn't enabled. Before 5.0.0, `-ExcludeNoneAccessEntries` had no effect, and the cmdlet returned nothing without `-Path` or for `-SecurityDescriptor`. diff --git a/Docs/Cmdlets/Move-Item2.md b/Docs/Cmdlets/Move-Item2.md index 06d409f..82549dd 100644 --- a/Docs/Cmdlets/Move-Item2.md +++ b/Docs/Cmdlets/Move-Item2.md @@ -24,7 +24,7 @@ The `Move-Item2` cmdlet moves the items in `-Path` to the location in `-Destinat How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and moves it into that folder. In every other case the value is the full path of the new item, which lets you move and rename in one step, or rename an item in place. `-Destination` is resolved against the current location once, when the cmdlet starts. -Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. +Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-WhatIf`, the cmdlet names an existing destination file in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`. diff --git a/Docs/Cmdlets/Remove-NTFSAudit.md b/Docs/Cmdlets/Remove-NTFSAudit.md index c8c7835..68bbeb9 100644 --- a/Docs/Cmdlets/Remove-NTFSAudit.md +++ b/Docs/Cmdlets/Remove-NTFSAudit.md @@ -302,6 +302,10 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `RemoveAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. +A security descriptor that was read without the Security privilege doesn't contain the audit entries. With such a descriptor, the cmdlet writes a `ReadSecurityError` and changes nothing, like `Get-NTFSAudit`; before 5.0.0, it wrote no error, and `-PassThru` returned nothing. + +A file or folder without audit entries can have no SACL at all. For such an item, the cmdlet writes nothing and no error; before 5.0.0, it failed with the error "(5) Access is denied". + If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. The cmdlet reports no error when no entry matches the supplied values. Compare the result with `Get-NTFSAudit` to confirm that the entry is gone. diff --git a/Docs/Cmdlets/Set-NTFSSecurityDescriptor.md b/Docs/Cmdlets/Set-NTFSSecurityDescriptor.md index f7c37e0..17f583a 100644 --- a/Docs/Cmdlets/Set-NTFSSecurityDescriptor.md +++ b/Docs/Cmdlets/Set-NTFSSecurityDescriptor.md @@ -25,7 +25,7 @@ Each descriptor remembers the item it was read from, and the cmdlet writes it ba 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. -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. If that fails as well, it 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 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. ## EXAMPLES diff --git a/Docs/Contributing/05-Releasing.md b/Docs/Contributing/05-Releasing.md index 116c7d7..fc91b16 100644 --- a/Docs/Contributing/05-Releasing.md +++ b/Docs/Contributing/05-Releasing.md @@ -39,7 +39,12 @@ Renew the API key before it expires, and update the secret. are the section `## [5.0.0] - ` of `CHANGELOG.md`. The tests in `Tests\Release.Tests.ps1` check that `CHANGELOG.md` has the -release notes for the version of the module manifest. +release notes for the version of the module manifest and test the release +scripts. The tests in `Tests\Repository.Tests.ps1` check the release metadata: +the description that the PowerShell Gallery shows, that the version isn't one +that the Gallery already has, and that `Docs/README.md` names no prerelease +version. Name a prerelease only in `CHANGELOG.md`, because the documentation +home outlives it. ## Publish a prerelease diff --git a/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs b/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs index 1e63063..5284330 100644 --- a/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs +++ b/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs @@ -166,10 +166,9 @@ namespace NTFSSecurity if (result.OperationFailed) { - var securityPrivilegeEnabled = securityPrivilege.Any(p => p.PrivilegeState == PrivilegeState.Enabled); - var message = securityPrivilegeEnabled ? - string.Format("Could not get effective permissions from machine '{0}'. The error is '{1}'", serverName, result.AuthzException.Message) : - string.Format("Could not get effective permissions from machine '{0}' maybe because the 'Security' privilege is not enabled which might be required. Enable the priviliges using 'Enable-Privileges'. The error was '{1}'", serverName, result.AuthzException.Message); + // The warning of BeginProcessing already names a missing or disabled Security privilege; the error names + // the cause that Windows reported (#109). + var message = string.Format("Could not get effective permissions from machine '{0}'. The error is '{1}'", serverName, result.AuthzException.Message); WriteError(new ErrorRecord(new Exception(message, result.AuthzException), "GetEffectiveAccessError", ErrorCategory.ReadError, target)); return; } diff --git a/NTFSSecurity/AuditCmdlets/AddAudit.cs b/NTFSSecurity/AuditCmdlets/AddAudit.cs index f8b8584..3e7e097 100644 --- a/NTFSSecurity/AuditCmdlets/AddAudit.cs +++ b/NTFSSecurity/AuditCmdlets/AddAudit.cs @@ -165,6 +165,11 @@ namespace NTFSSecurity { foreach (var sd in securityDescriptors) { + if (!TestAuditSection(sd)) + { + continue; + } + FileSystemAuditRule2.AddFileSystemAuditRule(sd, account.ToList(), accessRights, auditFlags, inheritanceFlags, propagationFlags); if (passThru == true) diff --git a/NTFSSecurity/AuditCmdlets/ClearAudit.cs b/NTFSSecurity/AuditCmdlets/ClearAudit.cs index a6f7614..88aef1e 100644 --- a/NTFSSecurity/AuditCmdlets/ClearAudit.cs +++ b/NTFSSecurity/AuditCmdlets/ClearAudit.cs @@ -99,6 +99,11 @@ namespace NTFSSecurity { foreach (var sd in securityDescriptors) { + if (!TestAuditSection(sd)) + { + continue; + } + FileSystemAuditRule2.RemoveFileSystemAuditRuleAll(sd); if (disableInheritance) FileSystemInheritanceInfo.DisableAuditInheritance(sd, true); diff --git a/NTFSSecurity/AuditCmdlets/Get-OrphanedAudit.cs b/NTFSSecurity/AuditCmdlets/Get-OrphanedAudit.cs index e4d2eec..fe1c43d 100644 --- a/NTFSSecurity/AuditCmdlets/Get-OrphanedAudit.cs +++ b/NTFSSecurity/AuditCmdlets/Get-OrphanedAudit.cs @@ -19,11 +19,8 @@ namespace NTFSSecurity.AuditCmdlets { foreach (var sd in securityDescriptors) { - if (!sd.HasAuditSection) + if (!TestAuditSection(sd)) { - var ex = new InvalidOperationException(string.Format( - "The security descriptor of '{0}' doesn't contain the audit entries, because it was read without the Security privilege.", sd.FullName)); - WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.InvalidData, sd)); continue; } diff --git a/NTFSSecurity/AuditCmdlets/GetAudit.cs b/NTFSSecurity/AuditCmdlets/GetAudit.cs index fabe976..f132702 100644 --- a/NTFSSecurity/AuditCmdlets/GetAudit.cs +++ b/NTFSSecurity/AuditCmdlets/GetAudit.cs @@ -120,11 +120,8 @@ namespace NTFSSecurity { foreach (var sd in securityDescriptors) { - if (!sd.HasAuditSection) + if (!TestAuditSection(sd)) { - var ex = new InvalidOperationException(string.Format( - "The security descriptor of '{0}' doesn't contain the audit entries, because it was read without the Security privilege.", sd.FullName)); - WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.InvalidData, sd)); continue; } diff --git a/NTFSSecurity/AuditCmdlets/RemoveAudit.cs b/NTFSSecurity/AuditCmdlets/RemoveAudit.cs index 9d88c2b..c4d2e1b 100644 --- a/NTFSSecurity/AuditCmdlets/RemoveAudit.cs +++ b/NTFSSecurity/AuditCmdlets/RemoveAudit.cs @@ -181,6 +181,11 @@ namespace NTFSSecurity { foreach (var sd in securityDescriptors) { + if (!TestAuditSection(sd)) + { + continue; + } + FileSystemAuditRule2.RemoveFileSystemAuditRule(sd, account.ToList(), accessRights, auditFlags, inheritanceFlags, propagationFlags, removeSpecific); if (passThru == true) diff --git a/NTFSSecurity/BaseCmdlets.cs b/NTFSSecurity/BaseCmdlets.cs index 0571c96..2e53149 100644 --- a/NTFSSecurity/BaseCmdlets.cs +++ b/NTFSSecurity/BaseCmdlets.cs @@ -23,6 +23,21 @@ namespace NTFSSecurity base.ProcessRecord(); } + // A security descriptor that was read without its audit entries, such as without the Security privilege, has + // nothing for the audit cmdlets to read or change. They report it the same way (#109). + internal bool TestAuditSection(FileSystemSecurity2 sd) + { + if (sd.HasAuditSection) + { + return true; + } + + var ex = new InvalidOperationException(string.Format( + "The security descriptor of '{0}' doesn't contain the audit entries, because it was read without the Security privilege.", sd.FullName)); + WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.InvalidData, sd)); + return false; + } + #region GetFileSystemInfo protected System.IO.FileSystemInfo GetFileSystemInfo(string path) { @@ -262,16 +277,16 @@ namespace NTFSSecurity privileges = (new PrivilegeControl()).GetPrivileges(); if (!TryEnablePrivilege(Privilege.TakeOwnership)) - WriteDebug("The privilige 'TakeOwnership' could not be enabled. Make sure your user account does have this privilige"); + WriteDebug("The privilege 'TakeOwnership' could not be enabled. Make sure your user account does have this privilege"); if (!TryEnablePrivilege(Privilege.Restore)) - WriteDebug("The privilige 'Restore' could not be enabled. Make sure your user account does have this privilige"); + WriteDebug("The privilege 'Restore' could not be enabled. Make sure your user account does have this privilege"); if (!TryEnablePrivilege(Privilege.Backup)) - WriteDebug("The privilige 'Backup' could not be enabled. Make sure your user account does have this privilige"); + WriteDebug("The privilege 'Backup' could not be enabled. Make sure your user account does have this privilege"); if (!TryEnablePrivilege(Privilege.Security)) - WriteDebug("The privilige 'Security' could not be enabled. Make sure your user account does have this privilige"); + WriteDebug("The privilege 'Security' could not be enabled. Make sure your user account does have this privilege"); if (!quite) { @@ -298,28 +313,17 @@ namespace NTFSSecurity // Refreshes the field that DisablePrivilege reads; it is null when BeginProcessing enabled nothing. privileges = privControl.GetPrivileges(); - if (privileges.Where(p => p.Privilege == Privilege.TakeOwnership) != null) - if (!TryDisablePrivilege(Privilege.TakeOwnership)) - WriteWarning("The privilige 'TakeOwnership' could not be disabled."); - else - WriteDebug("The privilige 'TakeOwnership' was disabled."); - - if (privileges.Where(p => p.Privilege == Privilege.Restore) != null) - if (!TryDisablePrivilege(Privilege.Restore)) - WriteWarning("The privilige 'Restore' could not be disabled."); - else - WriteDebug("The privilige 'Restore' was disabled."); + // Only the privileges that the access token holds; disabling another one fails. + foreach (var privilege in new[] { Privilege.TakeOwnership, Privilege.Restore, Privilege.Backup, Privilege.Security }) + { + if (!privileges.Any(p => p.Privilege == privilege)) + continue; - if (privileges.Where(p => p.Privilege == Privilege.Backup) != null) - if (!TryDisablePrivilege(Privilege.Backup)) - WriteWarning("The privilige 'Backup' could not be disabled."); + if (!TryDisablePrivilege(privilege)) + WriteWarning(string.Format("The privilege '{0}' could not be disabled.", privilege)); else - WriteDebug("The privilige 'Backup' was disabled."); - - if (!TryDisablePrivilege(Privilege.Security)) - WriteWarning("The privilige 'Security' could not be disabled."); - else - WriteDebug("The privilige 'Security' was disabled."); + WriteDebug(string.Format("The privilege '{0}' was disabled.", privilege)); + } } // These overloads hide the single-argument methods of Cmdlet, so a message without arguments must not be diff --git a/NTFSSecurity/ItemCmdlets/CopyItem2.cs b/NTFSSecurity/ItemCmdlets/CopyItem2.cs index 0775bc7..322f2b7 100644 --- a/NTFSSecurity/ItemCmdlets/CopyItem2.cs +++ b/NTFSSecurity/ItemCmdlets/CopyItem2.cs @@ -88,7 +88,20 @@ namespace NTFSSecurity actualDestination = destination; } - if (!force & File.Exists(actualDestination)) + var destinationExists = !force && File.Exists(actualDestination); + + // Report a conflict only for an operation that runs; -WhatIf names it in a verbose message (#108). + if (!ShouldProcess(resolvedPath, item is FileInfo ? "Copy File" : "Copy Directory")) + { + if (destinationExists) + { + WriteVerbose(string.Format("The destination '{0}' already exists; without -Force, the copy would fail", actualDestination)); + } + + continue; + } + + if (destinationExists) { WriteError(new ErrorRecord(new AlreadyExistsException(), "DestinationFileAlreadyExists", ErrorCategory.ResourceExists, actualDestination)); continue; @@ -96,33 +109,24 @@ namespace NTFSSecurity try { - var processed = false; FileSystemInfo copy = null; if (item is FileInfo) { - if (ShouldProcess(resolvedPath, "Copy File")) - { - copy = ((FileInfo)item).CopyTo(actualDestination, force ? CopyOptions.None : CopyOptions.FailIfExists, PathFormat.RelativePath); - WriteVerbose(string.Format("File '{0}' copied to '{1}'", resolvedPath, actualDestination)); - processed = true; - } + copy = ((FileInfo)item).CopyTo(actualDestination, force ? CopyOptions.None : CopyOptions.FailIfExists, PathFormat.RelativePath); + WriteVerbose(string.Format("File '{0}' copied to '{1}'", resolvedPath, actualDestination)); } else { - if (ShouldProcess(resolvedPath, "Copy Directory")) - { - // AlphaFS 2.2 copies into an existing folder only and otherwise fails with a - // DirectoryNotFoundException for the first file. - Directory.CreateDirectory(actualDestination); - copy = ((DirectoryInfo)item).CopyTo(actualDestination, force ? CopyOptions.None : CopyOptions.FailIfExists, PathFormat.RelativePath); - WriteVerbose(string.Format("Directory '{0}' copied to '{1}'", resolvedPath, actualDestination)); - processed = true; - } + // AlphaFS 2.2 copies into an existing folder only and otherwise fails with a + // DirectoryNotFoundException for the first file. + Directory.CreateDirectory(actualDestination); + copy = ((DirectoryInfo)item).CopyTo(actualDestination, force ? CopyOptions.None : CopyOptions.FailIfExists, PathFormat.RelativePath); + WriteVerbose(string.Format("Directory '{0}' copied to '{1}'", resolvedPath, actualDestination)); } // Write the object for the copy that CopyTo returns, not the source item. - if (passThru && processed) + if (passThru) WriteObject(copy); } catch (System.IO.IOException ex) diff --git a/NTFSSecurity/ItemCmdlets/MoveItem2.cs b/NTFSSecurity/ItemCmdlets/MoveItem2.cs index c83c9e2..e941e37 100644 --- a/NTFSSecurity/ItemCmdlets/MoveItem2.cs +++ b/NTFSSecurity/ItemCmdlets/MoveItem2.cs @@ -88,7 +88,20 @@ namespace NTFSSecurity actualDestination = destination; } - if (!force & File.Exists(actualDestination)) + var destinationExists = !force && File.Exists(actualDestination); + + // Report a conflict only for an operation that runs; -WhatIf names it in a verbose message (#108). + if (!ShouldProcess(resolvedPath, item is FileInfo ? "Move File" : "Move Directory")) + { + if (destinationExists) + { + WriteVerbose(string.Format("The destination '{0}' already exists; without -Force, the move would fail", actualDestination)); + } + + continue; + } + + if (destinationExists) { WriteError(new ErrorRecord(new AlreadyExistsException(), "DestinationFileAlreadyExists", ErrorCategory.ResourceExists, actualDestination)); continue; @@ -96,28 +109,18 @@ namespace NTFSSecurity try { - var processed = false; - if (item is FileInfo) { - if (ShouldProcess(resolvedPath, "Move File")) - { - ((FileInfo)item).MoveTo(actualDestination, force ? MoveOptions.ReplaceExisting : MoveOptions.CopyAllowed, PathFormat.RelativePath); - WriteVerbose(string.Format("File '{0}' moved to '{1}'", resolvedPath, actualDestination)); - processed = true; - } + ((FileInfo)item).MoveTo(actualDestination, force ? MoveOptions.ReplaceExisting : MoveOptions.CopyAllowed, PathFormat.RelativePath); + WriteVerbose(string.Format("File '{0}' moved to '{1}'", resolvedPath, actualDestination)); } else { - if (ShouldProcess(resolvedPath, "Move Directory")) - { - ((DirectoryInfo)item).MoveTo(actualDestination, force ? MoveOptions.ReplaceExisting : MoveOptions.CopyAllowed, PathFormat.RelativePath); - WriteVerbose(string.Format("Directory '{0}' moved to '{1}'", resolvedPath, actualDestination)); - processed = true; - } + ((DirectoryInfo)item).MoveTo(actualDestination, force ? MoveOptions.ReplaceExisting : MoveOptions.CopyAllowed, PathFormat.RelativePath); + WriteVerbose(string.Format("Directory '{0}' moved to '{1}'", resolvedPath, actualDestination)); } - if (passThru && processed) + if (passThru) WriteObject(item); } catch (System.IO.IOException ex) diff --git a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs index 9a9acb7..b688cd9 100644 --- a/NTFSSecurity/MiscCmdlets/GetFileHash2.cs +++ b/NTFSSecurity/MiscCmdlets/GetFileHash2.cs @@ -7,7 +7,7 @@ using Security2.FileSystem.FileInfo; namespace NTFSSecurity { [Cmdlet(VerbsCommon.Get, "FileHash2")] - [OutputType(typeof(FileInfo))] + [OutputType("Alphaleonis.Win32.Filesystem.FileInfo+Hash")] public class GetFileHash2 : BaseCmdlet { private HashAlgorithms algorithm = HashAlgorithms.SHA256; diff --git a/NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs b/NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs index 1fa10f4..a7e1d53 100644 --- a/NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs +++ b/NTFSSecurity/SecurityDescriptorCmdlets/SetSecurityDescriptor.cs @@ -62,14 +62,7 @@ namespace NTFSSecurity { try { - var ownerInfo = FileSystemOwner.GetOwner(sd.Item); - var previousOwner = ownerInfo.Owner; - - FileSystemOwner.SetOwner(sd.Item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - - sd.WriteChanges(); - - FileSystemOwner.SetOwner(sd.Item, previousOwner); + WriteChangesAsOwner(sd); } catch (Exception ex2) { @@ -83,5 +76,36 @@ namespace NTFSSecurity } } } + + // Like InvokeAsOwner, takes ownership for the write and sets the previous owner back on every exit path, but not + // after a successful write of a descriptor that sets the owner itself, which would undo that owner. + private void WriteChangesAsOwner(FileSystemSecurity2 sd) + { + var setsOwner = (sd.ChangedSections & AccessControlSections.Owner) == AccessControlSections.Owner; + var previousOwner = FileSystemOwner.GetOwner(sd.Item).Owner; + + FileSystemOwner.SetOwner(sd.Item, System.Security.Principal.WindowsIdentity.GetCurrent().User); + + var written = false; + try + { + sd.WriteChanges(); + written = true; + } + finally + { + if (!(written && setsOwner)) + { + try + { + FileSystemOwner.SetOwner(sd.Item, previousOwner); + } + catch (Exception ex) + { + WriteError(new ErrorRecord(ex, "RestoreOwnerError", ErrorCategory.WriteError, sd.Item)); + } + } + } + } } } diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 64cb669..49b6417 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -1502,6 +1502,7 @@ 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. Writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `AddAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. + A security descriptor that was read without the Security privilege doesn't contain the audit entries. With such a descriptor, the cmdlet writes a `ReadSecurityError` and changes nothing, like `Get-NTFSAudit`; before 5.0.0, it added the entry to the missing audit entries in memory and wrote no error. If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. In the `Path` parameter sets, the cmdlet reads and writes only the SACL of the item and leaves its owner, its group, and its DACL as they are. Before 5.0.0, it also wrote the owner and the DACL back, 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. In an elevated session, it could also store the inherited access entries of the item as explicit entries. `-Path` or `-SecurityDescriptor`, `-Account`, and `-AccessRights` are positional parameters at positions 1, 2, and 3, like in `Remove-NTFSAudit`. Before 5.0.0, `-Account` and `-AccessRights` were both declared at position 2, so a command that passed them by position failed. @@ -1917,6 +1918,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd 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. Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `ClearAclError` whose message states that a required privilege is not held by the client, and the item is left unchanged. Before 5.0.0, the cmdlet read the security descriptor without its SACL in that situation, found no audit entries to remove, and finished without an error although nothing was changed. In the `Path` parameter set, the cmdlet reads and writes only the SACL of the item and leaves its owner, its group, and its DACL as they are; it writes nothing for an item without a SACL. Before 5.0.0, it also wrote the owner back, 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. + A security descriptor that was read without the Security privilege doesn't contain the audit entries. With such a descriptor, the cmdlet writes a `ReadSecurityError` and changes nothing, like `Get-NTFSAudit`; before 5.0.0, it wrote no error. If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. @@ -1995,7 +1997,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd The `Copy-Item2` cmdlet copies the items in `-Path` to the location in `-Destination`. It is the long-path counterpart of the built-in `Copy-Item` cmdlet: it works through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), so source and destination may be longer than the 260-character `MAX_PATH` limit. How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and copies it into that folder. In every other case the value is the full path of the new item, which lets you copy and rename in one step. `-Destination` is resolved against the current location once, when the cmdlet starts. - Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it. With `-Force`, an existing file is replaced. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported. + Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it. With `-WhatIf`, it names an existing destination file in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, an existing file is replaced. Relative paths and the `.` and `..` notations in `-Path` are resolved against the current location, and wildcard characters are not supported. The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`. @@ -4155,10 +4157,10 @@ PS C:\> Disable-Privileges - Alphaleonis.Win32.Filesystem.FileInfo + Alphaleonis.Win32.Filesystem.FileInfo+Hash - For every hashed file, the cmdlet writes the file object of that file, decorated with the type name `Alphaleonis.Win32.Filesystem.FileInfo+Hash` and extended with the `Hash` and `Algorithm` note properties, so all regular file properties such as `FullName`, `Name`, and `Length` remain available. + For every hashed file, the cmdlet writes the file object of that file, decorated with the type name `Alphaleonis.Win32.Filesystem.FileInfo+Hash` and extended with the `Hash` and `Algorithm` note properties, so all regular file properties such as `FullName`, `Name`, and `Length` remain available. Before 5.0.0, the cmdlet declared `Alphaleonis.Win32.Filesystem.FileInfo` as its output type. @@ -4586,6 +4588,7 @@ PS C:\> Disable-Privileges If the ACL of an item cannot be read 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. Entries whose account cannot be translated into a name are returned with their SID. Use `Get-NTFSOrphanedAccess` to list only those entries. Before 5.0.0, after a path whose ACL could not be read, the cmdlet returned the entries of the previous item again. + For the root of a drive, such as `C:`, the cmdlets of the module read and change the root folder of the volume, like Explorer, `icacls`, and `Get-Acl`. Before 5.0.0, they read and changed the security descriptor of the drive itself, a device object with other entries. @@ -5148,7 +5151,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd 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. - Reading effective access needs the Security privilege. In a session that does not hold it, the cmdlet warns before it starts and the calculation may fail with an error. Use `Enable-Privileges` in an elevated session to enable the privilege, and `Get-Privileges` to see which privileges the session holds. + Reading effective access needs the Security privilege. In a session that does not hold it, the cmdlet warns before it starts and the calculation may fail with an error. Use `Enable-Privileges` in an elevated session to enable the privilege, and `Get-Privileges` to see which privileges the session holds. When the calculation fails, the error names the cause that Windows reported, such as a security descriptor without an owner; before 5.0.0, it blamed a missing Security privilege whenever the privilege wasn't enabled. Before 5.0.0, `-ExcludeNoneAccessEntries` had no effect, and the cmdlet returned nothing without `-Path` or for `-SecurityDescriptor`. @@ -6781,7 +6784,7 @@ PS C:\Data> Get-NTFSSecurityDescriptor The `Move-Item2` cmdlet moves the items in `-Path` to the location in `-Destination`. It is the long-path counterpart of the built-in `Move-Item` cmdlet: it works through the AlphaFS library (`Alphaleonis.Win32.Filesystem`), so source and destination may be longer than the 260-character `MAX_PATH` limit. Files and folders can both be moved, and a folder is moved with everything it contains. How `-Destination` is interpreted depends on what is already there. If the value names an existing folder, the cmdlet keeps the name of the source item and moves it into that folder. In every other case the value is the full path of the new item, which lets you move and rename in one step, or rename an item in place. `-Destination` is resolved against the current location once, when the cmdlet starts. - Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. + Without `-Force`, the cmdlet checks whether the destination file already exists and writes a `DestinationFileAlreadyExists` error instead of overwriting it; the move itself then runs with the `CopyAllowed` option, which allows a file to move to a different volume. With `-WhatIf`, the cmdlet names an existing destination file in a verbose message instead; before 5.0.0, it wrote the error also with `-WhatIf`. With `-Force`, the move runs with the `ReplaceExisting` option and overwrites an existing destination item. The cmdlet supports `-WhatIf` and `-Confirm`, and it writes nothing to the pipeline unless you specify `-PassThru $true`. @@ -9262,6 +9265,8 @@ 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. Reading and writing the SACL requires the Security privilege (`SeSecurityPrivilege`, "Manage auditing and security log"), so run this cmdlet in an elevated session of an account that holds that privilege. Without it, the cmdlet writes a non-terminating `RemoveAceError` whose message states that a required privilege is not held by the client, and the item is left unchanged. + A security descriptor that was read without the Security privilege doesn't contain the audit entries. With such a descriptor, the cmdlet writes a `ReadSecurityError` and changes nothing, like `Get-NTFSAudit`; before 5.0.0, it wrote no error, and `-PassThru` returned nothing. + A file or folder without audit entries can have no SACL at all. For such an item, the cmdlet writes nothing and no error; before 5.0.0, it failed with the error "(5) Access is denied". If the security descriptor cannot be read or written because access is denied, the cmdlet takes ownership of the item, repeats the operation, and restores the previous owner. If the second attempt fails as well, the cmdlet restores the previous owner and writes an error. Before 5.0.0, the account that ran the cmdlet stayed the owner of the item in that case. The cmdlet reports no error when no entry matches the supplied values. Compare the result with `Get-NTFSAudit` to confirm that the entry is gone. Before 5.0.0, the cmdlet had no `-RemoveSpecific` switch. @@ -9880,7 +9885,7 @@ PS C:\> Set-NTFSSecurityDescriptor -SecurityDescriptor $sd 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. 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. - 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. If that fails as well, it 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 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. diff --git a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs index 6264399..30f8432 100644 --- a/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs +++ b/Security2/FileSystem/FileSystemAccessRule2 Class/FileSystemAccessRule2.RemoveFileSystemAccessRules.cs @@ -52,31 +52,14 @@ namespace Security2 if (type == AccessControlType.Allow) rights = rights | FileSystemRights2.Synchronize; - FileSystemAccessRule ace = null; + // Only the DACL: Windows can return the owner with it, and writing that back fails for an owner that the user + // cannot assign (#34). + var sd = new FileSystemSecurity2(item, AccessControlSections.Access); - if (item as FileInfo != null) - { - var file = (FileInfo)item; - var sd = file.GetAccessControl(AccessControlSections.Access); - - ace = (FileSystemAccessRule)sd.AccessRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); - RemoveRule(sd, ace, removeSpecific); - - // Only the DACL: Windows can return the owner with it, and writing that back fails for an owner that the - // user cannot assign (#34). - file.SetAccessControl(sd, AccessControlSections.Access); - } - else - { - DirectoryInfo directory = (DirectoryInfo)item; - - var sd = directory.GetAccessControl(AccessControlSections.Access); - - ace = (FileSystemAccessRule)sd.AccessRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); - RemoveRule(sd, ace, removeSpecific); + var ace = (FileSystemAccessRule)sd.SecurityDescriptor.AccessRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); + RemoveRule(sd.SecurityDescriptor, ace, removeSpecific); - directory.SetAccessControl(sd, AccessControlSections.Access); - } + sd.Write(); } public static void RemoveFileSystemAccessRule(FileSystemInfo item, List accounts, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) @@ -117,25 +100,11 @@ namespace Security2 public static void RemoveFileSystemAccessRule(FileSystemInfo item, FileSystemAccessRule ace, bool removeSpecific = false) { - if (item as FileInfo != null) - { - var file = (FileInfo)item; - var sd = file.GetAccessControl(AccessControlSections.Access); - - RemoveRule(sd, ace, removeSpecific); - - file.SetAccessControl(sd, AccessControlSections.Access); - } - else - { - DirectoryInfo directory = (DirectoryInfo)item; - - var sd = directory.GetAccessControl(AccessControlSections.Access); + var sd = new FileSystemSecurity2(item, AccessControlSections.Access); - RemoveRule(sd, ace, removeSpecific); + RemoveRule(sd.SecurityDescriptor, ace, removeSpecific); - directory.SetAccessControl(sd, AccessControlSections.Access); - } + sd.Write(); } public static FileSystemAccessRule2 RemoveFileSystemAccessRule(FileSystemSecurity2 sd, IdentityReference2 account, FileSystemRights2 rights, AccessControlType type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) diff --git a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.RemoveFileSystemAuditRule.cs b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.RemoveFileSystemAuditRule.cs index b833a9e..74da114 100644 --- a/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.RemoveFileSystemAuditRule.cs +++ b/Security2/FileSystem/FileSystemAuditRule2 Class/FileSystemAuditRule2.RemoveFileSystemAuditRule.cs @@ -8,37 +8,18 @@ namespace Security2 { public static void RemoveFileSystemAuditRule(FileSystemInfo item, IdentityReference2 account, FileSystemRights2 rights, AuditFlags type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) { - FileSystemAuditRule ace = null; + // Only the SACL, so that no other section that Windows returns with it is written back (#34) + var sd = new FileSystemSecurity2(item, AccessControlSections.Audit); - if (item is FileInfo) + // An item without audit entries can have no SACL at all. Then there is nothing to remove, and Windows denies + // a write without any section: (5) Access is denied. + if (!sd.HasSystemAcl) { - var file = (FileInfo)item; - var sd = file.GetAccessControl(AccessControlSections.Audit); - - ace = (FileSystemAuditRule)sd.AuditRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); - - if (removeSpecific) - sd.RemoveAuditRuleSpecific(ace); - else - sd.RemoveAuditRule(ace); - - // Only the SACL, so that no other section that Windows returns with it is written back (#34) - file.SetAccessControl(sd, AccessControlSections.Audit); + return; } - else - { - DirectoryInfo directory = (DirectoryInfo)item; - - var sd = directory.GetAccessControl(AccessControlSections.Audit); - ace = (FileSystemAuditRule)sd.AuditRuleFactory(account, (int)rights, false, inheritanceFlags, propagationFlags, type); - if (removeSpecific) - sd.RemoveAuditRuleSpecific(ace); - else - sd.RemoveAuditRule(ace); - - directory.SetAccessControl(sd, AccessControlSections.Audit); - } + RemoveFileSystemAuditRule(sd, account, rights, type, inheritanceFlags, propagationFlags, removeSpecific); + sd.Write(); } public static void RemoveFileSystemAuditRule(FileSystemInfo item, List accounts, FileSystemRights2 rights, AuditFlags type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) @@ -51,25 +32,14 @@ namespace Security2 public static void RemoveFileSystemAuditRule(FileSystemInfo item, FileSystemAuditRule ace) { - if (item is FileInfo) + var sd = new FileSystemSecurity2(item, AccessControlSections.Audit); + if (!sd.HasSystemAcl) { - var file = (FileInfo)item; - var sd = file.GetAccessControl(AccessControlSections.Audit); - - sd.RemoveAuditRuleSpecific(ace); - - file.SetAccessControl(sd, AccessControlSections.Audit); + return; } - else - { - DirectoryInfo directory = (DirectoryInfo)item; - - var sd = directory.GetAccessControl(AccessControlSections.Audit); - sd.RemoveAuditRuleSpecific(ace); - - directory.SetAccessControl(sd, AccessControlSections.Audit); - } + sd.SecurityDescriptor.RemoveAuditRuleSpecific(ace); + sd.Write(); } public static void RemoveFileSystemAuditRule(string path, IdentityReference2 account, FileSystemRights2 rights, AuditFlags type, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags, bool removeSpecific = false) diff --git a/Security2/FileSystem/FileSystemInheritanceInfo.cs b/Security2/FileSystem/FileSystemInheritanceInfo.cs index 5de30f7..5fc9f9c 100644 --- a/Security2/FileSystem/FileSystemInheritanceInfo.cs +++ b/Security2/FileSystem/FileSystemInheritanceInfo.cs @@ -55,41 +55,20 @@ namespace Security2 public static FileSystemInheritanceInfo GetFileSystemInheritanceInfo(FileSystemInfo item) { - if (item is FileInfo) - { - - bool? areAuditRulesProtected = null; + bool? areAuditRulesProtected = null; - var areAccessRulesProtected = ((FileInfo)item).GetAccessControl(AccessControlSections.Access).AreAccessRulesProtected; + var areAccessRulesProtected = FileSystemSecurity2.GetSecurity(item, AccessControlSections.Access).AreAccessRulesProtected; - try - { - areAuditRulesProtected = ((FileInfo)item).GetAccessControl(AccessControlSections.Audit).AreAuditRulesProtected; - } - catch (System.IO.IOException) - { - //log that the security privilege is missing - } - - return new FileSystemInheritanceInfo(item, !areAccessRulesProtected, !areAuditRulesProtected); + try + { + areAuditRulesProtected = FileSystemSecurity2.GetSecurity(item, AccessControlSections.Audit).AreAuditRulesProtected; } - else + catch (System.IO.IOException) { - bool? areAuditRulesProtected = null; - - var areAccessRulesProtected = ((DirectoryInfo)item).GetAccessControl(AccessControlSections.Access).AreAccessRulesProtected; - - try - { - areAuditRulesProtected = ((DirectoryInfo)item).GetAccessControl(AccessControlSections.Audit).AreAuditRulesProtected; - } - catch (System.IO.IOException) - { - //log that the security privilege is missing - } - - return new FileSystemInheritanceInfo(item, !areAccessRulesProtected, !areAuditRulesProtected); + //log that the security privilege is missing } + + return new FileSystemInheritanceInfo(item, !areAccessRulesProtected, !areAuditRulesProtected); } public static FileSystemInheritanceInfo GetFileSystemInheritanceInfo(FileSystemSecurity2 sd) diff --git a/Security2/FileSystem/FileSystemSecurity2.cs b/Security2/FileSystem/FileSystemSecurity2.cs index 5d8f150..a5673b1 100644 --- a/Security2/FileSystem/FileSystemSecurity2.cs +++ b/Security2/FileSystem/FileSystemSecurity2.cs @@ -33,71 +33,35 @@ namespace Security2 public FileSystemSecurity2(FileSystemInfo item, AccessControlSections sections) { this.sections = sections; + this.item = item; + isFile = item is FileInfo; - if (item is FileInfo) - { - this.item = (FileInfo)item; - - sd = ((FileInfo)this.item).GetAccessControl(sections); - - isFile = true; - } - else - { - this.item = (DirectoryInfo)item; - - sd = ((DirectoryInfo)this.item).GetAccessControl(sections); - } + sd = GetSecurity(item, sections); RememberSections(); } public FileSystemSecurity2(FileSystemInfo item) { - if (item is FileInfo) - { - this.item = (FileInfo)item; - try - { - sd = ((FileInfo)this.item).GetAccessControl(AccessControlSections.All); - sections = AccessControlSections.All; - } - catch - { - try - { - sd = ((FileInfo)this.item).GetAccessControl(AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group); - sections = AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group; - } - catch - { - sd = ((FileInfo)this.item).GetAccessControl(AccessControlSections.Access); - sections = AccessControlSections.Access; - } - } + this.item = item; + isFile = item is FileInfo; - isFile = true; + try + { + sd = GetSecurity(item, AccessControlSections.All); + sections = AccessControlSections.All; } - else + catch { - this.item = (DirectoryInfo)item; try { - sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.All); - sections = AccessControlSections.All; + sd = GetSecurity(item, AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group); + sections = AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group; } catch { - try - { - sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group); - sections = AccessControlSections.Access | AccessControlSections.Owner | AccessControlSections.Group; - } - catch - { - sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.Access); - sections = AccessControlSections.Access; - } + sd = GetSecurity(item, AccessControlSections.Access); + sections = AccessControlSections.Access; } } @@ -106,15 +70,65 @@ namespace Security2 // entries. Read alone, the DACL keeps the flags. if (HasAuditSection) { - var accessSecurity = isFile - ? (FileSystemSecurity)((FileInfo)this.item).GetAccessControl(AccessControlSections.Access) - : ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.Access); + var accessSecurity = GetSecurity(item, AccessControlSections.Access); sd.SetSecurityDescriptorBinaryForm(accessSecurity.GetSecurityDescriptorBinaryForm(), AccessControlSections.Access); } RememberSections(); } + // For the root of a drive, the methods of DirectoryInfo read and write the security descriptor of the volume, + // a device object, instead of that of its root folder (#41). The methods that take the path keep the trailing + // backslash and reach the root folder. + internal static FileSystemSecurity GetSecurity(FileSystemInfo item, AccessControlSections sections) + { + var file = item as FileInfo; + if (file != null) + { + return file.GetAccessControl(sections); + } + + string root; + if (TryGetDriveRoot(item, out root)) + { + return Directory.GetAccessControl(root, sections); + } + + return ((DirectoryInfo)item).GetAccessControl(sections); + } + + internal static void SetSecurity(FileSystemInfo item, FileSystemSecurity security, AccessControlSections sections) + { + var file = item as FileInfo; + if (file != null) + { + file.SetAccessControl((FileSecurity)security, sections); + return; + } + + string root; + if (TryGetDriveRoot(item, out root)) + { + Directory.SetAccessControl(root, (DirectorySecurity)security, sections); + return; + } + + ((DirectoryInfo)item).SetAccessControl((DirectorySecurity)security, sections); + } + + private static bool TryGetDriveRoot(FileSystemInfo item, out string root) + { + var fullName = item.FullName.TrimEnd('\\'); + if (fullName.Length == 2 && fullName[1] == ':' && char.IsLetter(fullName[0])) + { + root = fullName + "\\"; + return true; + } + + root = null; + return false; + } + // Without the Security privilege, the security descriptor is read without its SACL. internal bool HasAuditSection { @@ -166,14 +180,7 @@ namespace Security2 // that is read alone, and writing them back fails for an owner that the user cannot assign (#34). public void Write() { - if (isFile) - { - ((FileInfo)item).SetAccessControl((FileSecurity)sd, sections); - } - else - { - ((DirectoryInfo)item).SetAccessControl((DirectorySecurity)sd, sections); - } + SetSecurity(item, sd, sections); RememberSections(); } @@ -188,28 +195,15 @@ namespace Security2 return; } - if (isFile) - { - ((FileInfo)item).SetAccessControl((FileSecurity)sd, changedSections); - } - else - { - ((DirectoryInfo)item).SetAccessControl((DirectorySecurity)sd, changedSections); - } + SetSecurity(item, sd, changedSections); RememberSections(); } + // Writes the sections that the descriptor was read with to another item, like Write(). public void Write(FileSystemInfo item) { - if (item is FileInfo) - { - ((FileInfo)item).SetAccessControl((FileSecurity)sd); - } - else - { - ((DirectoryInfo)item).SetAccessControl((DirectorySecurity)sd); - } + SetSecurity(item, sd, sections); } public void Write(string path) diff --git a/Tests/Access.Tests.ps1 b/Tests/Access.Tests.ps1 index b0f6a43..763a143 100644 --- a/Tests/Access.Tests.ps1 +++ b/Tests/Access.Tests.ps1 @@ -104,6 +104,33 @@ Describe 'Get-NTFSEffectiveAccess' { $result | Should -HaveCount 1 $result[0].FullName | Should -Be $effectiveFile } + + Context 'When the effective access cannot be calculated' { + BeforeAll { + $privateData['EnablePrivileges'] = $false + } + + AfterAll { + $privateData['EnablePrivileges'] = $enablePrivileges + } + + # Before 5.0.0-rc4, the cmdlet blamed a missing Security privilege for every failure while the privilege + # wasn't enabled (#109). A security descriptor without an owner is such a failure: the DACL of the file has the + # auto-inherit flag since Set-Acl wrote it, so Windows returns no owner when only the DACL is read. + It 'Should name the cause in the error, not the Security privilege' { + (Get-Privileges | Where-Object -Property Privilege -EQ -Value 'Security').PrivilegeState | Should -Not -Be 'Enabled' + $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( + (Get-Item2 -Path $effectiveFile), [System.Security.AccessControl.AccessControlSections]::Access + ) + $sd.SecurityDescriptor.GetOwner($sidType) | Should -BeNullOrEmpty + + Get-NTFSEffectiveAccess -SecurityDescriptor $sd -ErrorVariable accessErrors -ErrorAction SilentlyContinue -WarningAction SilentlyContinue | Out-Null + + $accessErrors | Should -HaveCount 1 + $accessErrors[0].FullyQualifiedErrorId | Should -BeLike 'GetEffectiveAccessError,*' + $accessErrors[0].Exception.Message | Should -Not -BeLike '*privilege*' + } + } } Describe 'Get-NTFSOrphanedAccess' { diff --git a/Tests/Audit.Tests.ps1 b/Tests/Audit.Tests.ps1 index b010a90..2ae8816 100644 --- a/Tests/Audit.Tests.ps1 +++ b/Tests/Audit.Tests.ps1 @@ -260,6 +260,47 @@ Describe 'Remove-NTFSAudit' { @($result | Where-Object -FilterScript { $_.Account.Sid -eq 'S-1-5-32-545' }) | Should -HaveCount 1 } } + + Context 'When the item has no SACL' { + # An item without audit entries can have no SACL at all, and Windows denies a write without any section: + # (5) Access is denied. Before 5.0.0-rc4, the cmdlet failed for such an item, although there was nothing to + # remove. + It 'Should write no error' -Skip:(-not $canReadAudit) { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'NoSacl' + Assert-TestSandboxPath -Sandbox $sandbox -Path $file + $audit = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( + (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Audit + ) + $audit.SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit') | Should -BeNullOrEmpty + + Remove-NTFSAudit -Path $file -Account 'Everyone' -AccessRights ReadData -InheritanceFlags None -PropagationFlags None -ErrorVariable removeErrors -ErrorAction SilentlyContinue + + $removeErrors | Should -BeNullOrEmpty + } + } +} + +Describe 'Audit cmdlets with a security descriptor without the audit entries' { + # Before 5.0.0-rc4, only Get-NTFSAudit reported a security descriptor that was read without the audit entries, such + # as without the Security privilege. The other audit cmdlets changed the missing SACL in memory and wrote no error, + # and -PassThru returned nothing (#109). + It ' should write an error and leave the descriptor without audit entries' -ForEach @( + @{ Command = 'Add-NTFSAudit'; Parameters = @{ Account = 'Everyone'; AccessRights = 'ReadData'; PassThru = $true } } + @{ Command = 'Remove-NTFSAudit'; Parameters = @{ Account = 'Everyone'; AccessRights = 'ReadData'; PassThru = $true } } + @{ Command = 'Clear-NTFSAudit'; Parameters = @{ DisableInheritance = $true } } + ) { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'AccessOnly' + $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( + (Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access + ) + + $result = @(& $Command -SecurityDescriptor $sd @Parameters -ErrorVariable auditErrors -ErrorAction SilentlyContinue) + + $result | Should -BeNullOrEmpty + $auditErrors | Should -HaveCount 1 + $auditErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*' + $sd.SecurityDescriptor.GetSecurityDescriptorSddlForm('Audit') | Should -BeNullOrEmpty + } } Describe 'Clear-NTFSAudit' { diff --git a/Tests/DriveRoot.Tests.ps1 b/Tests/DriveRoot.Tests.ps1 new file mode 100644 index 0000000..e39d751 --- /dev/null +++ b/Tests/DriveRoot.Tests.ps1 @@ -0,0 +1,42 @@ +<# + Tests the cmdlets of the module built in NTFSSecurity\bin\Release on the root folder of the system drive. The tests + only read, so they need no sandbox. +#> +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' +)] +param () + +BeforeAll { + $modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1' + Import-Module -Name $modulePath -Force -ErrorAction Stop + $root = [IO.Path]::GetPathRoot($env:SystemRoot) + $sidType = [System.Security.Principal.SecurityIdentifier] + $acl = Get-Acl -LiteralPath $root +} + +AfterAll { + Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue +} + +# Before 5.0.0-rc4, the cmdlets read the security descriptor of the drive, a device object, instead of that of its root +# folder, so they showed other entries than Explorer, icacls, and Get-Acl (#41). +Describe 'The root folder of a drive' { + It 'Get-NTFSAccess should return the access entries of the root folder' { + $expected = @($acl.GetAccessRules($true, $true, $sidType) | ForEach-Object -Process { $_.IdentityReference.Value } | Sort-Object) + + $entries = @(Get-NTFSAccess -Path $root) + + @($entries | ForEach-Object -Process { $_.Account.Sid } | Sort-Object) | Should -Be $expected + } + + It 'Get-NTFSSecurityDescriptor should read the DACL of the root folder' { + $sd = Get-NTFSSecurityDescriptor -Path $root + + $sd.SecurityDescriptor.GetSecurityDescriptorSddlForm('Access') | Should -Be $acl.GetSecurityDescriptorSddlForm('Access') + } + + It 'Get-NTFSOwner should return the owner of the root folder' { + (Get-NTFSOwner -Path $root).Owner.Sid | Should -Be $acl.GetOwner($sidType).Value + } +} diff --git a/Tests/ItemCmdlets.Tests.ps1 b/Tests/ItemCmdlets.Tests.ps1 index 4d181d6..36fd8e2 100644 --- a/Tests/ItemCmdlets.Tests.ps1 +++ b/Tests/ItemCmdlets.Tests.ps1 @@ -197,6 +197,28 @@ Describe 'Copy-Item2, Move-Item2, and Remove-Item2 with several paths' { $result | Should -BeNullOrEmpty $first | Should -Exist } + + # Before 5.0.0-rc4, an existing destination file produced a real error also with -WhatIf, which only previews the + # operation, so -WhatIf -ErrorAction Stop stopped the preview (#108). + It '<_> should write no error with -WhatIf when the destination file exists' -ForEach @('Copy-Item2', 'Move-Item2') { + $existing = Join-Path -Path $destination -ChildPath 'First.txt' + Set-Content -LiteralPath $existing -Value 'Existing' + + & $_ -Path $first -Destination $destination -WhatIf -ErrorVariable itemErrors -ErrorAction SilentlyContinue + + $itemErrors | Should -BeNullOrEmpty + $first | Should -Exist + Get-Content -LiteralPath $existing | Should -Be 'Existing' + } + + It '<_> should name the existing destination file in a verbose message with -WhatIf' -ForEach @('Copy-Item2', 'Move-Item2') { + $existing = Join-Path -Path $destination -ChildPath 'First.txt' + Set-Content -LiteralPath $existing -Value 'Existing' + + $messages = & $_ -Path $first -Destination $destination -WhatIf -Verbose -ErrorAction SilentlyContinue 4>&1 + + @($messages | Where-Object -FilterScript { "$_" -like "*'$existing' already exists*" }) | Should -HaveCount 1 + } } Describe 'Copy-Item2' { diff --git a/Tests/OutputTypes.Tests.ps1 b/Tests/OutputTypes.Tests.ps1 index 7708b67..0286333 100644 --- a/Tests/OutputTypes.Tests.ps1 +++ b/Tests/OutputTypes.Tests.ps1 @@ -16,7 +16,7 @@ BeforeDiscovery { $itemTypes = @('Alphaleonis.Win32.Filesystem.FileInfo', 'Alphaleonis.Win32.Filesystem.DirectoryInfo') $declaredTypes = @( @{ Name = 'Test-Path2'; Types = @('System.Boolean') } - @{ Name = 'Get-FileHash2'; Types = @('Alphaleonis.Win32.Filesystem.FileInfo') } + @{ Name = 'Get-FileHash2'; Types = @('Alphaleonis.Win32.Filesystem.FileInfo+Hash') } @{ Name = 'Add-NTFSAudit'; Types = @('Security2.FileSystemAuditRule2') } @{ Name = 'Remove-NTFSAudit'; Types = @('Security2.FileSystemAuditRule2') } @{ Name = 'Copy-Item2'; Types = $itemTypes } @@ -48,6 +48,16 @@ Describe 'Declared output types' { It ' should declare the type of the objects it writes' -ForEach $declaredTypes { @((Get-Command -Name $Name).OutputType.Name) | Should -Be $Types } + + # Before 5.0.0-rc4, Get-FileHash2 declared the AlphaFS FileInfo, although it writes objects with the type name + # that its format view uses (#111). + It 'Get-FileHash2 should declare the type name of the objects it writes' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Hash' + + $result = Get-FileHash2 -Path $file + + $result.PSObject.TypeNames[0] | Should -Be @((Get-Command -Name Get-FileHash2).OutputType.Name)[0] + } } Describe 'Privilege cmdlets with -PassThru' { diff --git a/Tests/Privileges.Tests.ps1 b/Tests/Privileges.Tests.ps1 index 70a5852..55b3c97 100644 --- a/Tests/Privileges.Tests.ps1 +++ b/Tests/Privileges.Tests.ps1 @@ -69,6 +69,30 @@ Describe 'Disable-Privileges' { $messages.Message | Should -Contain "The privileges 'TakeOwnership', 'Restore' and 'Backup' are now disabled." } } + + Context 'When the access token holds only some of the privileges' { + # Before 5.0.0-rc4, the cmdlet also tried to disable the privileges that the access token doesn't hold, and + # warned for each one that it couldn't disable it. A removed privilege can't be added back, so the test removes + # them in a child process. + It 'Should not warn about the privileges that the access token does not hold' -Skip:(-not $holdsPrivileges) { + $script = Join-Path -Path $sandbox -ChildPath 'Disable-PartialPrivileges.ps1' + Assert-TestSandboxPath -Sandbox $sandbox -Path $script + Set-Content -LiteralPath $script -Value @' +param ($ModulePath) +Import-Module -Name $ModulePath -ErrorAction Stop +$process = [System.Diagnostics.Process]::GetCurrentProcess() +[ProcessPrivileges.ProcessExtensions]::RemovePrivilege($process, [ProcessPrivileges.Privilege]::TakeOwnership) | Out-Null +[ProcessPrivileges.ProcessExtensions]::RemovePrivilege($process, [ProcessPrivileges.Privilege]::Security) | Out-Null +Enable-Privileges -ErrorAction SilentlyContinue -WarningAction SilentlyContinue +Disable-Privileges -WarningVariable privilegeWarnings -WarningAction SilentlyContinue +'WARNINGS:{0}' -f @($privilegeWarnings).Count +'@ + + $output = & (Get-Process -Id $PID).Path -NoProfile -NonInteractive -ExecutionPolicy Bypass -File $script -ModulePath ([IO.Path]::GetFullPath($modulePath)) + + $output | Should -Contain 'WARNINGS:0' + } + } } Describe 'Inheritance cmdlets' { diff --git a/Tests/SecurityDescriptor.Tests.ps1 b/Tests/SecurityDescriptor.Tests.ps1 index d23d26d..60152c9 100644 --- a/Tests/SecurityDescriptor.Tests.ps1 +++ b/Tests/SecurityDescriptor.Tests.ps1 @@ -180,4 +180,57 @@ Describe 'Set-NTFSSecurityDescriptor' { Should -Contain "No section of the security descriptor of '$($sd.FullName)' changed since it was read or last written; nothing is written" } } + + Context 'When the write is denied until the cmdlet takes ownership' { + BeforeAll { + $privateData['EnablePrivileges'] = $false + $currentUser = [System.Security.Principal.WindowsIdentity]::GetCurrent().User.Value + } + + AfterAll { + $privateData['EnablePrivileges'] = $enablePrivileges + } + + # Before 5.0.0-rc4, the cmdlet set the previous owner back after the write, which undid an owner that the + # descriptor set, and failed for a previous owner that the user can't assign. The deny entry for the user stops + # the first write; as the owner, the user may change the permissions. + It 'Should keep the owner that the descriptor sets when the write succeeds' -Skip:(-not $canAssignAnyOwner) { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'RetryOwner' + Add-TestDenyRule -Sandbox $sandbox -Path $file -Rights @{ $currentUser = 'ChangePermissions' } + Set-TestOwner -Sandbox $sandbox -Path $file -Sid $trustedInstaller + Get-RestorePrivilegeState | Should -Be 'Disabled' + $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')) + + Set-NTFSSecurityDescriptor -SecurityDescriptor $sd -ErrorVariable setErrors -ErrorAction SilentlyContinue + + $setErrors | Should -BeNullOrEmpty + (Get-Acl -LiteralPath $file).GetOwner($sidType).Value | Should -Be 'S-1-5-32-544' + } + } +} + +Describe 'FileSystemSecurity2.Write with another item' { + # Before 5.0.0-rc4, Write wrote every section that the descriptor held to the other item, also the owner that + # Windows returns with a DACL without the auto-inherit flag, which fails for an owner that the user can't assign. + It 'Should write only the sections that were read, given the item as <_>' -Skip:(-not $canAssignAnyOwner) -ForEach @( + 'FileSystemInfo', 'String' + ) { + $source = New-TestSandboxItem -Sandbox $sandbox -Name 'Source' + $target = New-TestSandboxItem -Sandbox $sandbox -Name 'Target' + Set-TestOwner -Sandbox $sandbox -Path $source -Sid $trustedInstaller + $targetOwner = (Get-Acl -LiteralPath $target).GetOwner($sidType).Value + Get-RestorePrivilegeState | Should -Be 'Disabled' + $sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList ( + (Get-Item2 -Path $source), [System.Security.AccessControl.AccessControlSections]::Access + ) + $sd.SecurityDescriptor.GetOwner($sidType).Value | Should -Be $trustedInstaller + Assert-TestSandboxPath -Sandbox $sandbox -Path $target + $destination = if ($_ -eq 'String') { $target } else { Get-Item2 -Path $target } + + { $sd.Write($destination) } | Should -Not -Throw + + (Get-Acl -LiteralPath $target).GetOwner($sidType).Value | Should -Be $targetOwner + } } diff --git a/Tests/TestHelpers.Tests.ps1 b/Tests/TestHelpers.Tests.ps1 index 355dff4..0b569c8 100644 --- a/Tests/TestHelpers.Tests.ps1 +++ b/Tests/TestHelpers.Tests.ps1 @@ -170,6 +170,32 @@ Describe 'Test helpers' { } } + Context 'Add-TestDenyRule' { + BeforeAll { + $sandbox = New-TestSandbox -Name 'Helpers' + } + + AfterAll { + Remove-TestSandbox -Sandbox $sandbox + } + + It 'Should add a deny entry to an item in the sandbox' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Deny' + + Add-TestDenyRule -Sandbox $sandbox -Path $file -Rights @{ 'S-1-5-32-546' = 'ReadData' } + + $rules = @((Get-Acl -LiteralPath $file).GetAccessRules($true, $false, [System.Security.Principal.SecurityIdentifier]) | + Where-Object -FilterScript { $_.IdentityReference.Value -eq 'S-1-5-32-546' }) + $rules | Should -HaveCount 1 + $rules[0].AccessControlType | Should -Be 'Deny' + } + + It 'Should refuse an item outside the sandbox' { + { Add-TestDenyRule -Sandbox $sandbox -Path "$sandbox-Other\File.txt" -Rights @{ 'S-1-5-32-546' = 'ReadData' } } | + Should -Throw -ExpectedMessage 'Refusing to change*' + } + } + Context 'Test-IsElevated and Test-PrivilegeHeld' { It 'Should tell whether the process is elevated' { Test-IsElevated | Should -BeOfType [bool] diff --git a/Tests/TestHelpers.psm1 b/Tests/TestHelpers.psm1 index 4ccdfdb..e956790 100644 --- a/Tests/TestHelpers.psm1 +++ b/Tests/TestHelpers.psm1 @@ -346,4 +346,5 @@ function Test-PrivilegeHeld { } Export-ModuleMember -Function New-TestSandbox, Assert-TestSandboxPath, Remove-TestSandbox, New-TestSandboxItem, - Block-TestReadPermission, Block-TestWritePermission, Set-TestOwner, Test-IsElevated, Test-PrivilegeHeld + Block-TestReadPermission, Block-TestWritePermission, Add-TestDenyRule, Set-TestOwner, Test-IsElevated, + Test-PrivilegeHeld