fix(postprocess): Fix multi-forest ADCS false positives BED-5572

packages/go/analysis/ad/adcs_forest_integration_test.go

@@ -0,0 +1,161 @@
+// Copyright 2026 Specter Ops, Inc.
+//
+// Licensed under the Apache License, Version 2.0
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build integration
+
+package ad_test
+
+import (
+	"testing"
+
+	"github.com/specterops/bloodhound/cmd/api/src/test/integration"
+	"github.com/specterops/bloodhound/packages/go/graphschema"
+	"github.com/specterops/bloodhound/packages/go/graphschema/ad"
+	"github.com/specterops/bloodhound/packages/go/graphschema/common"
+	"github.com/specterops/dawgs/graph"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// addEnabledHostingComputer creates an enabled computer in the given domain,
+// links it to the CA via HostsCAService, and returns the computer for tests that
+// need to model one host serving multiple EnterpriseCA nodes.
+func addEnabledHostingComputer(testCtx *integration.GraphTestContext, name, domainSID string, enterpriseCA *graph.Node) *graph.Node {
+	computer := testCtx.NewActiveDirectoryComputer(name, domainSID)
+	computer.Properties.Set(common.Enabled.String(), true)
+	testCtx.UpdateNode(computer)
+	testCtx.NewRelationship(computer, enterpriseCA, ad.HostsCAService)
+	return computer
+}
+
+// linkEnterpriseCAToDomain adds the per-domain edges that make a domain "chain
+// valid" (RootCAFor ∩ TrustedForNTAuth) for the CA. The per-CA EnterpriseCAFor
+// edge is created once by the caller; each domain gets its own NTAuthStore so the
+// TrustedForNTAuth edges don't collide.
+func linkEnterpriseCAToDomain(testCtx *integration.GraphTestContext, enterpriseCA, rootCA *graph.Node, domain *graph.Node, domainSID string) {
+	ntAuthStore := testCtx.NewActiveDirectoryNTAuthStore("NTAuthStore-"+domainSID, domainSID)
+
+	// RootCAFor path: domain <-RootCAFor- rootCA <-EnterpriseCAFor- enterpriseCA
+	testCtx.NewRelationship(rootCA, domain, ad.RootCAFor)
+
+	// TrustedForNTAuth path: domain <-NTAuthStoreFor- ntAuthStore <-TrustedForNTAuth- enterpriseCA
+	testCtx.NewRelationship(ntAuthStore, domain, ad.NTAuthStoreFor)
+	testCtx.NewRelationship(enterpriseCA, ntAuthStore, ad.TrustedForNTAuth)
+}
+
+// TestADCSForestScoping_UsesHostForestECAAndKeepsCrossForestDomain models
+// shared ADCS across two forests: one computer hosts the CA service for an
+// EnterpriseCA in its own forest and for a copied EnterpriseCA in another forest.
+// Only the host-forest EnterpriseCA should be retained, and it should still chain
+// to every domain reached by RootCAFor and TrustedForNTAuth.
+func TestADCSForestScoping_UsesHostForestECAAndKeepsCrossForestDomain(t *testing.T) {
+	testContext := integration.NewGraphTestContext(t, graphschema.DefaultGraphSchema())
+
+	var (
+		domainASID = integration.RandomDomainSID()
+		domainBSID = integration.RandomDomainSID()
+
+		hostForestEnterpriseCAID graph.ID
+		copiedEnterpriseCAID     graph.ID
+		domainAID                graph.ID
+		domainBID                graph.ID
+	)
+
+	testContext.DatabaseTestWithSetup(
+		func(harness *integration.HarnessDetails) error {
+			domainA := testContext.NewActiveDirectoryDomain("ForestA-Domain", domainASID, false, true)
+			domainB := testContext.NewActiveDirectoryDomain("ForestB-Domain", domainBSID, false, true)
+
+			// The real CA lives in forest A; a copied EnterpriseCA object exists in
+			// forest B and points at the same hosting computer.
+			hostForestEnterpriseCA := testContext.NewActiveDirectoryEnterpriseCA("SharedECA", domainASID)
+			copiedEnterpriseCA := testContext.NewActiveDirectoryEnterpriseCA("SharedECA-Copy", domainBSID)
+			rootCA := testContext.NewActiveDirectoryRootCA("SharedRootCA", domainASID)
+
+			testContext.NewRelationship(hostForestEnterpriseCA, rootCA, ad.EnterpriseCAFor)
+
+			// Valid cert chain to forest A (the CA's own forest)...
+			linkEnterpriseCAToDomain(testContext, hostForestEnterpriseCA, rootCA, domainA, domainASID)
+			// ...and a cross-forest chain into forest B (shared ADCS).
+			linkEnterpriseCAToDomain(testContext, hostForestEnterpriseCA, rootCA, domainB, domainBSID)
+
+			host := addEnabledHostingComputer(testContext, "HostA", domainASID, hostForestEnterpriseCA)
+			testContext.NewRelationship(host, copiedEnterpriseCA, ad.HostsCAService)
+
+			hostForestEnterpriseCAID = hostForestEnterpriseCA.ID
+			copiedEnterpriseCAID = copiedEnterpriseCA.ID
+			domainAID = domainA.ID
+			domainBID = domainB.ID
+			return nil
+		},
+		func(harness integration.HarnessDetails, db graph.Database) {
+			_, cache, err := FetchADCSPrereqs(db)
+			require.NoError(t, err)
+
+			chainedDomains := cache.GetECAHostedChainedDomains()
+
+			require.Contains(t, chainedDomains, hostForestEnterpriseCAID.Uint64(), "CA with an in-forest host should be retained")
+			assert.NotContains(t, chainedDomains, copiedEnterpriseCAID.Uint64(), "copied CA with only a cross-forest host should be skipped")
+			chains := chainedDomains[hostForestEnterpriseCAID.Uint64()]
+			assert.True(t, chains.Domains.Contains(domainAID.Uint64()), "in-forest domain should survive")
+			assert.True(t, chains.Domains.Contains(domainBID.Uint64()), "cross-forest chained domain should survive")
+		},
+	)
+}
+
+// TestADCSForestScoping_DropsCAWithOnlyCrossForestHost models a CA whose only
+// HostsCAService computer was matched across a forest boundary. With no hosting
+// computer in the CA's own forest, the CA should be dropped entirely.
+func TestADCSForestScoping_DropsCAWithOnlyCrossForestHost(t *testing.T) {
+	testContext := integration.NewGraphTestContext(t, graphschema.DefaultGraphSchema())
+
+	var (
+		domainASID = integration.RandomDomainSID()
+		domainBSID = integration.RandomDomainSID()
+
+		enterpriseCAID graph.ID
+	)
+
+	testContext.DatabaseTestWithSetup(
+		func(harness *integration.HarnessDetails) error {
+			domainA := testContext.NewActiveDirectoryDomain("ForestA-Domain", domainASID, false, true)
+			domainB := testContext.NewActiveDirectoryDomain("ForestB-Domain", domainBSID, false, true)
+
+			enterpriseCA := testContext.NewActiveDirectoryEnterpriseCA("SharedECA", domainASID)
+			rootCA := testContext.NewActiveDirectoryRootCA("SharedRootCA", domainASID)
+
+			// The CA chains up to its root once; the per-domain edges are added below.
+			testContext.NewRelationship(enterpriseCA, rootCA, ad.EnterpriseCAFor)
+
+			linkEnterpriseCAToDomain(testContext, enterpriseCA, rootCA, domainA, domainASID)
+			linkEnterpriseCAToDomain(testContext, enterpriseCA, rootCA, domainB, domainBSID)
+
+			// Only hosting computer lives in forest B (cross-forest from the CA).
+			addEnabledHostingComputer(testContext, "HostB", domainBSID, enterpriseCA)
+
+			enterpriseCAID = enterpriseCA.ID
+			return nil
+		},
+		func(harness integration.HarnessDetails, db graph.Database) {
+			_, cache, err := FetchADCSPrereqs(db)
+			require.NoError(t, err)
+
+			chainedDomains := cache.GetECAHostedChainedDomains()
+
+			assert.NotContains(t, chainedDomains, enterpriseCAID.Uint64(), "CA with no in-forest hosting computer should be skipped")
+		},
+	)
+}

packages/go/analysis/ad/adcscache.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"log/slog"
+	"strings"
 	"sync"
 
 	"github.com/specterops/bloodhound/packages/go/analysis/ad/wellknown"
@@ -138,6 +139,7 @@ type ADCSCache struct {
 	authStoreForChainValid          map[graph.ID]cardinality.Duplex[uint64] //Auth stores with a valid chain to the domain, key is domain ID
 	rootCAForChainValid             map[graph.ID]cardinality.Duplex[uint64] //Root CA with a valid chain to the domain, key is domain ID
 	hasHostingComputer              map[graph.ID]bool
+	hasInForestHostingComputer      map[graph.ID]bool // enterprise CA ID -> whether the CA has an enabled hosting computer inside its own forest
 
 	// ESC4-specific caches: principals with specific rights on cert templates, pre-computed to avoid per-ECA DB queries
 	certTemplateGenericWriters              map[graph.ID]CachedPrincipalSet         // principals with GenericWrite on a cert template
@@ -158,6 +160,7 @@ func NewADCSCache() *ADCSCache {
 		authStoreForChainValid:                  make(map[graph.ID]cardinality.Duplex[uint64]),
 		rootCAForChainValid:                     make(map[graph.ID]cardinality.Duplex[uint64]),
 		hasHostingComputer:                      make(map[graph.ID]bool),
+		hasInForestHostingComputer:              make(map[graph.ID]bool),
 		certTemplateEnrollers:                   make(map[graph.ID]CachedPrincipalSet),
 		certTemplateControllers:                 make(map[graph.ID]CachedPrincipalSet),
 		enterpriseCAEnrollers:                   make(map[graph.ID]CachedPrincipalSet),
@@ -275,6 +278,15 @@ func (s *ADCSCache) BuildCache(ctx context.Context, db graph.Database, enterpris
 
 		certTemplateMeasure()
 
+		// Index domains by SID so a CA or computer can be mapped to its forest via
+		// its domainsid. SIDs are upper-cased to match collected node identifiers.
+		domainsBySID := make(map[string]*graph.Node, len(s.domains))
+		for _, domain := range s.domains {
+			if sid, err := domain.Properties.Get(common.ObjectID.String()).String(); err == nil && sid != "" {
+				domainsBySID[strings.ToUpper(sid)] = domain
+			}
+		}
+
 		ecaMeasure := measure.ContextMeasure(
 			ctx,
 			slog.LevelInfo,
@@ -327,18 +339,55 @@ func (s *ADCSCache) BuildCache(ctx context.Context, db graph.Database, enterpris
 					attr.Error(err),
 				)
 			} else {
-				hasHostingComputer := false
+				// Resolve the CA's own forest; fall back to forest-agnostic behavior
+				// when it can't be determined.
+				forestDomains, err := resolveEnterpriseCAForest(tx, eca, domainsBySID)
+				if err != nil {
+					slog.WarnContext(
+						ctx,
+						"Error resolving forest for enterprise ca",
+						slog.Uint64("enterprise_ca", uint64(eca.ID)),
+						attr.Error(err),
+					)
+				}
+
+				var (
+					hasHostingComputer = false
+					hostInForest       = false
+				)
 
 				for _, computer := range hostingComputers.Slice() {
 					if enabled, err := computer.Properties.Get(common.Enabled.String()).Bool(); err != nil {
 						continue
-					} else if enabled {
-						hasHostingComputer = true
-						break
+					} else if !enabled {
+						continue
+					}
+
+					hasHostingComputer = true
+
+					// Only count a host that lives in the CA's forest; a shared CA can
+					// be linked to a computer in another forest.
+					if forestDomains != nil {
+						if computerSID, err := computer.Properties.Get(ad.DomainSID.String()).String(); err != nil || computerSID == "" {
+							// Without a domainsid we can't place the host in a forest, so it
+							// won't count as in-forest. Log it: if this is the CA's only host
+							// the CA is dropped, and the silent skip would be hard to diagnose.
+							slog.WarnContext(
+								ctx,
+								"Hosting computer is missing a domainsid; cannot determine whether it is in the CA's forest",
+								slog.Uint64("computer", uint64(computer.ID)),
+								slog.Uint64("enterprise_ca", uint64(eca.ID)),
+							)
+						} else if computerDomain, ok := domainsBySID[strings.ToUpper(computerSID)]; ok && forestDomains.Contains(computerDomain.ID.Uint64()) {
+							hostInForest = true
+						}
 					}
 				}
-				s.hasHostingComputer[eca.ID] = hasHostingComputer
 
+				s.hasHostingComputer[eca.ID] = hasHostingComputer
+				if forestDomains != nil {
+					s.hasInForestHostingComputer[eca.ID] = hostInForest
+				}
 			}
 		}
 
@@ -455,6 +504,34 @@ func (s *ADCSCache) BuildCache(ctx context.Context, db graph.Database, enterpris
 	return err
 }
 
+// resolveEnterpriseCAForest returns the domain IDs in the CA's forest (the
+// SameForestTrust closure of the CA's domain, resolved from its domainsid). It
+// returns a nil set when the forest can't be resolved, so callers fall back to
+// forest-agnostic behavior rather than dropping the CA.
+func resolveEnterpriseCAForest(tx graph.Transaction, eca *graph.Node, domainsBySID map[string]*graph.Node) (cardinality.Duplex[uint64], error) {
+	domainSID, err := eca.Properties.Get(ad.DomainSID.String()).String()
+	if err != nil || domainSID == "" {
+		return nil, nil
+	}
+
+	caDomain, ok := domainsBySID[strings.ToUpper(domainSID)]
+	if !ok {
+		return nil, nil
+	}
+
+	forestNodes, err := FetchNodesWithSameForestTrustRelationship(tx, caDomain)
+	if err != nil {
+		return nil, err
+	}
+
+	forestDomains := graph.NodeSetToDuplex(forestNodes)
+	// Always include the CA's own domain (the closure is just the seed when there
+	// are no SameForestTrust edges).
+	forestDomains.Add(caDomain.ID.Uint64())
+
+	return forestDomains, nil
+}
+
 func (s *ADCSCache) GetECAHostedChainedDomains() map[uint64]*EnterpriseCAChainedDomains {
 	s.mutex.RLock()
 	defer s.mutex.RUnlock()
@@ -464,15 +541,21 @@ func (s *ADCSCache) GetECAHostedChainedDomains() map[uint64]*EnterpriseCAChained
 	for _, enterpriseCA := range s.enterpriseCertAuthorities {
 		innerEnterpriseCA := enterpriseCA
 
+		// Require an enabled hosting computer; when the forest is known, require it
+		// in-forest. Drops CAs whose only host was matched across a forest boundary.
+		if hasInForestHostingComputer, forestKnown := s.hasInForestHostingComputer[innerEnterpriseCA.ID]; forestKnown {
+			if !hasInForestHostingComputer {
+				continue
+			}
+		} else if !s.hasHostingComputer[innerEnterpriseCA.ID] {
+			continue
+		}
+
 		targetDomains := NewEnterpriseCAChainedDomains(enterpriseCA)
 		for _, domain := range s.domains {
 			innerDomain := domain
 
-			if hasHost, ok := s.hasHostingComputer[innerEnterpriseCA.ID]; !ok {
-				continue
-			} else if !hasHost {
-				continue
-			} else if _, ok := s.rootCAForChainValid[innerDomain.ID]; !ok {
+			if _, ok := s.rootCAForChainValid[innerDomain.ID]; !ok {
 				continue
 			} else if _, ok := s.authStoreForChainValid[innerDomain.ID]; !ok {
 				continue