Skip to content

chore: Apply context pattern to other processors using shared state #310

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
definitelynotagoblin wants to merge 3 commits into
base: v4
Choose a base branch
from
Draft

chore: Apply context pattern to other processors using shared state #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1974409
fix: Make ACLProcessor guid cache singleton and thread-safe
definitelynotagoblin Aug 26, 2026
be826ec
feat: Apply new ProcessorContext pattern to ACLProcessor for shared s…
definitelynotagoblin Aug 27, 2026
dcd2b3f
chore: Apply context pattern to other processors using shared state
definitelynotagoblin Aug 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 84 additions & 13 deletions src/CommonLib/Processors/ACLProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,83 @@
using SharpHoundCommonLib.Enums;
using SharpHoundCommonLib.OutputTypes;
using System.Linq;
using System.Threading;

namespace SharpHoundCommonLib.Processors {
/// <summary>
/// Owns state shared by processor instances and gives that state an explicit lifetime.
/// </summary>
public sealed class ACLProcessorContext : IDisposable {
private readonly ACLProcessor.GuidCache _aclGuidCache = new();
private int _disposed;

/// <summary>
/// Creates an <see cref="ACLProcessor"/> that shares its GUID cache with other
/// ACL processors created by this context.
/// </summary>
public ACLProcessor CreateACLProcessor(ILdapUtils utils, ILogger log = null) {
if (Volatile.Read(ref _disposed) != 0) {
throw new ObjectDisposedException(nameof(ACLProcessorContext));
}

return new ACLProcessor(utils, _aclGuidCache, log);
}

/// <summary>
/// Clears the shared processor state. Processors created by this context must not
/// be used after the context is disposed.
/// </summary>
public void Dispose() {
if (Interlocked.Exchange(ref _disposed, 1) != 0) {
return;
}

_aclGuidCache.Dispose();
}
}

public class ACLProcessor {
private static readonly Dictionary<Label, string> BaseGuids;
private readonly ConcurrentDictionary<string, string> _guidMap = new();
private readonly ILogger _log;
private readonly ILdapUtils _utils;
private readonly ConcurrentHashSet _builtDomainCaches = new(StringComparer.OrdinalIgnoreCase);
private readonly object _lock = new();
private readonly GuidCache _guidCache;

internal sealed class GuidCache : IDisposable {
private readonly ConcurrentDictionary<string, string> _guidMap = new();
private readonly ConcurrentDictionary<string, Lazy<Task>> _buildTasks =
new(StringComparer.OrdinalIgnoreCase);
private int _disposed;

public Lazy<Task> GetOrAddBuildTask(string domain, Func<Lazy<Task>> buildTaskFactory) {
ThrowIfDisposed();
return _buildTasks.GetOrAdd(domain, _ => buildTaskFactory());
}

public void AddGuid(string guid, string name) {
ThrowIfDisposed();
_guidMap.TryAdd(guid, name);
}

public bool TryGetGuid(string guid, out string name) {
ThrowIfDisposed();
return _guidMap.TryGetValue(guid, out name);
}

public void Dispose() {
if (Interlocked.Exchange(ref _disposed, 1) != 0) {
return;
}

_buildTasks.Clear();
_guidMap.Clear();
}

private void ThrowIfDisposed() {
if (Volatile.Read(ref _disposed) != 0) {
throw new ObjectDisposedException(nameof(ACLProcessorContext));
}
}
}

static ACLProcessor() {
//Create a dictionary with the base GUIDs of each object type
Expand All @@ -42,9 +110,12 @@ static ACLProcessor() {
};
}

public ACLProcessor(ILdapUtils utils, ILogger log = null)
{
public ACLProcessor(ILdapUtils utils, ILogger log = null) : this(utils, new GuidCache(), log) {
}

internal ACLProcessor(ILdapUtils utils, GuidCache guidCache, ILogger log = null) {
_utils = utils;
_guidCache = guidCache;
_log = log ?? Logging.LogProvider.CreateLogger("ACLProc");
}

Expand Down Expand Up @@ -73,14 +144,14 @@ public override string ToString() {
/// LAPS
/// </summary>
private async Task BuildGuidCache(string domain) {
lock (_lock) {
if (_builtDomainCaches.Contains(domain)) {
return;
}
var buildTask = _guidCache.GetOrAddBuildTask(domain,
// The ExecutionAndPublication mode ensures that only one thread can execute the factory method at a time, and all other threads will wait for the result of that execution. This prevents multiple threads from building the cache simultaneously for the same domain.
() => new Lazy<Task>(() => BuildGuidCacheCore(domain), LazyThreadSafetyMode.ExecutionAndPublication));

_builtDomainCaches.Add(domain);
}
await buildTask.Value;
}

private async Task BuildGuidCacheCore(string domain) {
_log.LogInformation("Building GUID Cache for {Domain}", domain);
await foreach (var result in _utils.PagedQuery(new LdapQueryParameters {
DomainName = domain,
Expand Down Expand Up @@ -108,7 +179,7 @@ private async Task BuildGuidCache(string domain) {

if (name is LDAPProperties.LAPSPlaintextPassword or LDAPProperties.LAPSEncryptedPassword or LDAPProperties.LegacyLAPSPassword) {
_log.LogInformation("Found GUID for ACL Right {Name}: {Guid} in domain {Domain}", name, guid, domain);
_guidMap.TryAdd(guid, name);
_guidCache.AddGuid(guid, name);
}
} else {
_log.LogDebug("Error while building GUID cache for {Domain}: {Message}", domain, result.Error);
Expand Down Expand Up @@ -676,7 +747,7 @@ public async IAsyncEnumerable<ACE> ProcessACL(byte[] ntSecurityDescriptor, strin
IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid,
IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid,
};
else if (_guidMap.TryGetValue(aceType, out var lapsAttribute)) {
else if (_guidCache.TryGetGuid(aceType, out var lapsAttribute)) {
// Compare the retrieved attribute name against LDAPProperties values
if (lapsAttribute == LDAPProperties.LegacyLAPSPassword ||
lapsAttribute == LDAPProperties.LAPSPlaintextPassword ||
Expand Down
145 changes: 113 additions & 32 deletions src/CommonLib/Processors/GPOLocalGroupProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using System.Threading;
using System.Threading.Tasks;
using System.Xml.XPath;
using Microsoft.Extensions.Logging;
Expand All @@ -13,6 +14,38 @@
using SharpHoundCommonLib.OutputTypes;

namespace SharpHoundCommonLib.Processors {
/// <summary>
/// Owns state shared by GPOLocalGroupProcessor instances and gives that state an explicit lifetime.
/// </summary>
public sealed class GPOLocalGroupProcessorContext : IDisposable {
private readonly GPOLocalGroupProcessor.ActionCache _actionCache = new();
private int _disposed;

/// <summary>
/// Creates a <see cref="GPOLocalGroupProcessor"/> that shares its GPO action cache with other
/// processors created by this context.
/// </summary>
public GPOLocalGroupProcessor CreateGPOLocalGroupProcessor(ILdapUtils utils, ILogger log = null) {
if (Volatile.Read(ref _disposed) != 0) {
throw new ObjectDisposedException(nameof(GPOLocalGroupProcessorContext));
}

return new GPOLocalGroupProcessor(utils, _actionCache, log);
}

/// <summary>
/// Clears the shared processor state. Processors created by this context must not
/// be used after the context is disposed.
/// </summary>
public void Dispose() {
if (Interlocked.Exchange(ref _disposed, 1) != 0) {
return;
}

_actionCache.Dispose();
}
}

public class GPOLocalGroupProcessor {
private static readonly Regex KeyRegex = new(@"(.+?)\s*=(.*)", RegexOptions.Compiled);

Expand All @@ -30,8 +63,6 @@ public class GPOLocalGroupProcessor {
private static readonly Regex ExtractRid =
new(@"S-1-5-32-([0-9]{3})", RegexOptions.Compiled | RegexOptions.IgnoreCase);

private static readonly ConcurrentDictionary<string, List<GroupAction>> GpoActionCache = new();

private static readonly Dictionary<string, LocalGroupRids> ValidGroupNames =
new(StringComparer.OrdinalIgnoreCase) {
{ "Administrators", LocalGroupRids.Administrators },
Expand All @@ -43,9 +74,46 @@ public class GPOLocalGroupProcessor {
private readonly ILogger _log;

private readonly ILdapUtils _utils;
private readonly ActionCache _actionCache;

internal sealed class ActionCache : IDisposable {
private readonly ConcurrentDictionary<string, Lazy<Task<List<GroupAction>>>> _buildTasks =
new(StringComparer.OrdinalIgnoreCase);
private int _disposed;

public Lazy<Task<List<GroupAction>>> GetOrAddBuildTask(string distinguishedName,
Func<Lazy<Task<List<GroupAction>>>> buildTaskFactory) {
ThrowIfDisposed();
return _buildTasks.GetOrAdd(distinguishedName, _ => buildTaskFactory());
}

public void RemoveBuildTask(string distinguishedName, Lazy<Task<List<GroupAction>>> buildTask) {
ThrowIfDisposed();
((ICollection<KeyValuePair<string, Lazy<Task<List<GroupAction>>>>>)_buildTasks).Remove(
new KeyValuePair<string, Lazy<Task<List<GroupAction>>>>(distinguishedName, buildTask));
}

public void Dispose() {
if (Interlocked.Exchange(ref _disposed, 1) != 0) {
return;
}

_buildTasks.Clear();
}

public GPOLocalGroupProcessor(ILdapUtils utils, ILogger log = null) {
private void ThrowIfDisposed() {
if (Volatile.Read(ref _disposed) != 0) {
throw new ObjectDisposedException(nameof(GPOLocalGroupProcessorContext));
}
}
}

public GPOLocalGroupProcessor(ILdapUtils utils, ILogger log = null) : this(utils, new ActionCache(), log) {
}

internal GPOLocalGroupProcessor(ILdapUtils utils, ActionCache actionCache, ILogger log = null) {
_utils = utils;
_actionCache = actionCache;
_log = log ?? Logging.LogProvider.CreateLogger("GPOLocalGroupProc");
}

Expand Down Expand Up @@ -124,36 +192,22 @@ public async Task<ResultingGPOChanges> ReadGPOLocalGroups(string gpLink, string
foreach (var rid in Enum.GetValues(typeof(LocalGroupRids))) data[(LocalGroupRids)rid] = new GroupResults();

foreach (var linkDn in orderedLinks) {
if (!GpoActionCache.TryGetValue(linkDn.ToLower(), out var actions)) {
actions = new List<GroupAction>();

var gpoDomain = Helpers.DistinguishedNameToDomain(linkDn);
var result = await _utils.Query(new LdapQueryParameters() {
LDAPFilter = new LdapFilter().AddAllObjects().GetFilter(),
SearchScope = SearchScope.Base,
Attributes = [LDAPProperties.GPCFileSYSPath, LDAPProperties.Flags],
SearchBase = linkDn,
DomainName = gpoDomain
}).DefaultIfEmpty(LdapResult<IDirectoryObject>.Fail()).FirstOrDefaultAsync();

if (!result.IsSuccess) {
continue;
}

if (!result.Value.TryGetProperty(LDAPProperties.GPCFileSYSPath, out var filePath) ||
// Filter out GPOs that are disabled or the computer configuration is disabled
(result.Value.TryGetProperty(LDAPProperties.Flags, out var flags) && flags is "2" or "3")) {
GpoActionCache.TryAdd(linkDn, actions);
continue;
}

//Add the actions for each file. The GPO template file actions will override the XML file actions
await foreach (var item in ProcessGPOXmlFile(filePath, gpoDomain)) actions.Add(item);
await foreach (var item in ProcessGPOTemplateFile(filePath, gpoDomain)) actions.Add(item);
var buildTask = _actionCache.GetOrAddBuildTask(linkDn,
() => new Lazy<Task<List<GroupAction>>>(() => BuildGPOActionCache(linkDn),
LazyThreadSafetyMode.ExecutionAndPublication));
List<GroupAction> actions;
try {
actions = await buildTask.Value;
} catch {
_actionCache.RemoveBuildTask(linkDn, buildTask);
throw;
}

//Cache the actions for this GPO for later
GpoActionCache.TryAdd(linkDn.ToLower(), actions);
// Query failures are not cached so a later attempt can retry the GPO.
if (actions == null) {
_actionCache.RemoveBuildTask(linkDn, buildTask);
continue;
}

//If there are no actions, then we can move on from this GPO
if (actions.Count == 0)
Expand Down Expand Up @@ -248,6 +302,33 @@ public async Task<ResultingGPOChanges> ReadGPOLocalGroups(string gpLink, string
return ret;
}

private async Task<List<GroupAction>> BuildGPOActionCache(string linkDn) {
var actions = new List<GroupAction>();
var gpoDomain = Helpers.DistinguishedNameToDomain(linkDn);
var result = await _utils.Query(new LdapQueryParameters() {
LDAPFilter = new LdapFilter().AddAllObjects().GetFilter(),
SearchScope = SearchScope.Base,
Attributes = [LDAPProperties.GPCFileSYSPath, LDAPProperties.Flags],
SearchBase = linkDn,
DomainName = gpoDomain
}).DefaultIfEmpty(LdapResult<IDirectoryObject>.Fail()).FirstOrDefaultAsync();

if (!result.IsSuccess) {
return null;
}

if (!result.Value.TryGetProperty(LDAPProperties.GPCFileSYSPath, out var filePath) ||
// Filter out GPOs that are disabled or the computer configuration is disabled
(result.Value.TryGetProperty(LDAPProperties.Flags, out var flags) && flags is "2" or "3")) {
return actions;
}

//Add the actions for each file. The GPO template file actions will override the XML file actions
await foreach (var item in ProcessGPOXmlFile(filePath, gpoDomain)) actions.Add(item);
await foreach (var item in ProcessGPOTemplateFile(filePath, gpoDomain)) actions.Add(item);
return actions;
}

/// <summary>
/// Parses a GPO GptTmpl.inf file and pulls group membership changes out
/// </summary>
Expand Down Expand Up @@ -576,4 +657,4 @@ internal enum LocalGroupRids {
PSRemote = 580
}
}
}
}
Loading
Loading