From 15c2fc2ff78903867c5b6bc2911c48e6b7f93d91 Mon Sep 17 00:00:00 2001 From: Wesley Robert Maffly-Kipp Date: Thu, 16 May 2024 14:14:58 -0700 Subject: [PATCH 1/4] rough draft, working on tests --- cmd/api/src/queries/graph.go | 10 ++++++-- cmd/api/src/queries/graph_integration_test.go | 21 ++++++++++++++++ cmd/api/src/test/integration/harnesses.go | 24 +++++++++++++++++++ packages/go/dawgs/graph/node.go | 7 ++++-- 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/cmd/api/src/queries/graph.go b/cmd/api/src/queries/graph.go index 0ce092fec..ea975ee93 100644 --- a/cmd/api/src/queries/graph.go +++ b/cmd/api/src/queries/graph.go @@ -179,7 +179,10 @@ func (s *GraphQuery) GetAssetGroupComboNode(ctx context.Context, owningObjectID if assetGroupNodes, err := ops.FetchNodeSet(tx.Nodes().Filterf(func() graph.Criteria { filters := []graph.Criteria{ query.KindIn(query.Node(), azure.Entity, ad.Entity), - query.StringContains(query.NodeProperty(common.SystemTags.String()), assetGroupTag), + query.Or( + query.StringContains(query.NodeProperty(common.SystemTags.String()), assetGroupTag), + query.StringContains(query.NodeProperty(common.UserTags.String()), assetGroupTag), + ), } if owningObjectID != "" { @@ -233,7 +236,10 @@ func (s *GraphQuery) GetAssetGroupNodes(ctx context.Context, assetGroupTag strin if assetGroupNodes, err = ops.FetchNodeSet(tx.Nodes().Filterf(func() graph.Criteria { filters := []graph.Criteria{ query.KindIn(query.Node(), azure.Entity, ad.Entity), - query.StringContains(query.NodeProperty(common.SystemTags.String()), assetGroupTag), + query.Or( + query.StringContains(query.NodeProperty(common.SystemTags.String()), assetGroupTag), + query.StringContains(query.NodeProperty(common.UserTags.String()), assetGroupTag), + ), } return query.And(filters...) diff --git a/cmd/api/src/queries/graph_integration_test.go b/cmd/api/src/queries/graph_integration_test.go index 00b8c5ba3..514a48dde 100644 --- a/cmd/api/src/queries/graph_integration_test.go +++ b/cmd/api/src/queries/graph_integration_test.go @@ -287,6 +287,27 @@ func TestGetAssetGroupComboNode(t *testing.T) { }) } +func TestGetAssetGroupNodes(t *testing.T) { + testContext := integration.NewGraphTestContext(t, schema.DefaultGraphSchema()) + testContext.DatabaseTest(func(harness integration.HarnessDetails, db graph.Database) { + graphQuery := queries.NewGraphQuery(db, cache.Cache{}, config.Configuration{}) + + tierZeroNodes, err := graphQuery.GetAssetGroupNodes(context.Background(), ad.AdminTierZero) + require.Nil(t, err) + + customGroupNodes, err := graphQuery.GetAssetGroupNodes(context.Background(), "custom_tag") + require.Nil(t, err) + + require.True(t, tierZeroNodes.Contains(harness.AssetGroupNodesHarness.GroupB)) + require.True(t, tierZeroNodes.Contains(harness.AssetGroupNodesHarness.GroupC)) + require.Equal(t, 2, len(tierZeroNodes)) + + require.Contains(t, customGroupNodes, harness.AssetGroupNodesHarness.GroupD) + require.Contains(t, customGroupNodes, harness.AssetGroupNodesHarness.GroupE) + require.Equal(t, 2, len(customGroupNodes)) + }) +} + func TestGraphQuery_GetAllShortestPaths(t *testing.T) { testContext := integration.NewGraphTestContext(t, schema.DefaultGraphSchema()) testContext.DatabaseTestWithSetup( diff --git a/cmd/api/src/test/integration/harnesses.go b/cmd/api/src/test/integration/harnesses.go index 2de024e36..32827cb14 100644 --- a/cmd/api/src/test/integration/harnesses.go +++ b/cmd/api/src/test/integration/harnesses.go @@ -330,6 +330,29 @@ func (s *AssetGroupComboNodeHarness) Setup(testCtx *GraphTestContext) { testCtx.NewRelationship(s.GroupA, s.GroupB, ad.MemberOf) } +type AssetGroupNodesHarness struct { + GroupA *graph.Node + GroupB *graph.Node + GroupC *graph.Node + GroupD *graph.Node + GroupE *graph.Node +} + +func (s *AssetGroupNodesHarness) Setup(testCtx *GraphTestContext) { + domainSID := RandomDomainSID() + + s.GroupA = testCtx.NewActiveDirectoryGroup("GroupA", domainSID) + s.GroupB = testCtx.NewActiveDirectoryGroup("GroupB", domainSID) + s.GroupC = testCtx.NewActiveDirectoryGroup("GroupC", domainSID) + s.GroupD = testCtx.NewActiveDirectoryGroup("GroupD", domainSID) + s.GroupE = testCtx.NewActiveDirectoryGroup("GroupD", domainSID) + + s.GroupB.Properties.Set(common.SystemTags.String(), ad.AdminTierZero) + s.GroupC.Properties.Set(common.SystemTags.String(), ad.AdminTierZero) + s.GroupD.Properties.Set(common.UserTags.String(), "custom_tag") + s.GroupE.Properties.Set(common.UserTags.String(), "custom_tag another_tag") +} + type InboundControlHarness struct { ControlledUser *graph.Node ControlledGroup *graph.Node @@ -6833,6 +6856,7 @@ type HarnessDetails struct { OutboundControl OutboundControlHarness InboundControl InboundControlHarness AssetGroupComboNodeHarness AssetGroupComboNodeHarness + AssetGroupNodesHarness AssetGroupNodesHarness OUHarness OUContainedHarness MembershipHarness MembershipHarness ForeignHarness ForeignDomainHarness diff --git a/packages/go/dawgs/graph/node.go b/packages/go/dawgs/graph/node.go index 240d1ee84..4577ac1f4 100644 --- a/packages/go/dawgs/graph/node.go +++ b/packages/go/dawgs/graph/node.go @@ -18,11 +18,13 @@ package graph import ( "encoding/json" + "fmt" + "math" + "sync" + "github.com/RoaringBitmap/roaring" "github.com/RoaringBitmap/roaring/roaring64" "github.com/specterops/bloodhound/dawgs/util/size" - "math" - "sync" ) const ( @@ -183,6 +185,7 @@ func (s NodeSet) KindSet() NodeKindSet { // Contains returns true if the ID of the given Node is stored within this NodeSet. func (s NodeSet) Contains(node *Node) bool { + fmt.Printf("THIS IS THE NODE %+v", node) return s.ContainsID(node.ID) } From 08e6dda8e49483f236d140e4df105022b91e70b8 Mon Sep 17 00:00:00 2001 From: Wesley Robert Maffly-Kipp Date: Thu, 16 May 2024 15:59:03 -0700 Subject: [PATCH 2/4] tests passing --- cmd/api/src/queries/graph.go | 5 +---- cmd/api/src/queries/graph_integration_test.go | 9 ++++++--- cmd/api/src/test/integration/harnesses.go | 5 +++++ 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/cmd/api/src/queries/graph.go b/cmd/api/src/queries/graph.go index ea975ee93..205c7d5c8 100644 --- a/cmd/api/src/queries/graph.go +++ b/cmd/api/src/queries/graph.go @@ -179,10 +179,7 @@ func (s *GraphQuery) GetAssetGroupComboNode(ctx context.Context, owningObjectID if assetGroupNodes, err := ops.FetchNodeSet(tx.Nodes().Filterf(func() graph.Criteria { filters := []graph.Criteria{ query.KindIn(query.Node(), azure.Entity, ad.Entity), - query.Or( - query.StringContains(query.NodeProperty(common.SystemTags.String()), assetGroupTag), - query.StringContains(query.NodeProperty(common.UserTags.String()), assetGroupTag), - ), + query.StringContains(query.NodeProperty(common.SystemTags.String()), assetGroupTag), } if owningObjectID != "" { diff --git a/cmd/api/src/queries/graph_integration_test.go b/cmd/api/src/queries/graph_integration_test.go index 514a48dde..adff67d31 100644 --- a/cmd/api/src/queries/graph_integration_test.go +++ b/cmd/api/src/queries/graph_integration_test.go @@ -289,7 +289,10 @@ func TestGetAssetGroupComboNode(t *testing.T) { func TestGetAssetGroupNodes(t *testing.T) { testContext := integration.NewGraphTestContext(t, schema.DefaultGraphSchema()) - testContext.DatabaseTest(func(harness integration.HarnessDetails, db graph.Database) { + testContext.DatabaseTestWithSetup(func(harness *integration.HarnessDetails) error { + harness.AssetGroupNodesHarness.Setup(testContext) + return nil + }, func(harness integration.HarnessDetails, db graph.Database) { graphQuery := queries.NewGraphQuery(db, cache.Cache{}, config.Configuration{}) tierZeroNodes, err := graphQuery.GetAssetGroupNodes(context.Background(), ad.AdminTierZero) @@ -302,8 +305,8 @@ func TestGetAssetGroupNodes(t *testing.T) { require.True(t, tierZeroNodes.Contains(harness.AssetGroupNodesHarness.GroupC)) require.Equal(t, 2, len(tierZeroNodes)) - require.Contains(t, customGroupNodes, harness.AssetGroupNodesHarness.GroupD) - require.Contains(t, customGroupNodes, harness.AssetGroupNodesHarness.GroupE) + require.True(t, customGroupNodes.Contains(harness.AssetGroupNodesHarness.GroupD)) + require.True(t, customGroupNodes.Contains(harness.AssetGroupNodesHarness.GroupE)) require.Equal(t, 2, len(customGroupNodes)) }) } diff --git a/cmd/api/src/test/integration/harnesses.go b/cmd/api/src/test/integration/harnesses.go index 32827cb14..a21edc15d 100644 --- a/cmd/api/src/test/integration/harnesses.go +++ b/cmd/api/src/test/integration/harnesses.go @@ -351,6 +351,11 @@ func (s *AssetGroupNodesHarness) Setup(testCtx *GraphTestContext) { s.GroupC.Properties.Set(common.SystemTags.String(), ad.AdminTierZero) s.GroupD.Properties.Set(common.UserTags.String(), "custom_tag") s.GroupE.Properties.Set(common.UserTags.String(), "custom_tag another_tag") + + testCtx.UpdateNode(s.GroupB) + testCtx.UpdateNode(s.GroupC) + testCtx.UpdateNode(s.GroupD) + testCtx.UpdateNode(s.GroupE) } type InboundControlHarness struct { From 3ff0915c78980551785f047673a17e0c0dc77afd Mon Sep 17 00:00:00 2001 From: Wesley Robert Maffly-Kipp Date: Fri, 17 May 2024 10:41:16 -0700 Subject: [PATCH 3/4] removing unnecessary change --- packages/go/dawgs/graph/node.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/go/dawgs/graph/node.go b/packages/go/dawgs/graph/node.go index 4577ac1f4..240d1ee84 100644 --- a/packages/go/dawgs/graph/node.go +++ b/packages/go/dawgs/graph/node.go @@ -18,13 +18,11 @@ package graph import ( "encoding/json" - "fmt" - "math" - "sync" - "github.com/RoaringBitmap/roaring" "github.com/RoaringBitmap/roaring/roaring64" "github.com/specterops/bloodhound/dawgs/util/size" + "math" + "sync" ) const ( @@ -185,7 +183,6 @@ func (s NodeSet) KindSet() NodeKindSet { // Contains returns true if the ID of the given Node is stored within this NodeSet. func (s NodeSet) Contains(node *Node) bool { - fmt.Printf("THIS IS THE NODE %+v", node) return s.ContainsID(node.ID) } From d78005a169b73ac428cb292dfdeae358dc8a2f67 Mon Sep 17 00:00:00 2001 From: Wesley Robert Maffly-Kipp Date: Fri, 17 May 2024 12:59:46 -0700 Subject: [PATCH 4/4] updated to avoid matching substrings --- cmd/api/src/queries/graph.go | 14 +++++++++- cmd/api/src/queries/graph_integration_test.go | 15 ++++++---- cmd/api/src/test/integration/harnesses.go | 28 ++++++++++++------- 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/cmd/api/src/queries/graph.go b/cmd/api/src/queries/graph.go index 205c7d5c8..159d80ea9 100644 --- a/cmd/api/src/queries/graph.go +++ b/cmd/api/src/queries/graph.go @@ -24,6 +24,7 @@ import ( "fmt" "net/http" "net/url" + "slices" "sort" "strconv" "strings" @@ -244,7 +245,18 @@ func (s *GraphQuery) GetAssetGroupNodes(ctx context.Context, assetGroupTag strin return err } else { for _, node := range assetGroupNodes { - node.Properties.Set("type", analysis.GetNodeKindDisplayLabel(node)) + // We need to filter out nodes that do not contain an exact tag match + var ( + systemTags, _ = node.Properties.Get(common.SystemTags.String()).String() + userTags, _ = node.Properties.Get(common.UserTags.String()).String() + allTags = append(strings.Split(systemTags, " "), strings.Split(userTags, " ")...) + ) + + if !slices.Contains(allTags, assetGroupTag) { + assetGroupNodes.Remove(node.ID) + } else { + node.Properties.Set("type", analysis.GetNodeKindDisplayLabel(node)) + } } return nil } diff --git a/cmd/api/src/queries/graph_integration_test.go b/cmd/api/src/queries/graph_integration_test.go index adff67d31..02813468d 100644 --- a/cmd/api/src/queries/graph_integration_test.go +++ b/cmd/api/src/queries/graph_integration_test.go @@ -295,19 +295,24 @@ func TestGetAssetGroupNodes(t *testing.T) { }, func(harness integration.HarnessDetails, db graph.Database) { graphQuery := queries.NewGraphQuery(db, cache.Cache{}, config.Configuration{}) - tierZeroNodes, err := graphQuery.GetAssetGroupNodes(context.Background(), ad.AdminTierZero) + tierZeroNodes, err := graphQuery.GetAssetGroupNodes(context.Background(), harness.AssetGroupNodesHarness.TierZeroTag) require.Nil(t, err) - customGroupNodes, err := graphQuery.GetAssetGroupNodes(context.Background(), "custom_tag") + customGroup1Nodes, err := graphQuery.GetAssetGroupNodes(context.Background(), harness.AssetGroupNodesHarness.CustomTag1) + require.Nil(t, err) + + customGroup2Nodes, err := graphQuery.GetAssetGroupNodes(context.Background(), harness.AssetGroupNodesHarness.CustomTag2) require.Nil(t, err) require.True(t, tierZeroNodes.Contains(harness.AssetGroupNodesHarness.GroupB)) require.True(t, tierZeroNodes.Contains(harness.AssetGroupNodesHarness.GroupC)) require.Equal(t, 2, len(tierZeroNodes)) - require.True(t, customGroupNodes.Contains(harness.AssetGroupNodesHarness.GroupD)) - require.True(t, customGroupNodes.Contains(harness.AssetGroupNodesHarness.GroupE)) - require.Equal(t, 2, len(customGroupNodes)) + require.True(t, customGroup1Nodes.Contains(harness.AssetGroupNodesHarness.GroupD)) + require.Equal(t, 1, len(customGroup1Nodes)) + + require.True(t, customGroup2Nodes.Contains(harness.AssetGroupNodesHarness.GroupE)) + require.Equal(t, 1, len(customGroup2Nodes)) }) } diff --git a/cmd/api/src/test/integration/harnesses.go b/cmd/api/src/test/integration/harnesses.go index a21edc15d..f05974a07 100644 --- a/cmd/api/src/test/integration/harnesses.go +++ b/cmd/api/src/test/integration/harnesses.go @@ -331,26 +331,34 @@ func (s *AssetGroupComboNodeHarness) Setup(testCtx *GraphTestContext) { } type AssetGroupNodesHarness struct { - GroupA *graph.Node - GroupB *graph.Node - GroupC *graph.Node - GroupD *graph.Node - GroupE *graph.Node + GroupA *graph.Node + GroupB *graph.Node + GroupC *graph.Node + GroupD *graph.Node + GroupE *graph.Node + TierZeroTag string + CustomTag1 string + CustomTag2 string } func (s *AssetGroupNodesHarness) Setup(testCtx *GraphTestContext) { domainSID := RandomDomainSID() + // use one tag value that contains the other as a substring to test that we only match exactly + s.TierZeroTag = ad.AdminTierZero + s.CustomTag1 = "custom_tag" + s.CustomTag2 = "another_custom_tag" + s.GroupA = testCtx.NewActiveDirectoryGroup("GroupA", domainSID) s.GroupB = testCtx.NewActiveDirectoryGroup("GroupB", domainSID) s.GroupC = testCtx.NewActiveDirectoryGroup("GroupC", domainSID) s.GroupD = testCtx.NewActiveDirectoryGroup("GroupD", domainSID) - s.GroupE = testCtx.NewActiveDirectoryGroup("GroupD", domainSID) + s.GroupE = testCtx.NewActiveDirectoryGroup("GroupE", domainSID) - s.GroupB.Properties.Set(common.SystemTags.String(), ad.AdminTierZero) - s.GroupC.Properties.Set(common.SystemTags.String(), ad.AdminTierZero) - s.GroupD.Properties.Set(common.UserTags.String(), "custom_tag") - s.GroupE.Properties.Set(common.UserTags.String(), "custom_tag another_tag") + s.GroupB.Properties.Set(common.SystemTags.String(), s.TierZeroTag) + s.GroupC.Properties.Set(common.SystemTags.String(), s.TierZeroTag) + s.GroupD.Properties.Set(common.UserTags.String(), s.CustomTag1) + s.GroupE.Properties.Set(common.UserTags.String(), s.CustomTag2) testCtx.UpdateNode(s.GroupB) testCtx.UpdateNode(s.GroupC)