diff --git a/src/CommonLib/Helpers.cs b/src/CommonLib/Helpers.cs index 6b89ae9c9..19a51a37c 100644 --- a/src/CommonLib/Helpers.cs +++ b/src/CommonLib/Helpers.cs @@ -7,10 +7,6 @@ using System.Text.RegularExpressions; using SharpHoundCommonLib.Enums; using Microsoft.Extensions.Logging; -using System.IO; -using System.Security; -using SharpHoundCommonLib.Processors; -using Microsoft.Win32; using System.Threading.Tasks; namespace SharpHoundCommonLib { @@ -151,7 +147,7 @@ public static string DistinguishedNameToDomain(string distinguishedName) { } /// - /// Converts a domain name to a distinguished name using simple string substitution + /// Converts a domain name to a distinguished name using simple string substitution /// /// /// @@ -258,44 +254,6 @@ public static bool IsSidFiltered(string sid) { return false; } - public static RegistryResult GetRegistryKeyData(string target, string subkey, string subvalue, ILogger log) { - var data = new RegistryResult(); - - try { - var baseKey = OpenRemoteRegistry(target); - var value = baseKey.GetValue(subkey, subvalue); - data.Value = value; - - data.Collected = true; - } - catch (IOException e) { - log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", - target, subkey, subvalue); - data.FailureReason = "Target machine was not found or not connectable"; - } - catch (SecurityException e) { - log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", - target, subkey, subvalue); - data.FailureReason = "User does not have the proper permissions to perform this operation"; - } - catch (UnauthorizedAccessException e) { - log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", - target, subkey, subvalue); - data.FailureReason = "User does not have the necessary registry rights"; - } - catch (Exception e) { - log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", - target, subkey, subvalue); - data.FailureReason = e.Message; - } - - return data; - } - - public static IRegistryKey OpenRemoteRegistry(string target) { - return SHRegistryKey.Connect(RegistryHive.LocalMachine, target).GetAwaiter().GetResult(); - } - public static string[] AuthenticationOIDs = new string[] { CommonOids.ClientAuthentication, CommonOids.PKINITClientAuthentication, diff --git a/src/CommonLib/IRegistryAccessor.cs b/src/CommonLib/IRegistryAccessor.cs new file mode 100644 index 000000000..0a80174f1 --- /dev/null +++ b/src/CommonLib/IRegistryAccessor.cs @@ -0,0 +1,82 @@ +using System; +using System.IO; +using System.Security; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.Win32; +using SharpHoundCommonLib.Processors; + +namespace SharpHoundCommonLib { + public interface IRegistryAccessor { + public RegistryResult GetRegistryKeyData(string target, string subkey, string subvalue); + public IRegistryKey OpenRemoteRegistry(string target); + public Task Connect(RegistryHive hive, string machineName); + } + + public class RegistryAccessor : IRegistryAccessor { + private readonly ILogger _log; + private readonly AdaptiveTimeout _adaptiveTimeout; + + public RegistryAccessor(ILogger log = null) { + _log = log ?? Logging.LogProvider.CreateLogger(nameof(RegistryAccessor)); + _adaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromSeconds(10), _log); + } + + public RegistryResult GetRegistryKeyData(string target, string subkey, string subvalue) { + var data = new RegistryResult(); + + try { + using (var baseKey = OpenRemoteRegistry(target)) { + var value = baseKey.GetValue(subkey, subvalue); + data.Value = value; + data.Collected = true; + } + } + catch (IOException e) { + _log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", + target, subkey, subvalue); + data.FailureReason = "Target machine was not found or not connectable"; + } + catch (SecurityException e) { + _log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", + target, subkey, subvalue); + data.FailureReason = "User does not have the proper permissions to perform this operation"; + } + catch (UnauthorizedAccessException e) { + _log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", + target, subkey, subvalue); + data.FailureReason = "User does not have the necessary registry rights"; + } + catch (Exception e) { + _log.LogDebug(e, "Error getting data from registry for {Target}: {RegSubKey}:{RegValue}", + target, subkey, subvalue); + data.FailureReason = e.Message; + } + + return data; + } + + public IRegistryKey OpenRemoteRegistry(string target) { + return Connect(RegistryHive.LocalMachine, target).GetAwaiter().GetResult(); + } + + /// + /// Gets a handle to a remote registry. + /// + /// + /// + /// + /// + /// + /// + /// + /// + /// + public async Task Connect(RegistryHive hive, string machineName) { + var remoteKey = await _adaptiveTimeout.ExecuteWithTimeout((_) => RegistryKey.OpenRemoteBaseKey(hive, machineName)); + if (remoteKey.IsSuccess) + return new SHRegistryKey(remoteKey.Value); + throw new TimeoutException($"Failed to connect to registry on {machineName}: {remoteKey.Error}"); + } + } +} \ No newline at end of file diff --git a/src/CommonLib/IRegistryKey.cs b/src/CommonLib/IRegistryKey.cs index fd266265f..510efe32c 100644 --- a/src/CommonLib/IRegistryKey.cs +++ b/src/CommonLib/IRegistryKey.cs @@ -1,18 +1,16 @@ using System; -using System.Threading.Tasks; using Microsoft.Win32; namespace SharpHoundCommonLib { - public interface IRegistryKey { + public interface IRegistryKey: IDisposable { public object GetValue(string subkey, string name); public string[] GetSubKeyNames(); } - public class SHRegistryKey : IRegistryKey, IDisposable { + public class SHRegistryKey : IRegistryKey { private readonly RegistryKey _currentKey; - private static readonly AdaptiveTimeout _adaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromSeconds(10), Logging.LogProvider.CreateLogger(nameof(SHRegistryKey))); - - private SHRegistryKey(RegistryKey registryKey) { + + public SHRegistryKey(RegistryKey registryKey) { _currentKey = registryKey; } @@ -23,38 +21,8 @@ public object GetValue(string subkey, string name) { public string[] GetSubKeyNames() => _currentKey.GetSubKeyNames(); - /// - /// Gets a handle to a remote registry. - /// - /// - /// - /// - /// - /// - /// - /// - /// - /// - public static async Task Connect(RegistryHive hive, string machineName) { - var remoteKey = await _adaptiveTimeout.ExecuteWithTimeout((_) => RegistryKey.OpenRemoteBaseKey(hive, machineName)); - if (remoteKey.IsSuccess) - return new SHRegistryKey(remoteKey.Value); - throw new TimeoutException($"Failed to connect to registry on {machineName}: {remoteKey.Error}"); - } - public void Dispose() { _currentKey.Dispose(); } } - - public class MockRegistryKey : IRegistryKey { - public virtual object GetValue(string subkey, string name) { - //Unimplemented - return default; - } - - public virtual string[] GetSubKeyNames() { - throw new NotImplementedException(); - } - } } \ No newline at end of file diff --git a/src/CommonLib/Processors/CertAbuseProcessor.cs b/src/CommonLib/Processors/CertAbuseProcessor.cs index 86513f5af..d9e4286e9 100644 --- a/src/CommonLib/Processors/CertAbuseProcessor.cs +++ b/src/CommonLib/Processors/CertAbuseProcessor.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; using System.Security.AccessControl; using System.Security.Principal; using System.Text; @@ -8,7 +7,6 @@ using Microsoft.Extensions.Logging; using SharpHoundCommonLib.Enums; using SharpHoundCommonLib.OutputTypes; -using SharpHoundRPC; using SharpHoundRPC.Wrappers; using Encoder = Microsoft.Security.Application.Encoder; @@ -20,12 +18,14 @@ public class CertAbuseProcessor private readonly ILdapUtils _utils; private readonly AdaptiveTimeout _getMachineSidAdaptiveTimeout; private readonly AdaptiveTimeout _openSamServerAdaptiveTimeout; + private readonly IRegistryAccessor _registryAccessor; + public delegate Task ComputerStatusDelegate(CSVComputerStatus status); public event ComputerStatusDelegate ComputerStatusEvent; - - public CertAbuseProcessor(ILdapUtils utils, ILogger log = null) { + public CertAbuseProcessor(ILdapUtils utils, IRegistryAccessor registryAccessor, ILogger log = null) { _utils = utils; + _registryAccessor = registryAccessor; _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))); @@ -35,9 +35,10 @@ public CertAbuseProcessor(ILdapUtils utils, ILogger log = null) { /// This function should be called with the security data fetched from . /// The resulting ACEs will contain the owner of the CA as well as Management rights. /// - /// + /// /// /// + /// /// public async Task ProcessRegistryEnrollmentPermissions(string caName, string objectDomain, string computerName, string computerObjectId) { @@ -47,9 +48,23 @@ public async Task ProcessRegistryEnrollmentPermissions(str data.Collected = aceData.Collected; if (!aceData.Collected) { + await SendComputerStatus(new CSVComputerStatus { + Status = aceData.FailureReason, + Task = nameof(ProcessRegistryEnrollmentPermissions), + ComputerName = computerName, + ObjectId = computerObjectId, + }); + data.FailureReason = aceData.FailureReason; return data; } + + await SendComputerStatus(new CSVComputerStatus { + Status = CSVComputerStatus.StatusSuccess, + Task = nameof(ProcessRegistryEnrollmentPermissions), + ComputerName = computerName, + ObjectId = computerObjectId, + }); if (aceData.Value == null) { @@ -167,9 +182,23 @@ public async Task ProcessEAPermissions(string ret.Collected = regData.Collected; if (!ret.Collected) { + await SendComputerStatus(new CSVComputerStatus { + Status = regData.FailureReason, + Task = nameof(ProcessEAPermissions), + ComputerName = computerName, + ObjectId = computerObjectId, + }); + ret.FailureReason = regData.FailureReason; return ret; } + + await SendComputerStatus(new CSVComputerStatus { + Status = CSVComputerStatus.StatusSuccess, + Task = nameof(ProcessEAPermissions), + ComputerName = computerName, + ObjectId = computerObjectId, + }); if (regData.Value == null) { @@ -179,6 +208,12 @@ public async Task ProcessEAPermissions(string var isDomainController = await _utils.IsDomainController(computerObjectId, objectDomain); var machineSid = await GetMachineSid(computerName, computerObjectId); var descriptor = new RawSecurityDescriptor(regData.Value as byte[], 0); + + if (descriptor.DiscretionaryAcl is null) + { + return ret; + } + var enrollmentAgentRestrictions = new List(); foreach (var genericAce in descriptor.DiscretionaryAcl) { @@ -218,13 +253,12 @@ public async Task ProcessEAPermissions(string /// /// /// - [ExcludeFromCodeCoverage] private RegistryResult GetCASecurity(string target, string caName) { var regSubKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{caName}"; const string regValue = "Security"; - return Helpers.GetRegistryKeyData(target, regSubKey, regValue, _log); + return _registryAccessor.GetRegistryKeyData(target, regSubKey, regValue); } /// @@ -233,13 +267,12 @@ private RegistryResult GetCASecurity(string target, string caName) /// /// /// - [ExcludeFromCodeCoverage] private RegistryResult GetEnrollmentAgentRights(string target, string caName) { var regSubKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{caName}"; var regValue = "EnrollmentAgentRights"; - return Helpers.GetRegistryKeyData(target, regSubKey, regValue, _log); + return _registryAccessor.GetRegistryKeyData(target, regSubKey, regValue); } /// @@ -249,22 +282,36 @@ private RegistryResult GetEnrollmentAgentRights(string target, string caName) /// https://blog.keyfactor.com/hidden-dangers-certificate-subject-alternative-names-sans /// /// + /// /// - [ExcludeFromCodeCoverage] - public BoolRegistryAPIResult IsUserSpecifiesSanEnabled(string target, string caName) + public async Task IsUserSpecifiesSanEnabled(string target, string caName, string computerObjectId) { var ret = new BoolRegistryAPIResult(); - var subKey = + var regSubKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{caName}\\PolicyModules\\CertificateAuthority_MicrosoftDefault.Policy"; - const string subValue = "EditFlags"; - var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log); + const string regValue = "EditFlags"; + var data = _registryAccessor.GetRegistryKeyData(target, regSubKey, regValue); ret.Collected = data.Collected; if (!data.Collected) { + await SendComputerStatus(new CSVComputerStatus { + Status = data.FailureReason, + Task = nameof(IsUserSpecifiesSanEnabled), + ComputerName = target, + ObjectId = computerObjectId + }); + ret.FailureReason = data.FailureReason; return ret; } + + await SendComputerStatus(new CSVComputerStatus { + Status = CSVComputerStatus.StatusSuccess, + Task = nameof(IsUserSpecifiesSanEnabled), + ComputerName = target, + ObjectId = computerObjectId + }); if (data.Value == null) { @@ -278,28 +325,42 @@ public BoolRegistryAPIResult IsUserSpecifiesSanEnabled(string target, string caN } /// - /// This function checks a registry setting on the target host for the specified CA to see if role seperation is enabled. + /// This function checks a registry setting on the target host for the specified CA to see if role separation is enabled. /// If enabled, you cannot perform any CA actions if you have both ManageCA and ManageCertificates permissions. Only CA admins can modify the setting. /// /// https://www.itprotoday.com/security/q-how-can-i-make-sure-given-windows-account-assigned-only-single-certification-authority-ca /// /// + /// /// /// - [ExcludeFromCodeCoverage] - public BoolRegistryAPIResult RoleSeparationEnabled(string target, string caName) + public async Task IsRoleSeparationEnabled(string target, string caName, string computerObjectId) { var ret = new BoolRegistryAPIResult(); var regSubKey = $"SYSTEM\\CurrentControlSet\\Services\\CertSvc\\Configuration\\{caName}"; const string regValue = "RoleSeparationEnabled"; - var data = Helpers.GetRegistryKeyData(target, regSubKey, regValue, _log); + var data = _registryAccessor.GetRegistryKeyData(target, regSubKey, regValue); ret.Collected = data.Collected; if (!data.Collected) { + await SendComputerStatus(new CSVComputerStatus { + Status = data.FailureReason, + Task = nameof(IsRoleSeparationEnabled), + ComputerName = target, + ObjectId = computerObjectId + }); + ret.FailureReason = data.FailureReason; return ret; } + + await SendComputerStatus(new CSVComputerStatus { + Status = CSVComputerStatus.StatusSuccess, + Task = nameof(IsRoleSeparationEnabled), + ComputerName = target, + ObjectId = computerObjectId + }); if (data.Value == null) { @@ -405,6 +466,10 @@ await SendComputerStatus(new CSVComputerStatus computerObjectId, machineSid); var opaque = ace.GetOpaque(); + + if(opaque is null) + return (false, default); + var sidCount = BitConverter.ToUInt32(opaque, 0); index += 4; @@ -463,7 +528,6 @@ private async Task SendComputerStatus(CSVComputerStatus status) { if (ComputerStatusEvent is not null) await ComputerStatusEvent(status); } - } public class EnrollmentAgentRestriction diff --git a/src/CommonLib/Processors/ComputerSessionProcessor.cs b/src/CommonLib/Processors/ComputerSessionProcessor.cs index 729df1820..52c2a8c1c 100644 --- a/src/CommonLib/Processors/ComputerSessionProcessor.cs +++ b/src/CommonLib/Processors/ComputerSessionProcessor.cs @@ -24,6 +24,7 @@ public class ComputerSessionProcessor { private readonly string _localAdminPassword; private readonly AdaptiveTimeout _readUserSessionsAdaptiveTimeout; private readonly AdaptiveTimeout _readUserSessionsPriviledgedAdaptiveTimeout; + private readonly IRegistryAccessor _registryAccessor; public ComputerSessionProcessor(ILdapUtils utils, NativeMethods nativeMethods = null, ILogger log = null, string currentUserName = null, @@ -38,6 +39,7 @@ public ComputerSessionProcessor(ILdapUtils utils, _localAdminPassword = localAdminPassword; _readUserSessionsAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ReadUserSessions))); _readUserSessionsPriviledgedAdaptiveTimeout = new AdaptiveTimeout(maxTimeout: TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ReadUserSessionsPrivileged))); + _registryAccessor = new RegistryAccessor(); } public event ComputerStatusDelegate ComputerStatusEvent; @@ -293,7 +295,7 @@ public async Task ReadUserSessionsRegistry(string computerName _log.LogDebug("Running RegSessionEnum for {ObjectName}", computerName); try { - using (var key = await SHRegistryKey.Connect(RegistryHive.Users, computerName)) { + using (var key = await _registryAccessor.Connect(RegistryHive.Users, computerName)) { ret.Collected = true; await SendComputerStatus(new CSVComputerStatus { Status = CSVComputerStatus.StatusSuccess, diff --git a/src/CommonLib/Processors/DCRegistryProcessor.cs b/src/CommonLib/Processors/DCRegistryProcessor.cs index 4c229d025..2726a45a6 100644 --- a/src/CommonLib/Processors/DCRegistryProcessor.cs +++ b/src/CommonLib/Processors/DCRegistryProcessor.cs @@ -1,5 +1,4 @@ using SharpHoundCommonLib.OutputTypes; -using System; using System.Diagnostics.CodeAnalysis; using System.Threading.Tasks; using Microsoft.Extensions.Logging; @@ -9,12 +8,15 @@ namespace SharpHoundCommonLib.Processors public class DCRegistryProcessor { private readonly ILogger _log; + private readonly IRegistryAccessor _registryAccessor; + public readonly ILdapUtils _utils; public delegate Task ComputerStatusDelegate(CSVComputerStatus status); public DCRegistryProcessor(ILdapUtils utils, ILogger log = null) { _utils = utils; + _registryAccessor = new RegistryAccessor(log); _log = log ?? Logging.LogProvider.CreateLogger("DCRegProc"); } @@ -30,7 +32,7 @@ public IntRegistryAPIResult GetCertificateMappingMethods(string target) var ret = new IntRegistryAPIResult(); const string subKey = @"SYSTEM\CurrentControlSet\Control\SecurityProviders\Schannel"; const string subValue = "CertificateMappingMethods"; - var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log); + var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); ret.Collected = data.Collected; if (!data.Collected) @@ -62,7 +64,7 @@ public IntRegistryAPIResult GetStrongCertificateBindingEnforcement(string target var ret = new IntRegistryAPIResult(); const string subKey = @"SYSTEM\CurrentControlSet\Services\Kdc"; const string subValue = "StrongCertificateBindingEnforcement"; - var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log); + var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); ret.Collected = data.Collected; if (!data.Collected) @@ -94,7 +96,7 @@ public StrRegistryAPIResult GetVulnerableNetlogonSecurityDescriptor(string targe var ret = new StrRegistryAPIResult(); const string subKey = @"SYSTEM\CurrentControlSet\Services\Netlogon\Parameters"; const string subValue = "VulnerableChannelAllowList"; - var data = Helpers.GetRegistryKeyData(target, subKey, subValue, _log); + var data = _registryAccessor.GetRegistryKeyData(target, subKey, subValue); ret.Collected = data.Collected; if (!data.Collected) diff --git a/test/unit/CertAbuseProcessorTest.cs b/test/unit/CertAbuseProcessorTest.cs index 17efe86fd..663ece98d 100644 --- a/test/unit/CertAbuseProcessorTest.cs +++ b/test/unit/CertAbuseProcessorTest.cs @@ -1,141 +1,375 @@ using System; -using System.DirectoryServices; +using System.Collections.Generic; +using System.Security.AccessControl; +using System.Security.Principal; using System.Threading.Tasks; -using CommonLibTest.Facades; using Moq; -using Newtonsoft.Json; using SharpHoundCommonLib; using SharpHoundCommonLib.Processors; using Xunit; -using Xunit.Abstractions; +using Microsoft.Extensions.Logging; +using SharpHoundCommonLib.Enums; +using SharpHoundCommonLib.OutputTypes; -namespace CommonLibTest { - //TODO: Make these tests work - public class CertAbuseProcessorTest : IDisposable { - private const string CASecurityFixture = - "AQAUhCABAAAwAQAAFAAAAEQAAAACADAAAgAAAALAFAD//wAAAQEAAAAAAAEAAAAAAsAUAP//AAABAQAAAAAABQcAAAACANwABwAAAAADGAABAAAAAQIAAAAAAAUgAAAAIAIAAAADGAACAAAAAQIAAAAAAAUgAAAAIAIAAAADJAABAAAAAQUAAAAAAAUVAAAAIE+Qun9GhKV2SBaQAAIAAAADJAACAAAAAQUAAAAAAAUVAAAAIE+Qun9GhKV2SBaQAAIAAAADJAABAAAAAQUAAAAAAAUVAAAAIE+Qun9GhKV2SBaQBwIAAAADJAACAAAAAQUAAAAAAAUVAAAAIE+Qun9GhKV2SBaQBwIAAAADFAAAAgAAAQEAAAAAAAULAAAAAQIAAAAAAAUgAAAAIAIAAAECAAAAAAAFIAAAACACAAA="; +namespace CommonLibTest +{ + public class CertAbuseProcessorTest + { + private readonly Mock _mockLdapUtils; + private readonly Mock _mockRegistryAccessor; + 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 => + { + _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 }); + + var results = await _certAbuseProcessor.IsUserSpecifiesSanEnabled(TargetName, CAName, TargetDomainSid); + + //Validate result + Assert.True(results.Collected); + Assert.Equal(expectedResult, results.Value); + Assert.Null(results.FailureReason); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(CertAbuseProcessor.IsUserSpecifiesSanEnabled), _receivedCompStatus.Task); + 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 }); + + var results = await _certAbuseProcessor.IsUserSpecifiesSanEnabled(TargetName, CAName, TargetDomainSid); + + //Validate result + Assert.False(results.Collected); + Assert.Equal(FailureReason, results.FailureReason); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.IsUserSpecifiesSanEnabled), _receivedCompStatus.Task); + 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 }); + + var results = await _certAbuseProcessor.IsRoleSeparationEnabled(TargetName, CAName, TargetDomainSid); + + //Validate result + Assert.True(results.Collected); + Assert.Equal(expectedResult, results.Value); + Assert.Null(results.FailureReason); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(CertAbuseProcessor.IsRoleSeparationEnabled), _receivedCompStatus.Task); + 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 }); + + var results = await _certAbuseProcessor.IsRoleSeparationEnabled(TargetName, CAName, TargetDomainSid); + + //Validate result + Assert.False(results.Collected); + Assert.Equal(FailureReason, results.FailureReason); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.IsRoleSeparationEnabled), _receivedCompStatus.Task); + Assert.Equal(FailureReason, _receivedCompStatus.Status); + 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) { + 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", + subKey, + subValue)) + .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 }); + + var results = await _certAbuseProcessor.ProcessEAPermissions(CAName, DomainName, TargetName, TargetDomainSid); + + //Validate result + Assert.False(results.Collected); + Assert.Equal(FailureReason, results.FailureReason); - private readonly ITestOutputHelper _testOutputHelper; + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.ProcessEAPermissions), _receivedCompStatus.Task); + 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, + 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}); + + //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 }); + + var results = await _certAbuseProcessor.ProcessRegistryEnrollmentPermissions(CAName, DomainName, TargetName, TargetDomainSid); - public CertAbuseProcessorTest(ITestOutputHelper testOutputHelper) { - _testOutputHelper = testOutputHelper; + //Validate result + Assert.False(results.Collected); + Assert.Equal(FailureReason, results.FailureReason); + + //Validate CompStatus Log + Assert.Equal(TargetName, _receivedCompStatus.ComputerName); + Assert.Equal(nameof(_certAbuseProcessor.ProcessRegistryEnrollmentPermissions), _receivedCompStatus.Task); + 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); + Assert.Single(results.resolvedTemplates); + Assert.Contains(expectedTemplate, results.resolvedTemplates); + Assert.Single(results.unresolvedTemplates); + 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); + + var results = await _certAbuseProcessor.GetRegistryPrincipal( + sid, + DomainName, + TargetName, + true, + TargetDomainSid, + new SecurityIdentifier("S-1-5-18") + ); - public void Dispose() { + Assert.Equal((false, null), results); } + + [WindowsOnlyFact] + public async Task CertAbuseProcessor_GetRegistryPrincipal_ResolvedDomainController_ReturnsTrue() { + 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 results = await _certAbuseProcessor.GetRegistryPrincipal( + sid, + DomainName, + TargetName, + true, + TargetDomainSid, + new SecurityIdentifier("S-1-5-18") + ); - // [Fact] - // public void CertAbuseProcessor_GetCASecurity_HappyPath() - // { - // var mockProcessor = new Mock(new MockLDAPUtils(), null); - // - // var mockRegistryKey = new Mock(); - // mockRegistryKey.Setup(x => x.GetValue(It.IsAny(), It.IsAny())) - // .Returns(new byte[] { 0x20, 0x20 }); - // mockProcessor.Setup(x => x.OpenRemoteRegistry(It.IsAny())).Returns(mockRegistryKey.Object); - // - // var processor = mockProcessor.Object; - // var results = processor.GetCASecurity("testlab.local", "blah"); - // Assert.True(results.Collected); - // } - - // [Fact] - // public void CertAbuseProcessor_GetTrustedCerts_EmptyForNonRoot() - // { - // var mockUtils = new Mock(); - // mockUtils.Setup(x => x.IsForestRoot(It.IsAny())).Returns(false); - // var processor = new CertAbuseProcessor(mockUtils.Object); - // - // var results = processor.GetTrustedCerts("testlab.local"); - // Assert.Empty(results); - // } - // - // [Fact] - // public void CertAbuseProcessor_GetTrustedCerts_NullConfigPath_ReturnsEmpty() - // { - // var mockUtils = new Mock(); - // mockUtils.Setup(x => x.IsForestRoot(It.IsAny())).Returns(true); - // mockUtils.Setup(x => x.GetConfigurationPath(It.IsAny())).Returns((string)null); - // var processor = new CertAbuseProcessor(mockUtils.Object); - // - // var results = processor.GetTrustedCerts("testlab.local"); - // Assert.Empty(results); - // } - // - // [Fact] - // public void CertAbuseProcessor_GetRootCAs_EmptyForNonRoot() - // { - // var mockUtils = new Mock(); - // mockUtils.Setup(x => x.IsForestRoot(It.IsAny())).Returns(false); - // var processor = new CertAbuseProcessor(mockUtils.Object); - // - // var results = processor.GetRootCAs("testlab.local"); - // Assert.Empty(results); - // } - // - // [Fact] - // public void CertAbuseProcessor_GetRootCAs_NullConfigPath_ReturnsEmpty() - // { - // var mockUtils = new Mock(); - // mockUtils.Setup(x => x.IsForestRoot(It.IsAny())).Returns(true); - // mockUtils.Setup(x => x.GetConfigurationPath(It.IsAny())).Returns((string)null); - // var processor = new CertAbuseProcessor(mockUtils.Object); - // - // var results = processor.GetRootCAs("testlab.local"); - // Assert.Empty(results); - // } - - // [Fact] - // public async Task CertAbuseProcessor_ProcessCAPermissions_NullSecurity_ReturnsNull() - // { - // var processor = new CertAbuseProcessor(new MockLdapUtils()); - // - // var results = await processor.ProcessRegistryEnrollmentPermissions(null, "DUMPSTER.FIRE", null, "test"); - // - // Assert.Equal("Value cannot be null. (Parameter 'machineName')", results.FailureReason); - // Assert.False(results.Collected); - // Assert.Null(results.Data); - // } - - // [WindowsOnlyFact] - // public void CertAbuseProcessor_ProcessCAPermissions_ReturnsCorrectValues() - // { - // var mockUtils = new Mock(); - // var sd = new ActiveDirectorySecurityDescriptor(new ActiveDirectorySecurity()); - // mockUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(sd); - // var processor = new CertAbuseProcessor(mockUtils.Object); - // var bytes = Helpers.B64ToBytes(CASecurityFixture); - // - // var results = processor.ProcessCAPermissions(bytes, "TESTLAB.LOCAL", "test", false); - // _testOutputHelper.WriteLine(JsonConvert.SerializeObject(results, Formatting.Indented)); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.Owns && x.PrincipalSID == "TESTLAB.LOCAL-S-1-5-32-544" && - // x.PrincipalType == Label.Group && !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.Enroll && x.PrincipalSID == "TESTLAB.LOCAL-S-1-5-11" && - // !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.ManageCA && x.PrincipalSID == "TESTLAB.LOCAL-S-1-5-32-544" && - // !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.ManageCertificates && x.PrincipalSID == "TESTLAB.LOCAL-S-1-5-32-544" && - // !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.ManageCA && - // x.PrincipalSID == "S-1-5-21-3130019616-2776909439-2417379446-512" && - // !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.ManageCertificates && - // x.PrincipalSID == "S-1-5-21-3130019616-2776909439-2417379446-512" && - // !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.ManageCA && - // x.PrincipalSID == "S-1-5-21-3130019616-2776909439-2417379446-519" && - // !x.IsInherited); - // Assert.Contains(results, - // x => x.RightName == EdgeNames.ManageCertificates && - // x.PrincipalSID == "S-1-5-21-3130019616-2776909439-2417379446-519" && - // !x.IsInherited); - // } + Assert.Equal((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)), results); + } } } \ No newline at end of file diff --git a/test/unit/Facades/MockLdapUtils.cs b/test/unit/Facades/MockLdapUtils.cs index 8efd0eb07..1e5f1194d 100644 --- a/test/unit/Facades/MockLdapUtils.cs +++ b/test/unit/Facades/MockLdapUtils.cs @@ -744,7 +744,7 @@ public bool GetDomain(out Domain domain) { return (!results.IsNullOrEmpty(), results); } - + public Task<(bool Success, TypedPrincipal Principal)> ResolveCertTemplateByProperty(string propValue, string propName, string domainName) { throw new NotImplementedException(); } diff --git a/test/unit/Utils.cs b/test/unit/Utils.cs index 10e9d90a0..fe5255182 100644 --- a/test/unit/Utils.cs +++ b/test/unit/Utils.cs @@ -90,4 +90,12 @@ public WindowsOnlyFact() if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) Skip = "Ignore on non-Windows platforms"; } } + + public sealed class WindowsOnlyTheory : TheoryAttribute + { + public WindowsOnlyTheory() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) Skip = "Ignore on non-Windows platforms"; + } + } } \ No newline at end of file