Merge pull request #84 from BloodHoundAD/fix_bad_coll_method

ADCS Data collection consistency fixes
BloodHoundAD · Jan 19, 2024 · 38a81f3 · 38a81f3
2 parents 3b9947b + a1c335b
commit 38a81f3
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 89 deletions.
diff --git a/src/BaseContext.cs b/src/BaseContext.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.IO;
 using System.Threading;
 using System.Threading.Tasks;
@@ -64,6 +65,8 @@ public void UpdateLoopTime()
             CurrentLoopTime = $"{DateTime.Now:yyyyMMddHHmmss}";
         }
 
+        public HashSet<string> CollectedDomainSids { get; } = new();
+
         public async Task DoDelay()
         {
             if (Throttle == 0)

diff --git a/src/Client/Context.cs b/src/Client/Context.cs
@@ -75,5 +75,6 @@ public interface IContext
         string ResolveFileName(string filename, string extension, bool addTimestamp);
         EnumerationDomain[] Domains { get; set; }
         void UpdateLoopTime();
+        public HashSet<string> CollectedDomainSids { get; }
     }
 }
diff --git a/src/Producers/BaseProducer.cs b/src/Producers/BaseProducer.cs
@@ -130,7 +130,7 @@ protected LDAPData CreateDefaultNCData()
 
                 if ((methods & ResolvedCollectionMethod.DCRegistry) != 0)
                 {
-                    query = query.AddComputers();
+                    query = query.AddComputers(CommonFilters.DomainControllers);
                     props.AddRange(CommonProperties.ComputerMethodProps);
                 }
             }
@@ -169,6 +169,9 @@ protected LDAPData CreateConfigNCData()
             {
                 query = allObjectTypesQuery;
                 props.AddRange(CommonProperties.CertAbuseProps);
+                props.AddRange(CommonProperties.ObjectPropsProps);
+                props.AddRange(CommonProperties.ContainerProps);
+                props.AddRange(CommonProperties.ACLProps);
             }
 
             if ((methods & ResolvedCollectionMethod.Container) != 0)

diff --git a/src/Producers/LdapProducer.cs b/src/Producers/LdapProducer.cs
@@ -31,6 +31,11 @@ public override async Task Produce()
             var log = Context.Logger;
             var utils = Context.LDAPUtils;
 
+            if (string.IsNullOrEmpty(ldapData.Filter.GetFilter()))
+            {
+                return;
+            }
+
             if (Context.Flags.CollectAllProperties)
             {
                 log.LogDebug("CollectAllProperties set. Changing LDAP properties to *");
@@ -39,7 +44,7 @@ public override async Task Produce()
 
             foreach (var domain in Context.Domains)
             {
-                Context.Logger.LogInformation("Beginning LDAP search for {Domain}", domain);
+                Context.Logger.LogInformation("Beginning LDAP search for {Domain}", domain.Name);
                 //Do a basic  LDAP search and grab results
                 var successfulConnect = false;
                 try
@@ -59,14 +64,7 @@ public override async Task Produce()
                     continue;
                 }
 
-                await OutputChannel.Writer.WriteAsync(new Domain
-                {
-                    ObjectIdentifier = domain.DomainSid,
-                    Properties = new Dictionary<string, object>
-                    {
-                        { "collected", true },
-                    }
-                });
+                Context.CollectedDomainSids.Add(domain.DomainSid);
 
                 foreach (var searchResult in Context.LDAPUtils.QueryLDAP(ldapData.Filter.GetFilter(), SearchScope.Subtree,
                              ldapData.Props.Distinct().ToArray(), cancellationToken, domain.Name,
@@ -83,7 +81,6 @@ await OutputChannel.Writer.WriteAsync(new Domain
                     Context.Logger.LogTrace("Producer wrote {DistinguishedName} to channel", searchResult.DistinguishedName);
                 }
             }
-
         }
 
         /// <summary>
@@ -105,19 +102,18 @@ public override async Task ProduceConfigNC()
                 if (!configurationNCsCollected.Contains(configAdsPath))
                 {
                     Context.Logger.LogInformation("Beginning LDAP search for {Domain} Configuration NC", domain.Name);
+                    // Ensure we only collect the Configuration NC once per forest
+                    configurationNCsCollected.Add(configAdsPath);
 
                     //Do a basic LDAP search and grab results
                     foreach (var searchResult in Context.LDAPUtils.QueryLDAP(configNcData.Filter.GetFilter(), SearchScope.Subtree,
                                 configNcData.Props.Distinct().ToArray(), cancellationToken, domain.Name,
                                 adsPath: configAdsPath,
-                                includeAcl: (Context.ResolvedCollectionMethods & ResolvedCollectionMethod.ACL) != 0))
+                                includeAcl: (Context.ResolvedCollectionMethods & ResolvedCollectionMethod.ACL) != 0 || (Context.ResolvedCollectionMethods & ResolvedCollectionMethod.CertServices) != 0))
                     {
                         await Channel.Writer.WriteAsync(searchResult, cancellationToken);
                         Context.Logger.LogTrace("Producer wrote {DistinguishedName} to channel", searchResult.DistinguishedName);
                     }
-
-                    // Ensure we only collect the Configuration NC once per forest
-                    configurationNCsCollected.Add(configAdsPath);
                 }
                 else
                 {

diff --git a/src/Runtime/CollectionTask.cs b/src/Runtime/CollectionTask.cs
@@ -22,6 +22,7 @@ public class CollectionTask
         private readonly OutputWriter _outputWriter;
         private readonly BaseProducer _producer;
         private readonly List<Task> _taskPool = new();
+        private const string EnterpriseDCSuffix = "S-1-5-9";
 
         public CollectionTask(IContext context)
         {
@@ -82,7 +83,19 @@ internal async Task<string> StartCollection()
             _log.LogInformation("Consumers finished, closing output channel");
 
             foreach (var wkp in _context.LDAPUtils.GetWellKnownPrincipalOutput(_context.DomainName))
+            {
+                if (!wkp.ObjectIdentifier.EndsWith(EnterpriseDCSuffix))
+                {
+                    wkp.Properties["reconcile"] = false;
+                }
+                else if (wkp is Group g && g.Members.Length == 0)
+                {
+                    continue;
+                }
+
                 await _outputChannel.Writer.WriteAsync(wkp);
+            }
+
 
             _outputChannel.Writer.Complete();
             _compStatusChannel?.Writer.Complete();

diff --git a/src/Runtime/LDAPConsumer.cs b/src/Runtime/LDAPConsumer.cs
@@ -37,6 +37,11 @@ internal static async Task ConsumeSearchResults(Channel<ISearchResultEntry> inpu
                         watch.Elapsed.TotalMilliseconds, res.DisplayName);
                     if (processed == null)
                         continue;
+
+                    if (processed is Domain d && context.CollectedDomainSids.Contains(d.ObjectIdentifier))
+                    {
+                        d.Properties.Add("collected", true);
+                    }
                     await outputChannel.Writer.WriteAsync(processed);
                 }
                 catch (Exception e)