From 4d94463829526b0fc0dcd56eae0dacf69fd7b984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adil=20Alperen=20=C3=87iftci?= <134228585+adilalperenciftci@users.noreply.github.com> Date: Sun, 25 Oct 2026 05:01:55 +0300 Subject: [PATCH 1/3] feat: Collect dMSA patch prerequisites from domain controllers --- src/CommonLib/Enums/EdgeNames.cs | 2 + src/CommonLib/OutputTypes/Computer.cs | 4 +- src/CommonLib/Processors/ACEGuids.cs | 2 + src/CommonLib/Processors/ACLProcessor.cs | 44 +++++++++---- .../Processors/DCRegistryProcessor.cs | 63 ++++++++++++++++++- test/unit/ACLProcessorTest.cs | 20 ++++++ test/unit/DCRegistryProcessorTests.cs | 63 +++++++++++++++++++ 7 files changed, 185 insertions(+), 13 deletions(-) create mode 100644 test/unit/DCRegistryProcessorTests.cs diff --git a/src/CommonLib/Enums/EdgeNames.cs b/src/CommonLib/Enums/EdgeNames.cs index 833621bc1..a5d594958 100644 --- a/src/CommonLib/Enums/EdgeNames.cs +++ b/src/CommonLib/Enums/EdgeNames.cs @@ -24,6 +24,8 @@ public static class EdgeNames public const string WriteGPLink = "WriteGPLink"; public const string WriteAltSecurityIdentities = "WriteAltSecurityIdentities"; public const string WritePublicInformation = "WritePublicInformation"; + public const string WriteSupersededManagedAccountLink = "WriteSupersededManagedAccountLink"; + public const string WriteSupersededServiceAccountState = "WriteSupersededServiceAccountState"; public const string ServerIs = "ServerIs"; //CertAbuse edges diff --git a/src/CommonLib/OutputTypes/Computer.cs b/src/CommonLib/OutputTypes/Computer.cs index 2d91c3871..84c889eba 100644 --- a/src/CommonLib/OutputTypes/Computer.cs +++ b/src/CommonLib/OutputTypes/Computer.cs @@ -66,6 +66,8 @@ public class DCRegistryData { public IntRegistryAPIResult CertificateMappingMethods { get; set; } public IntRegistryAPIResult StrongCertificateBindingEnforcement { get; set; } public StrRegistryAPIResult VulnerableNetlogonSecurityDescriptor { get; set; } + public StrRegistryAPIResult CurrentBuildNumber { get; set; } + public IntRegistryAPIResult CurrentBuildRevision { get; set; } } public class ComputerStatus { @@ -85,4 +87,4 @@ public CSVComputerStatus GetCSVStatus(string computerName) { }; } } -} \ No newline at end of file +} diff --git a/src/CommonLib/Processors/ACEGuids.cs b/src/CommonLib/Processors/ACEGuids.cs index 378a3525e..62c29f80d 100644 --- a/src/CommonLib/Processors/ACEGuids.cs +++ b/src/CommonLib/Processors/ACEGuids.cs @@ -17,6 +17,8 @@ public class ACEGuids public const string WriteTitle = "bf967a55-0de6-11d0-a285-00aa003049e2"; // Not an edge, just used for testing public const string WriteAltSecurityIdentities = "00fbf30c-91fe-11d1-aebc-0000f80367c1"; public const string WritePublicInformation = "e48d0154-bcf8-11d1-8702-00c04fb96050"; + public const string WriteSupersededManagedAccountLink = "3752e002-43be-48c8-b3ca-2cb2fffbc8a1"; + public const string WriteSupersededServiceAccountState = "a4693096-a4c3-4ecd-b8cb-73cd6398b7a2"; //Cert abuse ACEs public const string PKINameFlag = "ea1dddc4-60ff-416e-8cc0-17cee534bce7"; diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index 8e0976ab0..d78ac1a98 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -996,17 +996,39 @@ or Label.IssuancePolicy IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, }; - else if (objectType is Label.User or Label.Computer && aceType == ACEGuids.WriteAltSecurityIdentities) - yield return new ACE { - PrincipalType = resolvedPrincipal.ObjectType, - PrincipalSID = resolvedPrincipal.ObjectIdentifier, - IsInherited = inherited, - RightName = EdgeNames.WriteAltSecurityIdentities, - InheritanceHash = aceInheritanceHash, - IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, - IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, - }; - else if (objectType is Label.User or Label.Computer && aceType == ACEGuids.WritePublicInformation) + else if (objectType is Label.User or Label.Computer && + aceType == ACEGuids.WriteSupersededManagedAccountLink) + yield return new ACE { + PrincipalType = resolvedPrincipal.ObjectType, + PrincipalSID = resolvedPrincipal.ObjectIdentifier, + IsInherited = inherited, + RightName = EdgeNames.WriteSupersededManagedAccountLink, + InheritanceHash = aceInheritanceHash, + IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, + IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, + }; + else if (objectType is Label.User or Label.Computer && + aceType == ACEGuids.WriteSupersededServiceAccountState) + yield return new ACE { + PrincipalType = resolvedPrincipal.ObjectType, + PrincipalSID = resolvedPrincipal.ObjectIdentifier, + IsInherited = inherited, + RightName = EdgeNames.WriteSupersededServiceAccountState, + InheritanceHash = aceInheritanceHash, + IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, + IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, + }; + else if (objectType is Label.User or Label.Computer && aceType == ACEGuids.WriteAltSecurityIdentities) + yield return new ACE { + PrincipalType = resolvedPrincipal.ObjectType, + PrincipalSID = resolvedPrincipal.ObjectIdentifier, + IsInherited = inherited, + RightName = EdgeNames.WriteAltSecurityIdentities, + InheritanceHash = aceInheritanceHash, + IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, + IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, + }; + else if (objectType is Label.User or Label.Computer && aceType == ACEGuids.WritePublicInformation) yield return new ACE { PrincipalType = resolvedPrincipal.ObjectType, diff --git a/src/CommonLib/Processors/DCRegistryProcessor.cs b/src/CommonLib/Processors/DCRegistryProcessor.cs index 2726a45a6..127d6ba1e 100644 --- a/src/CommonLib/Processors/DCRegistryProcessor.cs +++ b/src/CommonLib/Processors/DCRegistryProcessor.cs @@ -20,6 +20,67 @@ public DCRegistryProcessor(ILdapUtils utils, ILogger log = null) _log = log ?? Logging.LogProvider.CreateLogger("DCRegProc"); } + internal DCRegistryProcessor(ILdapUtils utils, IRegistryAccessor registryAccessor, ILogger log = null) + { + _utils = utils; + _registryAccessor = registryAccessor; + _log = log ?? Logging.LogProvider.CreateLogger("DCRegProc"); + } + + /// + /// Gets the Windows build number reported by a domain controller. + /// + /// Domain controller to query. + /// The CurrentBuildNumber registry value and collection status. + public StrRegistryAPIResult GetCurrentBuildNumber(string target) + { + var ret = new StrRegistryAPIResult(); + const string subKey = @"SOFTWARE\Microsoft\Windows NT\CurrentVersion"; + const string subValue = "CurrentBuildNumber"; + var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); + + ret.Collected = data.Collected; + if (!data.Collected) + { + ret.FailureReason = data.FailureReason; + return ret; + } + + if (data.Value != null) + { + ret.Value = (string)data.Value; + } + + return ret; + } + + /// + /// Gets the Windows update build revision reported by a domain controller. + /// + /// Domain controller to query. + /// The UBR registry value and collection status. + public IntRegistryAPIResult GetCurrentBuildRevision(string target) + { + var ret = new IntRegistryAPIResult(); + const string subKey = @"SOFTWARE\Microsoft\Windows NT\CurrentVersion"; + const string subValue = "UBR"; + var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); + + ret.Collected = data.Collected; + if (!data.Collected) + { + ret.FailureReason = data.FailureReason; + return ret; + } + + if (data.Value != null) + { + ret.Value = (int)data.Value; + } + + return ret; + } + /// /// This function gets the CertificateMappingMethods registry value stored on DCs. /// @@ -115,4 +176,4 @@ public StrRegistryAPIResult GetVulnerableNetlogonSecurityDescriptor(string targe return ret; } } -} \ No newline at end of file +} diff --git a/test/unit/ACLProcessorTest.cs b/test/unit/ACLProcessorTest.cs index 101dcdc6c..1c50f09bb 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -2438,6 +2438,26 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_Computer_WritePublicInfor Assert.Equal(actual.RightName, expectedRightName); } + [Theory] + [InlineData(Label.User, ACEGuids.WriteSupersededManagedAccountLink, EdgeNames.WriteSupersededManagedAccountLink)] + [InlineData(Label.Computer, ACEGuids.WriteSupersededManagedAccountLink, EdgeNames.WriteSupersededManagedAccountLink)] + [InlineData(Label.User, ACEGuids.WriteSupersededServiceAccountState, EdgeNames.WriteSupersededServiceAccountState)] + [InlineData(Label.Computer, ACEGuids.WriteSupersededServiceAccountState, EdgeNames.WriteSupersededServiceAccountState)] + public async Task ACLProcessor_ProcessACL_WriteProperty_DMSAVictimAttribute(Label objectType, + string attributeGuid, string expectedRightName) { + const string expectedPrincipalSid = "S-1-5-21-3130019616-2776909439-2417379446-512"; + var rule = CreateRuleDescriptor(expectedPrincipalSid, AccessControlType.Allow, + ActiveDirectoryRights.WriteProperty, objectType: new Guid(attributeGuid)); + rule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + + var processor = CreateCombinedAclProcessor(new[] { rule.Object }); + var result = await processor.ProcessACL(new byte[] { 1 }, _testDomainName, objectType, true).ToArrayAsync(); + + var actual = Assert.Single(result); + Assert.Equal(expectedPrincipalSid, actual.PrincipalSID); + Assert.Equal(expectedRightName, actual.RightName); + } + [Fact] public async Task ACLProcessor_ProcessACLWithCustomDenyAces_ReturnsRegularAcesAndDenyCounts() { var denyRule = CreateRuleDescriptor("S-1-5-21-3130019616-2776909439-2417379446-3100", diff --git a/test/unit/DCRegistryProcessorTests.cs b/test/unit/DCRegistryProcessorTests.cs new file mode 100644 index 000000000..cd29c97c4 --- /dev/null +++ b/test/unit/DCRegistryProcessorTests.cs @@ -0,0 +1,63 @@ +using Moq; +using SharpHoundCommonLib; +using SharpHoundCommonLib.Processors; +using Xunit; + +namespace CommonLibTest { + public class DCRegistryProcessorTests { + private const string CurrentVersionKey = @"SOFTWARE\Microsoft\Windows NT\CurrentVersion"; + private const string Target = "dc.test.local"; + + private readonly Mock _registryAccessor = new(); + private readonly DCRegistryProcessor _processor; + + public DCRegistryProcessorTests() { + _processor = new DCRegistryProcessor(Mock.Of(), _registryAccessor.Object); + } + + [Fact] + public void GetCurrentBuildNumber_ReturnsCollectedStringValue() { + _registryAccessor + .Setup(s => s.GetRegistryKeyData(Target, CurrentVersionKey, "CurrentBuildNumber")) + .Returns(new RegistryResult { + Collected = true, + Value = "26100" + }); + + var result = _processor.GetCurrentBuildNumber(Target); + + Assert.True(result.Collected); + Assert.Equal("26100", result.Value); + } + + [Fact] + public void GetCurrentBuildRevision_ReturnsCollectedIntegerValue() { + _registryAccessor + .Setup(s => s.GetRegistryKeyData(Target, CurrentVersionKey, "UBR")) + .Returns(new RegistryResult { + Collected = true, + Value = 4946 + }); + + var result = _processor.GetCurrentBuildRevision(Target); + + Assert.True(result.Collected); + Assert.Equal(4946, result.Value); + } + + [Fact] + public void GetCurrentBuildRevision_PreservesCollectionFailure() { + _registryAccessor + .Setup(s => s.GetRegistryKeyData(Target, CurrentVersionKey, "UBR")) + .Returns(new RegistryResult { + Collected = false, + FailureReason = "access denied" + }); + + var result = _processor.GetCurrentBuildRevision(Target); + + Assert.False(result.Collected); + Assert.Equal("access denied", result.FailureReason); + } + } +} From d975701b076d11c80b7761161a31e3ea78286918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adil=20Alperen=20=C3=87iftci?= Date: Tue, 27 Oct 2026 15:27:26 +0300 Subject: [PATCH 2/3] fix: preserve missing domain controller build data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I now report absent build values as collection failures instead of valid empty or zero data, with regression coverage for both registry values. Signed-off-by: Adil Alperen Çiftci --- .../Processors/DCRegistryProcessor.cs | 16 ++++++---- test/unit/DCRegistryProcessorTests.cs | 31 +++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/CommonLib/Processors/DCRegistryProcessor.cs b/src/CommonLib/Processors/DCRegistryProcessor.cs index 127d6ba1e..556d1efcb 100644 --- a/src/CommonLib/Processors/DCRegistryProcessor.cs +++ b/src/CommonLib/Processors/DCRegistryProcessor.cs @@ -39,18 +39,20 @@ public StrRegistryAPIResult GetCurrentBuildNumber(string target) const string subValue = "CurrentBuildNumber"; var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); - ret.Collected = data.Collected; if (!data.Collected) { ret.FailureReason = data.FailureReason; return ret; } - if (data.Value != null) + if (data.Value == null) { - ret.Value = (string)data.Value; + ret.FailureReason = "Registry value was not found"; + return ret; } + ret.Collected = true; + ret.Value = (string)data.Value; return ret; } @@ -66,18 +68,20 @@ public IntRegistryAPIResult GetCurrentBuildRevision(string target) const string subValue = "UBR"; var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); - ret.Collected = data.Collected; if (!data.Collected) { ret.FailureReason = data.FailureReason; return ret; } - if (data.Value != null) + if (data.Value == null) { - ret.Value = (int)data.Value; + ret.FailureReason = "Registry value was not found"; + return ret; } + ret.Collected = true; + ret.Value = (int)data.Value; return ret; } diff --git a/test/unit/DCRegistryProcessorTests.cs b/test/unit/DCRegistryProcessorTests.cs index cd29c97c4..92deb968a 100644 --- a/test/unit/DCRegistryProcessorTests.cs +++ b/test/unit/DCRegistryProcessorTests.cs @@ -59,5 +59,36 @@ public void GetCurrentBuildRevision_PreservesCollectionFailure() { Assert.False(result.Collected); Assert.Equal("access denied", result.FailureReason); } + + [Fact] + public void GetCurrentBuildNumber_ReportsMissingValue() { + _registryAccessor + .Setup(s => s.GetRegistryKeyData(Target, CurrentVersionKey, "CurrentBuildNumber")) + .Returns(new RegistryResult { + Collected = true, + Value = null + }); + + var result = _processor.GetCurrentBuildNumber(Target); + + Assert.False(result.Collected); + Assert.Null(result.Value); + Assert.Equal("Registry value was not found", result.FailureReason); + } + + [Fact] + public void GetCurrentBuildRevision_ReportsMissingValue() { + _registryAccessor + .Setup(s => s.GetRegistryKeyData(Target, CurrentVersionKey, "UBR")) + .Returns(new RegistryResult { + Collected = true, + Value = null + }); + + var result = _processor.GetCurrentBuildRevision(Target); + + Assert.False(result.Collected); + Assert.Equal("Registry value was not found", result.FailureReason); + } } } From 09344aa569dcc30e9579cee49dd7b02bbab64635 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adil=20Alperen=20=C3=87iftci?= Date: Tue, 27 Oct 2026 16:04:01 +0300 Subject: [PATCH 3/3] feat: collect delegated managed service accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I now classify delegated managed service accounts separately, collect their predecessor links, and emit CreateDMSA rights from access controls on valid OU and container parents. Signed-off-by: Adil Alperen Çiftci --- .../DirectoryObjectExtensions.cs | 10 +++- src/CommonLib/Enums/DataType.cs | 1 + src/CommonLib/Enums/EdgeNames.cs | 1 + src/CommonLib/Enums/LDAPProperties.cs | 1 + src/CommonLib/Enums/Labels.cs | 3 +- src/CommonLib/Enums/ObjectClass.cs | 1 + src/CommonLib/LdapQueries/CommonProperties.cs | 2 +- src/CommonLib/LdapUtils.cs | 19 ++++--- src/CommonLib/OutputTypes/DMSA.cs | 7 +++ src/CommonLib/Processors/ACEGuids.cs | 1 + src/CommonLib/Processors/ACLProcessor.cs | 18 ++++++- test/unit/ACLProcessorTest.cs | 50 +++++++++++++++++++ test/unit/DirectoryObjectTests.cs | 18 +++++++ test/unit/LDAPUtilsTest.cs | 24 +++++++++ test/unit/LdapProducerQueryGeneratorTest.cs | 8 +++ 15 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 src/CommonLib/OutputTypes/DMSA.cs diff --git a/src/CommonLib/DirectoryObjects/DirectoryObjectExtensions.cs b/src/CommonLib/DirectoryObjects/DirectoryObjectExtensions.cs index a5af2c693..2d0894e42 100644 --- a/src/CommonLib/DirectoryObjects/DirectoryObjectExtensions.cs +++ b/src/CommonLib/DirectoryObjects/DirectoryObjectExtensions.cs @@ -20,7 +20,15 @@ public static bool IsGMSA(this IDirectoryObject directoryObject) { return classes.Contains(ObjectClass.GMSAClass, StringComparer.InvariantCultureIgnoreCase); } - + + public static bool IsDMSA(this IDirectoryObject directoryObject) { + if (!directoryObject.TryGetArrayProperty(LDAPProperties.ObjectClass, out var classes)) { + return false; + } + + return classes.Contains(ObjectClass.DMSAClass, StringComparer.InvariantCultureIgnoreCase); + } + public static bool GetObjectIdentifier(this IDirectoryObject directoryObject, out string objectIdentifier) { // Builtin container has a SID (always "S-1-5-32"). We use the ObjectGUID as ID like for other containers if (directoryObject.TryGetArrayProperty(LDAPProperties.ObjectClass, out var objectClasses) && diff --git a/src/CommonLib/Enums/DataType.cs b/src/CommonLib/Enums/DataType.cs index 2553fd529..1e56eb940 100644 --- a/src/CommonLib/Enums/DataType.cs +++ b/src/CommonLib/Enums/DataType.cs @@ -18,5 +18,6 @@ public static class DataType public const string Sites = "sites"; public const string SiteServers = "siteservers"; public const string SiteSubnets = "sitesubnets"; + public const string DMSAs = "dmsas"; } } diff --git a/src/CommonLib/Enums/EdgeNames.cs b/src/CommonLib/Enums/EdgeNames.cs index a5d594958..a6ec133fc 100644 --- a/src/CommonLib/Enums/EdgeNames.cs +++ b/src/CommonLib/Enums/EdgeNames.cs @@ -26,6 +26,7 @@ public static class EdgeNames public const string WritePublicInformation = "WritePublicInformation"; public const string WriteSupersededManagedAccountLink = "WriteSupersededManagedAccountLink"; public const string WriteSupersededServiceAccountState = "WriteSupersededServiceAccountState"; + public const string CreateDMSA = "CreateDMSA"; public const string ServerIs = "ServerIs"; //CertAbuse edges diff --git a/src/CommonLib/Enums/LDAPProperties.cs b/src/CommonLib/Enums/LDAPProperties.cs index da1298d05..fcfb55c03 100644 --- a/src/CommonLib/Enums/LDAPProperties.cs +++ b/src/CommonLib/Enums/LDAPProperties.cs @@ -3,6 +3,7 @@ public static class LDAPProperties { public const string GroupMSAMembership = "msds-groupmsamembership"; + public const string ManagedAccountPrecededByLink = "msds-managedaccountprecededbylink"; public const string UserAccountControl = "useraccountcontrol"; public const string IsDeleted = "isdeleted"; public const string SAMAccountType = "samaccounttype"; diff --git a/src/CommonLib/Enums/Labels.cs b/src/CommonLib/Enums/Labels.cs index f961430a4..8b0f90c9e 100644 --- a/src/CommonLib/Enums/Labels.cs +++ b/src/CommonLib/Enums/Labels.cs @@ -21,6 +21,7 @@ public enum Label IssuancePolicy, Site, SiteServer, - SiteSubnet + SiteSubnet, + DMSA } } diff --git a/src/CommonLib/Enums/ObjectClass.cs b/src/CommonLib/Enums/ObjectClass.cs index 1113999df..ee9476fc3 100644 --- a/src/CommonLib/Enums/ObjectClass.cs +++ b/src/CommonLib/Enums/ObjectClass.cs @@ -14,6 +14,7 @@ public static class ObjectClass { public const string OIDContainerClass = "msPKI-Enterprise-Oid"; public const string GMSAClass = "msds-groupmanagedserviceaccount"; public const string MSAClass = "msds-managedserviceaccount"; + public const string DMSAClass = "msds-delegatedmanagedserviceaccount"; public const string SiteClass = "site"; public const string SiteServerClass = "server"; public const string SiteSubnetClass = "subnet"; diff --git a/src/CommonLib/LdapQueries/CommonProperties.cs b/src/CommonLib/LdapQueries/CommonProperties.cs index 5ed308d42..554bcf43f 100644 --- a/src/CommonLib/LdapQueries/CommonProperties.cs +++ b/src/CommonLib/LdapQueries/CommonProperties.cs @@ -62,7 +62,7 @@ public static class CommonProperties LDAPProperties.MinPwdLength, LDAPProperties.PwdProperties, LDAPProperties.MinPwdAge, LDAPProperties.MaxPwdAge, LDAPProperties.PwdHistoryLength, LDAPProperties.LockoutDuration, LDAPProperties.LockoutThreshold, LDAPProperties.LockOutObservationWindow, LDAPProperties.GroupType, - LDAPProperties.PrincipalName + LDAPProperties.PrincipalName, LDAPProperties.ManagedAccountPrecededByLink }; public static readonly string[] ContainerProps = diff --git a/src/CommonLib/LdapUtils.cs b/src/CommonLib/LdapUtils.cs index 2bc4a7745..9b8ddf056 100644 --- a/src/CommonLib/LdapUtils.cs +++ b/src/CommonLib/LdapUtils.cs @@ -1166,12 +1166,18 @@ internal static bool ResolveLabel(string objectIdentifier, string distinguishedN return true; } - //Override GMSA/MSA account to treat them as users for the graph - if (objectClasses != null && - (objectClasses.Contains(ObjectClass.MSAClass, StringComparer.OrdinalIgnoreCase) || - objectClasses.Contains(ObjectClass.GMSAClass, StringComparer.OrdinalIgnoreCase))) { - type = Label.User; - return true; + if (objectClasses != null) { + if (objectClasses.Contains(ObjectClass.DMSAClass, StringComparer.OrdinalIgnoreCase)) { + type = Label.DMSA; + return true; + } + + //Override GMSA/MSA account to treat them as users for the graph + if (objectClasses.Contains(ObjectClass.MSAClass, StringComparer.OrdinalIgnoreCase) || + objectClasses.Contains(ObjectClass.GMSAClass, StringComparer.OrdinalIgnoreCase)) { + type = Label.User; + return true; + } } if (samAccountType != null) { @@ -1344,6 +1350,7 @@ private static string ComputeDisplayName(IDirectoryObject directoryObject, strin string displayName; switch (label) { case Label.User: + case Label.DMSA: case Label.Group: case Label.Base: if (!string.IsNullOrWhiteSpace(samAccountName)) { diff --git a/src/CommonLib/OutputTypes/DMSA.cs b/src/CommonLib/OutputTypes/DMSA.cs new file mode 100644 index 000000000..8e649c79d --- /dev/null +++ b/src/CommonLib/OutputTypes/DMSA.cs @@ -0,0 +1,7 @@ +namespace SharpHoundCommonLib.OutputTypes +{ + public class DMSA : OutputBase + { + public TypedPrincipal PrecededBy { get; set; } + } +} diff --git a/src/CommonLib/Processors/ACEGuids.cs b/src/CommonLib/Processors/ACEGuids.cs index 62c29f80d..538d378d6 100644 --- a/src/CommonLib/Processors/ACEGuids.cs +++ b/src/CommonLib/Processors/ACEGuids.cs @@ -19,6 +19,7 @@ public class ACEGuids public const string WritePublicInformation = "e48d0154-bcf8-11d1-8702-00c04fb96050"; public const string WriteSupersededManagedAccountLink = "3752e002-43be-48c8-b3ca-2cb2fffbc8a1"; public const string WriteSupersededServiceAccountState = "a4693096-a4c3-4ecd-b8cb-73cd6398b7a2"; + public const string DMSAClass = "0feb936f-47b3-49f2-9386-1dedc2c23765"; //Cert abuse ACEs public const string PKINameFlag = "ea1dddc4-60ff-416e-8cc0-17cee534bce7"; diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index d78ac1a98..4dd3b5798 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -126,7 +126,8 @@ static ACLProcessor() { { Label.IssuancePolicy, "37cfd85c-6719-4ad8-8f9e-8678ba627563" }, { Label.Site, "bf967ab3-0de6-11d0-a285-00aa003049e2" }, { Label.SiteServer, "bf967a92-0de6-11d0-a285-00aa003049e2" }, - { Label.SiteSubnet, "b7b13124-b82e-11d0-afee-0000f80367c1" } + { Label.SiteSubnet, "b7b13124-b82e-11d0-afee-0000f80367c1" }, + { Label.DMSA, ACEGuids.DMSAClass } }; } @@ -724,6 +725,21 @@ await CountCustomDenyAce(ace, customDenyAceAccumulator, objectDomain, objectType } } + if (objectType is Label.OU or Label.Container && + (aceRights.HasFlag(ActiveDirectoryRights.CreateChild) || + aceRights.HasFlag(ActiveDirectoryRights.GenericAll)) && + aceType is ACEGuids.DMSAClass or ACEGuids.AllGuid or "") { + yield return new ACE { + PrincipalType = resolvedPrincipal.ObjectType, + PrincipalSID = resolvedPrincipal.ObjectIdentifier, + IsInherited = inherited, + RightName = EdgeNames.CreateDMSA, + InheritanceHash = aceInheritanceHash, + IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, + IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, + }; + } + //// This log is exceptionally noisy, disabling // _log.LogTrace("Processing ACE with rights {Rights} and guid {GUID} on object {Name}", aceRights, // aceType, objectName); diff --git a/test/unit/ACLProcessorTest.cs b/test/unit/ACLProcessorTest.cs index 1c50f09bb..630043f04 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -2458,6 +2458,56 @@ public async Task ACLProcessor_ProcessACL_WriteProperty_DMSAVictimAttribute(Labe Assert.Equal(expectedRightName, actual.RightName); } + [Theory] + [InlineData(ACEGuids.DMSAClass, Label.OU)] + [InlineData(ACEGuids.AllGuid, Label.OU)] + [InlineData(ACEGuids.DMSAClass, Label.Container)] + public async Task ACLProcessor_ProcessACL_CreateChild_DMSAOnParent(string objectGuid, Label parentType) { + const string expectedPrincipalSid = "S-1-5-21-3130019616-2776909439-2417379446-512"; + var rule = CreateRuleDescriptor(expectedPrincipalSid, AccessControlType.Allow, + ActiveDirectoryRights.CreateChild, objectType: new Guid(objectGuid)); + rule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + + var processor = CreateCombinedAclProcessor(new[] { rule.Object }); + var result = await processor.ProcessACL(new byte[] { 1 }, _testDomainName, parentType, false) + .ToArrayAsync(); + + var actual = Assert.Single(result); + Assert.Equal(expectedPrincipalSid, actual.PrincipalSID); + Assert.Equal(EdgeNames.CreateDMSA, actual.RightName); + } + + [Fact] + public async Task ACLProcessor_ProcessACL_GenericAll_OnOUIncludesCreateDMSA() { + const string expectedPrincipalSid = "S-1-5-21-3130019616-2776909439-2417379446-512"; + var rule = CreateRuleDescriptor(expectedPrincipalSid, AccessControlType.Allow, + ActiveDirectoryRights.GenericAll, objectType: new Guid(ACEGuids.AllGuid)); + rule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + + var processor = CreateCombinedAclProcessor(new[] { rule.Object }); + var result = await processor.ProcessACL(new byte[] { 1 }, _testDomainName, Label.OU, false) + .ToArrayAsync(); + + Assert.Contains(result, ace => ace.RightName == EdgeNames.CreateDMSA); + Assert.Contains(result, ace => ace.RightName == EdgeNames.GenericAll); + } + + [Fact] + public async Task ACLProcessor_ProcessACL_GenericAll_DMSA() { + const string expectedPrincipalSid = "S-1-5-21-3130019616-2776909439-2417379446-512"; + var rule = CreateRuleDescriptor(expectedPrincipalSid, AccessControlType.Allow, + ActiveDirectoryRights.GenericAll, objectType: new Guid(ACEGuids.AllGuid)); + rule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + + var processor = CreateCombinedAclProcessor(new[] { rule.Object }); + var result = await processor.ProcessACL(new byte[] { 1 }, _testDomainName, Label.DMSA, false) + .ToArrayAsync(); + + var actual = Assert.Single(result); + Assert.Equal(expectedPrincipalSid, actual.PrincipalSID); + Assert.Equal(EdgeNames.GenericAll, actual.RightName); + } + [Fact] public async Task ACLProcessor_ProcessACLWithCustomDenyAces_ReturnsRegularAcesAndDenyCounts() { var denyRule = CreateRuleDescriptor("S-1-5-21-3130019616-2776909439-2417379446-3100", diff --git a/test/unit/DirectoryObjectTests.cs b/test/unit/DirectoryObjectTests.cs index cde14c4c9..846ab98c1 100644 --- a/test/unit/DirectoryObjectTests.cs +++ b/test/unit/DirectoryObjectTests.cs @@ -114,6 +114,24 @@ public void Test_IsGMSA() { Assert.False(mock.IsMSA()); } + [Fact] + public void Test_IsDMSA() { + var attribs = new Dictionary { + { LDAPProperties.ObjectClass, new[] { "top", ObjectClass.DMSAClass } }, + }; + + var mock = new MockDirectoryObject("abc", attribs, "", ""); + Assert.True(mock.IsDMSA()); + + mock.Properties = new Dictionary { + { LDAPProperties.ObjectClass, new[] { "top" } }, + }; + Assert.False(mock.IsDMSA()); + + mock.Properties = new Dictionary(); + Assert.False(mock.IsDMSA()); + } + [Fact] public void Test_GetLabel_BadObjectID() { var attribs = new Dictionary { diff --git a/test/unit/LDAPUtilsTest.cs b/test/unit/LDAPUtilsTest.cs index 906318a1a..647723b15 100644 --- a/test/unit/LDAPUtilsTest.cs +++ b/test/unit/LDAPUtilsTest.cs @@ -224,6 +224,30 @@ public async Task Test_ResolveSearchResult_MSAGMSA() { Assert.False(result.Deleted); } + [Fact] + public async Task Test_ResolveSearchResult_DMSA() { + var utils = new MockLdapUtils(); + var attribs = new Dictionary { + { LDAPProperties.ObjectClass, new[] { "top", ObjectClass.DMSAClass } }, + { LDAPProperties.SAMAccountType, "805306369" }, + { LDAPProperties.SAMAccountName, "TESTDMSA$" } + }; + + const string sid = "S-1-5-21-3130019616-2776909439-2417379446-2106"; + const string dn = "CN=TESTDMSA,OU=SERVICE ACCOUNTS,DC=TESTLAB,DC=LOCAL"; + var mock = new MockDirectoryObject(dn, attribs, sid, new Guid().ToString()); + + var (success, result) = await LdapUtils.ResolveSearchResult(mock, utils); + + Assert.True(success); + Assert.Equal(sid, result.ObjectId); + Assert.Equal(Label.DMSA, result.ObjectType); + Assert.Equal("TESTDMSA$@TESTLAB.LOCAL", result.DisplayName); + Assert.Equal("S-1-5-21-3130019616-2776909439-2417379446", result.DomainSid); + Assert.Equal("TESTLAB.LOCAL", result.Domain); + Assert.False(result.Deleted); + } + [Fact] public async Task Test_ResolveSearchResult_TrustAccount() { var utils = new MockLdapUtils(); diff --git a/test/unit/LdapProducerQueryGeneratorTest.cs b/test/unit/LdapProducerQueryGeneratorTest.cs index 0a2f8c61f..f60005cbf 100644 --- a/test/unit/LdapProducerQueryGeneratorTest.cs +++ b/test/unit/LdapProducerQueryGeneratorTest.cs @@ -28,6 +28,14 @@ public void GenerateDefaultPartitionParameters_Container_IncludesBuiltinDomainFi Assert.Contains("(objectClass=builtinDomain)", result.Filter.GetFilter()); } + [Fact] + public void GenerateDefaultPartitionParameters_ObjectProps_IncludesDMSAPredecessor() + { + var result = LdapProducerQueryGenerator.GenerateDefaultPartitionParameters(CollectionMethod.ObjectProps); + + Assert.Contains(LDAPProperties.ManagedAccountPrecededByLink, result.Attributes); + } + [Fact] public void GenerateConfigurationPartitionParameters_Site_IncludesSiteFiltersAndProperties() {