From 828a521fdcc66be7aef628cbf04ea34aed8f1834 Mon Sep 17 00:00:00 2001 From: Raimund Andree Date: Mon, 5 Oct 2026 01:07:04 +0200 Subject: [PATCH] fix: make Get-NTFSEffectiveAccess honor its parameters Defect 15, all three parts: - -ExcludeNoneAccessEntries had no effect: the result was written in a finally block, so "continue" didn't skip it, and the check compared the rights with None although .NET adds Synchronize to every allow rule. A result is now written only after the checks, and Synchronize alone counts as no access. - Without -Path, BeginProcessing tested the path list for null, which it never is, so the cmdlet wrote nothing. It now uses the current location, like the other cmdlets. - ProcessRecord ignored the SecurityDescriptor parameter set. A new EffectiveAccess overload computes the effective access from an in-memory security descriptor; the item overload uses it. The Security privilege state that selects the error message was read into a local variable that hid the field; the field is now set. Tests/Access.Tests.ps1: 4 tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: AI Assistant --- CHANGELOG.md | 5 + Docs/Cmdlets/Get-NTFSEffectiveAccess.md | 10 +- .../AccessCmdlets/GetEffectiveAccess.cs | 123 +++++++++--------- NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml | 17 +-- Security2/EffectiveAccess.cs | 11 +- Tests/Access.Tests.ps1 | 42 ++++++ 6 files changed, 128 insertions(+), 80 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 995c230..89cc288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -98,5 +98,10 @@ The format is based on `-PropagationFlags`; `-AppliesTo` is now mandatory in the `Simple` parameter sets, so such a command uses the flag parameters and their defaults, as for a path +- Fix `Get-NTFSEffectiveAccess`: `-ExcludeNoneAccessEntries` now leaves out + items without access, the cmdlet uses the current location when `-Path` + is omitted, and `-SecurityDescriptor` returns the effective access of the + security descriptor; before, all three returned nothing or ignored the + parameter [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/Docs/Cmdlets/Get-NTFSEffectiveAccess.md b/Docs/Cmdlets/Get-NTFSEffectiveAccess.md index 78632c1..c10b202 100644 --- a/Docs/Cmdlets/Get-NTFSEffectiveAccess.md +++ b/Docs/Cmdlets/Get-NTFSEffectiveAccess.md @@ -33,7 +33,7 @@ The calculation covers the NTFS permissions of the item only. Share permissions When `-Account` is omitted, the account that runs the session is used. `-ServerName` selects the computer whose authorization manager resolves the group memberships of the account and defaults to `localhost`; when the remote authorization manager of the named computer cannot be reached, the cmdlet falls back to the local one and warns that the result is based on the group memberships known on this computer and may be inaccurate. Reading effective access relies on the Security privilege, and the cmdlet warns when the account does not hold it or the privilege is disabled. -Although `-Path` is optional, the cmdlet writes nothing when the parameter is omitted; pass a path or pipe items in. The `SecurityDescriptor` parameter set is accepted by the parameter binder but produces no output, so use `-Path` to query effective access. +When `-Path` is omitted, the cmdlet calculates the effective access to the current location. In the `SecurityDescriptor` parameter set, it calculates the effective access from a `Security2.FileSystemSecurity2` object that `Get-NTFSSecurityDescriptor` returned, without reading the item again. ## EXAMPLES @@ -89,7 +89,7 @@ Accept wildcard characters: False ### -ExcludeNoneAccessEntries -Indicates that items on which the account has no rights at all are left out of the result. In this release the switch does not suppress anything: the cmdlet writes a result for every item it processes, even when the calculated access mask is `None`. +Indicates that items on which the account has no rights at all are left out of the result. Because every calculated result includes the `Synchronize` right, an item counts as without rights when `Synchronize` is the only right. ```yaml Type: SwitchParameter @@ -105,7 +105,7 @@ Accept wildcard characters: False ### -Path -Specifies the path of one or more files or folders the effective access is calculated for. Relative paths are resolved against the current location. The parameter accepts pipeline input by value and by property name through its alias `FullName`. The cmdlet writes nothing when no path is supplied. +Specifies the path of one or more files or folders the effective access is calculated for. Relative paths are resolved against the current location. The parameter accepts pipeline input by value and by property name through its alias `FullName`. When you omit the parameter, the cmdlet uses the current location. ```yaml Type: String[] @@ -121,7 +121,7 @@ Accept wildcard characters: False ### -SecurityDescriptor -This parameter is accepted by the parameter binder but has no effect. The cmdlet produces no output in this parameter set; use `-Path` instead. +Specifies one or more security descriptors that `Get-NTFSSecurityDescriptor` returned. The cmdlet calculates the effective access from the in-memory object instead of reading the item again. A security descriptor contains information about the owner of the object, and the primary group of an object. The security descriptor also contains two access control lists (ACL). The first list is called the discretionary access control lists (DACL), and describes who should have access to an object and what type of access to grant. The second list is called the system access control lists (SACL) and defines what type of auditing to record for an object. @@ -182,6 +182,8 @@ When the module setting `EnablePrivileges` is `$true` (the default in the `Priva 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. +Before 5.0.0, `-ExcludeNoneAccessEntries` had no effect, and the cmdlet returned nothing without `-Path` or for `-SecurityDescriptor`. + ## RELATED LINKS [Get-NTFSAccess](Get-NTFSAccess.md) diff --git a/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs b/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs index d91a873..4fb3c23 100644 --- a/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs +++ b/NTFSSecurity/AccessCmdlets/GetEffectiveAccess.cs @@ -69,12 +69,7 @@ namespace NTFSSecurity { base.BeginProcessing(); - if (paths == null) - { - paths = new List() { GetVariableValue("PWD").ToString() }; - } - - var securityPrivilege = privControl.GetPrivileges().Where(priv => priv.Privilege == ProcessPrivileges.Privilege.Security); + securityPrivilege = privControl.GetPrivileges().Where(priv => priv.Privilege == ProcessPrivileges.Privilege.Security).ToList(); if (securityPrivilege.Count() == 0) { this.WriteWarning("The user does not hold the Security Privliege and might not be able to read the effective permissions"); @@ -90,10 +85,34 @@ namespace NTFSSecurity protected override void ProcessRecord() { - FileSystemInfo item = null; + if (ParameterSetName == "SecurityDescriptor") + { + foreach (var sd in securityDescriptors) + { + EffectiveAccessInfo result = null; - foreach (var path in paths) + try + { + result = EffectiveAccess.GetEffectiveAccess(sd, account, serverName); + } + catch (Exception ex) + { + WriteError(new ErrorRecord(ex, "ReadEffectivePermissionError", ErrorCategory.ReadError, sd)); + continue; + } + + WriteEffectiveAccess(result, sd.Item); + } + + return; + } + + // Like the other cmdlets, use the current location when -Path is omitted. + var targets = paths.Count > 0 ? paths : new List() { GetVariableValue("PWD").ToString() }; + + foreach (var path in targets) { + FileSystemInfo item = null; EffectiveAccessInfo result = null; try @@ -109,30 +128,7 @@ namespace NTFSSecurity try { result = EffectiveAccess.GetEffectiveAccess(item, account, serverName); - - if (!result.FromRemote) - { - WriteWarning("The effective rights can only be computed based on group membership on this" + - " computer. For more accurate results, calculate effective access rights on " + - "the target computer"); - } - if (result.OperationFailed && securityPrivilege == null) - { - var ex = new Exception(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), result.AuthzException); - WriteError(new ErrorRecord(ex, "GetEffectiveAccessError", ErrorCategory.ReadError, item)); - continue; - } - else if (result.OperationFailed) - { - var ex = new Exception(string.Format("Could not get effective permissions from machine '{0}'. The error is '{1}'", serverName, result.AuthzException.Message), result.AuthzException); - WriteError(new ErrorRecord(ex, "GetEffectiveAccessError", ErrorCategory.ReadError, item)); - continue; - } - - if (excludeNoneAccessEntries && result.Ace.AccessRights == FileSystemRights2.None) - continue; } - //not sure if the following catch block willb be invoked, testing needed. catch (UnauthorizedAccessException) { try @@ -142,53 +138,52 @@ namespace NTFSSecurity FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User); - //-------------------- - result = EffectiveAccess.GetEffectiveAccess(item, account, serverName); - if (!result.FromRemote) - { - WriteWarning("The effective rights can only be computed based on group membership on this" + - " computer. For more accurate results, calculate effective access rights on " + - "the target computer"); - } - if (result.OperationFailed && securityPrivilege == null) - { - var ex = new Exception(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), result.AuthzException); - WriteError(new ErrorRecord(ex, "GetEffectiveAccessError", ErrorCategory.ReadError, item)); - continue; - } - else if (result.OperationFailed) - { - var ex = new Exception(string.Format("Could not get effective permissions from machine '{0}'. The error is '{1}'", serverName, result.AuthzException.Message), result.AuthzException); - WriteError(new ErrorRecord(ex, "GetEffectiveAccessError", ErrorCategory.ReadError, item)); - continue; - } - - if (excludeNoneAccessEntries && result.Ace.AccessRights == FileSystemRights2.None) - continue; - - //-------------------- - FileSystemOwner.SetOwner(item, previousOwner); } catch (Exception ex2) { this.WriteError(new ErrorRecord(ex2, "ReadSecurityError", ErrorCategory.WriteError, path)); + continue; } } catch (Exception ex) { WriteError(new ErrorRecord(ex, "ReadEffectivePermissionError", ErrorCategory.ReadError, path)); + continue; } - finally - { - if (result != null) - { - WriteObject(result.Ace); - } - } + + WriteEffectiveAccess(result, item); } } + + private void WriteEffectiveAccess(EffectiveAccessInfo result, object target) + { + if (!result.FromRemote) + { + WriteWarning("The effective rights can only be computed based on group membership on this" + + " computer. For more accurate results, calculate effective access rights on " + + "the target computer"); + } + + 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); + WriteError(new ErrorRecord(new Exception(message, result.AuthzException), "GetEffectiveAccessError", ErrorCategory.ReadError, target)); + return; + } + + // .NET adds Synchronize to every allow rule, so an account without access has Synchronize only. + if (excludeNoneAccessEntries && (result.Ace.AccessRights & ~FileSystemRights2.Synchronize) == FileSystemRights2.None) + { + return; + } + + WriteObject(result.Ace); + } } } \ No newline at end of file diff --git a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml index 74bd974..bdc029f 100644 --- a/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml +++ b/NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml @@ -4922,7 +4922,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd Calculates the rights an account really has on a file or a folder and writes the result as a single `Security2.FileSystemAccessRule2` object per item. The cmdlet evaluates the complete discretionary access control list (DACL) of the item against the group memberships of the account with the Windows Authorization API, so allow entries, deny entries, and inherited entries are combined the same way the Windows access check combines them. This is the equivalent of the "Effective Access" tab of the advanced security dialog. The calculation covers the NTFS permissions of the item only. Share permissions are stored in a separate security descriptor and are not part of the result, so access over a network share can be more restrictive than this cmdlet reports. When `-Account` is omitted, the account that runs the session is used. `-ServerName` selects the computer whose authorization manager resolves the group memberships of the account and defaults to `localhost`; when the remote authorization manager of the named computer cannot be reached, the cmdlet falls back to the local one and warns that the result is based on the group memberships known on this computer and may be inaccurate. Reading effective access relies on the Security privilege, and the cmdlet warns when the account does not hold it or the privilege is disabled. - Although `-Path` is optional, the cmdlet writes nothing when the parameter is omitted; pass a path or pipe items in. The `SecurityDescriptor` parameter set is accepted by the parameter binder but produces no output, so use `-Path` to query effective access. + When `-Path` is omitted, the cmdlet calculates the effective access to the current location. In the `SecurityDescriptor` parameter set, it calculates the effective access from a `Security2.FileSystemSecurity2` object that `Get-NTFSSecurityDescriptor` returned, without reading the item again. @@ -4930,7 +4930,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd Path - Specifies the path of one or more files or folders the effective access is calculated for. Relative paths are resolved against the current location. The parameter accepts pipeline input by value and by property name through its alias `FullName`. The cmdlet writes nothing when no path is supplied. + Specifies the path of one or more files or folders the effective access is calculated for. Relative paths are resolved against the current location. The parameter accepts pipeline input by value and by property name through its alias `FullName`. When you omit the parameter, the cmdlet uses the current location. String[] @@ -4954,7 +4954,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd ExcludeNoneAccessEntries - Indicates that items on which the account has no rights at all are left out of the result. In this release the switch does not suppress anything: the cmdlet writes a result for every item it processes, even when the calculated access mask is `None`. + Indicates that items on which the account has no rights at all are left out of the result. Because every calculated result includes the `Synchronize` right, an item counts as without rights when `Synchronize` is the only right. SwitchParameter @@ -4980,7 +4980,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd SecurityDescriptor - This parameter is accepted by the parameter binder but has no effect. The cmdlet produces no output in this parameter set; use `-Path` instead. + Specifies one or more security descriptors that `Get-NTFSSecurityDescriptor` returned. The cmdlet calculates the effective access from the in-memory object instead of reading the item again. A security descriptor contains information about the owner of the object, and the primary group of an object. The security descriptor also contains two access control lists (ACL). The first list is called the discretionary access control lists (DACL), and describes who should have access to an object and what type of access to grant. The second list is called the system access control lists (SACL) and defines what type of auditing to record for an object. FileSystemSecurity2[] @@ -5005,7 +5005,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd ExcludeNoneAccessEntries - Indicates that items on which the account has no rights at all are left out of the result. In this release the switch does not suppress anything: the cmdlet writes a result for every item it processes, even when the calculated access mask is `None`. + Indicates that items on which the account has no rights at all are left out of the result. Because every calculated result includes the `Synchronize` right, an item counts as without rights when `Synchronize` is the only right. SwitchParameter @@ -5043,7 +5043,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd ExcludeNoneAccessEntries - Indicates that items on which the account has no rights at all are left out of the result. In this release the switch does not suppress anything: the cmdlet writes a result for every item it processes, even when the calculated access mask is `None`. + Indicates that items on which the account has no rights at all are left out of the result. Because every calculated result includes the `Synchronize` right, an item counts as without rights when `Synchronize` is the only right. SwitchParameter @@ -5055,7 +5055,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd Path - Specifies the path of one or more files or folders the effective access is calculated for. Relative paths are resolved against the current location. The parameter accepts pipeline input by value and by property name through its alias `FullName`. The cmdlet writes nothing when no path is supplied. + Specifies the path of one or more files or folders the effective access is calculated for. Relative paths are resolved against the current location. The parameter accepts pipeline input by value and by property name through its alias `FullName`. When you omit the parameter, the cmdlet uses the current location. String[] @@ -5067,7 +5067,7 @@ PS C:\> Get-NTFSAudit -SecurityDescriptor $sd SecurityDescriptor - This parameter is accepted by the parameter binder but has no effect. The cmdlet produces no output in this parameter set; use `-Path` instead. + Specifies one or more security descriptors that `Get-NTFSSecurityDescriptor` returned. The cmdlet calculates the effective access from the in-memory object instead of reading the item again. A security descriptor contains information about the owner of the object, and the primary group of an object. The security descriptor also contains two access control lists (ACL). The first list is called the discretionary access control lists (DACL), and describes who should have access to an object and what type of access to grant. The second list is called the system access control lists (SACL) and defines what type of auditing to record for an object. FileSystemSecurity2[] @@ -5130,6 +5130,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. + Before 5.0.0, `-ExcludeNoneAccessEntries` had no effect, and the cmdlet returned nothing without `-Path` or for `-SecurityDescriptor`. diff --git a/Security2/EffectiveAccess.cs b/Security2/EffectiveAccess.cs index 9527b2c..9e959f5 100644 --- a/Security2/EffectiveAccess.cs +++ b/Security2/EffectiveAccess.cs @@ -8,20 +8,23 @@ namespace Security2 public class EffectiveAccess { public static EffectiveAccessInfo GetEffectiveAccess(FileSystemInfo item, IdentityReference2 id, string serverName) + { + return GetEffectiveAccess(new FileSystemSecurity2(item), id, serverName); + } + + public static EffectiveAccessInfo GetEffectiveAccess(FileSystemSecurity2 sd, IdentityReference2 id, string serverName) { bool remoteServerAvailable = false; Exception authzAccessCheckException = null; var win32 = new Win32(); - var fss = new FileSystemSecurity2(item); - - var effectiveAccessMask = win32.GetEffectiveAccess(fss.SecurityDescriptor, id, serverName, out remoteServerAvailable, out authzAccessCheckException); + var effectiveAccessMask = win32.GetEffectiveAccess(sd.SecurityDescriptor, id, serverName, out remoteServerAvailable, out authzAccessCheckException); var ace = new FileSystemAccessRule((SecurityIdentifier)id, (FileSystemRights)effectiveAccessMask, AccessControlType.Allow); return new EffectiveAccessInfo( - new FileSystemAccessRule2(ace, item), + new FileSystemAccessRule2(ace, sd.Item), remoteServerAvailable, authzAccessCheckException); } diff --git a/Tests/Access.Tests.ps1 b/Tests/Access.Tests.ps1 index 3e6e2fd..33468f6 100644 --- a/Tests/Access.Tests.ps1 +++ b/Tests/Access.Tests.ps1 @@ -36,6 +36,48 @@ Describe 'Get-NTFSAccess' { } } +Describe 'Get-NTFSEffectiveAccess' { + BeforeAll { + $effectiveFile = New-TestSandboxItem -Sandbox $sandbox -Name 'Effective' + Assert-TestSandboxPath -Sandbox $sandbox -Path $effectiveFile + $acl = Get-Acl -LiteralPath $effectiveFile + $guests = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-5-32-546' + $acl.AddAccessRule((New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList ( + $guests, [System.Security.AccessControl.FileSystemRights]::FullControl, + [System.Security.AccessControl.AccessControlType]::Deny + ))) + Set-Acl -LiteralPath $effectiveFile -AclObject $acl + } + + It 'Should leave out an account without access when -ExcludeNoneAccessEntries is used' { + $result = @(Get-NTFSEffectiveAccess -Path $effectiveFile -Account 'S-1-5-32-546' -ExcludeNoneAccessEntries -WarningAction SilentlyContinue -ErrorAction Stop) + + $result | Should -BeNullOrEmpty + } + + It 'Should return an account with access when -ExcludeNoneAccessEntries is used' { + $result = @(Get-NTFSEffectiveAccess -Path $effectiveFile -ExcludeNoneAccessEntries -WarningAction SilentlyContinue -ErrorAction Stop) + + $result | Should -HaveCount 1 + } + + It 'Should use the current location when -Path is omitted' { + $result = @(Get-NTFSEffectiveAccess -WarningAction SilentlyContinue -ErrorAction Stop) + + $result | Should -HaveCount 1 + $result[0].FullName | Should -BeLike ('*\{0}' -f (Split-Path -Path $sandbox -Leaf)) + } + + It 'Should compute the effective access of a security descriptor' { + $sd = Get-NTFSSecurityDescriptor -Path $effectiveFile + + $result = @(Get-NTFSEffectiveAccess -SecurityDescriptor $sd -WarningAction SilentlyContinue -ErrorAction Stop) + + $result | Should -HaveCount 1 + $result[0].FullName | Should -Be $effectiveFile + } +} + Describe 'Security descriptor parameter sets' { BeforeAll { $folder = New-TestSandboxItem -Sandbox $sandbox -Name 'ParameterSets' -Directory