From 05c35e2f1f5aad84bf8907d717b3a4f69b363c21 Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Thu, 8 Sep 2022 13:05:25 -0600 Subject: [PATCH 1/7] chore: prevent my vscode settings from getting committed --- .gitignore | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index fadb6a16..6e28db82 100644 --- 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 From 1d39b34ea44b0eb6968762be902d10d8917a14b9 Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Tue, 20 Sep 2022 10:27:45 -0600 Subject: [PATCH 2/7] feat: added optional exception throwing Deduplicated some code between QueryLDAP implementations --- .../Exceptions/SharpHoundCommonException.cs | 10 + src/CommonLib/ILDAPUtils.cs | 8 +- src/CommonLib/LDAPUtils.cs | 241 ++++++++++++------ test/unit/ContainerProcessorTest.cs | 2 +- test/unit/DomainTrustProcessorTest.cs | 4 +- test/unit/Facades/MockLDAPUtils.cs | 10 +- test/unit/GPOLocalGroupProcessorTest.cs | 13 +- 7 files changed, 192 insertions(+), 96 deletions(-) create mode 100644 src/CommonLib/Exceptions/SharpHoundCommonException.cs diff --git a/src/CommonLib/Exceptions/SharpHoundCommonException.cs b/src/CommonLib/Exceptions/SharpHoundCommonException.cs new file mode 100644 index 00000000..14c9bdaa --- /dev/null +++ b/src/CommonLib/Exceptions/SharpHoundCommonException.cs @@ -0,0 +1,10 @@ +using System; + +namespace SharpHoundCommonLib.Exceptions; + +public class SharpHoundCommonException : Exception +{ + public SharpHoundCommonException() { } + public SharpHoundCommonException(string message) : base(message) { } + public SharpHoundCommonException(string message, Exception inner) : base(message, inner) { } +} \ No newline at end of file diff --git a/src/CommonLib/ILDAPUtils.cs b/src/CommonLib/ILDAPUtils.cs index 9f7bd27e..8a328c6f 100644 --- 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. /// + /// Throw exceptions rather than logging the errors directly /// All LDAP search results matching the specified parameters IEnumerable 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); /// /// Performs an LDAP query using the parameters specified by the user. @@ -118,10 +121,11 @@ IEnumerable QueryLDAP(string ldapFilter, SearchScope scope, /// Skip the connection cache and force a new connection. You must dispose of this connection /// yourself. /// + /// Throw exceptions rather than logging the errors directly /// All LDAP search results matching the specified parameters IEnumerable 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 index 4553c2a8..67576154 100644 --- a/src/CommonLib/LDAPUtils.cs +++ b/src/CommonLib/LDAPUtils.cs @@ -16,6 +16,7 @@ using SharpHoundCommonLib.LDAPQueries; using SharpHoundCommonLib.OutputTypes; using SharpHoundCommonLib.Processors; +using SharpHoundCommonLib.Exceptions; using Domain = System.DirectoryServices.ActiveDirectory.Domain; using SearchScope = System.DirectoryServices.Protocols.SearchScope; using SecurityMasks = System.DirectoryServices.Protocols.SecurityMasks; @@ -182,7 +183,7 @@ public string[] GetUserGlobalCatalogMatches(string name) return sids; var query = new LDAPFilter().AddUsers($"samaccountname={tempName}").GetFilter(); - var results = QueryLDAP(query, SearchScope.Subtree, new[] {"objectsid"}, globalCatalog: true) + var results = QueryLDAP(query, SearchScope.Subtree, new[] { "objectsid" }, globalCatalog: true) .Select(x => x.GetSid()).Where(x => x != null).ToArray(); Cache.AddGCCache(tempName, results); return results; @@ -358,9 +359,8 @@ public IEnumerable DoRangedRetrieval(string distinguishedName, string at var currentRange = $"{baseString};range={index}-*"; var complete = false; - var searchRequest = CreateSearchRequest($"{attributeName}=*", SearchScope.Base, new[] {currentRange}, - domainName, - distinguishedName); + var searchRequest = CreateSearchRequest($"{attributeName}=*", SearchScope.Base, new[] { currentRange }, + domainName, distinguishedName); if (searchRequest == null) yield break; @@ -369,7 +369,7 @@ public IEnumerable DoRangedRetrieval(string distinguishedName, string at { SearchResponse response; - response = (SearchResponse) conn.SendRequest(searchRequest); + response = (SearchResponse)conn.SendRequest(searchRequest); //If we ever get more than one response from here, something is horribly wrong if (response?.Entries.Count == 1) @@ -633,6 +633,65 @@ public TypedPrincipal ResolveDistinguishedName(string dn) }; } + /// + /// Setup LDAP query for filter + /// + /// LDAP filter + /// SearchScope to query + /// LDAP properties to fetch for each object + /// Domain to query + /// Include deleted objects + /// ADS path to limit the query too + /// Use the global catalog instead of the regular LDAP server + /// + /// Skip the connection cache and force a new connection. You must dispose of this connection + /// yourself. + /// + /// Tuple of LdapConnection, SearchRequest, and SharpHoundCommonException + public Tuple SetupLDAPQueryFilter(string ldapFilter, + SearchScope scope, string[] props, string domainName = null, bool showDeleted = false, + string adsPath = null, bool globalCatalog = false, bool skipCache = false) + { + _log.LogTrace("Creating ldap connection for {Target} with filter {Filter}", + globalCatalog ? "Global Catalog" : "DC", ldapFilter); + var task = globalCatalog + ? Task.Run(() => CreateGlobalCatalogConnection(domainName, _ldapConfig.AuthType)) + : Task.Run(() => CreateLDAPConnection(domainName, skipCache, _ldapConfig.AuthType)); + + LdapConnection conn; + try + { + conn = task.ConfigureAwait(false).GetAwaiter().GetResult(); + } + catch (Exception e) + { + var errorString = String.Format("Exception getting LDAP connection for {0} and domain {1}", ldapFilter, + domainName ?? "Default Domain"); + return Tuple.Create(null, null, + new SharpHoundCommonException(errorString, e)); + } + + if (conn == null) + { + var errorString = String.Format("LDAP connection is null for filter {0} and domain {1}", ldapFilter, + domainName ?? "Default Domain"); + return Tuple.Create(null, null, + new SharpHoundCommonException(errorString)); + } + + var request = CreateSearchRequest(ldapFilter, scope, props, domainName, adsPath, showDeleted); + + if (request == null) + { + var errorString = String.Format("Search request is null for filter {0} and domain {1}", ldapFilter, + domainName ?? "Default Domain"); + return Tuple.Create(null, null, + new SharpHoundCommonException(errorString)); + } + + return Tuple.Create(conn, request, null); + } + /// /// Queries LDAP using LDAPQueryOptions /// @@ -650,7 +709,8 @@ public IEnumerable QueryLDAP(LDAPQueryOptions options) options.ShowDeleted, options.AdsPath, options.GlobalCatalog, - options.SkipCache + options.SkipCache, + options.ThrowException ); } @@ -670,40 +730,32 @@ public IEnumerable QueryLDAP(LDAPQueryOptions options) /// Skip the connection cache and force a new connection. You must dispose of this connection /// yourself. /// + /// Throw exceptions rather than logging the errors directly /// All LDAP search results matching the specified parameters + /// + /// Thrown when an error occurs during LDAP query + /// public IEnumerable 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) { - _log.LogTrace("Creating ldap connection for {Target} with filter {Filter}", - globalCatalog ? "Global Catalog" : "DC", ldapFilter); - var task = globalCatalog - ? Task.Run(() => CreateGlobalCatalogConnection(domainName, _ldapConfig.AuthType)) - : Task.Run(() => CreateLDAPConnection(domainName, skipCache, _ldapConfig.AuthType)); - - LdapConnection conn; - try - { - conn = task.ConfigureAwait(false).GetAwaiter().GetResult(); - } - catch - { - yield break; - } - - if (conn == null) - { - _log.LogTrace("LDAP connection is null for filter {Filter} and domain {Domain}", ldapFilter, - domainName); - yield break; - } - - var request = CreateSearchRequest(ldapFilter, scope, props, domainName, adsPath, showDeleted); + // TODO: Find a better abstraction than a tuple of results + var setupTuple = SetupLDAPQueryFilter(ldapFilter, scope, props, domainName, includeAcl, adsPath, globalCatalog, skipCache); + var conn = setupTuple.Item1; + var request = setupTuple.Item2; + var error = setupTuple.Item3; - if (request == null) + if (error != null) { - _log.LogTrace("Search request is null for filter {Filter} and domain {Domain}", ldapFilter, domainName); - yield break; + if (throwException) + { + throw new SharpHoundCommonException("Failed to setup LDAP Query Filter", error); + } + else + { + _log.LogWarning(error, "Failed to setup LDAP Query Filter"); + } } var pageControl = new PageResultRequestControl(500); @@ -725,24 +777,41 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope try { _log.LogTrace("Sending LDAP request for {Filter}", ldapFilter); - response = (SearchResponse) conn.SendRequest(request); + response = (SearchResponse)conn.SendRequest(request); if (response != null) - pageResponse = (PageResultResponseControl) response.Controls + pageResponse = (PageResultResponseControl)response.Controls .Where(x => x is PageResultResponseControl).DefaultIfEmpty(null).FirstOrDefault(); } catch (LdapException le) { if (le.ErrorCode != 82) - _log.LogWarning(le, - "LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}", - le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName); + if (throwException) + { + throw new SharpHoundCommonException(String.Format( + "LDAP Exception in Loop: {0}. {1}. {2}. Filter: {3}. Domain: {4}.", + le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName), le); + } + else + { + _log.LogWarning(le, + "LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}", + le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName); + } yield break; } catch (Exception e) { - _log.LogWarning(e, "Exception in LDAP loop for {Filter} and {Domain}", ldapFilter, domainName); - yield break; + if (throwException) + { + throw new SharpHoundCommonException(String.Format("Exception in LDAP loop for {0} and {1}", + ldapFilter, domainName)); + } + else + { + _log.LogWarning(e, "Exception in LDAP loop for {Filter} and {Domain}", ldapFilter, domainName); + yield break; + } } if (cancellationToken.IsCancellationRequested) @@ -782,40 +851,32 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope /// Skip the connection cache and force a new connection. You must dispose of this connection /// yourself. /// + /// Throw exceptions rather than logging the errors directly /// All LDAP search results matching the specified parameters + /// + /// Thrown when an error occurs during LDAP query + /// public IEnumerable 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) { - _log.LogTrace("Creating ldap connection for {Target} with filter {Filter}", - globalCatalog ? "Global Catalog" : "DC", ldapFilter); - var task = globalCatalog - ? Task.Run(() => CreateGlobalCatalogConnection(domainName, _ldapConfig.AuthType)) - : Task.Run(() => CreateLDAPConnection(domainName, skipCache, _ldapConfig.AuthType)); - - LdapConnection conn; - try - { - conn = task.ConfigureAwait(false).GetAwaiter().GetResult(); - } - catch - { - yield break; - } - - if (conn == null) - { - _log.LogTrace("LDAP connection is null for filter {Filter} and domain {Domain}", ldapFilter, - domainName); - yield break; - } - - var request = CreateSearchRequest(ldapFilter, scope, props, domainName, adsPath, showDeleted); + // TODO: Find a better abstraction than a tuple of results + var setupTuple = SetupLDAPQueryFilter(ldapFilter, scope, props, domainName, includeAcl, adsPath, globalCatalog, skipCache); + var conn = setupTuple.Item1; + var request = setupTuple.Item2; + var error = setupTuple.Item3; - if (request == null) + if (error != null) { - _log.LogTrace("Search request is null for filter {Filter} and domain {Domain}", ldapFilter, domainName); - yield break; + if (throwException) + { + throw new SharpHoundCommonException("Failed to setup LDAP Query Filter", error); + } + else + { + _log.LogWarning(error, "Failed to setup LDAP Query Filter"); + yield break; + } } var pageControl = new PageResultRequestControl(500); @@ -834,23 +895,40 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope try { _log.LogTrace("Sending LDAP request for {Filter}", ldapFilter); - response = (SearchResponse) conn.SendRequest(request); + response = (SearchResponse)conn.SendRequest(request); if (response != null) - pageResponse = (PageResultResponseControl) response.Controls + pageResponse = (PageResultResponseControl)response.Controls .Where(x => x is PageResultResponseControl).DefaultIfEmpty(null).FirstOrDefault(); } catch (LdapException le) { if (le.ErrorCode != 82) - _log.LogWarning(le, - "LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}", - le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName); + if (throwException) + { + throw new SharpHoundCommonException(String.Format( + "LDAP Exception in Loop: {0}. {1}. {2}. Filter: {3}. Domain: {4}", + le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName), le); + } + else + { + _log.LogWarning(le, + "LDAP Exception in Loop: {ErrorCode}. {ServerErrorMessage}. {Message}. Filter: {Filter}. Domain: {Domain}", + le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName); + } yield break; } catch (Exception e) { - _log.LogWarning(e, "Exception in LDAP loop for {Filter} and {Domain}", ldapFilter, domainName); - yield break; + if (throwException) + { + throw new SharpHoundCommonException(String.Format( + "Exception in LDAP loop for {0} and {1}", ldapFilter, domainName ?? "Default Domain"), e); + } + else + { + _log.LogWarning(e, "Exception in LDAP loop for {Filter} and {Domain}", ldapFilter, domainName ?? "Default Domain"); + yield break; + } } if (response == null || pageResponse == null) continue; @@ -910,10 +988,13 @@ public bool TestLDAPConfig(string domain) filter.AddDomains(); var resDomain = GetDomain(domain)?.Name ?? domain; + _log.LogTrace("Testing LDAP connection for domain {Domain}", resDomain); var result = QueryLDAP(filter.GetFilter(), SearchScope.Subtree, CommonProperties.ObjectID, resDomain) .DefaultIfEmpty(null).FirstOrDefault(); + _log.LogTrace("Result object from LDAP connection test is {DN}", result?.DistinguishedName ?? "null"); + return result != null; } @@ -957,7 +1038,7 @@ private Group GetBaseEnterpriseDC(string domain) { var forest = GetForest(domain)?.Name; if (forest == null) _log.LogWarning("Error getting forest, ENTDC sid is likely incorrect"); - var g = new Group {ObjectIdentifier = $"{forest}-S-1-5-9".ToUpper()}; + var g = new Group { ObjectIdentifier = $"{forest}-S-1-5-9".ToUpper() }; g.Properties.Add("name", $"ENTERPRISE DOMAIN CONTROLLERS@{forest ?? "UNKNOWN"}".ToUpper()); g.Properties.Add("domainsid", GetSidFromDomainName(forest)); g.Properties.Add("domain", forest); @@ -983,7 +1064,7 @@ private string GetDomainNameFromSidLdap(string sid) //Search using objectsid first var result = QueryLDAP($"(&(objectclass=domain)(objectsid={hexSid}))", SearchScope.Subtree, - new[] {"distinguishedname"}, globalCatalog: true).DefaultIfEmpty(null).FirstOrDefault(); + new[] { "distinguishedname" }, globalCatalog: true).DefaultIfEmpty(null).FirstOrDefault(); if (result != null) { @@ -994,7 +1075,7 @@ private string GetDomainNameFromSidLdap(string sid) //Try trusteddomain objects with the securityidentifier attribute result = QueryLDAP($"(&(objectclass=trusteddomain)(securityidentifier={sid}))", SearchScope.Subtree, - new[] {"cn"}, globalCatalog: true).DefaultIfEmpty(null).FirstOrDefault(); + new[] { "cn" }, globalCatalog: true).DefaultIfEmpty(null).FirstOrDefault(); if (result != null) { @@ -1221,7 +1302,7 @@ private async Task CreateLDAPConnection(string domainName = null var port = _ldapConfig.GetPort(); var ident = new LdapDirectoryIdentifier(targetServer, port, false, false); - var connection = new LdapConnection(ident) {Timeout = new TimeSpan(0, 0, 5, 0)}; + var connection = new LdapConnection(ident) { Timeout = new TimeSpan(0, 0, 5, 0) }; if (_ldapConfig.Username != null) { var cred = new NetworkCredential(_ldapConfig.Username, _ldapConfig.Password); diff --git a/test/unit/ContainerProcessorTest.cs b/test/unit/ContainerProcessorTest.cs index e25acd0b..173f88d5 100644 --- 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(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny())).Returns(searchResults); + It.IsAny(), It.IsAny())).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 index 8f4283e9..ed91b9e9 100644 --- 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(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny())).Returns(searchResults); + It.IsAny(), It.IsAny())).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(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny())).Returns(searchResults); + It.IsAny(), It.IsAny())).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 index 67ab7dfd..8d92e58a 100644 --- 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() }; } @@ -1023,7 +1023,7 @@ public virtual IEnumerable QueryLDAP(LDAPQueryOptions option public virtual IEnumerable 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(); } @@ -1031,7 +1031,7 @@ public virtual IEnumerable QueryLDAP(string ldapFilter, Sear public virtual IEnumerable 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 CONTROLLERS@TESTLAB.LOCAL".ToUpper()); return g; } diff --git a/test/unit/GPOLocalGroupProcessorTest.cs b/test/unit/GPOLocalGroupProcessorTest.cs index 63954e06..cf24b2ae 100644 --- a/test/unit/GPOLocalGroupProcessorTest.cs +++ b/test/unit/GPOLocalGroupProcessorTest.cs @@ -122,6 +122,7 @@ public async Task GPOLocalGroupProcessor_ReadGPOLocalGroups_AffectedComputers_0( It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny() )).Returns(new List()); var processor = new GPOLocalGroupProcessor(mockLDAPUtils.Object); @@ -144,12 +145,12 @@ public async Task GPOLocalGroupProcessor_ReadGPOLocalGroups_Null_Gpcfilesyspath( var mockSearchResults = new List(); mockSearchResults.Add(mockSearchResultEntry.Object); mockLDAPUtils.Setup(x => x.QueryLDAP(new LDAPQueryOptions - { - Filter = "(samaccounttype=805306369)", - Scope = SearchScope.Subtree, - Properties = CommonProperties.ObjectSID, - AdsPath = null - })) + { + Filter = "(samaccounttype=805306369)", + Scope = SearchScope.Subtree, + Properties = CommonProperties.ObjectSID, + AdsPath = null + })) .Returns(mockSearchResults.ToArray()); var processor = new GPOLocalGroupProcessor(mockLDAPUtils.Object); From dc741441430bea0cde5a3bae5366aeb8478b270c Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Tue, 20 Sep 2022 13:45:47 -0600 Subject: [PATCH 3/7] fix: changed namespace syntax --- .../Exceptions/SharpHoundCommonException.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/CommonLib/Exceptions/SharpHoundCommonException.cs b/src/CommonLib/Exceptions/SharpHoundCommonException.cs index 14c9bdaa..72ea28b7 100644 --- a/src/CommonLib/Exceptions/SharpHoundCommonException.cs +++ b/src/CommonLib/Exceptions/SharpHoundCommonException.cs @@ -1,10 +1,11 @@ using System; -namespace SharpHoundCommonLib.Exceptions; - -public class SharpHoundCommonException : Exception +namespace SharpHoundCommonLib.Exceptions { - public SharpHoundCommonException() { } - public SharpHoundCommonException(string message) : base(message) { } - public SharpHoundCommonException(string message, Exception inner) : base(message, inner) { } -} \ No newline at end of file + public class SharpHoundCommonException : Exception + { + public SharpHoundCommonException() { } + public SharpHoundCommonException(string message) : base(message) { } + public SharpHoundCommonException(string message, Exception inner) : base(message, inner) { } + } +} From 6fc20a0d2bd1c20c0bd419f82598eec79dc434db Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Tue, 20 Sep 2022 14:11:58 -0600 Subject: [PATCH 4/7] chore: additional setup moved to SetupLDAPQueryFilter --- src/CommonLib/LDAPUtils.cs | 60 ++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/CommonLib/LDAPUtils.cs b/src/CommonLib/LDAPUtils.cs index 67576154..cc4fc1c9 100644 --- a/src/CommonLib/LDAPUtils.cs +++ b/src/CommonLib/LDAPUtils.cs @@ -639,6 +639,7 @@ public TypedPrincipal ResolveDistinguishedName(string dn) /// LDAP filter /// SearchScope to query /// LDAP properties to fetch for each object + /// Include the DACL and Owner values in the NTSecurityDescriptor /// Domain to query /// Include deleted objects /// ADS path to limit the query too @@ -647,9 +648,9 @@ public TypedPrincipal ResolveDistinguishedName(string dn) /// Skip the connection cache and force a new connection. You must dispose of this connection /// yourself. /// - /// Tuple of LdapConnection, SearchRequest, and SharpHoundCommonException - public Tuple SetupLDAPQueryFilter(string ldapFilter, - SearchScope scope, string[] props, string domainName = null, bool showDeleted = false, + /// Tuple of LdapConnection, SearchRequest, PageResultRequestControl and SharpHoundCommonException + public Tuple SetupLDAPQueryFilter(string ldapFilter, + SearchScope scope, string[] props, bool includeAcl = false, string domainName = null, bool showDeleted = false, string adsPath = null, bool globalCatalog = false, bool skipCache = false) { _log.LogTrace("Creating ldap connection for {Target} with filter {Filter}", @@ -667,16 +668,16 @@ public Tuple SetupLDAP { var errorString = String.Format("Exception getting LDAP connection for {0} and domain {1}", ldapFilter, domainName ?? "Default Domain"); - return Tuple.Create(null, null, - new SharpHoundCommonException(errorString, e)); + return Tuple.Create( + null, null, null, new SharpHoundCommonException(errorString, e)); } if (conn == null) { var errorString = String.Format("LDAP connection is null for filter {0} and domain {1}", ldapFilter, domainName ?? "Default Domain"); - return Tuple.Create(null, null, - new SharpHoundCommonException(errorString)); + return Tuple.Create( + null, null, null, new SharpHoundCommonException(errorString)); } var request = CreateSearchRequest(ldapFilter, scope, props, domainName, adsPath, showDeleted); @@ -685,11 +686,20 @@ public Tuple SetupLDAP { var errorString = String.Format("Search request is null for filter {0} and domain {1}", ldapFilter, domainName ?? "Default Domain"); - return Tuple.Create(null, null, - new SharpHoundCommonException(errorString)); + return Tuple.Create( + null, null, null, new SharpHoundCommonException(errorString)); } - return Tuple.Create(conn, request, null); + var pageControl = new PageResultRequestControl(500); + request.Controls.Add(pageControl); + + if (includeAcl) + request.Controls.Add(new SecurityDescriptorFlagControl + { + SecurityMasks = SecurityMasks.Dacl | SecurityMasks.Owner + }); + + return Tuple.Create(conn, request, pageControl, null); } /// @@ -741,10 +751,12 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope bool throwException = false) { // TODO: Find a better abstraction than a tuple of results - var setupTuple = SetupLDAPQueryFilter(ldapFilter, scope, props, domainName, includeAcl, adsPath, globalCatalog, skipCache); + var setupTuple = SetupLDAPQueryFilter( + ldapFilter, scope, props, includeAcl, domainName, includeAcl, adsPath, globalCatalog, skipCache); var conn = setupTuple.Item1; var request = setupTuple.Item2; - var error = setupTuple.Item3; + var pageControl = setupTuple.Item3; + var error = setupTuple.Item4; if (error != null) { @@ -758,15 +770,6 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope } } - var pageControl = new PageResultRequestControl(500); - request.Controls.Add(pageControl); - - if (includeAcl) - request.Controls.Add(new SecurityDescriptorFlagControl - { - SecurityMasks = SecurityMasks.Dacl | SecurityMasks.Owner - }); - PageResultResponseControl pageResponse = null; while (true) { @@ -861,10 +864,12 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope string adsPath = null, bool globalCatalog = false, bool skipCache = false, bool throwException = false) { // TODO: Find a better abstraction than a tuple of results - var setupTuple = SetupLDAPQueryFilter(ldapFilter, scope, props, domainName, includeAcl, adsPath, globalCatalog, skipCache); + var setupTuple = SetupLDAPQueryFilter( + ldapFilter, scope, props, includeAcl, domainName, includeAcl, adsPath, globalCatalog, skipCache); var conn = setupTuple.Item1; var request = setupTuple.Item2; - var error = setupTuple.Item3; + var pageControl = setupTuple.Item3; + var error = setupTuple.Item4; if (error != null) { @@ -879,15 +884,6 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope } } - var pageControl = new PageResultRequestControl(500); - request.Controls.Add(pageControl); - - if (includeAcl) - request.Controls.Add(new SecurityDescriptorFlagControl - { - SecurityMasks = SecurityMasks.Dacl | SecurityMasks.Owner - }); - PageResultResponseControl pageResponse = null; while (true) { From a0c1b5dc638bcf1cdeb508a2b1565afe5e0d7dd4 Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Tue, 20 Sep 2022 15:22:39 -0600 Subject: [PATCH 5/7] chore: added tests to ensure exception toggle behavior --- test/unit/LDAPUtilsTest.cs | 61 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/test/unit/LDAPUtilsTest.cs b/test/unit/LDAPUtilsTest.cs index 824a6ac0..b6ebbacb 100644 --- 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( + () => + { + 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( + () => + { + 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 From 52192403ce0dc6766e6c4daf8bf3c0cde0fb0713 Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Tue, 20 Sep 2022 15:46:36 -0600 Subject: [PATCH 6/7] chore: fix formatting so it doesn't get auto-formatted weird --- test/unit/GPOLocalGroupProcessorTest.cs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/unit/GPOLocalGroupProcessorTest.cs b/test/unit/GPOLocalGroupProcessorTest.cs index cf24b2ae..ef9e0496 100644 --- a/test/unit/GPOLocalGroupProcessorTest.cs +++ b/test/unit/GPOLocalGroupProcessorTest.cs @@ -144,13 +144,14 @@ public async Task GPOLocalGroupProcessor_ReadGPOLocalGroups_Null_Gpcfilesyspath( mockSearchResultEntry.Setup(x => x.GetSid()).Returns("teapot"); var mockSearchResults = new List(); mockSearchResults.Add(mockSearchResultEntry.Object); - mockLDAPUtils.Setup(x => x.QueryLDAP(new LDAPQueryOptions - { - Filter = "(samaccounttype=805306369)", - Scope = SearchScope.Subtree, - Properties = CommonProperties.ObjectSID, - AdsPath = null - })) + mockLDAPUtils.Setup(x => x.QueryLDAP( + new LDAPQueryOptions + { + Filter = "(samaccounttype=805306369)", + Scope = SearchScope.Subtree, + Properties = CommonProperties.ObjectSID, + AdsPath = null + })) .Returns(mockSearchResults.ToArray()); var processor = new GPOLocalGroupProcessor(mockLDAPUtils.Object); From 668606a8dc58874df36fbca25b8466e5ebc0f554 Mon Sep 17 00:00:00 2001 From: Alex Holms Date: Wed, 21 Sep 2022 11:29:12 -0600 Subject: [PATCH 7/7] chore: rename SharpHoundCommonException to be more specific to our need --- .../Exceptions/SharpHoundCommonException.cs | 8 ++-- src/CommonLib/LDAPUtils.cs | 38 +++++++++---------- test/unit/LDAPUtilsTest.cs | 4 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/CommonLib/Exceptions/SharpHoundCommonException.cs b/src/CommonLib/Exceptions/SharpHoundCommonException.cs index 72ea28b7..2e66668e 100644 --- a/src/CommonLib/Exceptions/SharpHoundCommonException.cs +++ b/src/CommonLib/Exceptions/SharpHoundCommonException.cs @@ -2,10 +2,10 @@ namespace SharpHoundCommonLib.Exceptions { - public class SharpHoundCommonException : Exception + public class LDAPQueryException : Exception { - public SharpHoundCommonException() { } - public SharpHoundCommonException(string message) : base(message) { } - public SharpHoundCommonException(string message, Exception inner) : base(message, inner) { } + public LDAPQueryException() { } + public LDAPQueryException(string message) : base(message) { } + public LDAPQueryException(string message, Exception inner) : base(message, inner) { } } } diff --git a/src/CommonLib/LDAPUtils.cs b/src/CommonLib/LDAPUtils.cs index cc4fc1c9..8d835622 100644 --- a/src/CommonLib/LDAPUtils.cs +++ b/src/CommonLib/LDAPUtils.cs @@ -648,8 +648,8 @@ public TypedPrincipal ResolveDistinguishedName(string dn) /// Skip the connection cache and force a new connection. You must dispose of this connection /// yourself. /// - /// Tuple of LdapConnection, SearchRequest, PageResultRequestControl and SharpHoundCommonException - public Tuple SetupLDAPQueryFilter(string ldapFilter, + /// Tuple of LdapConnection, SearchRequest, PageResultRequestControl and LDAPQueryException + public Tuple SetupLDAPQueryFilter(string ldapFilter, SearchScope scope, string[] props, bool includeAcl = false, string domainName = null, bool showDeleted = false, string adsPath = null, bool globalCatalog = false, bool skipCache = false) { @@ -668,16 +668,16 @@ public Tuple( - null, null, null, new SharpHoundCommonException(errorString, e)); + return Tuple.Create( + null, null, null, new LDAPQueryException(errorString, e)); } if (conn == null) { var errorString = String.Format("LDAP connection is null for filter {0} and domain {1}", ldapFilter, domainName ?? "Default Domain"); - return Tuple.Create( - null, null, null, new SharpHoundCommonException(errorString)); + return Tuple.Create( + null, null, null, new LDAPQueryException(errorString)); } var request = CreateSearchRequest(ldapFilter, scope, props, domainName, adsPath, showDeleted); @@ -686,8 +686,8 @@ public Tuple( - null, null, null, new SharpHoundCommonException(errorString)); + return Tuple.Create( + null, null, null, new LDAPQueryException(errorString)); } var pageControl = new PageResultRequestControl(500); @@ -699,7 +699,7 @@ public Tuple(conn, request, pageControl, null); + return Tuple.Create(conn, request, pageControl, null); } /// @@ -742,8 +742,8 @@ public IEnumerable QueryLDAP(LDAPQueryOptions options) /// /// Throw exceptions rather than logging the errors directly /// All LDAP search results matching the specified parameters - /// - /// Thrown when an error occurs during LDAP query + /// + /// Thrown when an error occurs during LDAP query (only when throwException = true) /// public IEnumerable QueryLDAP(string ldapFilter, SearchScope scope, string[] props, CancellationToken cancellationToken, string domainName = null, bool includeAcl = false, @@ -762,7 +762,7 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope { if (throwException) { - throw new SharpHoundCommonException("Failed to setup LDAP Query Filter", error); + throw new LDAPQueryException("Failed to setup LDAP Query Filter", error); } else { @@ -790,7 +790,7 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope if (le.ErrorCode != 82) if (throwException) { - throw new SharpHoundCommonException(String.Format( + throw new LDAPQueryException(String.Format( "LDAP Exception in Loop: {0}. {1}. {2}. Filter: {3}. Domain: {4}.", le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName), le); } @@ -807,7 +807,7 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope { if (throwException) { - throw new SharpHoundCommonException(String.Format("Exception in LDAP loop for {0} and {1}", + throw new LDAPQueryException(String.Format("Exception in LDAP loop for {0} and {1}", ldapFilter, domainName)); } else @@ -856,8 +856,8 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope /// /// Throw exceptions rather than logging the errors directly /// All LDAP search results matching the specified parameters - /// - /// Thrown when an error occurs during LDAP query + /// + /// Thrown when an error occurs during LDAP query (only when throwException = true) /// public IEnumerable QueryLDAP(string ldapFilter, SearchScope scope, string[] props, string domainName = null, bool includeAcl = false, bool showDeleted = false, @@ -875,7 +875,7 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope { if (throwException) { - throw new SharpHoundCommonException("Failed to setup LDAP Query Filter", error); + throw new LDAPQueryException("Failed to setup LDAP Query Filter", error); } else { @@ -901,7 +901,7 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope if (le.ErrorCode != 82) if (throwException) { - throw new SharpHoundCommonException(String.Format( + throw new LDAPQueryException(String.Format( "LDAP Exception in Loop: {0}. {1}. {2}. Filter: {3}. Domain: {4}", le.ErrorCode, le.ServerErrorMessage, le.Message, ldapFilter, domainName), le); } @@ -917,7 +917,7 @@ public IEnumerable QueryLDAP(string ldapFilter, SearchScope { if (throwException) { - throw new SharpHoundCommonException(String.Format( + throw new LDAPQueryException(String.Format( "Exception in LDAP loop for {0} and {1}", ldapFilter, domainName ?? "Default Domain"), e); } else diff --git a/test/unit/LDAPUtilsTest.cs b/test/unit/LDAPUtilsTest.cs index b6ebbacb..5e39cf4e 100644 --- a/test/unit/LDAPUtilsTest.cs +++ b/test/unit/LDAPUtilsTest.cs @@ -117,7 +117,7 @@ public void QueryLDAP_With_Exception() ThrowException = true }; - Assert.Throws( + Assert.Throws( () => { foreach (var sre in _utils.QueryLDAP(null, new SearchScope(), null, new CancellationToken(), null, false, false, null, false, false, true)) @@ -126,7 +126,7 @@ public void QueryLDAP_With_Exception() }; }); - Assert.Throws( + Assert.Throws( () => { foreach (var sre in _utils.QueryLDAP(options))