BED-5435 AddSelf, BED-5988 Mandatory Filters (#209)

* fix: properly account for AddSelf edges where the RightsGuid is AllGuid, which is a valid attack for this scenario
fix: Make our ldap filters actually respect mandatory filters properly

https://specterops.atlassian.net/browse/BED-5435
https://specterops.atlassian.net/browse/BED-5988

* chore: add unit tests to check that AddSelf edge is added when appropriate and not added if not

* chore: added unit test for BED-5988

---------

Co-authored-by: Rohan Vazarkar <rvazarkar@specterops.io>

Author: ktstrader

src/CommonLib/LdapQueries/LdapFilter.cs

@@ -255,8 +255,21 @@ public string GetFilter() {
             return filterPartsDistinct;
         }
 
+        private string MergeFilter(string filterA, string filterB) {
+            return $"(&{filterA}{filterB})";
+        }
+
         public IEnumerable<string> GetFilterList() {
-            return _filterParts.Distinct();
+            foreach (var filter in _filterParts.Distinct())
+            {
+                if (_mandatory.Count > 0) {
+                    foreach (var mandatory in _mandatory) {
+                        yield return MergeFilter(filter, mandatory);
+                    }
+                } else {
+                    yield return filter;
+                }
+            }
         }
     }
 }

src/CommonLib/Processors/ACLProcessor.cs

@@ -407,7 +407,7 @@ public async IAsyncEnumerable<ACE> ProcessACL(byte[] ntSecurityDescriptor, strin
                     if (aceRights.HasFlag(ActiveDirectoryRights.Self) &&
                         !aceRights.HasFlag(ActiveDirectoryRights.WriteProperty) &&
                         !aceRights.HasFlag(ActiveDirectoryRights.GenericWrite) && objectType == Label.Group &&
-                        aceType == ACEGuids.WriteMember)
+                        aceType is ACEGuids.WriteMember or ACEGuids.AllGuid)
                         aces.Add(new ACE
                         {
                             PrincipalType = resolvedPrincipal.ObjectType,

test/unit/ACLProcessorTest.cs

@@ -734,6 +734,78 @@ public async Task ACLProcessor_ProcessACL_Self() {
             Assert.False(actual.IsInherited);
             Assert.Equal(actual.RightName, expectedRightName);
         }
+        
+        [Fact]
+        public async Task ACLProcessor_ProcessACL_Self_AllGuid() {
+            var expectedPrincipalType = Label.Group;
+            var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512";
+            var expectedRightName = EdgeNames.AddSelf;
+
+            var mockLDAPUtils = new Mock<ILdapUtils>();
+            var mockSecurityDescriptor = new Mock<ActiveDirectorySecurityDescriptor>(MockBehavior.Loose, null);
+            var mockRule = new Mock<ActiveDirectoryRuleDescriptor>(MockBehavior.Loose, null);
+            var collection = new List<ActiveDirectoryRuleDescriptor>();
+            mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow);
+            mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny<string>())).Returns(true);
+            mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID);
+            mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.Self);
+            mockRule.Setup(x => x.ObjectType()).Returns(new Guid(ACEGuids.AllGuid));
+            collection.Add(mockRule.Object);
+
+            mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<Type>()))
+                .Returns(collection);
+            mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny<Type>())).Returns((string)null);
+            mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object);
+            mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny<string>(), It.IsAny<string>()))
+                .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)));
+            var mockData = new[] { LdapResult<IDirectoryObject>.Fail() };
+            mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny<LdapQueryParameters>(), It.IsAny<CancellationToken>()))
+                .Returns(mockData.ToAsyncEnumerable());
+
+            var processor = new ACLProcessor(mockLDAPUtils.Object);
+            var bytes = Utils.B64ToBytes(AddMemberSecurityDescriptor);
+            var result = await processor.ProcessACL(bytes, _testDomainName, Label.Group, false).ToArrayAsync();
+
+            Assert.Single(result);
+            var actual = result.First();
+            Assert.Equal(actual.PrincipalType, expectedPrincipalType);
+            Assert.Equal(actual.PrincipalSID, expectedPrincipalSID);
+            Assert.False(actual.IsInherited);
+            Assert.Equal(actual.RightName, expectedRightName);
+        }
+        
+        [Fact]
+        public async Task ACLProcessor_ProcessACL_NoAddSelfEdge() {
+            var expectedPrincipalType = Label.Group;
+            var expectedPrincipalSID = "S-1-5-21-3130019616-2776909439-2417379446-512";
+
+            var mockLDAPUtils = new Mock<ILdapUtils>();
+            var mockSecurityDescriptor = new Mock<ActiveDirectorySecurityDescriptor>(MockBehavior.Loose, null);
+            var mockRule = new Mock<ActiveDirectoryRuleDescriptor>(MockBehavior.Loose, null);
+            var collection = new List<ActiveDirectoryRuleDescriptor>();
+            mockRule.Setup(x => x.AccessControlType()).Returns(AccessControlType.Allow);
+            mockRule.Setup(x => x.IsAceInheritedFrom(It.IsAny<string>())).Returns(true);
+            mockRule.Setup(x => x.IdentityReference()).Returns(expectedPrincipalSID);
+            mockRule.Setup(x => x.ActiveDirectoryRights()).Returns(ActiveDirectoryRights.Self);
+            mockRule.Setup(x => x.ObjectType()).Returns(new Guid(ACEGuids.WriteAllowedToAct));
+            collection.Add(mockRule.Object);
+
+            mockSecurityDescriptor.Setup(m => m.GetAccessRules(It.IsAny<bool>(), It.IsAny<bool>(), It.IsAny<Type>()))
+                .Returns(collection);
+            mockSecurityDescriptor.Setup(m => m.GetOwner(It.IsAny<Type>())).Returns((string)null);
+            mockLDAPUtils.Setup(x => x.MakeSecurityDescriptor()).Returns(mockSecurityDescriptor.Object);
+            mockLDAPUtils.Setup(x => x.ResolveIDAndType(It.IsAny<string>(), It.IsAny<string>()))
+                .ReturnsAsync((true, new TypedPrincipal(expectedPrincipalSID, expectedPrincipalType)));
+            var mockData = new[] { LdapResult<IDirectoryObject>.Fail() };
+            mockLDAPUtils.Setup(x => x.PagedQuery(It.IsAny<LdapQueryParameters>(), It.IsAny<CancellationToken>()))
+                .Returns(mockData.ToAsyncEnumerable());
+
+            var processor = new ACLProcessor(mockLDAPUtils.Object);
+            var bytes = Utils.B64ToBytes(AddMemberSecurityDescriptor);
+            var result = await processor.ProcessACL(bytes, _testDomainName, Label.Group, false).ToArrayAsync();
+
+            Assert.Empty(result);
+        }
 
         [Fact]
         public async Task ACLProcessor_ProcessACL_ExtendedRight_Domain_Unmatched() {

test/unit/LDAPFilterTest.cs

@@ -73,6 +73,28 @@ public void LDAPFilter_GetFilterList()
                  i++;
             }
         }
+        
+        [Fact]
+        public void LDAPFilter_GetFilterList_MergeFilter()
+        {
+            var test = new LdapFilter();
+            test.AddUsers();
+            test.AddComputers();
+            string mergeFilter = "(objectclass=*)";
+            test.AddFilter(mergeFilter, true);
+            
+            IEnumerable<string> filters = test.GetFilterList();
+            
+            int i = 0;
+            string computerFilter = "(samaccounttype=805306369)";
+            string userFilter = "(|(samaccounttype=805306368)(samaccounttype=805306370))";
+            string[] expected = {$"(&{userFilter}{mergeFilter})", $"(&{computerFilter}{mergeFilter})"};
+
+            foreach (var filter in filters) {
+                Assert.Equal(expected[i], filter);
+                i++;
+            }
+        }
 
         #endregion
     }