Browse Source

fix: report unreadable audit entries in Get-NTFSAudit

Defect 4, both parts:

- Get-NTFSAudit kept the entries of the previous item and wrote them in a
  finally block, so a path whose security descriptor failed to read
  returned the previous item's entries again. Each item now starts empty,
  and entries are written only after a successful read.
- Without the Security privilege, the cmdlet read the descriptor without
  its SACL and returned nothing, like an item without audit entries. It
  now reads the SACL alone, so a missing privilege is a ReadSecurityError
  ("A required privilege is not held by the client"). A descriptor from
  Get-NTFSSecurityDescriptor that was read without the SACL gets the same
  error; FileSystemSecurity2 now records which sections it read
  (internal, visible to NTFSSecurity).

Tests/Audit.Tests.ps1 (new): 3 tests. The repeat test needs the Security
privilege to add an audit entry and runs in CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: AI Assistant <ai@example.com>
pull/100/head
Raimund Andree 1 week ago
parent
commit
ac9e57e349
  1. 3
      CHANGELOG.md
  2. 6
      Docs/Cmdlets/Get-NTFSAudit.md
  3. 42
      NTFSSecurity/AuditCmdlets/GetAudit.cs
  4. 5
      NTFSSecurity/en-US/NTFSSecurity.dll-Help.xml
  5. 12
      Security2/FileSystem/FileSystemSecurity2.cs
  6. 3
      Security2/Properties/AssemblyInfo.cs
  7. 100
      Tests/Audit.Tests.ps1

3
CHANGELOG.md

@ -59,5 +59,8 @@ The format is based on
`-Path` pointed to a file; it now returns the file, like `Get-ChildItem`
- Fix `Get-FileHash2`, which stopped at a folder in `-Path` and didn't hash
the files that followed it; folders are now skipped
- Fix `Get-NTFSAudit`, which returned nothing without the Security privilege
instead of an error, and which returned the entries of the previous item
again after a path whose security descriptor it couldn't read
[Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD

6
Docs/Cmdlets/Get-NTFSAudit.md

@ -173,16 +173,18 @@ You can pass an account name or a SID string to `-Account`, which the cmdlet con
### Security2.FileSystemAuditRule2
The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, or when the SACL cannot be read, the cmdlet returns nothing for that item.
The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, the cmdlet returns nothing for that item; when its SACL cannot be read, the cmdlet writes an error.
## NOTES
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 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 falls back to reading the security descriptor without its SACL; it then returns no audit entries and reports no error, which looks the same as an item that is not audited at all.
Reading 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 the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well.
If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back.
Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again.
## RELATED LINKS
[Add-NTFSAudit](Add-NTFSAudit.md)

42
NTFSSecurity/AuditCmdlets/GetAudit.cs

@ -79,13 +79,13 @@ namespace NTFSSecurity
protected override void ProcessRecord()
{
IEnumerable<FileSystemAuditRule2> acl = null;
FileSystemInfo item = null;
if (ParameterSetName == "Path")
{
foreach (var path in paths)
{
FileSystemInfo item = null;
IEnumerable<FileSystemAuditRule2> acl = null;
try
{
item = GetFileSystemInfo2(path);
@ -98,7 +98,7 @@ namespace NTFSSecurity
try
{
acl = FileSystemAuditRule2.GetFileSystemAuditRules(item, !excludeExplicit, !excludeInherited, getInheritedFrom);
acl = GetAuditRules(item);
}
catch (UnauthorizedAccessException)
{
@ -108,7 +108,7 @@ namespace NTFSSecurity
var previousOwner = ownerInfo.Owner;
FileSystemOwner.SetOwner(item, System.Security.Principal.WindowsIdentity.GetCurrent().User);
acl = FileSystemAuditRule2.GetFileSystemAuditRules(item, !excludeExplicit, !excludeInherited, getInheritedFrom);
acl = GetAuditRules(item);
FileSystemOwner.SetOwner(item, previousOwner);
}
catch (Exception ex2)
@ -122,26 +122,36 @@ namespace NTFSSecurity
WriteError(new ErrorRecord(ex, "ReadSecurityError", ErrorCategory.OpenError, path));
continue;
}
finally
WriteAuditRules(acl);
}
}
else
{
if (acl != null)
foreach (var sd in securityDescriptors)
{
if (account != null)
if (!sd.HasAuditSection)
{
acl = acl.Where(ace => ace.Account == account);
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;
}
acl.ForEach(ace => WriteObject(ace));
WriteAuditRules(FileSystemAuditRule2.GetFileSystemAuditRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom));
}
}
}
}
else
{
foreach (var sd in securityDescriptors)
private IEnumerable<FileSystemAuditRule2> GetAuditRules(FileSystemInfo item)
{
acl = FileSystemAuditRule2.GetFileSystemAuditRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom);
// Reading only the SACL fails without the Security privilege, instead of returning no entries.
var sd = new FileSystemSecurity2(item, System.Security.AccessControl.AccessControlSections.Audit);
return FileSystemAuditRule2.GetFileSystemAuditRules(sd, !excludeExplicit, !excludeInherited, getInheritedFrom);
}
private void WriteAuditRules(IEnumerable<FileSystemAuditRule2> acl)
{
if (account != null)
{
acl = acl.Where(ace => ace.Account == account);
@ -150,6 +160,4 @@ namespace NTFSSecurity
acl.ForEach(ace => WriteObject(ace));
}
}
}
}
}

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

@ -4832,15 +4832,16 @@ PS C:\&gt; Disable-Privileges</dev:code>
<maml:name>Security2.FileSystemAuditRule2</maml:name>
</dev:type>
<maml:description>
<maml:para>The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, or when the SACL cannot be read, the cmdlet returns nothing for that item.</maml:para>
<maml:para>The cmdlet returns one object per audit entry, with the audited account, the audited access rights, the audit flags, the inheritance and propagation flags, the `IsInherited` flag, and the `InheritedFrom` path. When an item has no audit entries, the cmdlet returns nothing for that item; when its SACL cannot be read, the cmdlet writes an error.</maml:para>
</maml:description>
</command:returnValue>
</command:returnValues>
<maml:alertSet>
<maml:alert>
<maml:para>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.</maml:para>
<maml:para>Reading 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 falls back to reading the security descriptor without its SACL; it then returns no audit entries and reports no error, which looks the same as an item that is not audited at all.</maml:para>
<maml:para>Reading 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 the non-terminating error `ReadSecurityError` for each item, which reports "A required privilege is not held by the client". `Get-NTFSSecurityDescriptor` reads a security descriptor without its SACL when the privilege is missing; for such a descriptor, the cmdlet writes a `ReadSecurityError` as well.</maml:para>
<maml:para>If the security descriptor cannot be read because access is denied, the cmdlet takes ownership of the item, reads the descriptor again, and restores the previous owner. If the second attempt fails as well, the cmdlet writes an error, and the ownership change is not rolled back.</maml:para>
<maml:para>Before 5.0.0, the cmdlet returned no entries and no error without the Security privilege, and after a path whose security descriptor could not be read, it returned the entries of the previous item again.</maml:para>
</maml:alert>
</maml:alertSet>
<command:examples>

12
Security2/FileSystem/FileSystemSecurity2.cs

@ -53,16 +53,19 @@ namespace Security2
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;
}
}
@ -74,21 +77,30 @@ namespace Security2
try
{
sd = ((DirectoryInfo)this.item).GetAccessControl(AccessControlSections.All);
sections = AccessControlSections.All;
}
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;
}
}
}
}
// Without the Security privilege, the security descriptor is read without its SACL.
internal bool HasAuditSection
{
get { return (sections & AccessControlSections.Audit) == AccessControlSections.Audit; }
}
public FileSystemSecurity SecurityDescriptor
{
get

3
Security2/Properties/AssemblyInfo.cs

@ -19,6 +19,9 @@ using System.Runtime.InteropServices;
// COM, set the ComVisible attribute to true on that type.
[assembly: ComVisible(false)]
// Lets the cmdlets tell whether a security descriptor was read with its SACL.
[assembly: InternalsVisibleTo("NTFSSecurity")]
// The following GUID is for the ID of the typelib if this project is exposed to COM
[assembly: Guid("d89dc40a-9b43-4bce-972d-b995df8d2820")]

100
Tests/Audit.Tests.ps1

@ -0,0 +1,100 @@
<#
Tests the audit cmdlets of the module built in NTFSSecurity\bin\Release on files in a sandbox folder. Reading
and changing audit entries needs the Security privilege; tests that need it skip without it and run in CI,
whose runners are elevated.
#>
[Diagnostics.CodeAnalysis.SuppressMessageAttribute(
'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.'
)]
param ()
BeforeDiscovery {
Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force
$canReadAudit = Test-PrivilegeHeld -Name 'SeSecurityPrivilege'
}
BeforeAll {
Import-Module -Name (Join-Path -Path $PSScriptRoot -ChildPath 'TestHelpers.psm1') -Force
$modulePath = Join-Path -Path $PSScriptRoot -ChildPath '..\NTFSSecurity\bin\Release\NTFSSecurity.psd1'
Import-Module -Name $modulePath -Force -ErrorAction Stop
$sandbox = New-TestSandbox -Name 'Audit'
Push-Location -LiteralPath $sandbox
function New-SandboxItem {
param (
[string] $Name,
[switch] $Directory
)
$path = Join-Path -Path $sandbox -ChildPath ('{0}-{1}' -f $Name, [guid]::NewGuid().ToString('N').Substring(0, 8))
Assert-TestSandboxPath -Sandbox $sandbox -Path $path
if ($Directory) {
New-Item -ItemType Directory -Path $path | Out-Null
}
else {
Set-Content -LiteralPath $path -Value 'Audit test'
}
$path
}
# Denies the owner, the current account, to read the security descriptor of the item.
function Deny-ReadPermission {
param ([string] $Path)
Assert-TestSandboxPath -Sandbox $sandbox -Path $Path
$acl = Get-Acl -LiteralPath $Path
$ownerRights = New-Object -TypeName 'System.Security.Principal.SecurityIdentifier' -ArgumentList 'S-1-3-4'
$rule = New-Object -TypeName 'System.Security.AccessControl.FileSystemAccessRule' -ArgumentList (
$ownerRights, [System.Security.AccessControl.FileSystemRights]::ReadPermissions, [System.Security.AccessControl.AccessControlType]::Deny
)
$acl.AddAccessRule($rule)
Set-Acl -LiteralPath $Path -AclObject $acl
}
}
AfterAll {
Pop-Location
Remove-TestSandbox -Sandbox $sandbox
Remove-Module -Name NTFSSecurity -Force -ErrorAction SilentlyContinue
}
Describe 'Get-NTFSAudit' {
Context 'When the audit entries cannot be read' {
It 'Should write an error without the Security privilege instead of returning nothing' -Skip:$canReadAudit {
$file = New-SandboxItem -Name 'NoPrivilege'
$entries = @(Get-NTFSAudit -Path $file -ErrorVariable auditErrors -ErrorAction SilentlyContinue)
$entries | Should -BeNullOrEmpty
$auditErrors | Should -HaveCount 1
$auditErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*'
}
It 'Should write an error for a security descriptor that was read without the audit entries' {
$file = New-SandboxItem -Name 'AccessOnly'
$sd = New-Object -TypeName 'Security2.FileSystemSecurity2' -ArgumentList (
(Get-Item2 -Path $file), [System.Security.AccessControl.AccessControlSections]::Access
)
$entries = @(Get-NTFSAudit -SecurityDescriptor $sd -ErrorVariable auditErrors -ErrorAction SilentlyContinue)
$entries | Should -BeNullOrEmpty
$auditErrors | Should -HaveCount 1
$auditErrors[0].FullyQualifiedErrorId | Should -BeLike 'ReadSecurityError,*'
}
}
Context 'When a path fails after a path with audit entries' {
# Before 5.0.0, the cmdlet wrote the entries of the previous item again for the failing path.
It 'Should return the entries of the first item once' -Skip:(-not $canReadAudit) {
$folder = New-SandboxItem -Name 'Audited' -Directory
$denied = New-SandboxItem -Name 'Denied'
Add-NTFSAudit -Path $folder -Account 'Everyone' -AccessRights Delete -AuditFlags Success
Deny-ReadPermission -Path $denied
$entries = @(Get-NTFSAudit -Path $folder, $denied -ExcludeInherited -ErrorAction SilentlyContinue)
@($entries | Where-Object -Property FullName -EQ -Value $folder) | Should -HaveCount 1
}
}
}
Loading…
Cancel
Save