Skip to content

Feature: Add collection of Active Directory Sites ACLs and data #186

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

Open
wants to merge 2 commits into
base: 2.X
Choose a base branch
from
+160 −11
Open

Feature: Add collection of Active Directory Sites ACLs and data #186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d54077
feat: Process data for Active Directory sites
q-roland Nov 6, 2025
ebc3743
chore: Add the "Site" collection method to help message
q-roland Nov 6, 2025
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
1 change: 1 addition & 0 deletions src/Client/Enums.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public enum CollectionMethodOptions
LdapServices,
SmbInfo,
NTLMRegistry,
Site,
// Re-introduce this when we're ready for Event Log collection
// EventLogs,
All
Expand Down
3 changes: 2 additions & 1 deletion src/Options.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class Options
// Options that affect what is collected
[Option('c', "collectionmethods", Default = new[] { "Default" },
HelpText =
"Collection Methods: Group, LocalGroup, LocalAdmin, RDP, DCOM, PSRemote, Session, Trusts, ACL, Container, ComputerOnly, GPOLocalGroup, LoggedOn, ObjectProps, SPNTargets, UserRights, Default, DCOnly, CARegistry, DCRegistry, CertServices, WebClientService, LdapServices, SmbInfo, NTLMRegistry, All")]
"Collection Methods: Group, LocalGroup, LocalAdmin, RDP, DCOM, PSRemote, Session, Trusts, ACL, Container, ComputerOnly, GPOLocalGroup, LoggedOn, ObjectProps, SPNTargets, UserRights, Default, DCOnly, CARegistry, DCRegistry, CertServices, Site, WebClientService, LdapServices, SmbInfo, NTLMRegistry, All")]
public IEnumerable<string> CollectionMethods { get; set; }

[Option('d', "domain", Default = null, HelpText = "Specify domain to enumerate")]
Expand Down Expand Up @@ -208,6 +208,7 @@ internal bool ResolveCollectionMethods(ILogger logger, out CollectionMethod reso
CollectionMethodOptions.LdapServices => CollectionMethod.LdapServices,
CollectionMethodOptions.SmbInfo => CollectionMethod.SmbInfo,
CollectionMethodOptions.NTLMRegistry => CollectionMethod.NTLMRegistry,
CollectionMethodOptions.Site => CollectionMethod.Site,
// Re-introduce this when we're ready for Event Log collection
// CollectionMethodOptions.EventLogs => CollectionMethod.EventLogs,
CollectionMethodOptions.All => CollectionMethod.All,
Expand Down
128 changes: 128 additions & 0 deletions src/Runtime/ObjectProcessors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public class ObjectProcessors {
private readonly SPNProcessors _spnProcessor;
private readonly WebClientServiceProcessor _webClientProcessor;
private readonly SmbProcessor _smbProcessor;
private readonly SiteProcessor _siteProcessor;
private readonly ConcurrentDictionary<string, RegistryProcessor> _registryProcessorMap = new();
public ObjectProcessors(IContext context, ILogger log) {
_context = context;
Expand All @@ -60,6 +61,7 @@ public ObjectProcessors(IContext context, ILogger log) {
_localGroupProcessor = new LocalGroupProcessor(context.LDAPUtils);
_webClientProcessor = new WebClientServiceProcessor(log);
_smbProcessor = new SmbProcessor(context.PortScanTimeout);
_siteProcessor = new SiteProcessor(context.LDAPUtils);
_methods = context.ResolvedCollectionMethods;
_cancellationToken = context.CancellationTokenSource.Token;
_log = log;
Expand Down Expand Up @@ -95,6 +97,12 @@ internal async Task<OutputBase> ProcessObject(IDirectoryObject entry,
return await ProcessCertTemplate(entry, resolvedSearchResult);
case Label.IssuancePolicy:
return await ProcessIssuancePolicy(entry, resolvedSearchResult);
case Label.Site:
return await ProcessSiteObject(entry, resolvedSearchResult);
case Label.SiteServer:
return await ProcessSiteServerObject(entry, resolvedSearchResult);
case Label.SiteSubnet:
return await ProcessSiteSubnetObject(entry, resolvedSearchResult);
case Label.Base:
return null;
default:
Expand Down Expand Up @@ -926,5 +934,125 @@ private async Task<IssuancePolicy> ProcessIssuancePolicy(IDirectoryObject entry,

return ret;
}

private async Task<Site> ProcessSiteObject(IDirectoryObject entry,
ResolvedSearchResult resolvedSearchResult)
{
var ret = new Site
{
ObjectIdentifier = resolvedSearchResult.ObjectId
};

ret.Properties = new Dictionary<string, object>(GetCommonProperties(entry, resolvedSearchResult));

if (_methods.HasFlag(CollectionMethod.ACL))
{
var aces = await _aclProcessor.ProcessACL(resolvedSearchResult, entry, true)
.ToArrayAsync(cancellationToken: _cancellationToken);
ret.Properties.Add("doesanyacegrantownerrights", aces.Any(ace => ace.IsPermissionForOwnerRightsSid));
ret.Properties.Add("doesanyinheritedacegrantownerrights", aces.Any(ace => ace.IsInheritedPermissionForOwnerRightsSid));
ret.Aces = aces;
ret.IsACLProtected = _aclProcessor.IsACLProtected(entry);
ret.Properties.Add("isaclprotected", ret.IsACLProtected);
}

if (_methods.HasFlag(CollectionMethod.ObjectProps))
{
ret.Properties =
ContextUtils.Merge(LdapPropertyProcessor.ReadSiteProperties(entry), ret.Properties);
if (_context.Flags.CollectAllProperties)
{
ret.Properties = ContextUtils.Merge(_ldapPropertyProcessor.ParseAllProperties(entry),
ret.Properties);
}
}

ret.Links = await _siteProcessor.ReadSiteGPLinks(resolvedSearchResult, entry).ToArrayAsync();

return ret;
}

private async Task<SiteServer> ProcessSiteServerObject(IDirectoryObject entry,
ResolvedSearchResult resolvedSearchResult)
{
var ret = new SiteServer
{
ObjectIdentifier = resolvedSearchResult.ObjectId
};

ret.Properties = new Dictionary<string, object>(GetCommonProperties(entry, resolvedSearchResult));


if (await _siteProcessor.GetContainingSiteForServer(entry) is (true, var container))
{
ret.ContainedBy = container;
}
Comment on lines +986 to +989
Copy link

@coderabbitai coderabbitai bot Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify: Should containing site lookup be conditional on Container collection method?

Lines 986-989 get the containing site for the server unconditionally. Other similar methods only retrieve containment relationships when the Container collection method is enabled:

  • ProcessUserObject (lines 205-212) checks Container flag
  • ProcessComputerObject (lines 274-279) checks Container flag
  • ProcessGroupObject (lines 482-487) checks Container flag

Run the following script to examine containment patterns:

🏁 Script executed:

#!/bin/bash
# Search for ContainedBy assignment patterns to verify conditional checks

rg -nP -B5 'ret\.ContainedBy\s*=' --type=cs | rg -A5 -B5 'ProcessObject|HasFlag.*Container'

Length of output: 6055

🏁 Script executed:

#!/bin/bash
# View ProcessSiteServerObject method around lines 975-1005 to verify Container check presence
sed -n '975,1005p' src/Runtime/ObjectProcessors.cs | cat -n

Length of output: 74

🏁 Script executed:

#!/bin/bash
# View ProcessSiteServerObject method around lines 975-1005
sed -n '975,1005p' src/Runtime/ObjectProcessors.cs | cat -n

Length of output: 1717

🏁 Script executed:

#!/bin/bash
# View ProcessSiteSubnetObject method around lines 1016-1056 to verify its containment pattern
sed -n '1016,1056p' src/Runtime/ObjectProcessors.cs | cat -n

Length of output: 2208

Wrap containing site lookups in Container collection method checks

Verification confirms the issue. All 12 other object processors guard ContainedBy assignments with HasFlag(CollectionMethod.Container) checks, but:

  1. ProcessSiteServerObject (lines 986-989): Retrieves containing site unconditionally—needs Container flag guard
  2. ProcessSiteSubnetObject (lines 1049-1052): Retrieves containing site inside the ObjectProps block instead of a Container block—should be conditional on Container flag (the comment about needing siteObject property explains data dependencies, but architectural responsibility for containment should follow Container flag, not ObjectProps)

Both deviations break consistency with ProcessUserObject, ProcessComputerObject, ProcessGroupObject, and all domain/certificate processors.

🤖 Prompt for AI Agents 
In src/Runtime/ObjectProcessors.cs around lines 986-989 and 1049-1052, the code
retrieves the containing site unconditionally (lines 986-989) and inside the
ObjectProps block (lines 1049-1052); wrap these containing-site lookups and the
assignments to ret.ContainedBy with a guard that checks
entry.Methods.HasFlag(CollectionMethod.Container) (i.e., only perform the
_siteProcessor.GetContainingSiteForServer / GetContainingSiteForSubnet calls and
set ret.ContainedBy when the Container flag is present), and move the subnet
lookup out of the ObjectProps block into the Container-guarded section so
containment logic matches the other processors.


if (_methods.HasFlag(CollectionMethod.ACL))
{
var aces = await _aclProcessor.ProcessACL(resolvedSearchResult, entry, true)
.ToArrayAsync(cancellationToken: _cancellationToken);
ret.Properties.Add("doesanyacegrantownerrights", aces.Any(ace => ace.IsPermissionForOwnerRightsSid));
ret.Properties.Add("doesanyinheritedacegrantownerrights", aces.Any(ace => ace.IsInheritedPermissionForOwnerRightsSid));
ret.Aces = aces;
ret.IsACLProtected = _aclProcessor.IsACLProtected(entry);
ret.Properties.Add("isaclprotected", ret.IsACLProtected);
}

if (_methods.HasFlag(CollectionMethod.ObjectProps))
{
ret.Properties =
ContextUtils.Merge(LdapPropertyProcessor.ReadSiteServerProperties(entry), ret.Properties);
if (_context.Flags.CollectAllProperties)
{
ret.Properties = ContextUtils.Merge(_ldapPropertyProcessor.ParseAllProperties(entry),
ret.Properties);
}
}

return ret;
}

private async Task<SiteSubnet> ProcessSiteSubnetObject(IDirectoryObject entry,
ResolvedSearchResult resolvedSearchResult)
{
var ret = new SiteSubnet
{
ObjectIdentifier = resolvedSearchResult.ObjectId
};

ret.Properties = new Dictionary<string, object>(GetCommonProperties(entry, resolvedSearchResult));

if (_methods.HasFlag(CollectionMethod.ACL))
{
var aces = await _aclProcessor.ProcessACL(resolvedSearchResult, entry, true)
.ToArrayAsync(cancellationToken: _cancellationToken);
ret.Properties.Add("doesanyacegrantownerrights", aces.Any(ace => ace.IsPermissionForOwnerRightsSid));
ret.Properties.Add("doesanyinheritedacegrantownerrights", aces.Any(ace => ace.IsInheritedPermissionForOwnerRightsSid));
ret.Aces = aces;
ret.IsACLProtected = _aclProcessor.IsACLProtected(entry);
ret.Properties.Add("isaclprotected", ret.IsACLProtected);
}

if (_methods.HasFlag(CollectionMethod.ObjectProps))
{
ret.Properties =
ContextUtils.Merge(LdapPropertyProcessor.ReadSiteSubnetProperties(entry), ret.Properties);
if (_context.Flags.CollectAllProperties)
{
ret.Properties = ContextUtils.Merge(_ldapPropertyProcessor.ParseAllProperties(entry),
ret.Properties);
}

// Can only deduce containing site for a subnet if we read the object properties, including siteObject

if (await _siteProcessor.GetContainingSiteForSubnet(ret.Properties) is (true, var container))
{
ret.ContainedBy = container;
}
}

return ret;
}
}
}
39 changes: 29 additions & 10 deletions src/Runtime/OutputWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
using System;
using ICSharpCode.SharpZipLib.Core;
using ICSharpCode.SharpZipLib.Zip;
using Microsoft.Extensions.Logging;
using Sharphound.Client;
using Sharphound.Writers;
using SharpHoundCommonLib.Enums;
using SharpHoundCommonLib.OutputTypes;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.DirectoryServices;
using System.IO;
using System.Linq;
using System.Threading.Channels;
using System.Threading.Tasks;
using System.Timers;
using ICSharpCode.SharpZipLib.Core;
using ICSharpCode.SharpZipLib.Zip;
using Microsoft.Extensions.Logging;
using Sharphound.Client;
using Sharphound.Writers;
using SharpHoundCommonLib.Enums;
using SharpHoundCommonLib.OutputTypes;

namespace Sharphound.Runtime
{
Expand All @@ -34,6 +35,9 @@ public class OutputWriter
private readonly JsonDataWriter<NTAuthStore> _nTAuthStoreOutput;
private readonly JsonDataWriter<CertTemplate> _certTemplateOutput;
private readonly JsonDataWriter<IssuancePolicy> _issuancePolicyOutput;
private readonly JsonDataWriter<Site> _siteOutput;
private readonly JsonDataWriter<SiteServer> _siteServerOutput;
private readonly JsonDataWriter<SiteSubnet> _siteSubnetOutput;


private int _completedCount;
Expand All @@ -57,6 +61,9 @@ public OutputWriter(IContext context, Channel<OutputBase> outputChannel)
_nTAuthStoreOutput = new JsonDataWriter<NTAuthStore>(_context, DataType.NTAuthStores);
_certTemplateOutput = new JsonDataWriter<CertTemplate>(_context, DataType.CertTemplates);
_issuancePolicyOutput = new JsonDataWriter<IssuancePolicy>(_context, DataType.IssuancePolicies);
_siteOutput = new JsonDataWriter<Site>(_context, DataType.Sites);
_siteServerOutput = new JsonDataWriter<SiteServer>(_context, DataType.SiteServers);
_siteSubnetOutput = new JsonDataWriter<SiteSubnet>(_context, DataType.SiteSubnets);

_runTimer = new Stopwatch();
_statusTimer = new Timer(_context.StatusInterval);
Expand Down Expand Up @@ -145,12 +152,20 @@ internal async Task<string> StartWriter()
case IssuancePolicy issuancePolicy:
await _issuancePolicyOutput.AcceptObject(issuancePolicy);
break;
case Site site:
await _siteOutput.AcceptObject(site);
break;
case SiteServer siteServer:
await _siteServerOutput.AcceptObject(siteServer);
break;
case SiteSubnet siteSubnet:
await _siteSubnetOutput.AcceptObject(siteSubnet);
break;
default:
throw new ArgumentOutOfRangeException(nameof(item));
}
}

Console.WriteLine("Closing writers");
return await FlushWriters();
}

Expand All @@ -169,6 +184,9 @@ private async Task<string> FlushWriters()
await _nTAuthStoreOutput.FlushWriter();
await _certTemplateOutput.FlushWriter();
await _issuancePolicyOutput.FlushWriter();
await _siteOutput.FlushWriter();
await _siteServerOutput.FlushWriter();
await _siteSubnetOutput.FlushWriter();
CloseOutput();
var fileName = ZipFiles();
return fileName;
Expand Down Expand Up @@ -198,7 +216,8 @@ private string ZipFiles()
_containerOutput.GetFilename(), _domainOutput.GetFilename(), _gpoOutput.GetFilename(),
_ouOutput.GetFilename(), _rootCAOutput.GetFilename(), _aIACAOutput.GetFilename(),
_enterpriseCAOutput.GetFilename(), _nTAuthStoreOutput.GetFilename(),
_certTemplateOutput.GetFilename(),_issuancePolicyOutput.GetFilename()
_certTemplateOutput.GetFilename(),_issuancePolicyOutput.GetFilename(),
_siteOutput.GetFilename(), _siteServerOutput.GetFilename(), _siteSubnetOutput.GetFilename(),
});

foreach (var entry in fileList.Where(x => !string.IsNullOrEmpty(x)))
Expand Down