Alter ReadGroupProperties to Add HasSIDHistory +Minor Version (#223)

* feat: Update ReadGroupProperties to Return HasSidHistory. Minor Version bump from Breaking Change

* test: Add Test for new Collection of SIDHistory on Groups

* chore: Creating a Shared ProcessSidHistory for Shared Logic

* chore: Rename ReadGroupProperties to ReadGroupPropertiesAsync

Author: Michael Cuomo

src/CommonLib/OutputTypes/Group.cs

@@ -5,5 +5,6 @@ namespace SharpHoundCommonLib.OutputTypes
     public class Group : OutputBase
     {
         public TypedPrincipal[] Members { get; set; } = Array.Empty<TypedPrincipal>();
+        public TypedPrincipal[] HasSIDHistory { get; set; } = Array.Empty<TypedPrincipal>();
     }
 }

src/CommonLib/Processors/LdapPropertyProcessor.cs

@@ -177,20 +177,33 @@ public static Dictionary<string, object> ReadOUProperties(IDirectoryObject entry
             var props = GetCommonProps(entry);
             return props;
         }
+        
+        public Task<GroupProperties> ReadGroupPropertiesAsync(IDirectoryObject entry,
+            ResolvedSearchResult searchResult) {
+            return ReadGroupPropertiesAsync(entry, searchResult.Domain);
+        }
 
         /// <summary>
         ///     Reads specific LDAP properties related to Groups
         /// </summary>
         /// <param name="entry"></param>
+        /// <param name="domain"></param>
         /// <returns></returns>
-        public static Dictionary<string, object> ReadGroupProperties(IDirectoryObject entry) {
+        public async Task<GroupProperties> ReadGroupPropertiesAsync(IDirectoryObject entry, string domain)
+        {
+            var groupProperties = new GroupProperties();
             var props = GetCommonProps(entry);
             entry.TryGetLongProperty(LDAPProperties.AdminCount, out var ac);
             props.Add("admincount", ac != 0);
             entry.TryGetLongProperty(LDAPProperties.GroupType, out var groupType);
             props.Add("groupscope", GetGroupScope(groupType));
+            entry.TryGetByteArrayProperty(LDAPProperties.SIDHistory, out var sh);
+            var (sidHistoryStrings, sidHistoryPrincipals) = await ProcessSidHistory(sh, domain);
+            groupProperties.SidHistory = sidHistoryPrincipals;
+            props.Add("sidhistory", sidHistoryStrings);
+            groupProperties.Props = props;
 
-            return props;
+            return groupProperties;
         }
 
         /// <summary>
@@ -307,25 +320,9 @@ await SendComputerStatus(new CSVComputerStatus {
             props.Add("supportedencryptiontypes", encryptionTypes);
 
             entry.TryGetByteArrayProperty(LDAPProperties.SIDHistory, out var sh);
-            var sidHistoryList = new List<string>();
-            var sidHistoryPrincipals = new List<TypedPrincipal>();
-            foreach (var sid in sh) {
-                string sSid;
-                try {
-                    sSid = new SecurityIdentifier(sid, 0).Value;
-                } catch {
-                    continue;
-                }
-
-                sidHistoryList.Add(sSid);
-
-                if (await _utils.ResolveIDAndType(sSid, domain) is (true, var res))
-                    sidHistoryPrincipals.Add(res);
-            }
-
-            userProps.SidHistory = sidHistoryPrincipals.Distinct().ToArray();
-
-            props.Add("sidhistory", sidHistoryList.ToArray());
+            var (sidHistoryStrings, sidHistoryPrincipals) = await ProcessSidHistory(sh, domain);
+            userProps.SidHistory = sidHistoryPrincipals;
+            props.Add("sidhistory", sidHistoryStrings);
 
             userProps.Props = props;
 
@@ -424,25 +421,9 @@ await SendComputerStatus(new CSVComputerStatus {
             props.Add("operatingsystem", os);
 
             entry.TryGetByteArrayProperty(LDAPProperties.SIDHistory, out var sh);
-            var sidHistoryList = new List<string>();
-            var sidHistoryPrincipals = new List<TypedPrincipal>();
-            foreach (var sid in sh) {
-                string sSid;
-                try {
-                    sSid = new SecurityIdentifier(sid, 0).Value;
-                } catch {
-                    continue;
-                }
-
-                sidHistoryList.Add(sSid);
-
-                if (await _utils.ResolveIDAndType(sSid, domain) is (true, var res))
-                    sidHistoryPrincipals.Add(res);
-            }
-
-            compProps.SidHistory = sidHistoryPrincipals.ToArray();
-
-            props.Add("sidhistory", sidHistoryList.ToArray());
+            var (sidHistoryStrings, sidHistoryPrincipals) = await ProcessSidHistory(sh, domain);
+            compProps.SidHistory = sidHistoryPrincipals;
+            props.Add("sidhistory", sidHistoryStrings);
 
             var smsaPrincipals = new List<TypedPrincipal>();
             if (entry.TryGetArrayProperty(LDAPProperties.HostServiceAccount, out var hsa)) {
@@ -789,6 +770,30 @@ private static List<string> ConvertEncryptionTypes(string encryptionTypes) {
 
             return supportedEncryptionTypes;
         }
+        
+        private async Task<(string[] sidHistoryStrings, TypedPrincipal[] sidHistoryPrincipals)> 
+            ProcessSidHistory(byte[][] sidHistory, string domain) {
+            var sidHistoryList = new List<string>();
+            var sidHistoryPrincipals = new List<TypedPrincipal>();
+                
+            if (sidHistory == null) return ([], []);
+                
+            foreach (var sid in sidHistory) {
+                string sSid;
+                try { 
+                    sSid = new SecurityIdentifier(sid, 0).Value;
+                } catch {
+                    continue;
+                }
+                        
+                sidHistoryList.Add(sSid);
+                        
+                if (await _utils.ResolveIDAndType(sSid, domain) is (true, var res))
+                    sidHistoryPrincipals.Add(res);
+            }
+                
+            return (sidHistoryList.ToArray(), sidHistoryPrincipals.Distinct().ToArray());
+        }
 
         private static string ConvertNanoDuration(long duration) {
             // In case duration is long.MinValue, Math.Abs will overflow.  Value represents Forever or Never
@@ -984,6 +989,12 @@ public ParsedCertificate(byte[] rawCertificate) {
         }
     }
 
+    public class GroupProperties
+    {
+        public Dictionary<string, object> Props { get; set; } = new();
+        public TypedPrincipal[] SidHistory { get; set; } = Array.Empty<TypedPrincipal>();
+    }
+
     public class UserProperties {
         public Dictionary<string, object> Props { get; set; } = new();
         public TypedPrincipal[] AllowedToDelegate { get; set; } = Array.Empty<TypedPrincipal>();

test/unit/LdapPropertyTests.cs

@@ -1,6 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.ComponentModel;
 using System.DirectoryServices;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
@@ -120,55 +119,81 @@ public void LDAPPropertyProcessor_ReadOUProperties_TestGoodData()
         }
 
         [Fact]
-        public void LDAPPropertyProcessor_ReadGroupProperties_TestGoodData()
+        public async Task LDAPPropertyProcessor_ReadGroupProperties_TestGoodData()
         {
             var mock = new MockDirectoryObject("CN\u003dDomain Admins,CN\u003dUsers,DC\u003dtestlab,DC\u003dlocal",
                 new Dictionary<string, object>
                 {
                     {"description", "Test"},
                     {"admincount", "1"}
                 }, "S-1-5-21-3130019616-2776909439-2417379446-512","");
+            var processor = new LdapPropertyProcessor(new MockLdapUtils());
 
-            var test = LdapPropertyProcessor.ReadGroupProperties(mock);
+            var groupProperties = await processor.ReadGroupPropertiesAsync(mock, "domain");
+            var test = groupProperties.Props;
             Assert.Contains("description", test.Keys);
             Assert.Equal("Test", test["description"] as string);
             Assert.Contains("admincount", test.Keys);
             Assert.True((bool)test["admincount"]);
         }
 
         [Fact]
-        public void LDAPPropertyProcessor_ReadGroupProperties_TestGoodData_FalseAdminCount()
+        public async Task LDAPPropertyProcessor_ReadGroupProperties_TestGoodData_FalseAdminCount()
         {
             var mock = new MockDirectoryObject("CN\u003dDomain Admins,CN\u003dUsers,DC\u003dtestlab,DC\u003dlocal",
                 new Dictionary<string, object>
                 {
                     {"description", "Test"},
                     {"admincount", "0"}
                 }, "S-1-5-21-3130019616-2776909439-2417379446-512","");
+            var processor = new LdapPropertyProcessor(new MockLdapUtils());
 
-            var test = LdapPropertyProcessor.ReadGroupProperties(mock);
+            var groupProperties = await processor.ReadGroupPropertiesAsync(mock, "domain");
+            var test = groupProperties.Props;
             Assert.Contains("description", test.Keys);
             Assert.Equal("Test", test["description"] as string);
             Assert.Contains("admincount", test.Keys);
             Assert.False((bool)test["admincount"]);
         }
 
         [Fact]
-        public void LDAPPropertyProcessor_ReadGroupProperties_NullAdminCount()
+        public async Task LDAPPropertyProcessor_ReadGroupProperties_NullAdminCount()
         {
             var mock = new MockDirectoryObject("CN\u003dDomain Admins,CN\u003dUsers,DC\u003dtestlab,DC\u003dlocal",
                 new Dictionary<string, object>
                 {
                     {"description", "Test"}
                 }, "S-1-5-21-3130019616-2776909439-2417379446-512","");
+            var processor = new LdapPropertyProcessor(new MockLdapUtils());
 
-            var test = LdapPropertyProcessor.ReadGroupProperties(mock);
+            var groupProperties = await processor.ReadGroupPropertiesAsync(mock, "domain");
+            var test = groupProperties.Props;
             Assert.Contains("description", test.Keys);
             Assert.Equal("Test", test["description"] as string);
             Assert.Contains("admincount", test.Keys);
             Assert.False((bool)test["admincount"]);
         }
 
+        [WindowsOnlyFact]
+        public async Task LDAPPropertyProcessor_ReadGroupProperties_Returns_HasSIDHistory()
+        {
+            var sid = new SecurityIdentifier("S-1-5-21-3130019616-2776909439-2417379446-519");
+            byte[] bytes = new byte[sid.BinaryLength];
+            sid.GetBinaryForm(bytes, 0);
+            var mock = new MockDirectoryObject("CN\u003dDomain Admins,CN\u003dUsers,DC\u003dtestlab,DC\u003dlocal",
+                new Dictionary<string, object>
+                {
+                    {"description", "Test"},
+                    {LDAPProperties.SIDHistory, new byte[][] { bytes }},
+                }, "S-1-5-21-3130019616-2776909439-2417379446-512","");
+            var processor = new LdapPropertyProcessor(new MockLdapUtils());
+
+            var groupProperties = await processor.ReadGroupPropertiesAsync(mock, "domain");
+            Assert.NotEmpty(groupProperties.SidHistory);
+            Assert.Equal("S-1-5-21-3130019616-2776909439-2417379446-519", groupProperties.SidHistory[0].ObjectIdentifier);
+            Assert.Equal(Label.Group, groupProperties.SidHistory[0].ObjectType);
+        }
+
         [Fact]
         public async Task LDAPPropertyProcessor_ReadUserProperties_TestTrustedToAuth()
         {