From 3e486f9d9a8f71e03a41646294a29a5821e105b3 Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Wed, 6 May 2026 05:17:48 -0400 Subject: [PATCH 1/4] Add session-scoped SID-to-objectClass cache in Set-RiskRating Replace seven identical Get-ADObject | Select-Object objectClass calls in Private\Set-RiskRating.ps1 with a new Get-SidObjectClass helper that wraps the query in a script-scoped hashtable cache (\). During a Locksmith scan, the same SID (Domain Users, Authenticated Users, Domain Computers, etc.) appears across many issues. Previously each issue evaluation issued a separate LDAP query for each SID. With the cache, every SID is resolved at most once per module session. Changes: - Add Get-SidObjectClass private helper at top of Set-RiskRating.ps1 - Replace all 7 Get-ADObject lookup sites with Get-SidObjectClass calls - Preserve exact return types and semantics for all call sites: - Pattern A (6 sites): returns PSCustomObject with objectClass property - Pattern B (1 site, ESC5): extracts .objectClass from the PSCustomObject - No changes to risk scoring logic, remediation, or test coverage Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Private/Set-RiskRating.ps1 | 58 +++++++++++++++++++++++++++++++++----- 1 file changed, 51 insertions(+), 7 deletions(-) diff --git a/Private/Set-RiskRating.ps1 b/Private/Set-RiskRating.ps1 index 1134682..33d976c 100644 --- a/Private/Set-RiskRating.ps1 +++ b/Private/Set-RiskRating.ps1 @@ -1,3 +1,47 @@ +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)) { + $script:SidObjectClassCache[$Sid] = Get-ADObject -Filter { objectSid -eq $Sid } | + Select-Object objectClass + } + + return $script:SidObjectClassCache[$Sid] +} + function Set-RiskRating { <# .SYNOPSIS @@ -67,7 +111,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 +170,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 +224,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 +271,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 +322,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 +358,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 +398,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 From a2961c464e6c72d5692470f9cab37ce2626abed7 Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Wed, 6 May 2026 05:33:06 -0400 Subject: [PATCH 2/4] fix: avoid caching transient AD lookup errors in Get-SidObjectClass Use -ErrorAction Stop with try/catch so that a transient ADWS error does not populate the cache with . Legitimate 'not found' (SID absent from AD) still returns and caches normally; transient errors emit Write-Warning and leave the key absent, allowing retry on the next call. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Private/Set-RiskRating.ps1 | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Private/Set-RiskRating.ps1 b/Private/Set-RiskRating.ps1 index 33d976c..9533c31 100644 --- a/Private/Set-RiskRating.ps1 +++ b/Private/Set-RiskRating.ps1 @@ -35,8 +35,16 @@ function Get-SidObjectClass { } if (-not $script:SidObjectClassCache.ContainsKey($Sid)) { - $script:SidObjectClassCache[$Sid] = Get-ADObject -Filter { objectSid -eq $Sid } | - Select-Object objectClass + 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 { + Write-Warning "Get-SidObjectClass: AD lookup failed for SID '$Sid': $_" + } } return $script:SidObjectClassCache[$Sid] From 1e7d0c781b34418a6758b7bbfee096062585f71a Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Wed, 6 May 2026 09:28:51 -0400 Subject: [PATCH 3/4] fix: mirror Get-SidObjectClass helper and call-site replacements in Invoke-Locksmith.ps1 Invoke-Locksmith.ps1 is the manually-maintained monolithic script that must stay in parity with the Private/*.ps1 sources. Apply the same Get-SidObjectClass helper (with try/catch error-handling) and replace the same 7 Get-ADObject | Select-Object objectClass call sites that were already applied to Private\Set-RiskRating.ps1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Invoke-Locksmith.ps1 | 66 +++++++++++++++++++++++++++++++++++++++----- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/Invoke-Locksmith.ps1 b/Invoke-Locksmith.ps1 index 9c2b6db..65fe0e2 100644 --- a/Invoke-Locksmith.ps1 +++ b/Invoke-Locksmith.ps1 @@ -3484,6 +3484,58 @@ 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 { + Write-Warning "Get-SidObjectClass: AD lookup failed for SID '$Sid': $_" + } + } + + return $script:SidObjectClassCache[$Sid] +} + function Set-RiskRating { <# .SYNOPSIS @@ -3553,7 +3605,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 +3667,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 +3726,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 +3777,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 +3832,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 +3870,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 +3912,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 From d384518a07fd22b15ad1a4396f888681b4704d32 Mon Sep 17 00:00:00 2001 From: Sam Erde <20478745+SamErde@users.noreply.github.com> Date: Wed, 6 May 2026 09:44:50 -0400 Subject: [PATCH 4/4] fix: use Write-Error to re-emit AD lookup failures to the error stream Replace Write-Warning with Write-Error -ErrorRecord $_ in the Get-SidObjectClass catch block so that AD lookup failures surface on the error stream and callers' $ErrorActionPreference (including Stop) is respected. Failed lookups are still not cached so each SID can be retried on the next call. Applied to both Private\Set-RiskRating.ps1 and Invoke-Locksmith.ps1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Invoke-Locksmith.ps1 | 5 ++++- Private/Set-RiskRating.ps1 | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Invoke-Locksmith.ps1 b/Invoke-Locksmith.ps1 index 65fe0e2..a5aa446 100644 --- a/Invoke-Locksmith.ps1 +++ b/Invoke-Locksmith.ps1 @@ -3529,7 +3529,10 @@ function Get-SidObjectClass { Select-Object objectClass $script:SidObjectClassCache[$Sid] = $result } catch { - Write-Warning "Get-SidObjectClass: AD lookup failed for SID '$Sid': $_" + # 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 $_ } } diff --git a/Private/Set-RiskRating.ps1 b/Private/Set-RiskRating.ps1 index 9533c31..e132a04 100644 --- a/Private/Set-RiskRating.ps1 +++ b/Private/Set-RiskRating.ps1 @@ -43,7 +43,10 @@ function Get-SidObjectClass { Select-Object objectClass $script:SidObjectClassCache[$Sid] = $result } catch { - Write-Warning "Get-SidObjectClass: AD lookup failed for SID '$Sid': $_" + # 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 $_ } }