fix: missing domains in query for GPOLocalGroups, tighten up an error…

… condition in the connection pool manager
SpecterOps · Jul 26, 2024 · d172461 · d172461
1 parent b114fb9
commit d172461
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 5 deletions.
diff --git a/src/CommonLib/ConnectionPoolManager.cs b/src/CommonLib/ConnectionPoolManager.cs
@@ -43,6 +43,9 @@ public void ReleaseConnection(LdapConnectionWrapper connectionWrapper, bool conn
 
         public async Task<(bool Success, LdapConnectionWrapper ConnectionWrapper, string Message)> GetLdapConnection(
             string identifier, bool globalCatalog) {
+            if (identifier == null) {
+                return (false, default, "Provided a null identifier for the connection");
+            }
             var resolved = ResolveIdentifier(identifier);
 
             if (!_pools.TryGetValue(resolved, out var pool)) {
@@ -72,8 +75,7 @@ private string ResolveIdentifier(string identifier) {
             if (_resolvedIdentifiers.TryGetValue(identifier, out var resolved)) {
                 return resolved;
             }
-
-
+
             if (GetDomainSidFromDomainName(identifier, out var sid)) {
                 _log.LogDebug("Resolved identifier {Identifier} to {Resolved}", identifier, sid);
                 _resolvedIdentifiers.TryAdd(identifier, sid);

diff --git a/src/CommonLib/Processors/GPOLocalGroupProcessor.cs b/src/CommonLib/Processors/GPOLocalGroupProcessor.cs
@@ -54,7 +54,7 @@ public Task<ResultingGPOChanges> ReadGPOLocalGroups(IDirectoryObject entry) {
                 return ReadGPOLocalGroups(links, dn);    
             }
 
-            return default;
+            return Task.FromResult(new ResultingGPOChanges());
         }
 
         public async Task<ResultingGPOChanges> ReadGPOLocalGroups(string gpLink, string distinguishedName) {
@@ -63,13 +63,15 @@ public async Task<ResultingGPOChanges> ReadGPOLocalGroups(string gpLink, string
             if (gpLink == null)
                 return ret;
 
+            var domain = Helpers.DistinguishedNameToDomain(distinguishedName);
             // First lets check if this OU actually has computers that it contains. If not, then we'll ignore it.
             // Its cheaper to fetch the affected computers from LDAP first and then process the GPLinks
             var affectedComputers = new List<TypedPrincipal>();
             await foreach (var result in _utils.Query(new LdapQueryParameters() {
                                LDAPFilter = new LdapFilter().AddComputersNoMSAs().GetFilter(),
                                Attributes = CommonProperties.ObjectSID,
-                               SearchBase = distinguishedName
+                               SearchBase = distinguishedName,
+                               DomainName = domain
                            })) {
                 if (!result.IsSuccess) {
                     break;
@@ -119,7 +121,8 @@ public async Task<ResultingGPOChanges> ReadGPOLocalGroups(string gpLink, string
                         LDAPFilter = new LdapFilter().AddAllObjects().GetFilter(),
                         SearchScope = SearchScope.Base,
                         Attributes = CommonProperties.GPCFileSysPath,
-                        SearchBase = linkDn
+                        SearchBase = linkDn,
+                        DomainName = gpoDomain
                     }).DefaultIfEmpty(LdapResult<IDirectoryObject>.Fail()).FirstOrDefaultAsync();
 
                     if (!result.IsSuccess) {