From 5fe9fab2a18374f43c1be7f70c5d43e90df3e09c Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 8 Oct 2024 17:11:17 -0400 Subject: [PATCH 1/7] fix: laps guids not being properly processed from schema --- src/CommonLib/Processors/ACLProcessor.cs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index af69068a..dbb471fa 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -18,7 +18,7 @@ public class ACLProcessor { private static readonly ConcurrentDictionary GuidMap = new(); private readonly ILogger _log; private readonly ILdapUtils _utils; - private static readonly HashSet BuiltDomainCaches = new(StringComparer.OrdinalIgnoreCase); + private static readonly ConcurrentHashSet BuiltDomainCaches = new(StringComparer.OrdinalIgnoreCase); static ACLProcessor() { //Create a dictionary with the base GUIDs of each object type @@ -50,8 +50,8 @@ public ACLProcessor(ILdapUtils utils, ILogger log = null) { /// LAPS /// private async Task BuildGuidCache(string domain) { - BuiltDomainCaches.Add(domain); - await foreach (var result in _utils.Query(new LdapQueryParameters { + _log.LogInformation("Building GUID Cache for {Domain}", domain); + await foreach (var result in _utils.PagedQuery(new LdapQueryParameters { DomainName = domain, LDAPFilter = "(schemaIDGUID=*)", NamingContext = NamingContext.Schema, @@ -59,13 +59,23 @@ private async Task BuildGuidCache(string domain) { })) { if (result.IsSuccess) { if (!result.Value.TryGetProperty(LDAPProperties.Name, out var name) || - !result.Value.TryGetGuid(out var guid)) { + !result.Value.TryGetByteProperty(LDAPProperties.SchemaIDGUID, out var schemaGuid)) { continue; } name = name.ToLower(); + string guid; + try + { + guid = new Guid(schemaGuid).ToString(); + } + catch + { + continue; + } + if (name is LDAPProperties.LAPSPassword or LDAPProperties.LegacyLAPSPassword) { - _log.LogDebug("Found GUID for ACL Right {Name}: {Guid} in domain {Domain}", name, guid, domain); + _log.LogInformation("Found GUID for ACL Right {Name}: {Guid} in domain {Domain}", name, guid, domain); GuidMap.TryAdd(guid, name); } } else { @@ -218,6 +228,7 @@ public async IAsyncEnumerable ProcessACL(byte[] ntSecurityDescriptor, strin Label objectType, bool hasLaps, string objectName = "") { if (!BuiltDomainCaches.Contains(objectDomain)) { + BuiltDomainCaches.Add(objectDomain); await BuildGuidCache(objectDomain); } From d76735ca9d8f4c3f7802455a9b9a9ced3a7aa136 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 8 Oct 2024 17:39:31 -0400 Subject: [PATCH 2/7] chore: fix tests, add test for laps aces --- test/unit/ACLProcessorTest.cs | 94 +++++++++++++++++++++++++++-------- 1 file changed, 73 insertions(+), 21 deletions(-) diff --git a/test/unit/ACLProcessorTest.cs b/test/unit/ACLProcessorTest.cs index f002a389..bd0360da 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -62,7 +62,7 @@ public async Task ACLProcessor_TestKnownDataAddMember() { var mockLdapUtils = new MockLdapUtils(); var mockUtils = new Mock(); var mockData = new[] { LdapResult.Fail() }; - mockUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); mockUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .Returns((string a, string b) => mockLdapUtils.ResolveIDAndType(a, b)); @@ -236,7 +236,7 @@ public async Task ACLProcessor_ProcessACL_Yields_Owns_ACE() { .ReturnsAsync((true, new TypedPrincipal(expectedSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -377,7 +377,7 @@ public async Task ACLProcessor_ProcessACL_GenericAll_Unmatched_Guid() { mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -410,7 +410,7 @@ public async Task ACLProcessor_ProcessACL_GenericAll() { mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -449,7 +449,7 @@ public async Task ACLProcessor_ProcessACL_WriteDacl() { mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -488,7 +488,7 @@ public async Task ACLProcessor_ProcessACL_WriteOwner() { mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -527,7 +527,7 @@ public async Task ACLProcessor_ProcessACL_Self() { mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -565,7 +565,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_Domain_Unmatched() { mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -598,7 +598,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_Domain_DSReplicationGetC mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -636,7 +636,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_Domain_All() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -675,9 +675,9 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_Domain_DSReplicationGetC mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); var mockData = new[] { LdapResult.Fail() }; - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(mockData.ToAsyncEnumerable()); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -715,7 +715,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_User_Unmatched() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -748,7 +748,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_User_UserForceChangePass mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -786,7 +786,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_User_All() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -823,7 +823,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_Computer_NoLAPS() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -856,7 +856,7 @@ public async Task ACLProcessor_ProcessACL_ExtendedRight_Computer_All() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -893,7 +893,7 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_Unmatched() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -926,7 +926,7 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_User_All() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -964,7 +964,7 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_User_WriteMember() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -1004,7 +1004,7 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_Computer_WriteAllowedToAc mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - mockLDAPUtils.Setup(x => x.Query(It.IsAny(), It.IsAny())) + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) .Returns(Array.Empty>().ToAsyncEnumerable); var processor = new ACLProcessor(mockLDAPUtils.Object); @@ -1018,6 +1018,58 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_Computer_WriteAllowedToAc Assert.False(actual.IsInherited); Assert.Equal(actual.RightName, expectedRightName); } + + [Fact] + public async Task ACLProcessor_ProcessACL_LAPS_Computer() { + var expectedPrincipalType = Label.Group; + var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + var expectedRightName = EdgeNames.ReadLAPSPassword; + + var mockLDAPUtils = new Mock(); + var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); + var mockRule = new Mock(MockBehavior.Loose, null); + var collection = new List(); + mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow); + mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID); + mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.ExtendedRight); + var lapsGuid = Guid.NewGuid(); + mockRule.Setup(x => x.ObjectType()).Returns(lapsGuid); + collection.Add(mockRule.Object); + + mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(collection); + mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); + mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + + + var searchResults = new[] + { + //These first 4 should be filtered by our DN filters + LdapResult.Ok(new MockDirectoryObject( + "CN=7868d4c8-ac41-4e05-b401-776280e8e9f1,CN=Operations,CN=DomainUpdates,CN=System,DC=testlab,DC=local" + , new Dictionary() + { + {LDAPProperties.SchemaIDGUID, lapsGuid.ToByteArray()}, + {LDAPProperties.Name, LDAPProperties.LegacyLAPSPassword} + }, null,null)), + }; + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(searchResults.ToAsyncEnumerable); + + var processor = new ACLProcessor(mockLDAPUtils.Object); + var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); + var result = await processor.ProcessACL(bytes, _testDomainName, Label.Computer, true).ToArrayAsync(); + + Assert.Single(result); + var actual = result.First(); + Assert.Equal(actual.PrincipalType, expectedPrincipalType); + Assert.Equal(actual.PrincipalSID, expectedPrincipalSID); + Assert.False(actual.IsInherited); + Assert.Equal(actual.RightName, expectedRightName); + } [Fact] public void GetInheritedAceHashes_NullSD_Empty() { From 5ad24318c84f5b6be93e42fec0ed1b35d6bad5b7 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 8 Oct 2024 17:47:50 -0400 Subject: [PATCH 3/7] chore: fix comment --- test/unit/ACLProcessorTest.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/unit/ACLProcessorTest.cs b/test/unit/ACLProcessorTest.cs index bd0360da..a4d7c48b 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -1043,13 +1043,12 @@ public async Task ACLProcessor_ProcessACL_LAPS_Computer() { mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - + //Return a directory object from pagedquery for the schemaid to simulate LAPS var searchResults = new[] { - //These first 4 should be filtered by our DN filters LdapResult.Ok(new MockDirectoryObject( - "CN=7868d4c8-ac41-4e05-b401-776280e8e9f1,CN=Operations,CN=DomainUpdates,CN=System,DC=testlab,DC=local" + "abc123" , new Dictionary() { {LDAPProperties.SchemaIDGUID, lapsGuid.ToByteArray()}, From 6e75eb7e0594fdbc8a11c8b7a68a858b31cf6f0d Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 8 Oct 2024 17:54:54 -0400 Subject: [PATCH 4/7] chore: bump to 8.0 as 7.0 is deprecated --- test/unit/CommonLibTest.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/CommonLibTest.csproj b/test/unit/CommonLibTest.csproj index 717d5c93..cd4ca2f3 100644 --- a/test/unit/CommonLibTest.csproj +++ b/test/unit/CommonLibTest.csproj @@ -1,7 +1,7 @@ - net7.0 + net8.0 false true ..\..\docfx\coverage\ From 21b82532d80e1c85e5cc01ff7cc5dcac5059c222 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 8 Oct 2024 17:59:20 -0400 Subject: [PATCH 5/7] chore: comment out test for now --- test/unit/ACLProcessorTest.cs | 100 +++++++++++++++++----------------- 1 file changed, 50 insertions(+), 50 deletions(-) diff --git a/test/unit/ACLProcessorTest.cs b/test/unit/ACLProcessorTest.cs index a4d7c48b..eafd3422 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -1019,56 +1019,56 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_Computer_WriteAllowedToAc Assert.Equal(actual.RightName, expectedRightName); } - [Fact] - public async Task ACLProcessor_ProcessACL_LAPS_Computer() { - var expectedPrincipalType = Label.Group; - var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; - var expectedRightName = EdgeNames.ReadLAPSPassword; - - var mockLDAPUtils = new Mock(); - var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); - var mockRule = new Mock(MockBehavior.Loose, null); - var collection = new List(); - mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow); - mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); - mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID); - mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.ExtendedRight); - var lapsGuid = Guid.NewGuid(); - mockRule.Setup(x => x.ObjectType()).Returns(lapsGuid); - collection.Add(mockRule.Object); - - mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns(collection); - mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); - mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); - mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) - .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - - //Return a directory object from pagedquery for the schemaid to simulate LAPS - var searchResults = new[] - { - LdapResult.Ok(new MockDirectoryObject( - "abc123" - , new Dictionary() - { - {LDAPProperties.SchemaIDGUID, lapsGuid.ToByteArray()}, - {LDAPProperties.Name, LDAPProperties.LegacyLAPSPassword} - }, null,null)), - }; - mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) - .Returns(searchResults.ToAsyncEnumerable); - - var processor = new ACLProcessor(mockLDAPUtils.Object); - var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); - var result = await processor.ProcessACL(bytes, _testDomainName, Label.Computer, true).ToArrayAsync(); - - Assert.Single(result); - var actual = result.First(); - Assert.Equal(actual.PrincipalType, expectedPrincipalType); - Assert.Equal(actual.PrincipalSID, expectedPrincipalSID); - Assert.False(actual.IsInherited); - Assert.Equal(actual.RightName, expectedRightName); - } + // [Fact] + // public async Task ACLProcessor_ProcessACL_LAPS_Computer() { + // var expectedPrincipalType = Label.Group; + // var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + // var expectedRightName = EdgeNames.ReadLAPSPassword; + // + // var mockLDAPUtils = new Mock(); + // var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); + // var mockRule = new Mock(MockBehavior.Loose, null); + // var collection = new List(); + // mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow); + // mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + // mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID); + // mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.ExtendedRight); + // var lapsGuid = Guid.NewGuid(); + // mockRule.Setup(x => x.ObjectType()).Returns(lapsGuid); + // collection.Add(mockRule.Object); + // + // mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) + // .Returns(collection); + // mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); + // mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + // mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) + // .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + // + // //Return a directory object from pagedquery for the schemaid to simulate LAPS + // var searchResults = new[] + // { + // LdapResult.Ok(new MockDirectoryObject( + // "abc123" + // , new Dictionary() + // { + // {LDAPProperties.SchemaIDGUID, lapsGuid.ToByteArray()}, + // {LDAPProperties.Name, LDAPProperties.LegacyLAPSPassword} + // }, null,null)), + // }; + // mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + // .Returns(searchResults.ToAsyncEnumerable); + // + // var processor = new ACLProcessor(mockLDAPUtils.Object); + // var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); + // var result = await processor.ProcessACL(bytes, _testDomainName, Label.Computer, true).ToArrayAsync(); + // + // Assert.Single(result); + // var actual = result.First(); + // Assert.Equal(actual.PrincipalType, expectedPrincipalType); + // Assert.Equal(actual.PrincipalSID, expectedPrincipalSID); + // Assert.False(actual.IsInherited); + // Assert.Equal(actual.RightName, expectedRightName); + // } [Fact] public void GetInheritedAceHashes_NullSD_Empty() { From 6927f71c561fcc8253c320694e0cd5ba84b109de Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 8 Oct 2024 18:04:21 -0400 Subject: [PATCH 6/7] fix: change from static to instance so tests work --- src/CommonLib/Processors/ACLProcessor.cs | 12 +-- test/unit/ACLProcessorTest.cs | 100 +++++++++++------------ 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index dbb471fa..7afbb9d5 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -15,10 +15,10 @@ namespace SharpHoundCommonLib.Processors { public class ACLProcessor { private static readonly Dictionary BaseGuids; - private static readonly ConcurrentDictionary GuidMap = new(); + private readonly ConcurrentDictionary _guidMap = new(); private readonly ILogger _log; private readonly ILdapUtils _utils; - private static readonly ConcurrentHashSet BuiltDomainCaches = new(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentHashSet _builtDomainCaches = new(StringComparer.OrdinalIgnoreCase); static ACLProcessor() { //Create a dictionary with the base GUIDs of each object type @@ -76,7 +76,7 @@ private async Task BuildGuidCache(string domain) { if (name is LDAPProperties.LAPSPassword or LDAPProperties.LegacyLAPSPassword) { _log.LogInformation("Found GUID for ACL Right {Name}: {Guid} in domain {Domain}", name, guid, domain); - GuidMap.TryAdd(guid, name); + _guidMap.TryAdd(guid, name); } } else { _log.LogDebug("Error while building GUID cache for {Domain}: {Message}", domain, result.Error); @@ -227,8 +227,8 @@ 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); + if (!_builtDomainCaches.Contains(objectDomain)) { + _builtDomainCaches.Add(objectDomain); await BuildGuidCache(objectDomain); } @@ -299,7 +299,7 @@ public async IAsyncEnumerable ProcessACL(byte[] ntSecurityDescriptor, strin aceInheritanceHash = CalculateInheritanceHash(ir, aceRights, aceType, ace.InheritedObjectType()); } - GuidMap.TryGetValue(aceType, out var mappedGuid); + _guidMap.TryGetValue(aceType, out var mappedGuid); _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 eafd3422..a4d7c48b 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -1019,56 +1019,56 @@ public async Task ACLProcessor_ProcessACL_GenericWrite_Computer_WriteAllowedToAc Assert.Equal(actual.RightName, expectedRightName); } - // [Fact] - // public async Task ACLProcessor_ProcessACL_LAPS_Computer() { - // var expectedPrincipalType = Label.Group; - // var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; - // var expectedRightName = EdgeNames.ReadLAPSPassword; - // - // var mockLDAPUtils = new Mock(); - // var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); - // var mockRule = new Mock(MockBehavior.Loose, null); - // var collection = new List(); - // mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow); - // mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); - // mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID); - // mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.ExtendedRight); - // var lapsGuid = Guid.NewGuid(); - // mockRule.Setup(x => x.ObjectType()).Returns(lapsGuid); - // collection.Add(mockRule.Object); - // - // mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) - // .Returns(collection); - // mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); - // mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); - // mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) - // .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - // - // //Return a directory object from pagedquery for the schemaid to simulate LAPS - // var searchResults = new[] - // { - // LdapResult.Ok(new MockDirectoryObject( - // "abc123" - // , new Dictionary() - // { - // {LDAPProperties.SchemaIDGUID, lapsGuid.ToByteArray()}, - // {LDAPProperties.Name, LDAPProperties.LegacyLAPSPassword} - // }, null,null)), - // }; - // mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) - // .Returns(searchResults.ToAsyncEnumerable); - // - // var processor = new ACLProcessor(mockLDAPUtils.Object); - // var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); - // var result = await processor.ProcessACL(bytes, _testDomainName, Label.Computer, true).ToArrayAsync(); - // - // Assert.Single(result); - // var actual = result.First(); - // Assert.Equal(actual.PrincipalType, expectedPrincipalType); - // Assert.Equal(actual.PrincipalSID, expectedPrincipalSID); - // Assert.False(actual.IsInherited); - // Assert.Equal(actual.RightName, expectedRightName); - // } + [Fact] + public async Task ACLProcessor_ProcessACL_LAPS_Computer() { + var expectedPrincipalType = Label.Group; + var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + var expectedRightName = EdgeNames.ReadLAPSPassword; + + var mockLDAPUtils = new Mock(); + var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); + var mockRule = new Mock(MockBehavior.Loose, null); + var collection = new List(); + mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow); + mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny())).Returns(true); + mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID); + mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.ExtendedRight); + var lapsGuid = Guid.NewGuid(); + mockRule.Setup(x => x.ObjectType()).Returns(lapsGuid); + collection.Add(mockRule.Object); + + mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(collection); + mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); + mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + + //Return a directory object from pagedquery for the schemaid to simulate LAPS + var searchResults = new[] + { + LdapResult.Ok(new MockDirectoryObject( + "abc123" + , new Dictionary() + { + {LDAPProperties.SchemaIDGUID, lapsGuid.ToByteArray()}, + {LDAPProperties.Name, LDAPProperties.LegacyLAPSPassword} + }, null,null)), + }; + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(searchResults.ToAsyncEnumerable); + + var processor = new ACLProcessor(mockLDAPUtils.Object); + var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); + var result = await processor.ProcessACL(bytes, _testDomainName, Label.Computer, true).ToArrayAsync(); + + Assert.Single(result); + var actual = result.First(); + Assert.Equal(actual.PrincipalType, expectedPrincipalType); + Assert.Equal(actual.PrincipalSID, expectedPrincipalSID); + Assert.False(actual.IsInherited); + Assert.Equal(actual.RightName, expectedRightName); + } [Fact] public void GetInheritedAceHashes_NullSD_Empty() { From 3ce123d53551489f63d453942e1e9e68d7e78120 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Tue, 15 Oct 2024 11:26:37 -0400 Subject: [PATCH 7/7] chore: fix tests --- test/unit/ACLProcessorTest.cs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/test/unit/ACLProcessorTest.cs b/test/unit/ACLProcessorTest.cs index a4d7c48b..5b367205 100644 --- a/test/unit/ACLProcessorTest.cs +++ b/test/unit/ACLProcessorTest.cs @@ -212,8 +212,13 @@ public async Task ACLProcessor_ProcessGMSAReaders_Null_PrincipalID() { } [Fact] - public async Task ACLProcessor_ProcessACL_Null_NTSecurityDescriptor() { - var processor = new ACLProcessor(new MockLdapUtils()); + public async Task ACLProcessor_ProcessACL_Null_NTSecurityDescriptor() + { + var mock = new Mock(); + mock.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(AsyncEnumerable.Empty>()); + var processor = new ACLProcessor(mock.Object); + var result = await processor.ProcessACL(null, _testDomainName, Label.User, false).ToArrayAsync(); Assert.Empty(result); @@ -261,6 +266,8 @@ public async Task ACLProcessor_ProcessACL_Null_SID() { .Returns(collection); mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(AsyncEnumerable.Empty>()); var processor = new ACLProcessor(mockLDAPUtils.Object); var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); @@ -279,6 +286,8 @@ public async Task ACLProcessor_ProcessACL_Null_ACE() { .Returns(collection); mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(AsyncEnumerable.Empty>()); var processor = new ACLProcessor(mockLDAPUtils.Object); var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); @@ -300,6 +309,8 @@ public async Task ACLProcessor_ProcessACL_Deny_ACE() { .Returns(collection); mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(AsyncEnumerable.Empty>()); var processor = new ACLProcessor(mockLDAPUtils.Object); var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); @@ -322,6 +333,8 @@ public async Task ACLProcessor_ProcessACL_Unmatched_Inheritance_ACE() { .Returns(collection); mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(AsyncEnumerable.Empty>()); var processor = new ACLProcessor(mockLDAPUtils.Object); var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor); @@ -345,6 +358,8 @@ public async Task ACLProcessor_ProcessACL_Null_SID_ACE() { .Returns(collection); mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny())).Returns((string)null); mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny(), It.IsAny())) + .Returns(AsyncEnumerable.Empty>()); var processor = new ACLProcessor(mockLDAPUtils.Object); var bytes = Utils.B64ToBytes(UnProtectedUserNtSecurityDescriptor);