From 9e03f0c76e75e78d8a91b5411610067247bbe7a1 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Mon, 4 Nov 2024 09:52:43 -0500 Subject: [PATCH] Fix properties with incorrect types (#173) * wip: fix bad data types in sh * chore: revert formatting to k&r * chore: fix more formatting * fix: elevate try/catch on principalcontext calls to fix exceptions https://github.com/BloodHoundAD/SharpHound/issues/120 * chore: add lock on buildguidcache --- src/CommonLib/LdapUtils.cs | 38 ++++---- src/CommonLib/OutputTypes/DomainTrust.cs | 2 +- src/CommonLib/Processors/ACLProcessor.cs | 15 +++- .../Processors/DomainTrustProcessor.cs | 2 +- .../Processors/LdapPropertyProcessor.cs | 89 ++++++++++++------- 5 files changed, 89 insertions(+), 57 deletions(-) diff --git a/src/CommonLib/LdapUtils.cs b/src/CommonLib/LdapUtils.cs index 17293d3b..e29defc7 100644 --- a/src/CommonLib/LdapUtils.cs +++ b/src/CommonLib/LdapUtils.cs @@ -168,8 +168,8 @@ public IAsyncEnumerable> PagedQuery(LdapQueryParame //pass } - using (var ctx = new PrincipalContext(ContextType.Domain)) { - try { + try { + using (var ctx = new PrincipalContext(ContextType.Domain)) { var principal = Principal.FindByIdentity(ctx, IdentityType.Sid, sid); if (principal != null) { var entry = ((DirectoryEntry)principal.GetUnderlyingObject()).ToDirectoryObject(); @@ -178,10 +178,11 @@ public IAsyncEnumerable> PagedQuery(LdapQueryParame return (true, type); } } - } catch { - //pass } + } catch { + //pass } + return (false, Label.Base); } @@ -212,8 +213,8 @@ public IAsyncEnumerable> PagedQuery(LdapQueryParame //pass } - using (var ctx = new PrincipalContext(ContextType.Domain)) { - try { + try { + using (var ctx = new PrincipalContext(ContextType.Domain)) { var principal = Principal.FindByIdentity(ctx, IdentityType.Guid, guid); if (principal != null) { var entry = ((DirectoryEntry)principal.GetUnderlyingObject()).ToDirectoryObject(); @@ -222,10 +223,11 @@ public IAsyncEnumerable> PagedQuery(LdapQueryParame return (true, type); } } - } catch { - //pass } + } catch { + //pass } + return (false, Label.Base); } @@ -345,8 +347,8 @@ public IAsyncEnumerable> PagedQuery(LdapQueryParame return (true, domainName); } - using (var ctx = new PrincipalContext(ContextType.Domain)) { - try { + try { + using (var ctx = new PrincipalContext(ContextType.Domain)) { var principal = Principal.FindByIdentity(ctx, IdentityType.Sid, sid); if (principal != null) { var dn = principal.DistinguishedName; @@ -355,10 +357,11 @@ public IAsyncEnumerable> PagedQuery(LdapQueryParame return (true, Helpers.DistinguishedNameToDomain(dn)); } } - } catch { - //pass } + } catch { + //pass } + return (false, string.Empty); } @@ -877,8 +880,8 @@ public async Task IsDomainController(string computerObjectId, string domai return (true, principal); } - using (var ctx = new PrincipalContext(ContextType.Domain)) { - try { + try { + using (var ctx = new PrincipalContext(ContextType.Domain)) { var lookupPrincipal = Principal.FindByIdentity(ctx, IdentityType.DistinguishedName, distinguishedName); if (lookupPrincipal != null) { @@ -895,12 +898,13 @@ public async Task IsDomainController(string computerObjectId, string domai } } - return (false, default); - } catch { - _unresolvablePrincipals.Add(distinguishedName); return (false, default); } + } catch { + _unresolvablePrincipals.Add(distinguishedName); + return (false, default); } + } public async Task<(bool Success, string DSHeuristics)> GetDSHueristics(string domain, string dn) { diff --git a/src/CommonLib/OutputTypes/DomainTrust.cs b/src/CommonLib/OutputTypes/DomainTrust.cs index 9c62a82c..446df81d 100644 --- a/src/CommonLib/OutputTypes/DomainTrust.cs +++ b/src/CommonLib/OutputTypes/DomainTrust.cs @@ -9,7 +9,7 @@ public class DomainTrust public bool IsTransitive { get; set; } public bool SidFilteringEnabled { get; set; } public bool TGTDelegationEnabled { get; set; } - public string TrustAttributes { get; set; } + public long TrustAttributes { get; set; } public TrustDirection TrustDirection { get; set; } public TrustType TrustType { get; set; } } diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index 34aff91f..45af8136 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -19,6 +19,7 @@ public class ACLProcessor { private readonly ILogger _log; private readonly ILdapUtils _utils; private readonly ConcurrentHashSet _builtDomainCaches = new(StringComparer.OrdinalIgnoreCase); + private readonly object _lock = new(); static ACLProcessor() { //Create a dictionary with the base GUIDs of each object type @@ -50,6 +51,14 @@ public ACLProcessor(ILdapUtils utils, ILogger log = null) { /// LAPS /// private async Task BuildGuidCache(string domain) { + lock (_lock) { + if (_builtDomainCaches.Contains(domain)) { + return; + } + + _builtDomainCaches.Add(domain); + } + _log.LogInformation("Building GUID Cache for {Domain}", domain); await foreach (var result in _utils.PagedQuery(new LdapQueryParameters { DomainName = domain, @@ -83,6 +92,7 @@ private async Task BuildGuidCache(string domain) { _log.LogDebug("Error while building GUID cache for {Domain}: {Message}", domain, result.Error); } } + } /// @@ -236,10 +246,7 @@ public IEnumerable GetInheritedAceHashes(byte[] ntSecurityDescriptor, st public async IAsyncEnumerable ProcessACL(byte[] ntSecurityDescriptor, string objectDomain, Label objectType, bool hasLaps, string objectName = "") { - if (!_builtDomainCaches.Contains(objectDomain)) { - _builtDomainCaches.Add(objectDomain); - await BuildGuidCache(objectDomain); - } + await BuildGuidCache(objectDomain); if (ntSecurityDescriptor == null) { _log.LogDebug("Security Descriptor is null for {Name}", objectName); diff --git a/src/CommonLib/Processors/DomainTrustProcessor.cs b/src/CommonLib/Processors/DomainTrustProcessor.cs index 16681f9d..42ab5862 100644 --- a/src/CommonLib/Processors/DomainTrustProcessor.cs +++ b/src/CommonLib/Processors/DomainTrustProcessor.cs @@ -71,7 +71,7 @@ public async IAsyncEnumerable EnumerateDomainTrusts(string domain) continue; } - trust.TrustAttributes = ta.ToString(); + trust.TrustAttributes = ta; attributes = (TrustAttributes) ta; trust.IsTransitive = !attributes.HasFlag(TrustAttributes.NonTransitive); diff --git a/src/CommonLib/Processors/LdapPropertyProcessor.cs b/src/CommonLib/Processors/LdapPropertyProcessor.cs index c12764b1..852619f7 100644 --- a/src/CommonLib/Processors/LdapPropertyProcessor.cs +++ b/src/CommonLib/Processors/LdapPropertyProcessor.cs @@ -57,17 +57,33 @@ private static Dictionary GetCommonProps(IDirectoryObject entry) /// /// /// - public async Task> ReadDomainProperties(IDirectoryObject entry, string domain) - { + public async Task> ReadDomainProperties(IDirectoryObject entry, string domain) { var props = GetCommonProps(entry); + if (entry.TryGetProperty(LDAPProperties.ExpirePasswordsOnSmartCardOnlyAccounts, out var expirePassword) && + bool.TryParse(expirePassword, out var expirePasswordBool)) { + props.Add("expirepasswordsonsmartcardonlyaccounts", expirePasswordBool); + } + + if (entry.TryGetLongProperty(LDAPProperties.MachineAccountQuota, out var machineAccountQuota)) { + props.Add("machineaccountquota", machineAccountQuota); + } + + if (entry.TryGetLongProperty(LDAPProperties.MinPwdLength, out var minPwdLength)) { + props.Add("minpwdlength", minPwdLength); + } + + if (entry.TryGetLongProperty(LDAPProperties.PwdProperties, out var pwdProperties)) { + props.Add("pwdproperties", pwdProperties); + } + + if (entry.TryGetLongProperty(LDAPProperties.PwdHistoryLength, out var pwdHistoryLength)) { + props.Add("pwdhistorylength", pwdHistoryLength); + } - props.Add("expirepasswordsonsmartcardonlyaccounts", entry.GetProperty(LDAPProperties.ExpirePasswordsOnSmartCardOnlyAccounts)); - props.Add("machineaccountquota", entry.GetProperty(LDAPProperties.MachineAccountQuota)); - props.Add("minpwdlength", entry.GetProperty(LDAPProperties.MinPwdLength)); - props.Add("pwdproperties", entry.GetProperty(LDAPProperties.PwdProperties)); - props.Add("pwdhistorylength", entry.GetProperty(LDAPProperties.PwdHistoryLength)); - props.Add("lockoutthreshold", entry.GetProperty(LDAPProperties.LockoutThreshold)); + if (entry.TryGetLongProperty(LDAPProperties.LockoutThreshold, out var lockoutThreshold)) { + props.Add("lockoutthreshold", lockoutThreshold); + } if (entry.TryGetLongProperty(LDAPProperties.MinPwdAge, out var minpwdage)) { var duration = ConvertNanoDuration(minpwdage); @@ -75,27 +91,32 @@ public async Task> ReadDomainProperties(IDirectoryObj props.Add("minpwdage", duration); } } + if (entry.TryGetLongProperty(LDAPProperties.MaxPwdAge, out var maxpwdage)) { var duration = ConvertNanoDuration(maxpwdage); if (duration != null) { props.Add("maxpwdage", duration); } } + if (entry.TryGetLongProperty(LDAPProperties.LockoutDuration, out var lockoutduration)) { var duration = ConvertNanoDuration(lockoutduration); if (duration != null) { props.Add("lockoutduration", duration); } } + if (entry.TryGetLongProperty(LDAPProperties.LockOutObservationWindow, out var lockoutobservationwindow)) { var duration = ConvertNanoDuration(lockoutobservationwindow); if (duration != null) { props.Add("lockoutobservationwindow", lockoutobservationwindow); } } + if (!entry.TryGetLongProperty(LDAPProperties.DomainFunctionalLevel, out var functionalLevel)) { functionalLevel = -1; } + props.Add("functionallevel", FunctionalLevelToString((int)functionalLevel)); var dn = entry.GetProperty(LDAPProperties.DistinguishedName); @@ -623,7 +644,7 @@ public Dictionary ParseAllProperties(IDirectoryObject entry) { if (testBytes == null || testBytes.Length == 0) { continue; } - + // SIDs try { var sid = new SecurityIdentifier(testBytes, 0); @@ -707,8 +728,7 @@ private static object BestGuessConvert(string value) { return value; } - private static List ConvertEncryptionTypes(string encryptionTypes) - { + private static List ConvertEncryptionTypes(string encryptionTypes) { if (encryptionTypes == null) { return null; } @@ -719,36 +739,36 @@ private static List ConvertEncryptionTypes(string encryptionTypes) supportedEncryptionTypes.Add("Not defined"); } - if ((encryptionTypesInt & KerberosEncryptionTypes.DES_CBC_CRC) == KerberosEncryptionTypes.DES_CBC_CRC) - { + if ((encryptionTypesInt & KerberosEncryptionTypes.DES_CBC_CRC) == KerberosEncryptionTypes.DES_CBC_CRC) { supportedEncryptionTypes.Add("DES-CBC-CRC"); } - if ((encryptionTypesInt & KerberosEncryptionTypes.DES_CBC_MD5) == KerberosEncryptionTypes.DES_CBC_MD5) - { + + if ((encryptionTypesInt & KerberosEncryptionTypes.DES_CBC_MD5) == KerberosEncryptionTypes.DES_CBC_MD5) { supportedEncryptionTypes.Add("DES-CBC-MD5"); } - if ((encryptionTypesInt & KerberosEncryptionTypes.RC4_HMAC_MD5) == KerberosEncryptionTypes.RC4_HMAC_MD5) - { + + if ((encryptionTypesInt & KerberosEncryptionTypes.RC4_HMAC_MD5) == KerberosEncryptionTypes.RC4_HMAC_MD5) { supportedEncryptionTypes.Add("RC4-HMAC-MD5"); } - if ((encryptionTypesInt & KerberosEncryptionTypes.AES128_CTS_HMAC_SHA1_96) == KerberosEncryptionTypes.AES128_CTS_HMAC_SHA1_96) - { + + if ((encryptionTypesInt & KerberosEncryptionTypes.AES128_CTS_HMAC_SHA1_96) == + KerberosEncryptionTypes.AES128_CTS_HMAC_SHA1_96) { supportedEncryptionTypes.Add("AES128-CTS-HMAC-SHA1-96"); } - if ((encryptionTypesInt & KerberosEncryptionTypes.AES256_CTS_HMAC_SHA1_96) == KerberosEncryptionTypes.AES256_CTS_HMAC_SHA1_96) - { + + if ((encryptionTypesInt & KerberosEncryptionTypes.AES256_CTS_HMAC_SHA1_96) == + KerberosEncryptionTypes.AES256_CTS_HMAC_SHA1_96) { supportedEncryptionTypes.Add("AES256-CTS-HMAC-SHA1-96"); } return supportedEncryptionTypes; } - private static string ConvertNanoDuration(long duration) - { + private static string ConvertNanoDuration(long duration) { // In case duration is long.MinValue, Math.Abs will overflow. Value represents Forever or Never if (duration == long.MinValue) { return "Forever"; - // And if the value is positive, it indicates an error code + // And if the value is positive, it indicates an error code } else if (duration > 0) { return null; } @@ -761,20 +781,19 @@ private static string ConvertNanoDuration(long duration) List timeComponents = new List(); // Add each time component if it's greater than zero - if (durationSpan.Days > 0) - { + if (durationSpan.Days > 0) { timeComponents.Add($"{durationSpan.Days} {(durationSpan.Days == 1 ? "day" : "days")}"); } - if (durationSpan.Hours > 0) - { + + if (durationSpan.Hours > 0) { timeComponents.Add($"{durationSpan.Hours} {(durationSpan.Hours == 1 ? "hour" : "hours")}"); } - if (durationSpan.Minutes > 0) - { + + if (durationSpan.Minutes > 0) { timeComponents.Add($"{durationSpan.Minutes} {(durationSpan.Minutes == 1 ? "minute" : "minutes")}"); } - if (durationSpan.Seconds > 0) - { + + if (durationSpan.Seconds > 0) { timeComponents.Add($"{durationSpan.Seconds} {(durationSpan.Seconds == 1 ? "second" : "seconds")}"); } @@ -888,10 +907,12 @@ public ParsedCertificate(byte[] rawCertificate) { foreach (var cert in chain.ChainElements) temp.Add(cert.Certificate.Thumbprint); Chain = temp.ToArray(); } catch (Exception e) { - Logging.LogProvider.CreateLogger("ParsedCertificate").LogWarning(e, "Failed to read certificate chain for certificate {Name} with Algo {Algorithm}", name, parsedCertificate.SignatureAlgorithm.FriendlyName); + Logging.LogProvider.CreateLogger("ParsedCertificate").LogWarning(e, + "Failed to read certificate chain for certificate {Name} with Algo {Algorithm}", name, + parsedCertificate.SignatureAlgorithm.FriendlyName); Chain = Array.Empty(); } - + // Extensions var extensions = parsedCertificate.Extensions;