diff --git a/Invoke-Locksmith.ps1 b/Invoke-Locksmith.ps1 index 9c2b6db..a5aa446 100644 --- a/Invoke-Locksmith.ps1 +++ b/Invoke-Locksmith.ps1 @@ -3484,6 +3484,61 @@ function Set-AdditionalTemplateProperty { } } +function Get-SidObjectClass { + <# + .SYNOPSIS + Returns the AD objectClass for a given SID, using a session-scoped cache to avoid repeated LDAP queries. + + .DESCRIPTION + Wraps Get-ADObject with a script-scoped hashtable cache so that repeated SID-to-objectClass lookups + within a single Locksmith scan run hit Active Directory only once per unique SID. Common principals + such as Domain Users, Authenticated Users, and Domain Computers appear across many issues and + benefit most from this cache. + + .PARAMETER Sid + The SID string to look up. + + .OUTPUTS + PSCustomObject with an objectClass property, or $null if the SID is not found in AD. + + .NOTES + The cache ($script:SidObjectClassCache) persists for the lifetime of the module session. It is + intentionally not cleared between issues so that common principals are resolved only once per + scan run. + + .EXAMPLE + $objectClassInfo = Get-SidObjectClass -Sid 'S-1-5-21-...-512' + #> + [CmdletBinding()] + [OutputType([PSCustomObject])] + param ( + [Parameter(Mandatory)] + [string]$Sid + ) + + if ($null -eq $script:SidObjectClassCache) { + $script:SidObjectClassCache = @{} + } + + if (-not $script:SidObjectClassCache.ContainsKey($Sid)) { + try { + # Use -ErrorAction Stop so transient ADWS/AD errors throw rather than producing + # $null output. Only cache a result (including $null for a genuine "not found") + # on success; leave the key absent on error so the next call can retry. + $result = Get-ADObject -Filter { objectSid -eq $Sid } -ErrorAction Stop | + Select-Object objectClass + $script:SidObjectClassCache[$Sid] = $result + } catch { + # Re-emit to the error stream so callers can detect/handle failures and so + # $ErrorActionPreference is honoured. The key is intentionally left absent + # so a subsequent call can retry the AD query. + Write-Error -ErrorRecord $_ + } + } + + return $script:SidObjectClassCache[$Sid] +} + function Set-RiskRating { <# .SYNOPSIS @@ -3553,7 +3608,7 @@ function Set-RiskRating { if ($Issue.Technique -eq 'ESC7') { # If an Issue can be tied to a principal, the principal's objectClass impacts the Issue's risk $SID = $Issue.IdentityReferenceSID.ToString() - $IdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $SID } | Select-Object objectClass + $IdentityReferenceObjectClass = Get-SidObjectClass -Sid $SID if ($Issue.IdentityReferenceSID -match $UnsafeUsers) { # Authenticated Users, Domain Users, Domain Computers etc. are very risky @@ -3615,7 +3670,7 @@ function Set-RiskRating { # If an Issue can be tied to a principal, the principal's objectClass impacts the Issue's risk $SID = $Issue.IdentityReferenceSID.ToString() - $IdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $SID } | Select-Object objectClass + $IdentityReferenceObjectClass = Get-SidObjectClass -Sid $SID if ($Issue.IdentityReferenceSID -match $UnsafeUsers) { @@ -3674,7 +3729,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $SafeUsers) { # Safe Users are admins. Authenticating as an admin is bad. $Principals += $esc.IdentityReference.Value @@ -3725,7 +3780,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $SafeUsers) { # Safe Users are admins. Authenticating as an admin is bad. $Principals += $esc.IdentityReference.Value @@ -3780,7 +3835,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $UnsafeUsers) { # Unsafe Users are large groups. $Principals += $esc.IdentityReference.Value @@ -3818,7 +3873,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $UnsafeUsers) { # Unsafe Users are large groups. $Principals += $esc.IdentityReference.Value @@ -3860,7 +3915,7 @@ function Set-RiskRating { } } $OtherIssueSID = $OtherIssue.IdentityReferenceSID.ToString() - $OtherIssueIdentityReferenceObjectClass = (Get-ADObject -Filter { objectSid -eq $OtherIssueSID } | Select-Object objectClass).objectClass + $OtherIssueIdentityReferenceObjectClass = (Get-SidObjectClass -Sid $OtherIssueSID).objectClass if ($OtherIssueSID -match $UnsafeUsers) { # Unsafe Users are large groups. $Principals += $OtherIssue.IdentityReference.Value diff --git a/Private/Set-RiskRating.ps1 b/Private/Set-RiskRating.ps1 index 1134682..e132a04 100644 --- a/Private/Set-RiskRating.ps1 +++ b/Private/Set-RiskRating.ps1 @@ -1,3 +1,58 @@ +function Get-SidObjectClass { + <# + .SYNOPSIS + Returns the AD objectClass for a given SID, using a session-scoped cache to avoid repeated LDAP queries. + + .DESCRIPTION + Wraps Get-ADObject with a script-scoped hashtable cache so that repeated SID-to-objectClass lookups + within a single Locksmith scan run hit Active Directory only once per unique SID. Common principals + such as Domain Users, Authenticated Users, and Domain Computers appear across many issues and + benefit most from this cache. + + .PARAMETER Sid + The SID string to look up. + + .OUTPUTS + PSCustomObject with an objectClass property, or $null if the SID is not found in AD. + + .NOTES + The cache ($script:SidObjectClassCache) persists for the lifetime of the module session. It is + intentionally not cleared between issues so that common principals are resolved only once per + scan run. + + .EXAMPLE + $objectClassInfo = Get-SidObjectClass -Sid 'S-1-5-21-...-512' + #> + [CmdletBinding()] + [OutputType([PSCustomObject])] + param ( + [Parameter(Mandatory)] + [string]$Sid + ) + + if ($null -eq $script:SidObjectClassCache) { + $script:SidObjectClassCache = @{} + } + + if (-not $script:SidObjectClassCache.ContainsKey($Sid)) { + try { + # Use -ErrorAction Stop so transient ADWS/AD errors throw rather than producing + # $null output. Only cache a result (including $null for a genuine "not found") + # on success; leave the key absent on error so the next call can retry. + $result = Get-ADObject -Filter { objectSid -eq $Sid } -ErrorAction Stop | + Select-Object objectClass + $script:SidObjectClassCache[$Sid] = $result + } catch { + # Re-emit to the error stream so callers can detect/handle failures and so + # $ErrorActionPreference is honoured. The key is intentionally left absent + # so a subsequent call can retry the AD query. + Write-Error -ErrorRecord $_ + } + } + + return $script:SidObjectClassCache[$Sid] +} + function Set-RiskRating { <# .SYNOPSIS @@ -67,7 +122,7 @@ function Set-RiskRating { if ($Issue.Technique -eq 'ESC7') { # If an Issue can be tied to a principal, the principal's objectClass impacts the Issue's risk $SID = $Issue.IdentityReferenceSID.ToString() - $IdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $SID } | Select-Object objectClass + $IdentityReferenceObjectClass = Get-SidObjectClass -Sid $SID if ($Issue.IdentityReferenceSID -match $UnsafeUsers) { # Authenticated Users, Domain Users, Domain Computers etc. are very risky @@ -126,7 +181,7 @@ function Set-RiskRating { # If an Issue can be tied to a principal, the principal's objectClass impacts the Issue's risk $SID = $Issue.IdentityReferenceSID.ToString() - $IdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $SID } | Select-Object objectClass + $IdentityReferenceObjectClass = Get-SidObjectClass -Sid $SID if ($Issue.IdentityReferenceSID -match $UnsafeUsers) { @@ -180,7 +235,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $SafeUsers) { # Safe Users are admins. Authenticating as an admin is bad. $Principals += $esc.IdentityReference.Value @@ -227,7 +282,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $SafeUsers) { # Safe Users are admins. Authenticating as an admin is bad. $Principals += $esc.IdentityReference.Value @@ -278,7 +333,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $UnsafeUsers) { # Unsafe Users are large groups. $Principals += $esc.IdentityReference.Value @@ -314,7 +369,7 @@ function Set-RiskRating { } } $escSID = $esc.IdentityReferenceSID.ToString() - $escIdentityReferenceObjectClass = Get-ADObject -Filter { objectSid -eq $escSID } | Select-Object objectClass + $escIdentityReferenceObjectClass = Get-SidObjectClass -Sid $escSID if ($escSID -match $UnsafeUsers) { # Unsafe Users are large groups. $Principals += $esc.IdentityReference.Value @@ -354,7 +409,7 @@ function Set-RiskRating { } } $OtherIssueSID = $OtherIssue.IdentityReferenceSID.ToString() - $OtherIssueIdentityReferenceObjectClass = (Get-ADObject -Filter { objectSid -eq $OtherIssueSID } | Select-Object objectClass).objectClass + $OtherIssueIdentityReferenceObjectClass = (Get-SidObjectClass -Sid $OtherIssueSID).objectClass if ($OtherIssueSID -match $UnsafeUsers) { # Unsafe Users are large groups. $Principals += $OtherIssue.IdentityReference.Value