From 1968f8d108be22e52906dd3cd15ddd62cf0544ba Mon Sep 17 00:00:00 2001 From: Emmanuel Robles <31362194+emmanuelrobles@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:33:15 -0500 Subject: [PATCH 1/8] added tests for helpers and init common lib (#265) * added tests for helpers and init common lib * added a comment * coderabbit suggestion * removed comment * made some test the were using cache run in sequence so they dont step on eachother * made some test the were using cache run in sequence so they dont step on eachother * cleanup the UT a bit --- .../CacheTestsCollectionDefinition.cs | 9 + test/unit/CommonLibHelperTests.cs | 78 +++++ test/unit/CommonLibTests.cs | 91 ++++++ .../unit/UserRightsAssignmentProcessorTest.cs | 295 +++++++++--------- 4 files changed, 328 insertions(+), 145 deletions(-) create mode 100644 test/unit/CollectionDefinitions/CacheTestsCollectionDefinition.cs create mode 100644 test/unit/CommonLibTests.cs diff --git a/test/unit/CollectionDefinitions/CacheTestsCollectionDefinition.cs b/test/unit/CollectionDefinitions/CacheTestsCollectionDefinition.cs new file mode 100644 index 000000000..c03cbc593 --- /dev/null +++ b/test/unit/CollectionDefinitions/CacheTestsCollectionDefinition.cs @@ -0,0 +1,9 @@ +using Xunit; + +namespace CommonLibTest.CollectionDefinitions; + +/// +/// Test that use cache cannot run in parallel, they can have flaky behavior +/// +[CollectionDefinition(nameof(CacheTestCollectionDefinition), DisableParallelization = true)] +public class CacheTestCollectionDefinition{} diff --git a/test/unit/CommonLibHelperTests.cs b/test/unit/CommonLibHelperTests.cs index e2cfe24f3..0cad80e12 100644 --- a/test/unit/CommonLibHelperTests.cs +++ b/test/unit/CommonLibHelperTests.cs @@ -1,4 +1,5 @@ using System; +using System.Security.Principal; using System.Text; using System.Threading.Tasks; using SharpHoundCommonLib; @@ -293,5 +294,82 @@ public async Task RetryOnException_SucceedsOnLastAttempt() { Assert.True(success); } + + [Fact] + public void DomainNameToDistinguishedName_DotsBecomeDcComponents() + { + var result = Helpers.DomainNameToDistinguishedName("test.local"); + Assert.Equal("DC=test,DC=local", result); + } + + [Theory] + [InlineData("S-1-5-32-544", "\\01\\02\\00\\00\\00\\00\\00\\05\\20\\00\\00\\00\\20\\02\\00\\00")] + public void ConvertSidToHexSid_ValidSid_MatchesSecurityIdentifierBinaryForm(string sid, string expectedHexSid) + { + // Arrange & Act + var actual = Helpers.ConvertSidToHexSid(sid); + + // Assert + Assert.Equal(expectedHexSid, actual); + return; + + static string BuildExpectedHexSid(string sid) + { + var securityIdentifier = new SecurityIdentifier(sid); + var sidBytes = new byte[securityIdentifier.BinaryLength]; + securityIdentifier.GetBinaryForm(sidBytes, 0); + return $"\\{BitConverter.ToString(sidBytes).Replace('-', '\\')}"; + } + } + + [Fact] + public void ConvertSidToHexSid_InvalidSid_Throws() + { + Assert.ThrowsAny(() => Helpers.ConvertSidToHexSid("NOT-A-SID")); + } + + [Theory] + [InlineData("s-1-5-18")] + [InlineData("S-1-5-18")] + public void IsSidFiltered_FilteredWellKnownSidsCaseInsensitive_ReturnsTrue(string sid) + { + Assert.True(Helpers.IsSidFiltered(sid)); + } + + [Theory] + [InlineData("S-1-5-80-1234567890")] + [InlineData("S-1-5-82-1234567890")] + [InlineData("S-1-5-90-0")] + [InlineData("S-1-5-96-0")] + public void IsSidFiltered_FilteredPrefixes_ReturnsTrue(string sid) + { + Assert.True(Helpers.IsSidFiltered(sid)); + } + + [Theory] + [InlineData("S-1-5-21-1234567890")] + [InlineData("S-1-5-21")] + public void IsSidFiltered_ReturnsFalse(string sid) + { + Assert.False(Helpers.IsSidFiltered(sid)); + } + + [Fact] + public void ConvertLdapTimeToLong_Null_ReturnsMinusOne() + { + Assert.Equal(-1, Helpers.ConvertLdapTimeToLong(null)); + } + + [Fact] + public void ConvertLdapTimeToLong_InvalidNumber_ThrowsFormatException() + { + Assert.Throws(() => Helpers.ConvertLdapTimeToLong("not-a-number")); + } + + [Fact] + public void ConvertLdapTimeToLong_ValidNumber_Parses() + { + Assert.Equal(123456789L, Helpers.ConvertLdapTimeToLong("123456789")); + } } } \ No newline at end of file diff --git a/test/unit/CommonLibTests.cs b/test/unit/CommonLibTests.cs new file mode 100644 index 000000000..8f1f595e0 --- /dev/null +++ b/test/unit/CommonLibTests.cs @@ -0,0 +1,91 @@ +using System; +using System.Reflection; +using CommonLibTest.CollectionDefinitions; +using Microsoft.Extensions.Logging; +using Moq; +using SharpHoundCommonLib; +using Xunit; + +namespace CommonLibTest; + +[Collection(nameof(CacheTestCollectionDefinition))] +public class CommonLibTests +{ + + public CommonLibTests() + { + ResetCommonLibState(); + } + + [Fact] + public void InitializeCommonLib_FirstCallWithoutCache_CreatesAndSetsCacheInstance() + { + // Arrange & Act + CommonLib.InitializeCommonLib(); + + // Assert + var cache = Cache.GetCacheInstance(); + Assert.NotNull(cache); + Assert.NotNull(cache.IdToTypeCache); + Assert.NotNull(cache.ValueToIdCache); + Assert.NotNull(cache.GlobalCatalogCache); + Assert.NotNull(cache.MachineSidCache); + Assert.NotNull(cache.SIDToDomainCache); + } + + [Fact] + public void InitializeCommonLib_UsesProvidedInstance() + { + // Arrange + var provided = Cache.CreateNewCache(); + + // Act + CommonLib.InitializeCommonLib(cache: provided); + + // Assert + Assert.Same(provided, Cache.GetCacheInstance()); + } + + [Fact] + public void InitializeCommonLib_2Calls_LogsWarningAndDoesNotReplaceCache() + { + // Arrange + var cache1 = Cache.CreateNewCache(); + CommonLib.InitializeCommonLib(cache: cache1); + + var cache2 = Cache.CreateNewCache(); + var logger = new Mock(); + + // Act + CommonLib.InitializeCommonLib(logger.Object, cache2); + + // Assert + Assert.Same(cache1, Cache.GetCacheInstance()); // cache1 should be then one used since lib was already initialized + + logger.Verify(x => x.Log( + LogLevel.Warning, + It.IsAny(), + It.Is((v, _) => + v.ToString() != null && + v.ToString().Contains("already initialized", StringComparison.InvariantCultureIgnoreCase)), + It.IsAny(), + It.IsAny>()), + Times.Once()); + } + + private static void ResetCommonLibState() + { + // Reset CommonLib._initialized (private static) + var commonLibType = typeof(CommonLib); + var initializedField = commonLibType.GetField("_initialized", + BindingFlags.Static | BindingFlags.NonPublic); + + if (initializedField == null) + throw new InvalidOperationException("CommonLib _initialized field not found"); + + initializedField.SetValue(null, false); + + // Reset cache singleton so tests don't leak state into each other + Cache.SetCacheInstance(null); + } +} \ No newline at end of file diff --git a/test/unit/UserRightsAssignmentProcessorTest.cs b/test/unit/UserRightsAssignmentProcessorTest.cs index 459c06bf4..14fbc9f8c 100644 --- a/test/unit/UserRightsAssignmentProcessorTest.cs +++ b/test/unit/UserRightsAssignmentProcessorTest.cs @@ -1,146 +1,151 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using CommonLibTest.Facades; -using CommonLibTest.Facades.LSAMocks.DCMocks; -using CommonLibTest.Facades.LSAMocks.WorkstationMocks; -using Moq; -using Newtonsoft.Json; -using SharpHoundCommonLib; -using SharpHoundCommonLib.Enums; -using SharpHoundCommonLib.Processors; -using SharpHoundRPC; -using Xunit; -using Xunit.Abstractions; - -namespace CommonLibTest -{ - public class UserRightsAssignmentProcessorTest - { - private readonly ITestOutputHelper _testOutputHelper; - - public UserRightsAssignmentProcessorTest(ITestOutputHelper testOutputHelper) - { - _testOutputHelper = testOutputHelper; - } - - [WindowsOnlyFact] - public async Task UserRightsAssignmentProcessor_TestWorkstation() - { - var mockProcessor = new Mock(new MockLdapUtils(), null); - var mockLSAPolicy = new MockWorkstationLSAPolicy(); - mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); - var processor = mockProcessor.Object; - var machineDomainSid = $"{Consts.MockDomainSid}-1001"; - var results = await processor.GetUserRightsAssignments("win10.testlab.local", machineDomainSid, "testlab.local", false) - .ToArrayAsync(); - - var privilege = results[0]; - Assert.Equal(LSAPrivileges.RemoteInteractiveLogon, privilege.Privilege); - Assert.Equal(3, results[0].Results.Length); - var adminResult = privilege.Results.First(x => x.ObjectIdentifier.EndsWith("-544")); - Assert.Equal($"{machineDomainSid}-544", adminResult.ObjectIdentifier); - Assert.Equal(Label.LocalGroup, adminResult.ObjectType); - var rdpResult = privilege.Results.First(x => x.ObjectIdentifier.EndsWith("-555")); - Assert.Equal($"{machineDomainSid}-555", rdpResult.ObjectIdentifier); - Assert.Equal(Label.LocalGroup, rdpResult.ObjectType); - } - - [WindowsOnlyFact] - public async Task UserRightsAssignmentProcessor_TestDC() - { - var mockProcessor = new Mock(new MockLdapUtils(), null); - var mockLSAPolicy = new MockDCLSAPolicy(); - mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); - var processor = mockProcessor.Object; - var machineDomainSid = $"{Consts.MockDomainSid}-1000"; - var results = await processor.GetUserRightsAssignments("primary.testlab.local", machineDomainSid, "testlab.local", true) - .ToArrayAsync(); - - var privilege = results[0]; - _testOutputHelper.WriteLine(JsonConvert.SerializeObject(privilege)); - Assert.Equal(LSAPrivileges.RemoteInteractiveLogon, privilege.Privilege); - Assert.Single(results[0].Results); - var adminResult = privilege.Results.First(x => x.ObjectIdentifier.EndsWith("-544")); - Assert.Equal("TESTLAB.LOCAL-S-1-5-32-544", adminResult.ObjectIdentifier); - Assert.Equal(Label.Group, adminResult.ObjectType); - } - - // Obsolete by AdaptiveTimeout - // [Fact] - // public async Task UserRightsAssignmentProcessor_TestTimeout() { - // var mockProcessor = new Mock(new MockLdapUtils(), null); - // mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(()=> { - // Task.Delay(100).Wait(); - // return NtStatus.StatusAccessDenied; - // }); - // var processor = mockProcessor.Object; - // var machineDomainSid = $"{Consts.MockDomainSid}-1000"; - // var receivedStatus = new List(); - // processor.ComputerStatusEvent += status => { - // receivedStatus.Add(status); - // return Task.CompletedTask; - // }; - // var results = await processor.GetUserRightsAssignments("primary.testlab.local", machineDomainSid, "testlab.local", true, null) - // .ToArrayAsync(); - // Assert.Empty(results); - // Assert.Single(receivedStatus); - // var status = receivedStatus[0]; - // Assert.Equal("Timeout", status.Status); - // } - - [WindowsOnlyFact] - public async Task UserRightsAssignmentProcessor_TestGetLocalDomainInformationFail() - { - var mockProcessor = new Mock(new MockLdapUtils(), null); - var mockLSAPolicy = new MockFailLSAPolicy_GetLocalDomainInformation(); - mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(()=> { - Task.Delay(100).Wait(); - return NtStatus.StatusAccessDenied; - }); - mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); - var processor = mockProcessor.Object; - var machineDomainSid = $"{Consts.MockDomainSid}-1001"; - var receivedStatus = new List(); - processor.ComputerStatusEvent += async status => { - receivedStatus.Add(status); - }; - var results = await processor.GetUserRightsAssignments("win10.testlab.local", machineDomainSid, "testlab.local", false) - .ToArrayAsync(); - - Assert.Empty(results); - Assert.Single(receivedStatus); - var status = receivedStatus[0]; - Assert.Equal("StatusAccessDenied", status.Status); - Assert.Equal("LSAGetMachineSID", status.Task); - } - - [WindowsOnlyFact] - public async Task UserRightsAssignmentProcessor_TestGetResolvedPrincipalsWithPrivilegeFail() - { - var mockProcessor = new Mock(new MockLdapUtils(), null); - var mockLSAPolicy = new MockFailLSAPolicy_GetResolvedPrincipalsWithPrivilege(); - mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); - var processor = mockProcessor.Object; - var machineDomainSid = $"{Consts.MockDomainSid}-1001"; - var receivedStatus = new List(); - processor.ComputerStatusEvent += async status => { - receivedStatus.Add(status); - }; - var results = await processor.GetUserRightsAssignments("win10.testlab.local", machineDomainSid, "testlab.local", false) - .ToArrayAsync(); - - Assert.Single(results); - - var result = results[0]; - Assert.False(result.Collected); - Assert.Equal("LSAEnumerateAccountsWithUserRights returned StatusAccessDenied", result.FailureReason); - Assert.Single(receivedStatus); - var status = receivedStatus[0]; - Assert.Equal("StatusAccessDenied", status.Status); - Assert.Equal("LSAEnumerateAccountsWithUserRight", status.Task); - } - } +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using CommonLibTest.CollectionDefinitions; +using CommonLibTest.Facades; +using CommonLibTest.Facades.LSAMocks.DCMocks; +using CommonLibTest.Facades.LSAMocks.WorkstationMocks; +using Moq; +using Newtonsoft.Json; +using SharpHoundCommonLib; +using SharpHoundCommonLib.Enums; +using SharpHoundCommonLib.Processors; +using SharpHoundRPC; +using Xunit; +using Xunit.Abstractions; + +namespace CommonLibTest +{ + [Collection(nameof(CacheTestCollectionDefinition))] + public class UserRightsAssignmentProcessorTest + { + private readonly ITestOutputHelper _testOutputHelper; + + public UserRightsAssignmentProcessorTest(ITestOutputHelper testOutputHelper) + { + _testOutputHelper = testOutputHelper; + + //reseting cache + Cache.SetCacheInstance(null); + } + + [WindowsOnlyFact] + public async Task UserRightsAssignmentProcessor_TestWorkstation() + { + var mockProcessor = new Mock(new MockLdapUtils(), null); + var mockLSAPolicy = new MockWorkstationLSAPolicy(); + mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); + var processor = mockProcessor.Object; + var machineDomainSid = $"{Consts.MockDomainSid}-1001"; + var results = await processor.GetUserRightsAssignments("win10.testlab.local", machineDomainSid, "testlab.local", false) + .ToArrayAsync(); + + var privilege = results[0]; + Assert.Equal(LSAPrivileges.RemoteInteractiveLogon, privilege.Privilege); + Assert.Equal(3, results[0].Results.Length); + var adminResult = privilege.Results.First(x => x.ObjectIdentifier.EndsWith("-544")); + Assert.Equal($"{machineDomainSid}-544", adminResult.ObjectIdentifier); + Assert.Equal(Label.LocalGroup, adminResult.ObjectType); + var rdpResult = privilege.Results.First(x => x.ObjectIdentifier.EndsWith("-555")); + Assert.Equal($"{machineDomainSid}-555", rdpResult.ObjectIdentifier); + Assert.Equal(Label.LocalGroup, rdpResult.ObjectType); + } + + [WindowsOnlyFact] + public async Task UserRightsAssignmentProcessor_TestDC() + { + var mockProcessor = new Mock(new MockLdapUtils(), null); + var mockLSAPolicy = new MockDCLSAPolicy(); + mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); + var processor = mockProcessor.Object; + var machineDomainSid = $"{Consts.MockDomainSid}-1000"; + var results = await processor.GetUserRightsAssignments("primary.testlab.local", machineDomainSid, "testlab.local", true) + .ToArrayAsync(); + + var privilege = results[0]; + _testOutputHelper.WriteLine(JsonConvert.SerializeObject(privilege)); + Assert.Equal(LSAPrivileges.RemoteInteractiveLogon, privilege.Privilege); + Assert.Single(results[0].Results); + var adminResult = privilege.Results.First(x => x.ObjectIdentifier.EndsWith("-544")); + Assert.Equal("TESTLAB.LOCAL-S-1-5-32-544", adminResult.ObjectIdentifier); + Assert.Equal(Label.Group, adminResult.ObjectType); + } + + // Obsolete by AdaptiveTimeout + // [Fact] + // public async Task UserRightsAssignmentProcessor_TestTimeout() { + // var mockProcessor = new Mock(new MockLdapUtils(), null); + // mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(()=> { + // Task.Delay(100).Wait(); + // return NtStatus.StatusAccessDenied; + // }); + // var processor = mockProcessor.Object; + // var machineDomainSid = $"{Consts.MockDomainSid}-1000"; + // var receivedStatus = new List(); + // processor.ComputerStatusEvent += status => { + // receivedStatus.Add(status); + // return Task.CompletedTask; + // }; + // var results = await processor.GetUserRightsAssignments("primary.testlab.local", machineDomainSid, "testlab.local", true, null) + // .ToArrayAsync(); + // Assert.Empty(results); + // Assert.Single(receivedStatus); + // var status = receivedStatus[0]; + // Assert.Equal("Timeout", status.Status); + // } + + [WindowsOnlyFact] + public async Task UserRightsAssignmentProcessor_TestGetLocalDomainInformationFail() + { + var mockProcessor = new Mock(new MockLdapUtils(), null); + var mockLSAPolicy = new MockFailLSAPolicy_GetLocalDomainInformation(); + mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(()=> { + Task.Delay(100).Wait(); + return NtStatus.StatusAccessDenied; + }); + mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); + var processor = mockProcessor.Object; + var machineDomainSid = $"{Consts.MockDomainSid}-1001"; + var receivedStatus = new List(); + processor.ComputerStatusEvent += async status => { + receivedStatus.Add(status); + }; + var results = await processor.GetUserRightsAssignments("win10.testlab.local", machineDomainSid, "testlab.local", false) + .ToArrayAsync(); + + Assert.Empty(results); + Assert.Single(receivedStatus); + var status = receivedStatus[0]; + Assert.Equal("StatusAccessDenied", status.Status); + Assert.Equal("LSAGetMachineSID", status.Task); + } + + [WindowsOnlyFact] + public async Task UserRightsAssignmentProcessor_TestGetResolvedPrincipalsWithPrivilegeFail() + { + var mockProcessor = new Mock(new MockLdapUtils(), null); + var mockLSAPolicy = new MockFailLSAPolicy_GetResolvedPrincipalsWithPrivilege(); + mockProcessor.Setup(x => x.OpenLSAPolicy(It.IsAny())).Returns(mockLSAPolicy); + var processor = mockProcessor.Object; + var machineDomainSid = $"{Consts.MockDomainSid}-1001"; + var receivedStatus = new List(); + processor.ComputerStatusEvent += async status => { + receivedStatus.Add(status); + }; + var results = await processor.GetUserRightsAssignments("win10.testlab.local", machineDomainSid, "testlab.local", false) + .ToArrayAsync(); + + Assert.Single(results); + + var result = results[0]; + Assert.False(result.Collected); + Assert.Equal("LSAEnumerateAccountsWithUserRights returned StatusAccessDenied", result.FailureReason); + Assert.Single(receivedStatus); + var status = receivedStatus[0]; + Assert.Equal("StatusAccessDenied", status.Status); + Assert.Equal("LSAEnumerateAccountsWithUserRight", status.Task); + } + } } \ No newline at end of file From 8837f50d3db43252d03bab0d230d24dafef3359d Mon Sep 17 00:00:00 2001 From: Jim Sykora <14374121+JimSycurity@users.noreply.github.com> Date: Fri, 19 Dec 2025 10:18:27 -0600 Subject: [PATCH 2/8] Include Membership property set in AddSelf and AddMember edges (#266) * feat: add membership property set BED-7069 --- src/CommonLib/Processors/ACEGuids.cs | 1 + src/CommonLib/Processors/ACLProcessor.cs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/CommonLib/Processors/ACEGuids.cs b/src/CommonLib/Processors/ACEGuids.cs index ebb0e5b11..fb205d7a1 100644 --- a/src/CommonLib/Processors/ACEGuids.cs +++ b/src/CommonLib/Processors/ACEGuids.cs @@ -8,6 +8,7 @@ public class ACEGuids public const string UserForceChangePassword = "00299570-246d-11d0-a768-00aa006e0529"; public const string AllGuid = "00000000-0000-0000-0000-000000000000"; public const string WriteMember = "bf9679c0-0de6-11d0-a285-00aa003049e2"; + public const string MembershipPropertySet = "bc0ac240-79a9-11d0-9020-00c04fc2d4cf"; // property set https://learn.microsoft.com/en-us/windows/win32/adschema/r-membership public const string WriteAllowedToAct = "3f78c3e5-f79a-46bd-a0b8-9d18116ddc79"; public const string WriteSPN = "f3a64788-5306-11d1-a9c5-0000f80367c1"; public const string AddKeyPrincipal = "5b47d60f-6090-40b2-9f37-2a4de88f3063"; diff --git a/src/CommonLib/Processors/ACLProcessor.cs b/src/CommonLib/Processors/ACLProcessor.cs index 383b69aff..da3a615b4 100644 --- a/src/CommonLib/Processors/ACLProcessor.cs +++ b/src/CommonLib/Processors/ACLProcessor.cs @@ -584,7 +584,7 @@ public async IAsyncEnumerable ProcessACL(byte[] ntSecurityDescriptor, strin if (aceRights.HasFlag(ActiveDirectoryRights.Self) && !aceRights.HasFlag(ActiveDirectoryRights.WriteProperty) && !aceRights.HasFlag(ActiveDirectoryRights.GenericWrite) && objectType == Label.Group && - aceType is ACEGuids.WriteMember or ACEGuids.AllGuid) + aceType is ACEGuids.WriteMember or ACEGuids.MembershipPropertySet or ACEGuids.AllGuid) yield return new ACE { PrincipalType = resolvedPrincipal.ObjectType, PrincipalSID = resolvedPrincipal.ObjectIdentifier, @@ -786,7 +786,7 @@ or Label.NTAuthStore IsPermissionForOwnerRightsSid = isPermissionForOwnerRightsSid, IsInheritedPermissionForOwnerRightsSid = isInheritedPermissionForOwnerRightsSid, }; - else if (objectType == Label.Group && aceType == ACEGuids.WriteMember) + else if (objectType == Label.Group && (aceType is ACEGuids.WriteMember or ACEGuids.MembershipPropertySet)) yield return new ACE { PrincipalType = resolvedPrincipal.ObjectType, PrincipalSID = resolvedPrincipal.ObjectIdentifier, From 5657e2c154ef1ae0d6c7972f0f8b59bd1e14cccd Mon Sep 17 00:00:00 2001 From: Lucas Falslev Date: Mon, 29 Dec 2025 08:55:55 -0700 Subject: [PATCH 3/8] Test GetRegistryPrincipal --- test/unit/CertAbuseProcessorTest.cs | 112 +++++++++++++++++++++++++--- 1 file changed, 101 insertions(+), 11 deletions(-) diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index 663ece98d..b7bf370a3 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -331,13 +331,9 @@ public async Task CertAbuseProcessor_ProcessCertTemplates_ReturnsResolvedAndUnre Assert.Contains(invalidCN, results.unresolvedTemplates); } - [WindowsOnlyTheory] - [InlineData("S-1-5-80")] - [InlineData("S-1-5-82")] - [InlineData("S-1-5-90")] - [InlineData("S-1-5-96")] - public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFilteredSID(string sidValue) { - var sid = new SecurityIdentifier(sidValue); + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFilteredSID() { + var sid = new SecurityIdentifier("S-1-5-3"); var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, @@ -345,31 +341,125 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFiltere TargetName, true, TargetDomainSid, - new SecurityIdentifier("S-1-5-18") + null ); Assert.Equal((false, null), results); + _mockLdapUtils.VerifyNoOtherCalls(); } [WindowsOnlyFact] - public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvedDomainController_ReturnsTrue() { + public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsResolveIDAndType_ForDomainController() { var expectedPrincipalType = Label.Group; var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; - var sid = new SecurityIdentifier(expectedPrincipalSID); _mockLdapUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + var sid = new SecurityIdentifier(expectedPrincipalSID); + var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, DomainName, TargetName, true, TargetDomainSid, - new SecurityIdentifier("S-1-5-18") + null ); Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); + + _mockLdapUtils.Verify( + x => x.ResolveIDAndType(It.IsAny(), It.IsAny()), + Times.Once); + + _mockLdapUtils.VerifyNoOtherCalls(); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsConvertLocalWellKnownPrincipal_ForNonDomainController() { + var expectedPrincipalType = Label.Group; + var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + + _mockLdapUtils.Setup(x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + + var sid = new SecurityIdentifier(expectedPrincipalSID); + + var results = await _certAbuseProcessor.GetRegistryPrincipal( + sid, + DomainName, + TargetName, + false, + TargetDomainSid, + null + ); + + Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); + + _mockLdapUtils.Verify( + x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + _mockLdapUtils.VerifyNoOtherCalls(); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToLocalPrincipal_ForLocalSID() { + var expectedPrincipalType = Label.LocalGroup; + var expectedPrincipalSID = $"{TargetDomainSid}-123"; + + _mockLdapUtils.Setup(x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((false, null)); + + var sid = new SecurityIdentifier(expectedPrincipalSID); + + var results = await _certAbuseProcessor.GetRegistryPrincipal( + sid, + DomainName, + TargetName, + false, + TargetDomainSid, + new SecurityIdentifier(TargetDomainSid) + ); + + Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); + + _mockLdapUtils.Verify( + x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + _mockLdapUtils.VerifyNoOtherCalls(); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToDomainPrincipal() { + var expectedPrincipalType = Label.Group; + var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + + _mockLdapUtils.Setup(x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((false, null)); + _mockLdapUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + + var sid = new SecurityIdentifier(expectedPrincipalSID); + + var results = await _certAbuseProcessor.GetRegistryPrincipal( + sid, + DomainName, + TargetName, + false, + TargetDomainSid, + null + ); + + Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); + + _mockLdapUtils.Verify( + x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + _mockLdapUtils.Verify( + x => x.ResolveIDAndType(It.IsAny(), It.IsAny()), + Times.Once); + _mockLdapUtils.VerifyNoOtherCalls(); } } } \ No newline at end of file From 830c1ce35e95cbd9184eda297a9c86d6521e11c8 Mon Sep 17 00:00:00 2001 From: Lucas Falslev Date: Wed, 31 Dec 2025 09:17:22 -0700 Subject: [PATCH 4/8] test OpenSAMServer --- .../Processors/CertAbuseProcessor.cs | 18 +- .../Processors/LocalGroupProcessor.cs | 6 +- src/SharpHoundRPC/SAMRPCNative/SAMMethods.cs | 2 - .../Wrappers/ISAMServerAccessor.cs | 27 +++ src/SharpHoundRPC/Wrappers/SAMServer.cs | 13 -- test/unit/CertAbuseProcessorTest.cs | 199 +++++++++++------- 6 files changed, 165 insertions(+), 100 deletions(-) create mode 100644 src/SharpHoundRPC/Wrappers/ISAMServerAccessor.cs diff --git a/src/CommonLib/Processors/CertAbuseProcessor.cs b/src/CommonLib/Processors/CertAbuseProcessor.cs index d9e4286e9..093437bb1 100644 --- a/src/CommonLib/Processors/CertAbuseProcessor.cs +++ b/src/CommonLib/Processors/CertAbuseProcessor.cs @@ -19,16 +19,18 @@ public class CertAbuseProcessor private readonly AdaptiveTimeout _getMachineSidAdaptiveTimeout; private readonly AdaptiveTimeout _openSamServerAdaptiveTimeout; private readonly IRegistryAccessor _registryAccessor; + private readonly ISAMServerAccessor _samServerAccessor; public delegate Task ComputerStatusDelegate(CSVComputerStatus status); public event ComputerStatusDelegate ComputerStatusEvent; - public CertAbuseProcessor(ILdapUtils utils, IRegistryAccessor registryAccessor, ILogger log = null) { + public CertAbuseProcessor(ILdapUtils utils, IRegistryAccessor registryAccessor, ISAMServerAccessor samServerAccessor, ILogger log = null) { _utils = utils; _registryAccessor = registryAccessor; + _samServerAccessor = samServerAccessor; _log = log ?? Logging.LogProvider.CreateLogger("CAProc"); _getMachineSidAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMServer.GetMachineSid))); - _openSamServerAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(SAMServer.OpenServer))); + _openSamServerAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMServerAccessor.OpenServer))); } /// @@ -411,7 +413,7 @@ private async Task GetMachineSid(string computerName, string { SecurityIdentifier machineSid = null; - //Try to get the machine sid for the computer if its not already cached + //Try to get the machine sid for the computer if it's not already cached if (!Cache.GetMachineSid(computerObjectId, out var tempMachineSid)) { // Open a handle to the server @@ -441,7 +443,7 @@ await SendComputerStatus(new CSVComputerStatus Task = "GetMachineSid", ObjectId = computerObjectId, }); - //If we can't get a machine sid, we wont be able to make local principals with unique object ids, or differentiate local/domain objects + //If we can't get a machine sid, we won't be able to make local principals with unique object ids, or differentiate local/domain objects _log.LogWarning("Unable to get machineSid for {Computer}: {Status}", computerName, getMachineSidResult.SError); return null; } @@ -515,13 +517,7 @@ await SendComputerStatus(new CSVComputerStatus public virtual SharpHoundRPC.Result OpenSamServer(string computerName) { - var result = _openSamServerAdaptiveTimeout.ExecuteRPCWithTimeout((_) => SAMServer.OpenServer(computerName)).GetAwaiter().GetResult(); - if (result.IsFailed) - { - return SharpHoundRPC.Result.Fail(result.SError); - } - - return SharpHoundRPC.Result.Ok(result.Value); + return _openSamServerAdaptiveTimeout.ExecuteRPCWithTimeout((_) => _samServerAccessor.OpenServer(computerName)).GetAwaiter().GetResult(); } private async Task SendComputerStatus(CSVComputerStatus status) diff --git a/src/CommonLib/Processors/LocalGroupProcessor.cs b/src/CommonLib/Processors/LocalGroupProcessor.cs index 93454b174..c2048b04f 100644 --- a/src/CommonLib/Processors/LocalGroupProcessor.cs +++ b/src/CommonLib/Processors/LocalGroupProcessor.cs @@ -16,6 +16,7 @@ public class LocalGroupProcessor public delegate Task ComputerStatusDelegate(CSVComputerStatus status); private readonly ILogger _log; private readonly ILdapUtils _utils; + private readonly ISAMServerAccessor _samServerAccessor; private readonly AdaptiveTimeout _getMachineSidAdaptiveTimeout; private readonly AdaptiveTimeout _openSamServerAdaptiveTimeout; private readonly AdaptiveTimeout _getDomainsAdaptiveTimeout; @@ -27,9 +28,10 @@ public class LocalGroupProcessor public LocalGroupProcessor(ILdapUtils utils, ILogger log = null) { _utils = utils; + _samServerAccessor = new SAMServerAccessor(); _log = log ?? Logging.LogProvider.CreateLogger("LocalGroupProcessor"); _getMachineSidAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMServer.GetMachineSid))); - _openSamServerAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(SAMServer.OpenServer))); + _openSamServerAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMServerAccessor.OpenServer))); _getDomainsAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMServer.GetDomains))); _openDomainAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMServer.OpenDomain))); _getAliasesAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ISAMDomain.GetAliases))); @@ -42,7 +44,7 @@ public LocalGroupProcessor(ILdapUtils utils, ILogger log = null) { public virtual SharpHoundRPC.Result OpenSamServer(string computerName) { - var result = _openSamServerAdaptiveTimeout.ExecuteRPCWithTimeout((_) => SAMServer.OpenServer(computerName)).GetAwaiter().GetResult(); + var result = _openSamServerAdaptiveTimeout.ExecuteRPCWithTimeout((_) => _samServerAccessor.OpenServer(computerName)).GetAwaiter().GetResult(); if (result.IsFailed) { return SharpHoundRPC.Result.Fail(result.SError); diff --git a/src/SharpHoundRPC/SAMRPCNative/SAMMethods.cs b/src/SharpHoundRPC/SAMRPCNative/SAMMethods.cs index e17de5b64..5fc5edc77 100644 --- a/src/SharpHoundRPC/SAMRPCNative/SAMMethods.cs +++ b/src/SharpHoundRPC/SAMRPCNative/SAMMethods.cs @@ -1,8 +1,6 @@ using System; -using System.Collections.Generic; using System.Runtime.InteropServices; using System.Security; -using System.Security.Principal; using SharpHoundRPC.Handles; using SharpHoundRPC.Shared; diff --git a/src/SharpHoundRPC/Wrappers/ISAMServerAccessor.cs b/src/SharpHoundRPC/Wrappers/ISAMServerAccessor.cs new file mode 100644 index 000000000..65851a212 --- /dev/null +++ b/src/SharpHoundRPC/Wrappers/ISAMServerAccessor.cs @@ -0,0 +1,27 @@ +using SharpHoundRPC.SAMRPCNative; + +namespace SharpHoundRPC.Wrappers +{ + public interface ISAMServerAccessor + { + Result OpenServer(string computerName, SAMEnums.SamAccessMasks requestedConnectAccess = + SAMEnums.SamAccessMasks.SamServerConnect | + SAMEnums.SamAccessMasks.SamServerEnumerateDomains | + SAMEnums.SamAccessMasks.SamServerLookupDomain); + } + + public class SAMServerAccessor : ISAMServerAccessor + { + public Result OpenServer(string computerName, SAMEnums.SamAccessMasks requestedConnectAccess = + SAMEnums.SamAccessMasks.SamServerConnect | + SAMEnums.SamAccessMasks.SamServerEnumerateDomains | + SAMEnums.SamAccessMasks.SamServerLookupDomain) + { + var (status, handle) = SAMMethods.SamConnect(computerName, requestedConnectAccess); + + return status.IsError() + ? status + : new SAMServer(handle, computerName); + } + } +} \ No newline at end of file diff --git a/src/SharpHoundRPC/Wrappers/SAMServer.cs b/src/SharpHoundRPC/Wrappers/SAMServer.cs index 872faa497..e405d1a88 100644 --- a/src/SharpHoundRPC/Wrappers/SAMServer.cs +++ b/src/SharpHoundRPC/Wrappers/SAMServer.cs @@ -22,19 +22,6 @@ public SAMServer(SAMHandle handle, string computerName) : base(handle) public string ComputerName { get; } - public static Result OpenServer(string computerName, SAMEnums.SamAccessMasks requestedConnectAccess = - SAMEnums.SamAccessMasks.SamServerConnect | - SAMEnums.SamAccessMasks - .SamServerEnumerateDomains | - SAMEnums.SamAccessMasks.SamServerLookupDomain) - { - var (status, handle) = SAMMethods.SamConnect(computerName, requestedConnectAccess); - - return status.IsError() - ? status - : new SAMServer(handle, computerName); - } - public Result> GetDomains() { var (status, rids, count) = SAMMethods.SamEnumerateDomainsInSamServer(Handle); diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index b7bf370a3..51aada231 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -7,51 +7,57 @@ using SharpHoundCommonLib; using SharpHoundCommonLib.Processors; using Xunit; -using Microsoft.Extensions.Logging; using SharpHoundCommonLib.Enums; using SharpHoundCommonLib.OutputTypes; +using SharpHoundRPC.SAMRPCNative; +using SharpHoundRPC.Wrappers; namespace CommonLibTest { - public class CertAbuseProcessorTest + public class CertAbuseProcessorTest { private readonly Mock _mockLdapUtils; private readonly Mock _mockRegistryAccessor; + private readonly Mock _mockSAMServerAccessor; private readonly CertAbuseProcessor _certAbuseProcessor; - + private const string DomainName = "TEST.LOCAL"; private const string CAName = "TEST-CA"; private const string TargetName = "target.test.local"; private const string TargetDomainSid = "S-1-5-21-123456789-123456789-123456789"; private const string FailureReason = "Registry Lookup Failure"; - + private CSVComputerStatus _receivedCompStatus; - + public CertAbuseProcessorTest() { _mockLdapUtils = new Mock(); _mockRegistryAccessor = new Mock(); - _certAbuseProcessor = new CertAbuseProcessor(_mockLdapUtils.Object, _mockRegistryAccessor.Object); - - _certAbuseProcessor.ComputerStatusEvent += status => - { + _mockSAMServerAccessor = new Mock(); + _certAbuseProcessor = new CertAbuseProcessor(_mockLdapUtils.Object, _mockRegistryAccessor.Object, _mockSAMServerAccessor.Object); + + _certAbuseProcessor.ComputerStatusEvent += status => { _receivedCompStatus = status; return Task.CompletedTask; }; } - + [Theory] [InlineData(0x00040000, true)] [InlineData(0x00000000, false)] public async Task CertAbuseProcessor_IsUserSpecifiesSanEnabled_ReturnsResult(int editFlags, bool expectedResult) { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}\\PolicyModules\\CertificateAuthority_MicrosoftDefault.Policy"; const string subValue = "EditFlags"; - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( TargetName, subKey, subValue)) - .Returns(new RegistryResult { Collected = true, Value = editFlags }); + .Returns(new RegistryResult + { + Collected = true, + Value = editFlags + }); var results = await _certAbuseProcessor.IsUserSpecifiesSanEnabled(TargetName, CAName, TargetDomainSid); @@ -66,18 +72,22 @@ public async Task CertAbuseProcessor_IsUserSpecifiesSanEnabled_ReturnsResult(int Assert.Equal(CSVComputerStatus.StatusSuccess, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [Fact] public async Task CertAbuseProcessor_IsUserSpecifiesSanEnabled_HandlesFailedLookup() { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}\\PolicyModules\\CertificateAuthority_MicrosoftDefault.Policy"; const string subValue = "EditFlags"; - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( TargetName, subKey, subValue)) - .Returns(new RegistryResult { Collected = false, FailureReason = FailureReason }); + .Returns(new RegistryResult + { + Collected = false, + FailureReason = FailureReason + }); var results = await _certAbuseProcessor.IsUserSpecifiesSanEnabled(TargetName, CAName, TargetDomainSid); @@ -91,20 +101,24 @@ public async Task CertAbuseProcessor_IsUserSpecifiesSanEnabled_HandlesFailedLook Assert.Equal(FailureReason, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [Theory] [InlineData(1, true)] [InlineData(0, false)] public async Task CertAbuseProcessor_IsRoleSeparationEnabled_ReturnsResult(int roleSeparationEnabled, bool expectedResult) { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "RoleSeparationEnabled"; - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( TargetName, subKey, subValue)) - .Returns(new RegistryResult { Collected = true, Value = roleSeparationEnabled }); + .Returns(new RegistryResult + { + Collected = true, + Value = roleSeparationEnabled + }); var results = await _certAbuseProcessor.IsRoleSeparationEnabled(TargetName, CAName, TargetDomainSid); @@ -119,18 +133,22 @@ public async Task CertAbuseProcessor_IsRoleSeparationEnabled_ReturnsResult(int r Assert.Equal(CSVComputerStatus.StatusSuccess, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [Fact] public async Task CertAbuseProcessor_IsRoleSeparationEnabled_HandlesFailedLookup() { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "RoleSeparationEnabled"; - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( TargetName, subKey, subValue)) - .Returns(new RegistryResult { Collected = false, FailureReason = FailureReason }); + .Returns(new RegistryResult + { + Collected = false, + FailureReason = FailureReason + }); var results = await _certAbuseProcessor.IsRoleSeparationEnabled(TargetName, CAName, TargetDomainSid); @@ -154,10 +172,10 @@ public static IEnumerable ProcessEAPermissionsTestData() { false, null ); - + var daclWithNullOpaque = new RawAcl(2, 1); daclWithNullOpaque.InsertAce(0, nullOpaqueAce); - + var emptyOpaqueAce = new CommonAce( AceFlags.None, AceQualifier.AccessAllowed, @@ -169,7 +187,7 @@ public static IEnumerable ProcessEAPermissionsTestData() { var daclWithEmptyOpaque = new RawAcl(2, 1); daclWithEmptyOpaque.InsertAce(0, emptyOpaqueAce); - + return new List { new object[] { null }, //null dacl @@ -178,13 +196,13 @@ public static IEnumerable ProcessEAPermissionsTestData() { new object[] { daclWithEmptyOpaque }, //dacl is callback true, empty opaque }; } - + [WindowsOnlyTheory] [MemberData(nameof(ProcessEAPermissionsTestData))] public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl dacl) { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "EnrollmentAgentRights"; - + //setup binary security descriptor as registry value var descriptor = new RawSecurityDescriptor( ControlFlags.DiscretionaryAclPresent, @@ -192,7 +210,7 @@ public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl da null, null, dacl); - + var regValue = new byte[descriptor.BinaryLength]; descriptor.GetBinaryForm(regValue, 0); @@ -201,33 +219,41 @@ public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl da "localhost", subKey, subValue)) - .Returns(new RegistryResult { Collected = true, Value = regValue }); - + .Returns(new RegistryResult + { + Collected = true, + Value = regValue + }); + var results = await _certAbuseProcessor.ProcessEAPermissions(CAName, DomainName, "localhost", TargetDomainSid); - + //Validate result Assert.True(results.Collected); Assert.Empty(results.Restrictions); Assert.Null(results.FailureReason); - + //Validate CompStatus Log Assert.Equal("localhost", _receivedCompStatus.ComputerName); Assert.Equal(nameof(CertAbuseProcessor.ProcessEAPermissions), _receivedCompStatus.Task); Assert.Equal(CSVComputerStatus.StatusSuccess, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [Fact] public async Task CertAbuseProcessor_ProcessEAPermissions_HandlesFailedLookup() { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "EnrollmentAgentRights"; - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( TargetName, subKey, subValue)) - .Returns(new RegistryResult { Collected = false, FailureReason = FailureReason }); + .Returns(new RegistryResult + { + Collected = false, + FailureReason = FailureReason + }); var results = await _certAbuseProcessor.ProcessEAPermissions(CAName, DomainName, TargetName, TargetDomainSid); @@ -241,61 +267,69 @@ public async Task CertAbuseProcessor_ProcessEAPermissions_HandlesFailedLookup() Assert.Equal(FailureReason, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [WindowsOnlyFact] public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_ReturnsEmpty_WhenNoOwnerAndNoRules() { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "Security"; - + //setup binary security descriptor as registry value var descriptor = new RawSecurityDescriptor( ControlFlags.DiscretionaryAclPresent, - null, //owner is null + null, //owner is null null, null, null); - + byte[] regValue = new byte[descriptor.BinaryLength]; descriptor.GetBinaryForm(regValue, 0); - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( "localhost", subKey, subValue)) - .Returns(new RegistryResult { Collected = true, Value = regValue}); - + .Returns(new RegistryResult + { + Collected = true, + Value = regValue + }); + //get access rules returns empty var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) .Returns([]); _mockLdapUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); - + var results = await _certAbuseProcessor.ProcessRegistryEnrollmentPermissions(CAName, DomainName, "localhost", TargetDomainSid); //Validate result Assert.True(results.Collected); Assert.Empty(results.Data); Assert.Null(results.FailureReason); - + //Validate CompStatus Log Assert.Equal("localhost", _receivedCompStatus.ComputerName); Assert.Equal(nameof(CertAbuseProcessor.ProcessRegistryEnrollmentPermissions), _receivedCompStatus.Task); Assert.Equal(CSVComputerStatus.StatusSuccess, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [Fact] public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_HandlesFailedLookup() { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "Security"; - + _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( TargetName, subKey, subValue)) - .Returns(new RegistryResult { Collected = false, FailureReason = FailureReason }); + .Returns(new RegistryResult + { + Collected = false, + FailureReason = FailureReason + }); var results = await _certAbuseProcessor.ProcessRegistryEnrollmentPermissions(CAName, DomainName, TargetName, TargetDomainSid); @@ -309,19 +343,19 @@ public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_Handle Assert.Equal(FailureReason, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - + [Fact] public async Task CertAbuseProcessor_ProcessCertTemplates_ReturnsResolvedAndUnresolvedTemplates() { const string validCN = "ValidCN"; const string invalidCN = "InvalidCN"; - + _mockLdapUtils .Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync((string cn, string _, string _) => cn == validCN ? (true, new TypedPrincipal("test guid", Label.CertTemplate)) : (false, null)); - + var results = await _certAbuseProcessor.ProcessCertTemplates([validCN, invalidCN], DomainName); var expectedTemplate = new TypedPrincipal("test guid", Label.CertTemplate); @@ -330,11 +364,11 @@ public async Task CertAbuseProcessor_ProcessCertTemplates_ReturnsResolvedAndUnre Assert.Single(results.unresolvedTemplates); Assert.Contains(invalidCN, results.unresolvedTemplates); } - + [WindowsOnlyFact] public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFilteredSID() { var sid = new SecurityIdentifier("S-1-5-3"); - + var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, DomainName, @@ -347,17 +381,17 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ReturnsFalseForFiltere Assert.Equal((false, null), results); _mockLdapUtils.VerifyNoOtherCalls(); } - + [WindowsOnlyFact] public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsResolveIDAndType_ForDomainController() { var expectedPrincipalType = Label.Group; var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; - + _mockLdapUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - + var sid = new SecurityIdentifier(expectedPrincipalSID); - + var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, DomainName, @@ -368,24 +402,24 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsResolveIDAndType_ ); Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); - + _mockLdapUtils.Verify( x => x.ResolveIDAndType(It.IsAny(), It.IsAny()), Times.Once); - + _mockLdapUtils.VerifyNoOtherCalls(); } - + [WindowsOnlyFact] public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsConvertLocalWellKnownPrincipal_ForNonDomainController() { var expectedPrincipalType = Label.Group; var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; - + _mockLdapUtils.Setup(x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - + var sid = new SecurityIdentifier(expectedPrincipalSID); - + var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, DomainName, @@ -396,23 +430,23 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_CallsConvertLocalWellK ); Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); - + _mockLdapUtils.Verify( x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); _mockLdapUtils.VerifyNoOtherCalls(); } - + [WindowsOnlyFact] public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToLocalPrincipal_ForLocalSID() { var expectedPrincipalType = Label.LocalGroup; var expectedPrincipalSID = $"{TargetDomainSid}-123"; - + _mockLdapUtils.Setup(x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync((false, null)); - + var sid = new SecurityIdentifier(expectedPrincipalSID); - + var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, DomainName, @@ -423,25 +457,25 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToLocalPrincip ); Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); - + _mockLdapUtils.Verify( x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); _mockLdapUtils.VerifyNoOtherCalls(); } - + [WindowsOnlyFact] public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToDomainPrincipal() { var expectedPrincipalType = Label.Group; var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; - + _mockLdapUtils.Setup(x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny())) .ReturnsAsync((false, null)); _mockLdapUtils.Setup(x => x.ResolveIDAndType(It.IsAny(), It.IsAny())) .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); - + var sid = new SecurityIdentifier(expectedPrincipalSID); - + var results = await _certAbuseProcessor.GetRegistryPrincipal( sid, DomainName, @@ -452,7 +486,7 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToDomainPrinci ); Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); - + _mockLdapUtils.Verify( x => x.ConvertLocalWellKnownPrincipal(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); @@ -461,5 +495,26 @@ public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvesToDomainPrinci Times.Once); _mockLdapUtils.VerifyNoOtherCalls(); } + + [Fact] + public void CertAbuseProcessor_OpenSamServer_CallsOpenServer_Failure() { + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Fail("Connection Failed")); + + var result = _certAbuseProcessor.OpenSamServer(TargetName); + + Assert.True(result.IsFailed); + } + + [Fact] + public void CertAbuseProcessor_OpenSamServer_CallsOpenServer_Success() { + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Ok(new SAMServer(null, "TestServer"))); + + var result = _certAbuseProcessor.OpenSamServer(TargetName); + + Assert.True(result.IsSuccess); + Assert.IsType(result.Value); + } } } \ No newline at end of file From 43207573a8da9fcbcea9b4bc22f423bd78fee09e Mon Sep 17 00:00:00 2001 From: Lucas Falslev Date: Fri, 2 Jan 2026 05:57:20 -0700 Subject: [PATCH 5/8] test GetMachineSid --- .../Processors/CertAbuseProcessor.cs | 13 +++- test/unit/CertAbuseProcessorTest.cs | 73 +++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/src/CommonLib/Processors/CertAbuseProcessor.cs b/src/CommonLib/Processors/CertAbuseProcessor.cs index 093437bb1..9474e41fc 100644 --- a/src/CommonLib/Processors/CertAbuseProcessor.cs +++ b/src/CommonLib/Processors/CertAbuseProcessor.cs @@ -409,7 +409,7 @@ await _utils.ResolveIDAndType(sid.Value, computerDomain) is (true, var resolvedP return await _utils.ResolveIDAndType(sid.Value, computerDomain); } - private async Task GetMachineSid(string computerName, string computerObjectId) + internal async Task GetMachineSid(string computerName, string computerObjectId) { SecurityIdentifier machineSid = null; @@ -423,7 +423,7 @@ private async Task GetMachineSid(string computerName, string _log.LogTrace("OpenServer failed on {ComputerName}: {Error}", computerName, openServerResult.SError); await SendComputerStatus(new CSVComputerStatus { - Task = "SamConnect", + Task = nameof(OpenSamServer), ComputerName = computerName, Status = openServerResult.SError, ObjectId = computerObjectId, @@ -440,7 +440,7 @@ await SendComputerStatus(new CSVComputerStatus { Status = getMachineSidResult.SError, ComputerName = computerName, - Task = "GetMachineSid", + Task = nameof(GetMachineSid), ObjectId = computerObjectId, }); //If we can't get a machine sid, we won't be able to make local principals with unique object ids, or differentiate local/domain objects @@ -448,6 +448,13 @@ await SendComputerStatus(new CSVComputerStatus return null; } + await SendComputerStatus(new CSVComputerStatus { + Status = CSVComputerStatus.StatusSuccess, + Task = nameof(GetMachineSid), + ComputerName = computerName, + ObjectId = computerObjectId + }); + machineSid = getMachineSidResult.Value; Cache.AddMachineSid(computerObjectId, machineSid.Value); } diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index 51aada231..7b23ffb64 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Security.AccessControl; using System.Security.Principal; +using System.Threading; using System.Threading.Tasks; using Moq; using SharpHoundCommonLib; @@ -39,6 +40,8 @@ public CertAbuseProcessorTest() { _receivedCompStatus = status; return Task.CompletedTask; }; + + Cache.SetCacheInstance(Cache.CreateNewCache()); } [Theory] @@ -516,5 +519,75 @@ public void CertAbuseProcessor_OpenSamServer_CallsOpenServer_Success() { Assert.True(result.IsSuccess); Assert.IsType(result.Value); } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetMachineSid_ReturnsCachedValue() { + Cache.AddMachineSid(TargetDomainSid, TargetDomainSid); + + var result = await _certAbuseProcessor.GetMachineSid(TargetName, TargetDomainSid); + + Assert.Equal(TargetDomainSid, result.Value); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetMachineSid_OpenSAMFailure_ReturnsNull() { + var error = "Connection Failed"; + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Fail(error)); + + var result = await _certAbuseProcessor.GetMachineSid(TargetName, TargetDomainSid); + + //Validate result + Assert.Null(result); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.OpenSamServer), _receivedCompStatus.Task); + Assert.Equal(error, _receivedCompStatus.Status); + Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetMachineSid_GetMachineSidFailure_ReturnsNull() { + var mockSamServer = new Mock(); + var error = "Sid Lookup Failed"; + + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Ok(mockSamServer.Object)); + mockSamServer.Setup(x => x.GetMachineSid(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Fail(error)); + + var result = await _certAbuseProcessor.GetMachineSid(TargetName, TargetDomainSid); + + //Validate result + Assert.Null(result); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.GetMachineSid), _receivedCompStatus.Task); + Assert.Equal(error, _receivedCompStatus.Status); + Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetMachineSid_ReturnsSid() { + var mockSamServer = new Mock(); + + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Ok(mockSamServer.Object)); + mockSamServer.Setup(x => x.GetMachineSid(It.IsAny(), It.IsAny())) + .Returns(new SecurityIdentifier(TargetDomainSid)); + + var result = await _certAbuseProcessor.GetMachineSid(TargetName, TargetDomainSid); + + //Validate result + Assert.Equal(TargetDomainSid, result.Value); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.GetMachineSid), _receivedCompStatus.Task); + Assert.Equal(ComputerStatus.Success, _receivedCompStatus.Status); + Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); + } } } \ No newline at end of file From 6e7d2c6133c6cea783be9aded67cc1eb67bf529c Mon Sep 17 00:00:00 2001 From: Lucas Falslev Date: Fri, 9 Jan 2026 13:57:45 -0700 Subject: [PATCH 6/8] return EAPermission restriction when AllCertificates is true --- .../Processors/CertAbuseProcessor.cs | 74 ++++++----- test/unit/CertAbuseProcessorTest.cs | 115 +++++++++++++++++- 2 files changed, 157 insertions(+), 32 deletions(-) diff --git a/src/CommonLib/Processors/CertAbuseProcessor.cs b/src/CommonLib/Processors/CertAbuseProcessor.cs index 9474e41fc..7da4f46c5 100644 --- a/src/CommonLib/Processors/CertAbuseProcessor.cs +++ b/src/CommonLib/Processors/CertAbuseProcessor.cs @@ -466,26 +466,28 @@ await SendComputerStatus(new CSVComputerStatus { return machineSid; } - private async Task<(bool success, EnrollmentAgentRestriction restriction)> CreateEnrollmentAgentRestriction(QualifiedAce ace, string computerDomain, string computerName, bool isDomainController, string computerObjectId, SecurityIdentifier machineSid) { + internal async Task<(bool success, EnrollmentAgentRestriction restriction)> CreateEnrollmentAgentRestriction(QualifiedAce ace, string computerDomain, string computerName, bool isDomainController, string computerObjectId, SecurityIdentifier machineSid) + { + var opaque = ace.GetOpaque(); + + if(opaque is null) + return (false, default); + var targets = new List(); var index = 0; var accessType = ace.AceType.ToString(); var agent = await GetRegistryPrincipal(ace.SecurityIdentifier, computerDomain, computerName, isDomainController, computerObjectId, machineSid); - - var opaque = ace.GetOpaque(); - - if(opaque is null) - return (false, default); var sidCount = BitConverter.ToUInt32(opaque, 0); index += 4; for (var i = 0; i < sidCount; i++) { var sid = new SecurityIdentifier(opaque, index); - if (await GetRegistryPrincipal(sid, computerDomain, computerName, isDomainController, computerObjectId, - machineSid) is (true, var regPrincipal)) { + if (await GetRegistryPrincipal(sid, computerDomain, computerName, isDomainController, computerObjectId, machineSid) + is (true, var regPrincipal)) + { targets.Add(regPrincipal); } @@ -494,31 +496,41 @@ await SendComputerStatus(new CSVComputerStatus { var finalTargets = targets.ToArray(); var allTemplates = index >= opaque.Length; - if (index < opaque.Length) { - var template = Encoding.Unicode.GetString(opaque, index, opaque.Length - index - 2).Replace("\u0000", string.Empty); - if (await _utils.ResolveCertTemplateByProperty(Encoder.LdapFilterEncode(template), LDAPProperties.CanonicalName, computerDomain) is (true, var resolvedTemplate)) { - return (true, new EnrollmentAgentRestriction { - Template = resolvedTemplate, - Agent = agent.Principal, - AllTemplates = allTemplates, - AccessType = accessType, - Targets = finalTargets - }); - } - - if (await _utils.ResolveCertTemplateByProperty( - Encoder.LdapFilterEncode(template), LDAPProperties.CertTemplateOID, computerDomain) is - (true, var resolvedOidTemplate)) { - return (true, new EnrollmentAgentRestriction { - Template = resolvedOidTemplate, - Agent = agent.Principal, - AllTemplates = allTemplates, - AccessType = accessType, - Targets = finalTargets - }); - } + + if (allTemplates) { + return (true, new EnrollmentAgentRestriction { + Agent = agent.Principal, + AllTemplates = allTemplates, + AccessType = accessType, + Targets = finalTargets + }); + } + + var template = Encoding.Unicode.GetString(opaque, index, opaque.Length - index - 2).Replace("\u0000", string.Empty); + if (await _utils.ResolveCertTemplateByProperty(Encoder.LdapFilterEncode(template), LDAPProperties.CanonicalName, computerDomain) + is (true, var resolvedTemplate)) + { + return (true, new EnrollmentAgentRestriction { + Template = resolvedTemplate, + Agent = agent.Principal, + AllTemplates = allTemplates, + AccessType = accessType, + Targets = finalTargets + }); } + if (await _utils.ResolveCertTemplateByProperty(Encoder.LdapFilterEncode(template), LDAPProperties.CertTemplateOID, computerDomain) + is (true, var resolvedOidTemplate)) + { + return (true, new EnrollmentAgentRestriction { + Template = resolvedOidTemplate, + Agent = agent.Principal, + AllTemplates = allTemplates, + AccessType = accessType, + Targets = finalTargets + }); + } + return (false, default); } diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index 7b23ffb64..036d7ba13 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -4,6 +4,7 @@ using System.Security.Principal; using System.Threading; using System.Threading.Tasks; +using CommonLibTest.CollectionDefinitions; using Moq; using SharpHoundCommonLib; using SharpHoundCommonLib.Processors; @@ -15,6 +16,7 @@ namespace CommonLibTest { + [Collection(nameof(CacheTestCollectionDefinition))] public class CertAbuseProcessorTest { private readonly Mock _mockLdapUtils; @@ -507,6 +509,7 @@ public void CertAbuseProcessor_OpenSamServer_CallsOpenServer_Failure() { var result = _certAbuseProcessor.OpenSamServer(TargetName); Assert.True(result.IsFailed); + Assert.Null(result.Value); } [Fact] @@ -527,9 +530,10 @@ public async Task CertAbuseProcessor_GetMachineSid_ReturnsCachedValue() { var result = await _certAbuseProcessor.GetMachineSid(TargetName, TargetDomainSid); Assert.Equal(TargetDomainSid, result.Value); + Assert.Null(_receivedCompStatus); } - [WindowsOnlyFact] + [Fact] public async Task CertAbuseProcessor_GetMachineSid_OpenSAMFailure_ReturnsNull() { var error = "Connection Failed"; _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) @@ -589,5 +593,114 @@ public async Task CertAbuseProcessor_GetMachineSid_ReturnsSid() { Assert.Equal(ComputerStatus.Success, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NullOpaque_ReturnsFalse() { + var nullOpaqueAce = new CommonAce( + AceFlags.None, + AceQualifier.AccessAllowed, + 0x0000, + new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), + false, + null + ); + var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(nullOpaqueAce, TargetName, DomainName, false, TargetDomainSid, sid); + + //Validate result + Assert.False(result.success); + Assert.Null(result.restriction); + _mockLdapUtils.VerifyNoOtherCalls(); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_UnresolvedTemplate_ReturnsFalse() { + var emptyOpaqueAce = new CommonAce( + AceFlags.None, + AceQualifier.AccessAllowed, + 0x0000, + new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), + true, + new byte[] + { + 1, 0, 0, 0, //sid count + 1, 0, 0, 0, 0, 0, 0, 0, //target sid + 0, 0, 0, 0 //template + } + ); + var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + _mockLdapUtils.Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((false, null)); + + var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(emptyOpaqueAce, TargetName, DomainName, false, TargetDomainSid, sid); + + //Validate result + Assert.False(result.success); + Assert.Null(result.restriction); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NoTemplate_ReturnsAllTemplates() { + var emptyOpaqueAce = new CommonAce( + AceFlags.None, + AceQualifier.AccessAllowed, + 0x0000, + new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), + true, + new byte[] + { + 2, 0, 0, 0, //sid count + 1, 0, 0, 0, 0, 0, 0, 0, //target sid S-1-0 + 1, 0, 0, 0, 0, 0, 0, 1 //target sid S-1-1 + } + ); + var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + _mockLdapUtils.Setup(x => x.ResolveIDAndType("S-1-0", It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal("S-1-0", Label.User))); + _mockLdapUtils.Setup(x => x.ResolveIDAndType("S-1-1", It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal("S-1-1", Label.User))); + + var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(emptyOpaqueAce, nameof(Label.User), DomainName, false, TargetDomainSid, sid); + + //Validate result + Assert.True(result.success); + // Assert.Contains("S-1-0", result.restriction.Targets); + Assert.True(result.restriction.AllTemplates); + Assert.Null(result.restriction.Template); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_ReturnsTemplate() { + var expectedPrincipalType = Label.Group; + var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + + var emptyOpaqueAce = new CommonAce( + AceFlags.None, + AceQualifier.AccessAllowed, + 0x0000, + new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), + true, + new byte[] + { + 1, 0, 0, 0, //sid count + 1, 0, 0, 0, 0, 0, 0, 0, //target sid + 77, 0, 97, 0, 99, 0, 104, 0, 105, 0, 110, 0, 101, 0, 0, 0 //Computer Template + } + ); + var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + _mockLdapUtils.Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + + var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(emptyOpaqueAce, TargetName, DomainName, false, TargetDomainSid, sid); + + //Validate result + Assert.True(result.success); + Assert.False(result.restriction.AllTemplates); + Assert.NotNull(result.restriction.Template); + } } } \ No newline at end of file From 3d14868c4c89d0b8e6264440ac007268cff86dd4 Mon Sep 17 00:00:00 2001 From: Lucas Falslev Date: Fri, 9 Jan 2026 16:24:50 -0700 Subject: [PATCH 7/8] finish CreateEnrollmentAgentRestriction tests --- test/unit/CertAbuseProcessorTest.cs | 60 ++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 10 deletions(-) diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index 036d7ba13..b1c7147d6 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -652,30 +652,32 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_NoTemplate new byte[] { 2, 0, 0, 0, //sid count - 1, 0, 0, 0, 0, 0, 0, 0, //target sid S-1-0 - 1, 0, 0, 0, 0, 0, 0, 1 //target sid S-1-1 + 1, 0, 0, 0, 0, 0, 0, 1, //target sid S-1-1 + 1, 0, 0, 0, 0, 0, 0, 3 //target sid S-1-3 } ); var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - _mockLdapUtils.Setup(x => x.ResolveIDAndType("S-1-0", It.IsAny())) - .ReturnsAsync((true, new TypedPrincipal("S-1-0", Label.User))); _mockLdapUtils.Setup(x => x.ResolveIDAndType("S-1-1", It.IsAny())) .ReturnsAsync((true, new TypedPrincipal("S-1-1", Label.User))); + _mockLdapUtils.Setup(x => x.ResolveIDAndType("S-1-3", It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal("S-1-3", Label.User))); var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(emptyOpaqueAce, nameof(Label.User), DomainName, false, TargetDomainSid, sid); //Validate result Assert.True(result.success); - // Assert.Contains("S-1-0", result.restriction.Targets); Assert.True(result.restriction.AllTemplates); Assert.Null(result.restriction.Template); + Assert.Equal(2, result.restriction.Targets.Length); + Assert.Contains(result.restriction.Targets, t => t.ObjectIdentifier == "S-1-1"); + Assert.Contains(result.restriction.Targets, t => t.ObjectIdentifier == "S-1-3"); } [WindowsOnlyFact] - public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_ReturnsTemplate() { - var expectedPrincipalType = Label.Group; - var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512"; + public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_WithCanonicalName_ReturnsTemplate() { + var expectedPrincipalType = Label.CertTemplate; + var templateOID = "E4B7F0B1-27E5-4C0F-A5C9-641A67171D05"; var emptyOpaqueAce = new CommonAce( AceFlags.None, @@ -692,8 +694,43 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_ReturnsTem ); var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); - _mockLdapUtils.Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), It.IsAny(), It.IsAny())) - .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType))); + _mockLdapUtils.Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), LDAPProperties.CanonicalName, It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(templateOID, expectedPrincipalType))); + + var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(emptyOpaqueAce, TargetName, DomainName, false, TargetDomainSid, sid); + + Assert.True(result.success); + Assert.False(result.restriction.AllTemplates); + Assert.NotNull(result.restriction.Template); + _mockLdapUtils.Verify( + x => x.ResolveCertTemplateByProperty(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Once); + } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_WithCertTemplateOID_ReturnsTemplate() { + var expectedPrincipalType = Label.CertTemplate; + var templateOID = "E4B7F0B1-27E5-4C0F-A5C9-641A67171D05"; + + var emptyOpaqueAce = new CommonAce( + AceFlags.None, + AceQualifier.AccessAllowed, + 0x0000, + new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), + true, + new byte[] + { + 1, 0, 0, 0, //sid count + 1, 0, 0, 0, 0, 0, 0, 0, //target sid + 77, 0, 97, 0, 99, 0, 104, 0, 105, 0, 110, 0, 101, 0, 0, 0 //Computer Template + } + ); + var sid = new SecurityIdentifier(WellKnownSidType.BuiltinUsersSid, null); + + _mockLdapUtils.Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), LDAPProperties.CanonicalName, It.IsAny())) + .ReturnsAsync((false, null)); + _mockLdapUtils.Setup(x => x.ResolveCertTemplateByProperty(It.IsAny(), LDAPProperties.CertTemplateOID, It.IsAny())) + .ReturnsAsync((true, new TypedPrincipal(templateOID, expectedPrincipalType))); var result = await _certAbuseProcessor.CreateEnrollmentAgentRestriction(emptyOpaqueAce, TargetName, DomainName, false, TargetDomainSid, sid); @@ -701,6 +738,9 @@ public async Task CertAbuseProcessor_CreateEnrollmentAgentRestriction_ReturnsTem Assert.True(result.success); Assert.False(result.restriction.AllTemplates); Assert.NotNull(result.restriction.Template); + _mockLdapUtils.Verify( + x => x.ResolveCertTemplateByProperty(It.IsAny(), It.IsAny(), It.IsAny()), + Times.Exactly(2)); } } } \ No newline at end of file From b93c93df8dc700e1bf5674fc92b63e393434de38 Mon Sep 17 00:00:00 2001 From: Lucas Falslev Date: Mon, 12 Jan 2026 09:20:38 -0700 Subject: [PATCH 8/8] fix tests --- test/unit/CertAbuseProcessorTest.cs | 171 +++++++++++++++++----------- 1 file changed, 103 insertions(+), 68 deletions(-) diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index b1c7147d6..ade1fc34c 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -168,69 +168,22 @@ public async Task CertAbuseProcessor_IsRoleSeparationEnabled_HandlesFailedLookup Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } - public static IEnumerable ProcessEAPermissionsTestData() { - var nullOpaqueAce = new CommonAce( - AceFlags.None, - AceQualifier.AccessAllowed, - 0x0000, - new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), - false, - null - ); - - var daclWithNullOpaque = new RawAcl(2, 1); - daclWithNullOpaque.InsertAce(0, nullOpaqueAce); - - var emptyOpaqueAce = new CommonAce( - AceFlags.None, - AceQualifier.AccessAllowed, - 0x0000, - new SecurityIdentifier(WellKnownSidType.BuiltinAdministratorsSid, null), - true, - new byte[] { 0, 0, 0, 0 } - ); - - var daclWithEmptyOpaque = new RawAcl(2, 1); - daclWithEmptyOpaque.InsertAce(0, emptyOpaqueAce); - - return new List - { - new object[] { null }, //null dacl - new object[] { new RawAcl(2, 0) }, //empty dacl - new object[] { daclWithNullOpaque }, //dacl is callback false, null opaque - new object[] { daclWithEmptyOpaque }, //dacl is callback true, empty opaque - }; - } - - [WindowsOnlyTheory] - [MemberData(nameof(ProcessEAPermissionsTestData))] - public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl dacl) { + [Fact] + public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmptyResult() { const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; const string subValue = "EnrollmentAgentRights"; - //setup binary security descriptor as registry value - var descriptor = new RawSecurityDescriptor( - ControlFlags.DiscretionaryAclPresent, - null, - null, - null, - dacl); - - var regValue = new byte[descriptor.BinaryLength]; - descriptor.GetBinaryForm(regValue, 0); - _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( - "localhost", + TargetName, subKey, subValue)) .Returns(new RegistryResult { - Collected = true, - Value = regValue + Collected = true }); - var results = await _certAbuseProcessor.ProcessEAPermissions(CAName, DomainName, "localhost", TargetDomainSid); + var results = await _certAbuseProcessor.ProcessEAPermissions(CAName, DomainName, TargetName, TargetDomainSid); //Validate result Assert.True(results.Collected); @@ -238,7 +191,7 @@ public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl da Assert.Null(results.FailureReason); //Validate CompStatus Log - Assert.Equal("localhost", _receivedCompStatus.ComputerName); + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); Assert.Equal(nameof(CertAbuseProcessor.ProcessEAPermissions), _receivedCompStatus.Task); Assert.Equal(CSVComputerStatus.StatusSuccess, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); @@ -272,26 +225,37 @@ public async Task CertAbuseProcessor_ProcessEAPermissions_HandlesFailedLookup() Assert.Equal(FailureReason, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } + + public static IEnumerable ProcessEAPermissionsTestData() { + return new List + { + new object[] { null }, //null dacl + new object[] { new RawAcl(2, 0) }, //empty dacl + }; + } - [WindowsOnlyFact] - public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_ReturnsEmpty_WhenNoOwnerAndNoRules() { + [WindowsOnlyTheory] + [MemberData(nameof(ProcessEAPermissionsTestData))] + public async Task CertAbuseProcessor_ProcessEAPermissions_ReturnsEmpty(RawAcl dacl) { + var mockSamServer = new Mock(); + const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; - const string subValue = "Security"; + const string subValue = "EnrollmentAgentRights"; //setup binary security descriptor as registry value var descriptor = new RawSecurityDescriptor( ControlFlags.DiscretionaryAclPresent, - null, //owner is null null, null, - null); + null, + dacl); - byte[] regValue = new byte[descriptor.BinaryLength]; + var regValue = new byte[descriptor.BinaryLength]; descriptor.GetBinaryForm(regValue, 0); _mockRegistryAccessor .Setup(ra => ra.GetRegistryKeyData( - "localhost", + TargetName, subKey, subValue)) .Returns(new RegistryResult @@ -299,22 +263,44 @@ public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_Return Collected = true, Value = regValue }); + + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Ok(mockSamServer.Object)); + mockSamServer.Setup(x => x.GetMachineSid(It.IsAny(), It.IsAny())) + .Returns(new SecurityIdentifier(TargetDomainSid)); - //get access rules returns empty - var mockSecurityDescriptor = new Mock(MockBehavior.Loose, null); - mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) - .Returns([]); - _mockLdapUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + var results = await _certAbuseProcessor.ProcessEAPermissions(CAName, DomainName, TargetName, TargetDomainSid); - var results = await _certAbuseProcessor.ProcessRegistryEnrollmentPermissions(CAName, DomainName, "localhost", TargetDomainSid); + //Validate result + Assert.True(results.Collected); + Assert.Empty(results.Restrictions); + Assert.Null(results.FailureReason); + } - //Validate result + [Fact] + public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_ReturnsEmptyResult() { + const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; + const string subValue = "Security"; + + _mockRegistryAccessor + .Setup(ra => ra.GetRegistryKeyData( + TargetName, + subKey, + subValue)) + .Returns(new RegistryResult + { + Collected = true + }); + + var results = await _certAbuseProcessor.ProcessRegistryEnrollmentPermissions(CAName, DomainName, TargetName, TargetDomainSid); + + //Validate result Assert.True(results.Collected); Assert.Empty(results.Data); Assert.Null(results.FailureReason); //Validate CompStatus Log - Assert.Equal("localhost", _receivedCompStatus.ComputerName); + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); Assert.Equal(nameof(CertAbuseProcessor.ProcessRegistryEnrollmentPermissions), _receivedCompStatus.Task); Assert.Equal(CSVComputerStatus.StatusSuccess, _receivedCompStatus.Status); Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); @@ -349,6 +335,55 @@ public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_Handle Assert.Equal(TargetDomainSid, _receivedCompStatus.ObjectId); } + [WindowsOnlyFact] + public async Task CertAbuseProcessor_ProcessRegistryEnrollmentPermissions_ReturnsEmpty_WhenNoOwnerAndNoRules() { + var mockSecurityDescriptor = new Mock(null); + var mockSamServer = new Mock(); + + const string subKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{CAName}"; + const string subValue = "Security"; + + //setup binary security descriptor as registry value + var descriptor = new RawSecurityDescriptor( + ControlFlags.DiscretionaryAclPresent, + null, + null, + null, + null); + + byte[] regValue = new byte[descriptor.BinaryLength]; + descriptor.GetBinaryForm(regValue, 0); + + _mockRegistryAccessor + .Setup(ra => ra.GetRegistryKeyData( + TargetName, + subKey, + subValue)) + .Returns(new RegistryResult + { + Collected = true, + Value = regValue + }); + + _mockLdapUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object); + + _mockSAMServerAccessor.Setup(x => x.OpenServer(It.IsAny(), It.IsAny())) + .Returns(SharpHoundRPC.Result.Ok(mockSamServer.Object)); + mockSamServer.Setup(x => x.GetMachineSid(It.IsAny(), It.IsAny())) + .Returns(new SecurityIdentifier(TargetDomainSid)); + + //get access rules returns empty + mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns([]); + + var results = await _certAbuseProcessor.ProcessRegistryEnrollmentPermissions(CAName, DomainName, TargetName, TargetDomainSid); + + //Validate result + Assert.True(results.Collected); + Assert.Empty(results.Data); + Assert.Null(results.FailureReason); + } + [Fact] public async Task CertAbuseProcessor_ProcessCertTemplates_ReturnsResolvedAndUnresolvedTemplates() { const string validCN = "ValidCN";