Skip to content

Commit

Permalink
More fixes (#147)
Browse files Browse the repository at this point in the history
* fix: fix our match for CertificationAuthority class to use >= instead of > to solve NTAuthCertificates node

* chore: fix another potential case confusion

* fix: missing properties in generator

* fix: exclude group policy container objects from container ldap filter

* fix: use long property instead of int
chore: temporarily add enterprise dc group back in

* fix: keep unresolved principals in enrollment agent aces
  • Loading branch information
rvazarkar authored Jul 30, 2024
1 parent 1ea7d1e commit 73cd00b
Show file tree
Hide file tree
Showing 12 changed files with 96 additions and 37 deletions.
4 changes: 2 additions & 2 deletions src/CommonLib/DirectoryObjects/DirectoryEntryWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ public bool TryGetByteArrayProperty(string propertyName, out byte[][] value) {
return true;
}

public bool TryGetIntProperty(string propertyName, out int value) {
public bool TryGetLongProperty(string propertyName, out long value) {
value = 0;
if (!CheckCache(propertyName)) return false;

if (!TryGetProperty(propertyName, out var s)) {
return false;
}

return int.TryParse(s, out value);
return long.TryParse(s, out value);
}

public bool TryGetCertificateArrayProperty(string propertyName, out X509Certificate2[] value) {
Expand Down
8 changes: 4 additions & 4 deletions src/CommonLib/DirectoryObjects/DirectoryObjectExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,15 @@ public static bool GetLabel(this IDirectoryObject directoryObject, out Label typ
return false;
}

if (!directoryObject.TryGetIntProperty(LDAPProperties.Flags, out var flags)) {
if (!directoryObject.TryGetLongProperty(LDAPProperties.Flags, out var flags)) {
flags = 0;
}

directoryObject.TryGetDistinguishedName(out var distinguishedName);
directoryObject.TryGetProperty(LDAPProperties.SAMAccountType, out var samAccountType);
directoryObject.TryGetArrayProperty(LDAPProperties.ObjectClass, out var objectClasses);

return LdapUtils.ResolveLabel(objectIdentifier, distinguishedName, samAccountType, objectClasses, flags,
return LdapUtils.ResolveLabel(objectIdentifier, distinguishedName, samAccountType, objectClasses, (int)flags,
out type);
}

Expand All @@ -56,12 +56,12 @@ public static bool IsDeleted(this IDirectoryObject directoryObject) {
}

public static bool HasLAPS(this IDirectoryObject directoryObject) {
if (directoryObject.TryGetIntProperty(LDAPProperties.LAPSExpirationTime, out var lapsExpiration) &&
if (directoryObject.TryGetLongProperty(LDAPProperties.LAPSExpirationTime, out var lapsExpiration) &&
lapsExpiration > 0) {
return true;
}

if (directoryObject.TryGetIntProperty(LDAPProperties.LegacyLAPSExpirationTime, out var legacyLapsExpiration) &&
if (directoryObject.TryGetLongProperty(LDAPProperties.LegacyLAPSExpirationTime, out var legacyLapsExpiration) &&
legacyLapsExpiration > 0) {
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/CommonLib/DirectoryObjects/IDirectoryObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public interface IDirectoryObject {
bool TryGetByteProperty(string propertyName, out byte[] value);
bool TryGetArrayProperty(string propertyName, out string[] value);
bool TryGetByteArrayProperty(string propertyName, out byte[][] value);
bool TryGetIntProperty(string propertyName, out int value);
bool TryGetLongProperty(string propertyName, out long value);
bool TryGetCertificateArrayProperty(string propertyName, out X509Certificate2[] value);
bool TryGetSecurityIdentifier(out string securityIdentifier);
bool TryGetGuid(out string guid);
Expand Down
9 changes: 5 additions & 4 deletions src/CommonLib/DirectoryObjects/SearchResultEntryWrapper.cs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
using System;
using System.Collections.Generic;
using System.DirectoryServices.Protocols;
using System.Linq;
using System.Runtime.Serialization;
using System.Security.Cryptography.X509Certificates;
using System.Security.Principal;

namespace SharpHoundCommonLib;

[DataContract]
public class SearchResultEntryWrapper : IDirectoryObject {
[DataMember]
private readonly SearchResultEntry _entry;

public SearchResultEntryWrapper(SearchResultEntry entry) {
Expand Down Expand Up @@ -82,13 +83,13 @@ public bool TryGetByteArrayProperty(string propertyName, out byte[][] value) {
return true;
}

public bool TryGetIntProperty(string propertyName, out int value) {
public bool TryGetLongProperty(string propertyName, out long value) {
if (!TryGetProperty(propertyName, out var raw)) {
value = 0;
return false;
}

return int.TryParse(raw, out value);
return long.TryParse(raw, out value);
}

public bool TryGetCertificateArrayProperty(string propertyName, out X509Certificate2[] value) {
Expand Down
4 changes: 4 additions & 0 deletions src/CommonLib/LdapProducerQueryGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ public static GeneratedLdapParameters GenerateDefaultPartitionParameters(Collect
properties.AddRange(CommonProperties.ACLProps);
}

if (methods.HasFlag(CollectionMethod.ObjectProps)) {
properties.AddRange(CommonProperties.ObjectPropsProps);
}

if (methods.IsComputerCollectionSet()) {
properties.AddRange(CommonProperties.ComputerMethodProps);
}
Expand Down
2 changes: 1 addition & 1 deletion src/CommonLib/LdapQueries/LdapFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public LdapFilter AddDomains(params string[] conditions) {
/// <param name="conditions"></param>
/// <returns></returns>
public LdapFilter AddContainers(params string[] conditions) {
_filterParts.Add(BuildString("(objectClass=container)", conditions));
_filterParts.Add(BuildString("(&(!(objectClass=groupPolicyContainer))(objectClass=container))", conditions));

return this;
}
Expand Down
50 changes: 43 additions & 7 deletions src/CommonLib/LdapUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
using SharpHoundCommonLib.Processors;
using SharpHoundRPC.NetAPINative;
using Domain = System.DirectoryServices.ActiveDirectory.Domain;
using Group = SharpHoundCommonLib.OutputTypes.Group;
using SearchScope = System.DirectoryServices.Protocols.SearchScope;
using SecurityMasks = System.DirectoryServices.Protocols.SecurityMasks;

Expand Down Expand Up @@ -478,7 +479,7 @@ await _connectionPool.GetLdapConnectionForServer(

public async Task<(bool Success, TypedPrincipal Principal)>
ResolveIDAndType(string identifier, string objectDomain) {
if (identifier.Contains("0ACNF")) {
if (identifier.IndexOf("0ACNF", StringComparison.OrdinalIgnoreCase) >= 0) {
return (false, new TypedPrincipal(identifier, Label.Base));
}

Expand Down Expand Up @@ -905,7 +906,7 @@ private SearchRequest CreateSearchRequest(string distinguishedName, string ldapF
try {
var account = new NTAccount(domainName, name);
var sid = (SecurityIdentifier)account.Translate(typeof(SecurityIdentifier));
domainSid = sid.AccountDomainSid.ToString();
domainSid = sid.AccountDomainSid.ToString().ToUpper();
Cache.AddDomainSidMapping(domainName, domainSid);
return (true, domainSid);
} catch {
Expand All @@ -919,7 +920,7 @@ private SearchRequest CreateSearchRequest(string distinguishedName, string ldapF
}).DefaultIfEmpty(LdapResult<IDirectoryObject>.Fail()).FirstOrDefaultAsync();

if (result.IsSuccess && result.Value.TryGetSecurityIdentifier(out var securityIdentifier)) {
domainSid = new SecurityIdentifier(securityIdentifier).AccountDomainSid.Value;
domainSid = new SecurityIdentifier(securityIdentifier).AccountDomainSid.Value.ToUpper();
Cache.AddDomainSidMapping(domainName, domainSid);
return (true, domainSid);
}
Expand Down Expand Up @@ -1394,6 +1395,41 @@ public async IAsyncEnumerable<OutputBase> GetWellKnownPrincipalOutput() {
output.ObjectIdentifier = wkp.Key;
yield return output;
}

await foreach (var entdc in GetEnterpriseDCGroups()) {
yield return entdc;
}
}

private async IAsyncEnumerable<Group> GetEnterpriseDCGroups() {
var grouped = new Dictionary<string, List<string>>();
var forestSidToName = new Dictionary<string, string>();
foreach (var domainSid in DomainControllers.GroupBy(x => new SecurityIdentifier(x.Key).AccountDomainSid.Value)) {
if (await GetDomainNameFromSid(domainSid.Key) is (true, var domainName) &&
await GetForest(domainName) is (true, var forestName) &&
await GetDomainSidFromDomainName(forestName) is (true, var forestDomainSid)) {
forestSidToName.Add(forestDomainSid, forestName);
if (grouped.ContainsKey(forestDomainSid)) {
foreach (var k in domainSid) {
grouped[forestDomainSid].Add(k.Key);
}
} else {
grouped[forestDomainSid] = new List<string>();
foreach (var k in domainSid) {
grouped[forestDomainSid].Add(k.Key);
}
}
}
}

foreach (var f in grouped) {
var group = new Group() { ObjectIdentifier = $"{f.Key}-S-1-5-9" };
group.Properties.Add("name", $"ENTERPRISE DOMAIN CONTROLLERS@{forestSidToName[f.Key]}".ToUpper());
group.Properties.Add("domainsid", f.Key);
group.Properties.Add("domain", forestSidToName[f.Key]);
group.Members = f.Value.Select(x => new TypedPrincipal(x, Label.Computer)).ToArray();
yield return group;
}
}

public void SetLdapConfig(LdapConfig config) {
Expand Down Expand Up @@ -1516,11 +1552,11 @@ internal static bool ResolveLabel(string objectIdentifier, string distinguishedN
type = Label.EnterpriseCA;
else if (objectClasses.Contains(ObjectClass.CertificationAuthorityClass,
StringComparer.OrdinalIgnoreCase)) {
if (distinguishedName.IndexOf(DirectoryPaths.RootCALocation, StringComparison.OrdinalIgnoreCase) > 0)
if (distinguishedName.IndexOf(DirectoryPaths.RootCALocation, StringComparison.OrdinalIgnoreCase) >= 0)
type = Label.RootCA;
if (distinguishedName.IndexOf(DirectoryPaths.AIACALocation, StringComparison.OrdinalIgnoreCase) > 0)
if (distinguishedName.IndexOf(DirectoryPaths.AIACALocation, StringComparison.OrdinalIgnoreCase) >= 0)
type = Label.AIACA;
if (distinguishedName.IndexOf(DirectoryPaths.NTAuthStoreLocation, StringComparison.OrdinalIgnoreCase) >
if (distinguishedName.IndexOf(DirectoryPaths.NTAuthStoreLocation, StringComparison.OrdinalIgnoreCase) >=
0)
type = Label.NTAuthStore;
} else if (objectClasses.Contains(ObjectClass.OIDContainerClass, StringComparer.OrdinalIgnoreCase)) {
Expand Down Expand Up @@ -1551,7 +1587,7 @@ internal static bool ResolveLabel(string objectIdentifier, string distinguishedN
return (true, res);
}

if (directoryObject.TryGetIntProperty(LDAPProperties.UserAccountControl, out var rawUac)) {
if (directoryObject.TryGetLongProperty(LDAPProperties.UserAccountControl, out var rawUac)) {
var flags = (UacFlags)rawUac;
if (flags.HasFlag(UacFlags.ServerTrustAccount)) {
res.IsDomainController = true;
Expand Down
6 changes: 4 additions & 2 deletions src/CommonLib/Processors/CertAbuseProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,10 @@ public async Task<AceRegistryAPIResult> ProcessRegistryEnrollmentPermissions(str
principalDomain = objectDomain;
}
var (resSuccess, resolvedPrincipal) = await GetRegistryPrincipal(new SecurityIdentifier(principalSid), principalDomain, computerName, isDomainController, computerObjectId, machineSid);
if (!resSuccess)
continue;
if (!resSuccess) {
resolvedPrincipal.ObjectType = Label.Base;
resolvedPrincipal.ObjectIdentifier = principalSid;
}
var isInherited = rule.IsInherited();

var cARights = (CertificationAuthorityRights)rule.ActiveDirectoryRights();
Expand Down
4 changes: 2 additions & 2 deletions src/CommonLib/Processors/DomainTrustProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public async IAsyncEnumerable<DomainTrust> EnumerateDomainTrusts(string domain)

trust.TargetDomainSid = sid;

if (!entry.TryGetIntProperty(LDAPProperties.TrustDirection, out var td)) {
if (!entry.TryGetLongProperty(LDAPProperties.TrustDirection, out var td)) {
_log.LogTrace("Failed to convert trustdirection for target: {Domain}", domain);
continue;
}
Expand All @@ -66,7 +66,7 @@ public async IAsyncEnumerable<DomainTrust> EnumerateDomainTrusts(string domain)

TrustAttributes attributes;

if (!entry.TryGetIntProperty(LDAPProperties.TrustAttributes, out var ta)) {
if (!entry.TryGetLongProperty(LDAPProperties.TrustAttributes, out var ta)) {
_log.LogTrace("Failed to convert trustattributes for target: {Domain}", domain);
continue;
}
Expand Down
26 changes: 13 additions & 13 deletions src/CommonLib/Processors/LdapPropertyProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ private static Dictionary<string, object> GetCommonProps(IDirectoryObject entry)
public static Dictionary<string, object> ReadDomainProperties(IDirectoryObject entry) {
var props = GetCommonProps(entry);

if (!entry.TryGetIntProperty(LDAPProperties.DomainFunctionalLevel, out var functionalLevel)) {
if (!entry.TryGetLongProperty(LDAPProperties.DomainFunctionalLevel, out var functionalLevel)) {
functionalLevel = -1;
}

props.Add("functionallevel", FunctionalLevelToString(functionalLevel));
props.Add("functionallevel", FunctionalLevelToString((int)functionalLevel));

return props;
}
Expand Down Expand Up @@ -119,7 +119,7 @@ public static Dictionary<string, object> ReadOUProperties(IDirectoryObject entry
/// <returns></returns>
public static Dictionary<string, object> ReadGroupProperties(IDirectoryObject entry) {
var props = GetCommonProps(entry);
entry.TryGetIntProperty(LDAPProperties.AdminCount, out var ac);
entry.TryGetLongProperty(LDAPProperties.AdminCount, out var ac);
props.Add("admincount", ac != 0);
return props;
}
Expand Down Expand Up @@ -150,7 +150,7 @@ public async Task<UserProperties> ReadUserProperties(IDirectoryObject entry, str
var props = GetCommonProps(entry);

var uacFlags = (UacFlags)0;
if (entry.TryGetIntProperty(LDAPProperties.UserAccountControl, out var uac)) {
if (entry.TryGetLongProperty(LDAPProperties.UserAccountControl, out var uac)) {
uacFlags = (UacFlags)uac;
}

Expand Down Expand Up @@ -213,7 +213,7 @@ public async Task<UserProperties> ReadUserProperties(IDirectoryObject entry, str
props.Add("sfupassword", entry.GetProperty(LDAPProperties.MsSFU30Password));
props.Add("logonscript", entry.GetProperty(LDAPProperties.ScriptPath));

entry.TryGetIntProperty(LDAPProperties.AdminCount, out var ac);
entry.TryGetLongProperty(LDAPProperties.AdminCount, out var ac);
props.Add("admincount", ac != 0);

entry.TryGetByteArrayProperty(LDAPProperties.SIDHistory, out var sh);
Expand Down Expand Up @@ -258,7 +258,7 @@ public async Task<ComputerProperties> ReadComputerProperties(IDirectoryObject en
var props = GetCommonProps(entry);

var flags = (UacFlags)0;
if (entry.TryGetIntProperty(LDAPProperties.UserAccountControl, out var uac)) {
if (entry.TryGetLongProperty(LDAPProperties.UserAccountControl, out var uac)) {
flags = (UacFlags)uac;
}

Expand Down Expand Up @@ -399,7 +399,7 @@ public static Dictionary<string, object> ReadAIACAProperties(IDirectoryObject en

public static Dictionary<string, object> ReadEnterpriseCAProperties(IDirectoryObject entry) {
var props = GetCommonProps(entry);
if (entry.TryGetIntProperty("flags", out var flags))
if (entry.TryGetLongProperty("flags", out var flags))
props.Add("flags", (PKICertificateAuthorityFlags)flags);
props.Add("caname", entry.GetProperty(LDAPProperties.Name));
props.Add("dnshostname", entry.GetProperty(LDAPProperties.DNSHostName));
Expand Down Expand Up @@ -438,21 +438,21 @@ public static Dictionary<string, object> ReadCertTemplateProperties(IDirectoryOb
props.Add("validityperiod", ConvertPKIPeriod(entry.GetByteProperty(LDAPProperties.PKIExpirationPeriod)));
props.Add("renewalperiod", ConvertPKIPeriod(entry.GetByteProperty(LDAPProperties.PKIOverlappedPeriod)));

if (entry.TryGetIntProperty(LDAPProperties.TemplateSchemaVersion, out var schemaVersion))
if (entry.TryGetLongProperty(LDAPProperties.TemplateSchemaVersion, out var schemaVersion))
props.Add("schemaversion", schemaVersion);

props.Add("displayname", entry.GetProperty(LDAPProperties.DisplayName));
props.Add("oid", entry.GetProperty(LDAPProperties.CertTemplateOID));

if (entry.TryGetIntProperty(LDAPProperties.PKIEnrollmentFlag, out var enrollmentFlagsRaw)) {
if (entry.TryGetLongProperty(LDAPProperties.PKIEnrollmentFlag, out var enrollmentFlagsRaw)) {
var enrollmentFlags = (PKIEnrollmentFlag)enrollmentFlagsRaw;

props.Add("enrollmentflag", enrollmentFlags);
props.Add("requiresmanagerapproval", enrollmentFlags.HasFlag(PKIEnrollmentFlag.PEND_ALL_REQUESTS));
props.Add("nosecurityextension", enrollmentFlags.HasFlag(PKIEnrollmentFlag.NO_SECURITY_EXTENSION));
}

if (entry.TryGetIntProperty(LDAPProperties.PKINameFlag, out var nameFlagsRaw)) {
if (entry.TryGetLongProperty(LDAPProperties.PKINameFlag, out var nameFlagsRaw)) {
var nameFlags = (PKICertificateNameFlag)nameFlagsRaw;

props.Add("certificatenameflag", nameFlags);
Expand Down Expand Up @@ -481,11 +481,11 @@ public static Dictionary<string, object> ReadCertTemplateProperties(IDirectoryOb
entry.TryGetArrayProperty(LDAPProperties.CertificatePolicy, out var certificatePolicy);
props.Add("certificatepolicy", certificatePolicy);

if (entry.TryGetIntProperty(LDAPProperties.NumSignaturesRequired, out var authorizedSignatures))
if (entry.TryGetLongProperty(LDAPProperties.NumSignaturesRequired, out var authorizedSignatures))
props.Add("authorizedsignatures", authorizedSignatures);

var hasUseLegacyProvider = false;
if (entry.TryGetIntProperty(LDAPProperties.PKIPrivateKeyFlag, out var privateKeyFlagsRaw)) {
if (entry.TryGetLongProperty(LDAPProperties.PKIPrivateKeyFlag, out var privateKeyFlagsRaw)) {
var privateKeyFlags = (PKIPrivateKeyFlag)privateKeyFlagsRaw;
hasUseLegacyProvider = privateKeyFlags.HasFlag(PKIPrivateKeyFlag.USE_LEGACY_PROVIDER);
}
Expand All @@ -494,7 +494,7 @@ public static Dictionary<string, object> ReadCertTemplateProperties(IDirectoryOb

props.Add("applicationpolicies",
ParseCertTemplateApplicationPolicies(appPolicies,
schemaVersion, hasUseLegacyProvider));
(int)schemaVersion, hasUseLegacyProvider));
entry.TryGetArrayProperty(LDAPProperties.IssuancePolicies, out var issuancePolicies);
props.Add("issuancepolicies", issuancePolicies);

Expand Down
13 changes: 13 additions & 0 deletions test/unit/DirectoryObjectTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,19 @@ public void Test_GetLabel_CertificationAuthorityObjects() {
Assert.True(mock.GetLabel(out label));
Assert.Equal(Label.NTAuthStore, label);
}

[Fact]
public void Test_GetLabel_NTAuthCertificateObject() {
var attribs = new Dictionary<string, object> {
{ LDAPProperties.ObjectClass, new[] { "top", ObjectClass.CertificationAuthorityClass } },
};

var mock = new MockDirectoryObject($"{DirectoryPaths.NTAuthStoreLocation.ToUpper()},DC=Testlab,DC=local",
attribs,
"123456", new Guid().ToString());
Assert.True(mock.GetLabel(out var label));
Assert.Equal(Label.NTAuthStore, label);
}

[Fact]
public void Test_GetLabel_NoLabel() {
Expand Down
Loading

0 comments on commit 73cd00b

Please sign in to comment.