Merge pull request #40 from BloodHoundAD/support-exceptions-in-QueryL…

…DAP-UAR Support exceptions in QueryLDAP methods (user_assignment_rights edition)
SpecterOps · Sep 28, 2022 · 1660da3 · 1660da3
2 parents 513b813 + 668606a
commit 1660da3
Show file tree

Hide file tree

Showing 9 changed files with 263 additions and 112 deletions.
diff --git a/.gitignore b/.gitignore
@@ -131,11 +131,7 @@ tags
 # Visual Studio Code
 ################
 
-.vscode/*
-!.vscode/settings.json
-!.vscode/tasks.json
-!.vscode/launch.json
-!.vscode/extensions.json
+.vscode
 *.code-workspace
 
 # Local History for Visual Studio Code

diff --git a/src/CommonLib/Exceptions/SharpHoundCommonException.cs b/src/CommonLib/Exceptions/SharpHoundCommonException.cs
@@ -0,0 +1,11 @@
+using System;
+
+namespace SharpHoundCommonLib.Exceptions
+{
+    public class LDAPQueryException : Exception
+    {
+        public LDAPQueryException() { }
+        public LDAPQueryException(string message) : base(message) { }
+        public LDAPQueryException(string message, Exception inner) : base(message, inner) { }
+    }
+}
diff --git a/src/CommonLib/ILDAPUtils.cs b/src/CommonLib/ILDAPUtils.cs
@@ -24,6 +24,7 @@ public struct LDAPQueryOptions
         public string AdsPath;
         public bool GlobalCatalog;
         public bool SkipCache;
+        public bool ThrowException;
     }
 
     public interface ILDAPUtils
@@ -98,10 +99,12 @@ public interface ILDAPUtils
         ///     Skip the connection cache and force a new connection. You must dispose of this connection
         ///     yourself.
         /// </param>
+        /// <param name="throwException">Throw exceptions rather than logging the errors directly</param>
         /// <returns>All LDAP search results matching the specified parameters</returns>
         IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, SearchScope scope,
             string[] props, CancellationToken cancellationToken, string domainName = null, bool includeAcl = false,
-            bool showDeleted = false, string adsPath = null, bool globalCatalog = false, bool skipCache = false);
+            bool showDeleted = false, string adsPath = null, bool globalCatalog = false, bool skipCache = false,
+            bool throwException = false);
 
         /// <summary>
         ///     Performs an LDAP query using the parameters specified by the user.
@@ -118,10 +121,11 @@ IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, SearchScope scope,
         ///     Skip the connection cache and force a new connection. You must dispose of this connection
         ///     yourself.
         /// </param>
+        /// <param name="throwException">Throw exceptions rather than logging the errors directly</param>
         /// <returns>All LDAP search results matching the specified parameters</returns>
         IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, SearchScope scope,
             string[] props, string domainName = null, bool includeAcl = false, bool showDeleted = false,
-            string adsPath = null, bool globalCatalog = false, bool skipCache = false);
+            string adsPath = null, bool globalCatalog = false, bool skipCache = false, bool throwException = false);
 
         Forest GetForest(string domainName = null);
 

diff --git a/src/CommonLib/LDAPUtils.cs b/src/CommonLib/LDAPUtils.cs
diff --git a/test/unit/ContainerProcessorTest.cs b/test/unit/ContainerProcessorTest.cs
@@ -100,7 +100,7 @@ public void ContainerProcessor_GetContainerChildObjects_ReturnsCorrectData()
 
             mock.Setup(x => x.QueryLDAP(It.IsAny<string>(), It.IsAny<SearchScope>(), It.IsAny<string[]>(),
                 It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<bool>(),
-                It.IsAny<bool>())).Returns(searchResults);
+                It.IsAny<bool>(), It.IsAny<bool>())).Returns(searchResults);
 
             var processor = new ContainerProcessor(mock.Object);
             var test = processor.GetContainerChildObjects(_testGpLinkString).ToArray();

diff --git a/test/unit/DomainTrustProcessorTest.cs b/test/unit/DomainTrustProcessorTest.cs
@@ -39,7 +39,7 @@ public void DomainTrustProcessor_EnumerateDomainTrusts_HappyPath()
 
             mockUtils.Setup(x => x.QueryLDAP(It.IsAny<string>(), It.IsAny<SearchScope>(), It.IsAny<string[]>(),
                 It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<bool>(),
-                It.IsAny<bool>())).Returns(searchResults);
+                It.IsAny<bool>(), It.IsAny<bool>())).Returns(searchResults);
             var processor = new DomainTrustProcessor(mockUtils.Object);
             var test = processor.EnumerateDomainTrusts("testlab.local").ToArray();
             Assert.Single(test);
@@ -96,7 +96,7 @@ public void DomainTrustProcessor_EnumerateDomainTrusts_SadPaths()
 
             mockUtils.Setup(x => x.QueryLDAP(It.IsAny<string>(), It.IsAny<SearchScope>(), It.IsAny<string[]>(),
                 It.IsAny<string>(), It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<string>(), It.IsAny<bool>(),
-                It.IsAny<bool>())).Returns(searchResults);
+                It.IsAny<bool>(), It.IsAny<bool>())).Returns(searchResults);
             var processor = new DomainTrustProcessor(mockUtils.Object);
             var test = processor.EnumerateDomainTrusts("testlab.local");
             Assert.Empty(test);

diff --git a/test/unit/Facades/MockLDAPUtils.cs b/test/unit/Facades/MockLDAPUtils.cs
@@ -40,10 +40,10 @@ public string[] GetUserGlobalCatalogMatches(string name)
             name = name.ToLower();
             return name switch
             {
-                "dfm" => new[] {"S-1-5-21-3130019616-2776909439-2417379446-1105"},
+                "dfm" => new[] { "S-1-5-21-3130019616-2776909439-2417379446-1105" },
                 "administrator" => new[]
                     {"S-1-5-21-3130019616-2776909439-2417379446-500", "S-1-5-21-3084884204-958224920-2707782874-500"},
-                "admin" => new[] {"S-1-5-21-3130019616-2776909439-2417379446-2116"},
+                "admin" => new[] { "S-1-5-21-3130019616-2776909439-2417379446-2116" },
                 _ => Array.Empty<string>()
             };
         }
@@ -1023,15 +1023,15 @@ public virtual IEnumerable<ISearchResultEntry> QueryLDAP(LDAPQueryOptions option
         public virtual IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, SearchScope scope, string[] props,
             CancellationToken cancellationToken,
             string domainName = null, bool includeAcl = false, bool showDeleted = false, string adsPath = null,
-            bool globalCatalog = false, bool skipCache = false)
+            bool globalCatalog = false, bool skipCache = false, bool throwException = false)
         {
             throw new NotImplementedException();
         }
 
         public virtual IEnumerable<ISearchResultEntry> QueryLDAP(string ldapFilter, SearchScope scope, string[] props,
             string domainName = null,
             bool includeAcl = false, bool showDeleted = false, string adsPath = null, bool globalCatalog = false,
-            bool skipCache = false)
+            bool skipCache = false, bool throwException = false)
         {
             throw new NotImplementedException();
         }
@@ -1049,7 +1049,7 @@ public ActiveDirectorySecurityDescriptor MakeSecurityDescriptor()
 
         private Group GetBaseEnterpriseDC()
         {
-            var g = new Group {ObjectIdentifier = "TESTLAB.LOCAL-S-1-5-9".ToUpper()};
+            var g = new Group { ObjectIdentifier = "TESTLAB.LOCAL-S-1-5-9".ToUpper() };
             g.Properties.Add("name", "ENTERPRISE DOMAIN [email protected]".ToUpper());
             return g;
         }

diff --git a/test/unit/GPOLocalGroupProcessorTest.cs b/test/unit/GPOLocalGroupProcessorTest.cs
@@ -122,6 +122,7 @@ public async Task GPOLocalGroupProcessor_ReadGPOLocalGroups_AffectedComputers_0(
                 It.IsAny<bool>(),
                 It.IsAny<string>(),
                 It.IsAny<bool>(),
+                It.IsAny<bool>(),
                 It.IsAny<bool>()
             )).Returns(new List<ISearchResultEntry>());
             var processor = new GPOLocalGroupProcessor(mockLDAPUtils.Object);
@@ -143,7 +144,8 @@ public async Task GPOLocalGroupProcessor_ReadGPOLocalGroups_Null_Gpcfilesyspath(
             mockSearchResultEntry.Setup(x => x.GetSid()).Returns("teapot");
             var mockSearchResults = new List<ISearchResultEntry>();
             mockSearchResults.Add(mockSearchResultEntry.Object);
-            mockLDAPUtils.Setup(x => x.QueryLDAP(new LDAPQueryOptions
+            mockLDAPUtils.Setup(x => x.QueryLDAP(
+                new LDAPQueryOptions
                 {
                     Filter = "(samaccounttype=805306369)",
                     Scope = SearchScope.Subtree,

diff --git a/test/unit/LDAPUtilsTest.cs b/test/unit/LDAPUtilsTest.cs
@@ -3,6 +3,9 @@
 using Moq;
 using SharpHoundCommonLib;
 using SharpHoundCommonLib.Enums;
+using SharpHoundCommonLib.Exceptions;
+using System.DirectoryServices.Protocols;
+using System.Threading;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -106,6 +109,64 @@ public void GetWellKnownPrincipal_WithDomain_ConvertsSID()
             Assert.Equal($"{_testDomainName}-S-1-5-32-544", typedPrincipal.ObjectIdentifier);
         }
 
+        [Fact]
+        public void QueryLDAP_With_Exception()
+        {
+            var options = new LDAPQueryOptions
+            {
+                ThrowException = true
+            };
+
+            Assert.Throws<LDAPQueryException>(
+                () =>
+                {
+                    foreach (var sre in _utils.QueryLDAP(null, new SearchScope(), null, new CancellationToken(), null, false, false, null, false, false, true))
+                    {
+                        // We shouldn't reach this anyway, and all we care about is if exceptions are bubbling
+                    };
+                });
+
+            Assert.Throws<LDAPQueryException>(
+                () =>
+                {
+                    foreach (var sre in _utils.QueryLDAP(options))
+                    {
+                        // We shouldn't reach this anyway, and all we care about is if exceptions are bubbling
+                    };
+                });
+        }
+
+        [Fact]
+        public void QueryLDAP_Without_Exception()
+        {
+            Exception exception;
+
+            var options = new LDAPQueryOptions
+            {
+                ThrowException = false
+            };
+
+            exception = Record.Exception(
+                () =>
+                {
+                    foreach (var sre in _utils.QueryLDAP(null, new SearchScope(), null, new CancellationToken()))
+                    {
+                        // We shouldn't reach this anyway, and all we care about is if exceptions are bubbling
+                    };
+                });
+            Assert.Null(exception);
+
+            exception = Record.Exception(
+                () =>
+                {
+                    foreach (var sre in _utils.QueryLDAP(options))
+                    {
+                        // We shouldn't reach this anyway, and all we care about is if exceptions are bubbling
+                    };
+                });
+            Assert.Null(exception);
+        }
+
         #endregion
 
         #region Structural