Improve NTLM Collection Logging BED-7374

src/CommonLib/Processors/ComputerAvailability.cs

@@ -95,11 +95,19 @@ await SendComputerStatus(new CSVComputerStatus {
                 };
             }
 
-            if (_skipPortScan)
+            if (_skipPortScan) {
+                await SendComputerStatus(new CSVComputerStatus {
+                    Status = CSVComputerStatus.StatusSuccess,
+                    Task = "ComputerAvailability",
+                    ComputerName = computerName,
+                    ObjectId = objectId,
+                });
+
                 return new ComputerStatus {
                     Connectable = true,
                     Error = null
                 };
+            }
 
             if (!await _scanner.CheckPort(computerName)) {
                 _log.LogTrace("{ComputerName} is not available because port 445 is unavailable", computerName);

src/CommonLib/Processors/RegistryProcessor.cs

@@ -6,20 +6,24 @@
 using System;
 using System.Linq;
 using System.Threading.Tasks;
-using static SharpHoundCommonLib.Helpers;
 
 namespace SharpHoundCommonLib.Processors;
 
 public class RegistryProcessor {
+    public delegate Task ComputerStatusDelegate(CSVComputerStatus status);
+
     private readonly ILogger _log;
     private readonly IPortScanner _portScanner;
+    private readonly IStrategyExecutor _registryCollector; 
+    private readonly AdaptiveTimeout _registryAdaptiveTimeout = new(maxTimeout:TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ReadRegistrySettings)));
     private readonly ICollectionStrategy<RegistryQueryResult, RegistryQuery>[] _strategies;
     private readonly RegistryQuery[] _queries;
-    private readonly AdaptiveTimeout _registryAdaptiveTimeout = new(maxTimeout:TimeSpan.FromMinutes(2), Logging.LogProvider.CreateLogger(nameof(ReadRegistrySettings)));
 
-    public RegistryProcessor(ILogger log, string domain) {
+    public RegistryProcessor(ILogger log, IStrategyExecutor registryCollector, string domain) {
         _log = log ?? Logging.LogProvider.CreateLogger("RegistryProcessor");
         _portScanner = new PortScanner();
+        _registryCollector = registryCollector;
+
         _strategies = [
             // Higher priority at the top of the list
             new DotNetWmiRegistryStrategy(_portScanner, domain),
@@ -50,12 +54,13 @@ public RegistryProcessor(ILogger log, string domain) {
         ];
     }
 
+    public event ComputerStatusDelegate ComputerStatusEvent;
+
     public async Task<APIResult<RegistryData>> ReadRegistrySettings(string targetMachine) {
         var output = new RegistryData();
 
         try {
-            var registryCollector = new StrategyExecutor();
-            var result = await _registryAdaptiveTimeout.ExecuteWithTimeout(async (_) => await registryCollector
+            var result = await _registryAdaptiveTimeout.ExecuteWithTimeout(async (_) => await _registryCollector
                 .CollectAsync(targetMachine, _queries, _strategies)
                 .ConfigureAwait(false));
 
@@ -65,6 +70,32 @@ public async Task<APIResult<RegistryData>> ReadRegistrySettings(string targetMac
 
             var collectedData = result.Value;
 
+            foreach (var attempt in collectedData.FailureAttempts ?? []) {
+                _log.LogTrace("ReadRegistry failed on {ComputerName} using {Strategy}: {Error}", targetMachine, attempt.StrategyType.Name, attempt.FailureReason);
+                await SendComputerStatus(new CSVComputerStatus
+                {
+                    Task = $"{nameof(ReadRegistrySettings)} - {attempt.StrategyType.Name}",
+                    ComputerName = targetMachine,
+                    Status = attempt.FailureReason
+                });
+            }
+
+            if (!collectedData.WasSuccessful) {
+                var msg = collectedData.FailureAttempts is null 
+                    ? "Failed to read registry settings"
+                    : string.Join("\n",
+                        collectedData.FailureAttempts.Select(a => $"{a.StrategyType.Name}: {a.FailureReason ?? ""}"));
+
+                return APIResult<RegistryData>.Failure(msg);
+            }
+
+            await SendComputerStatus(new CSVComputerStatus
+            {
+                Task = $"{nameof(ReadRegistrySettings)} - {collectedData.SuccessfulStrategy?.Name ?? ""}",
+                ComputerName = targetMachine,
+                Status = CSVComputerStatus.StatusSuccess
+            });
+
             foreach (var key in collectedData.Results ?? []) {
                 if (!key.ValueExists)
                     continue;
@@ -101,13 +132,6 @@ public async Task<APIResult<RegistryData>> ReadRegistrySettings(string targetMac
                 }
             }
 
-            // If all strategies failed, need to report errors.
-            if (collectedData.FailureAttempts.Count() == _strategies.Length) {
-                string msg = string.Join("\n",
-                    collectedData.FailureAttempts.Select(a => $"{a.StrategyType.Name}: {a.FailureReason ?? ""}"));
-                return APIResult<RegistryData>.Failure(msg);
-            }
-
             return APIResult<RegistryData>.Success(output);
         } catch (Exception ex) {
             _log.LogError(
@@ -118,4 +142,8 @@ public async Task<APIResult<RegistryData>> ReadRegistrySettings(string targetMac
             return APIResult<RegistryData>.Failure(ex.ToString());
         }
     }
+
+    private async Task SendComputerStatus(CSVComputerStatus status) {
+        if (ComputerStatusEvent is not null) await ComputerStatusEvent.Invoke(status);
+    }
 }

src/SharpHoundRPC/Registry/DotNetWmiRegistryStrategy.cs

@@ -26,7 +26,6 @@ public class DotNetWmiRegistryStrategy : ICollectionStrategy<RegistryQueryResult
         /// </summary>
         public bool UseKerberos { get; set; } = true;
 
-
         /// <summary>
         /// Creates a new WMI registry strategy
         /// </summary>
@@ -38,6 +37,9 @@ public DotNetWmiRegistryStrategy(IPortScanner scanner, string domain) {
         }
 
         public async Task<(bool, string)> CanExecute(string targetMachine) {
+            if (string.IsNullOrEmpty(targetMachine)) {
+                throw new ArgumentException("Target machine cannot be null or empty", nameof(targetMachine));
+            }
             try {
                 var isOpen = await _portScanner.CheckPort(targetMachine, EpMapperPort, throwError: true);
                 return (isOpen, string.Empty);

src/SharpHoundRPC/Registry/NativeUtils.cs

@@ -1,10 +1,8 @@
 using System;
 using System.Collections.Generic;
 using System.ComponentModel;
-using System.Linq;
 using System.Runtime.InteropServices;
 using System.Text;
-using System.Threading.Tasks;
 using static SharpHoundRPC.NetAPINative.NetAPIEnums;
 
 namespace SharpHoundRPC.Registry {

src/SharpHoundRPC/Registry/RemoteRegistryStrategy.cs

@@ -1,7 +1,7 @@
 #nullable enable
 namespace SharpHoundRPC.Registry {
     using Microsoft.Win32;
-    using SharpHoundRPC.PortScanner;
+    using PortScanner;
     using System;
     using System.Collections.Generic;
     using System.Linq;
@@ -38,7 +38,6 @@ public async Task<IEnumerable<RegistryQueryResult>> ExecuteAsync(
             if (queries == null || !queries.Any())
                 throw new ArgumentException("Queries cannot be null or empty", nameof(queries));
 
-
             return await Task.Run(() => {
                 var results = new List<RegistryQueryResult>();
 

src/SharpHoundRPC/Registry/StrategyExecutor.cs

@@ -4,8 +4,15 @@ namespace SharpHoundRPC.Registry {
     using System.Collections.Generic;
     using System.Threading.Tasks;
 
-
-    public class StrategyExecutor {
+    public interface IStrategyExecutor
+    {
+        Task<StrategyExecutorResult<T>> CollectAsync<T, TQuery>(
+            string targetMachine,
+            IEnumerable<TQuery> queries,
+            IEnumerable<ICollectionStrategy<T, TQuery>> strategies);
+    }
+
+    public class StrategyExecutor : IStrategyExecutor {
         public async Task<StrategyExecutorResult<T>> CollectAsync<T, TQuery>(
             string targetMachine,
             IEnumerable<TQuery> queries,
@@ -25,13 +32,11 @@ public async Task<StrategyExecutorResult<T>> CollectAsync<T, TQuery>(
                 try {
                     var results = await strategy.ExecuteAsync(targetMachine, queries).ConfigureAwait(false);
 
-                    attempt.WasSuccessful = true;
-                    attempt.Results = results;
-
                     return new StrategyExecutorResult<T> {
                         Results = results,
                         FailureAttempts = attempts,
-                        WasSuccessful = true
+                        WasSuccessful = true,
+                        SuccessfulStrategy =  strategy.GetType(),
                     };
                 } catch (Exception ex) {
                     attempt.FailureReason = $"Collector failed: {ex.Message}.\nInner Exception: {ex.InnerException}";

src/SharpHoundRPC/Registry/StrategyExecutorResult.cs

@@ -1,12 +1,16 @@
 #nullable enable
-namespace SharpHoundRPC.Registry {
-    using System.Collections.Generic;
 
+using System;
+using System.Collections.Generic;
+
+namespace SharpHoundRPC.Registry {
 
     public class StrategyExecutorResult<T> {
         public IEnumerable<T>? Results { get; set; } = null;
         public IEnumerable<StrategyResult<T>>? FailureAttempts { get; set; } = null;
         public bool WasSuccessful = false;
+        public Type? SuccessfulStrategy { get; set; } = null;
     }
-#nullable disable
-}
+}
+
+#nullable disable

test/unit/Facades/MockExtentions.cs

@@ -32,4 +32,16 @@ public static void VerifyLog<T>(this Mock<ILogger<T>> mockLogger, LogLevel logLe
                 It.IsAny<Func<It.IsAnyType, Exception?, string>>()),
             Times.Once);
     }
+
+    public static void VerifyNoLogs<T>(this Mock<ILogger<T>> mockLogger, LogLevel logLevel)
+    {
+        mockLogger.Verify(
+            x => x.Log(
+                logLevel,
+                It.IsAny<EventId>(),
+                It.IsAny<It.IsAnyType>(),
+                It.IsAny<Exception>(),
+                It.IsAny<Func<It.IsAnyType, Exception, string>>()),
+            Times.Never);
+    }
 }