Skip to content

Commit

Permalink
Fix properties with incorrect types (#173)
Browse files Browse the repository at this point in the history
* 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

BloodHoundAD/SharpHound#120

* chore: add lock on buildguidcache
  • Loading branch information
rvazarkar authored Nov 4, 2024
1 parent 276237f commit 9e03f0c
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 57 deletions.
38 changes: 21 additions & 17 deletions src/CommonLib/LdapUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ public IAsyncEnumerable<LdapResult<IDirectoryObject>> 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();
Expand All @@ -178,10 +178,11 @@ public IAsyncEnumerable<LdapResult<IDirectoryObject>> PagedQuery(LdapQueryParame
return (true, type);
}
}
} catch {
//pass
}
} catch {
//pass
}


return (false, Label.Base);
}
Expand Down Expand Up @@ -212,8 +213,8 @@ public IAsyncEnumerable<LdapResult<IDirectoryObject>> 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();
Expand All @@ -222,10 +223,11 @@ public IAsyncEnumerable<LdapResult<IDirectoryObject>> PagedQuery(LdapQueryParame
return (true, type);
}
}
} catch {
//pass
}
} catch {
//pass
}


return (false, Label.Base);
}
Expand Down Expand Up @@ -345,8 +347,8 @@ public IAsyncEnumerable<LdapResult<IDirectoryObject>> 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;
Expand All @@ -355,10 +357,11 @@ public IAsyncEnumerable<LdapResult<IDirectoryObject>> PagedQuery(LdapQueryParame
return (true, Helpers.DistinguishedNameToDomain(dn));
}
}
} catch {
//pass
}
} catch {
//pass
}


return (false, string.Empty);
}
Expand Down Expand Up @@ -877,8 +880,8 @@ public async Task<bool> 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) {
Expand All @@ -895,12 +898,13 @@ public async Task<bool> 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) {
Expand Down
2 changes: 1 addition & 1 deletion src/CommonLib/OutputTypes/DomainTrust.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
}
Expand Down
15 changes: 11 additions & 4 deletions src/CommonLib/Processors/ACLProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -50,6 +51,14 @@ public ACLProcessor(ILdapUtils utils, ILogger log = null) {
/// LAPS
/// </summary>
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,
Expand Down Expand Up @@ -83,6 +92,7 @@ private async Task BuildGuidCache(string domain) {
_log.LogDebug("Error while building GUID cache for {Domain}: {Message}", domain, result.Error);
}
}

}

/// <summary>
Expand Down Expand Up @@ -236,10 +246,7 @@ public IEnumerable<string> GetInheritedAceHashes(byte[] ntSecurityDescriptor, st
public async IAsyncEnumerable<ACE> 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);
Expand Down
2 changes: 1 addition & 1 deletion src/CommonLib/Processors/DomainTrustProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public async IAsyncEnumerable<DomainTrust> EnumerateDomainTrusts(string domain)
continue;
}

trust.TrustAttributes = ta.ToString();
trust.TrustAttributes = ta;
attributes = (TrustAttributes) ta;

trust.IsTransitive = !attributes.HasFlag(TrustAttributes.NonTransitive);
Expand Down
89 changes: 55 additions & 34 deletions src/CommonLib/Processors/LdapPropertyProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,45 +57,66 @@ private static Dictionary<string, object> GetCommonProps(IDirectoryObject entry)
/// </summary>
/// <param name="entry"></param>
/// <returns></returns>
public async Task<Dictionary<string, object>> ReadDomainProperties(IDirectoryObject entry, string domain)
{
public async Task<Dictionary<string, object>> 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);
if (duration != null) {
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);
Expand Down Expand Up @@ -623,7 +644,7 @@ public Dictionary<string, object> ParseAllProperties(IDirectoryObject entry) {
if (testBytes == null || testBytes.Length == 0) {
continue;
}

// SIDs
try {
var sid = new SecurityIdentifier(testBytes, 0);
Expand Down Expand Up @@ -707,8 +728,7 @@ private static object BestGuessConvert(string value) {
return value;
}

private static List<string> ConvertEncryptionTypes(string encryptionTypes)
{
private static List<string> ConvertEncryptionTypes(string encryptionTypes) {
if (encryptionTypes == null) {
return null;
}
Expand All @@ -719,36 +739,36 @@ private static List<string> 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;
}
Expand All @@ -761,20 +781,19 @@ private static string ConvertNanoDuration(long duration)
List<string> timeComponents = new List<string>();

// 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")}");
}

Expand Down Expand Up @@ -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<string>();
}


// Extensions
var extensions = parsedCertificate.Extensions;
Expand Down

0 comments on commit 9e03f0c

Please sign in to comment.