From 19841db45e60f306d233bbf53f2baf1f843c0126 Mon Sep 17 00:00:00 2001 From: Rohan Vazarkar Date: Mon, 5 Feb 2024 17:26:56 -0500 Subject: [PATCH] Remove ADCS feature flag gates (#388) * chore: gate adcs feature flag to b edges, remove elsewhere, make flag non user-updateable * chore: fix tests * chore: delete commented code --- cmd/api/src/api/registration/v2.go | 23 +++--- .../api/v2/file_uploads_integration_test.go | 1 - cmd/api/src/daemons/datapipe/convertors.go | 6 +- cmd/api/src/daemons/datapipe/ingest.go | 80 +++++++------------ cmd/api/src/model/appcfg/flag.go | 2 +- .../test/integration/harnesses/harnessgen.py | 2 +- packages/go/analysis/ad/adcs.go | 50 ++++++------ 7 files changed, 72 insertions(+), 92 deletions(-) diff --git a/cmd/api/src/api/registration/v2.go b/cmd/api/src/api/registration/v2.go index 64b7820909..70761dfdb9 100644 --- a/cmd/api/src/api/registration/v2.go +++ b/cmd/api/src/api/registration/v2.go @@ -30,7 +30,6 @@ import ( "github.com/specterops/bloodhound/src/auth" "github.com/specterops/bloodhound/src/config" "github.com/specterops/bloodhound/src/database" - "github.com/specterops/bloodhound/src/model/appcfg" ) func samlWriteAPIErrorResponse(request *http.Request, response http.ResponseWriter, statusCode int, message string) { @@ -150,7 +149,7 @@ func NewV2API(cfg config.Configuration, resources v2.Resources, routerInst *rout routerInst.GET("/api/v2/pathfinding", resources.GetPathfindingResult).Queries("start_node", "{start_node}", "end_node", "{end_node}").RequirePermissions(permissions.GraphDBRead), routerInst.GET("/api/v2/graphs/shortest-path", resources.GetShortestPath).Queries(params.StartNode.String(), params.StartNode.RouteMatcher(), params.EndNode.String(), params.EndNode.RouteMatcher()).RequirePermissions(permissions.GraphDBRead), - routerInst.GET("/api/v2/graphs/edge-composition", resources.GetEdgeComposition).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. + routerInst.GET("/api/v2/graphs/edge-composition", resources.GetEdgeComposition).RequirePermissions(permissions.GraphDBRead), // TODO discuss if this should be a post endpoint routerInst.GET("/api/v2/graph-search", resources.GetSearchResult).RequirePermissions(permissions.GraphDBRead), @@ -218,24 +217,24 @@ func NewV2API(cfg config.Configuration, resources v2.Resources, routerInst *rout routerInst.GET(fmt.Sprintf("/api/v2/gpos/{%s}/ous", api.URIPathVariableObjectID), resources.ListADGPOAffectedContainers).RequirePermissions(permissions.GraphDBRead), // AIACA Entity API - routerInst.GET(fmt.Sprintf("/api/v2/aiacas/{%s}", api.URIPathVariableObjectID), resources.GetAIACAEntityInfo).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. - routerInst.GET(fmt.Sprintf("/api/v2/aiacas/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. + routerInst.GET(fmt.Sprintf("/api/v2/aiacas/{%s}", api.URIPathVariableObjectID), resources.GetAIACAEntityInfo).RequirePermissions(permissions.GraphDBRead), + routerInst.GET(fmt.Sprintf("/api/v2/aiacas/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead), // RootCA Entity API - routerInst.GET(fmt.Sprintf("/api/v2/rootcas/{%s}", api.URIPathVariableObjectID), resources.GetRootCAEntityInfo).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. - routerInst.GET(fmt.Sprintf("/api/v2/rootcas/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. + routerInst.GET(fmt.Sprintf("/api/v2/rootcas/{%s}", api.URIPathVariableObjectID), resources.GetRootCAEntityInfo).RequirePermissions(permissions.GraphDBRead), + routerInst.GET(fmt.Sprintf("/api/v2/rootcas/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead), // EnterpriseCA Entity API - routerInst.GET(fmt.Sprintf("/api/v2/enterprisecas/{%s}", api.URIPathVariableObjectID), resources.GetEnterpriseCAEntityInfo).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. - routerInst.GET(fmt.Sprintf("/api/v2/enterprisecas/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. + routerInst.GET(fmt.Sprintf("/api/v2/enterprisecas/{%s}", api.URIPathVariableObjectID), resources.GetEnterpriseCAEntityInfo).RequirePermissions(permissions.GraphDBRead), + routerInst.GET(fmt.Sprintf("/api/v2/enterprisecas/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead), // NTAuthStore Entity API - routerInst.GET(fmt.Sprintf("/api/v2/ntauthstores/{%s}", api.URIPathVariableObjectID), resources.GetNTAuthStoreEntityInfo).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. - routerInst.GET(fmt.Sprintf("/api/v2/ntauthstores/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. + routerInst.GET(fmt.Sprintf("/api/v2/ntauthstores/{%s}", api.URIPathVariableObjectID), resources.GetNTAuthStoreEntityInfo).RequirePermissions(permissions.GraphDBRead), + routerInst.GET(fmt.Sprintf("/api/v2/ntauthstores/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead), // CertTemplate Entity API - routerInst.GET(fmt.Sprintf("/api/v2/certtemplates/{%s}", api.URIPathVariableObjectID), resources.GetCertTemplateEntityInfo).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. - routerInst.GET(fmt.Sprintf("/api/v2/certtemplates/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead).CheckFeatureFlag(resources.DB, appcfg.FeatureAdcs), // TODO: Cleanup #ADCSFeatureFlag after full launch. + routerInst.GET(fmt.Sprintf("/api/v2/certtemplates/{%s}", api.URIPathVariableObjectID), resources.GetCertTemplateEntityInfo).RequirePermissions(permissions.GraphDBRead), + routerInst.GET(fmt.Sprintf("/api/v2/certtemplates/{%s}/controllers", api.URIPathVariableObjectID), resources.ListADEntityControllers).RequirePermissions(permissions.GraphDBRead), // OU Entity API routerInst.GET(fmt.Sprintf("/api/v2/ous/{%s}", api.URIPathVariableObjectID), resources.GetOUEntityInfo).RequirePermissions(permissions.GraphDBRead), diff --git a/cmd/api/src/api/v2/file_uploads_integration_test.go b/cmd/api/src/api/v2/file_uploads_integration_test.go index 712b8a69a2..6400f09844 100644 --- a/cmd/api/src/api/v2/file_uploads_integration_test.go +++ b/cmd/api/src/api/v2/file_uploads_integration_test.go @@ -173,7 +173,6 @@ func Test_FileUploadWorkFlowVersion6(t *testing.T) { func Test_FileUploadVersion6AllOptionADCS(t *testing.T) { testCtx := integration.NewFOSSContext(t) - testCtx.ToggleFeatureFlag("adcs") testCtx.SendFileIngest([]string{ "v6/all/aiacas.json", diff --git a/cmd/api/src/daemons/datapipe/convertors.go b/cmd/api/src/daemons/datapipe/convertors.go index 1ebf98a701..12423c3c36 100644 --- a/cmd/api/src/daemons/datapipe/convertors.go +++ b/cmd/api/src/daemons/datapipe/convertors.go @@ -21,7 +21,7 @@ import ( "github.com/specterops/bloodhound/graphschema/ad" ) -func convertComputerData(data []ein.Computer, adcsEnabled bool) ConvertedData { +func convertComputerData(data []ein.Computer) ConvertedData { converted := ConvertedData{} for _, computer := range data { @@ -57,9 +57,7 @@ func convertComputerData(data []ein.Computer, adcsEnabled bool) ConvertedData { } } - if adcsEnabled { - converted.NodeProps = append(converted.NodeProps, ein.ParseDCRegistryData(computer)) - } + converted.NodeProps = append(converted.NodeProps, ein.ParseDCRegistryData(computer)) converted.NodeProps = append(converted.NodeProps, baseNodeProp) } diff --git a/cmd/api/src/daemons/datapipe/ingest.go b/cmd/api/src/daemons/datapipe/ingest.go index fe80784502..2e2b80cb73 100644 --- a/cmd/api/src/daemons/datapipe/ingest.go +++ b/cmd/api/src/daemons/datapipe/ingest.go @@ -18,7 +18,6 @@ package datapipe import ( "encoding/json" - "fmt" "io" "strings" "time" @@ -29,7 +28,6 @@ import ( "github.com/specterops/bloodhound/graphschema/azure" "github.com/specterops/bloodhound/graphschema/common" "github.com/specterops/bloodhound/log" - "github.com/specterops/bloodhound/src/model/appcfg" ) func (s *Daemon) ReadWrapper(batch graph.Batch, reader io.Reader) error { @@ -60,12 +58,6 @@ func (s *Daemon) IngestAzureData(batch graph.Batch, converted ConvertedAzureData } func (s *Daemon) IngestWrapper(batch graph.Batch, wrapper DataWrapper) error { - // TODO: Cleanup #ADCSFeatureFlag after full launch. - adcsFlag, err := s.db.GetFlagByKey(appcfg.FeatureAdcs) - if err != nil { - return fmt.Errorf("error getting ADCS feature flag: %w", err) - } - switch wrapper.Metadata.Type { case DataTypeComputer: // We should not be getting anything with Version < 5 at this point, and we don't want to ingest it if we do as post-processing will blow it away anyways @@ -75,7 +67,7 @@ func (s *Daemon) IngestWrapper(batch graph.Batch, wrapper DataWrapper) error { if err := json.Unmarshal(wrapper.Payload, &computerData); err != nil { return err } else { - converted := convertComputerData(computerData, adcsFlag.Enabled) + converted := convertComputerData(computerData) s.IngestBasicData(batch, converted) } } @@ -143,58 +135,48 @@ func (s *Daemon) IngestWrapper(batch graph.Batch, wrapper DataWrapper) error { } case DataTypeAIACA: - if adcsFlag.Enabled { - var aiacaData []ein.AIACA - if err := json.Unmarshal(wrapper.Payload, &aiacaData); err != nil { - return err - } else { - converted := convertAIACAData(aiacaData) - s.IngestBasicData(batch, converted) - } + var aiacaData []ein.AIACA + if err := json.Unmarshal(wrapper.Payload, &aiacaData); err != nil { + return err + } else { + converted := convertAIACAData(aiacaData) + s.IngestBasicData(batch, converted) } case DataTypeRootCA: - if adcsFlag.Enabled { - var rootcaData []ein.RootCA - if err := json.Unmarshal(wrapper.Payload, &rootcaData); err != nil { - return err - } else { - converted := convertRootCAData(rootcaData) - s.IngestBasicData(batch, converted) - } + var rootcaData []ein.RootCA + if err := json.Unmarshal(wrapper.Payload, &rootcaData); err != nil { + return err + } else { + converted := convertRootCAData(rootcaData) + s.IngestBasicData(batch, converted) } case DataTypeEnterpriseCA: - if adcsFlag.Enabled { - var enterprisecaData []ein.EnterpriseCA - if err := json.Unmarshal(wrapper.Payload, &enterprisecaData); err != nil { - return err - } else { - converted := convertEnterpriseCAData(enterprisecaData) - s.IngestBasicData(batch, converted) - } + var enterprisecaData []ein.EnterpriseCA + if err := json.Unmarshal(wrapper.Payload, &enterprisecaData); err != nil { + return err + } else { + converted := convertEnterpriseCAData(enterprisecaData) + s.IngestBasicData(batch, converted) } case DataTypeNTAuthStore: - if adcsFlag.Enabled { - var ntauthstoreData []ein.NTAuthStore - if err := json.Unmarshal(wrapper.Payload, &ntauthstoreData); err != nil { - return err - } else { - converted := convertNTAuthStoreData(ntauthstoreData) - s.IngestBasicData(batch, converted) - } + var ntauthstoreData []ein.NTAuthStore + if err := json.Unmarshal(wrapper.Payload, &ntauthstoreData); err != nil { + return err + } else { + converted := convertNTAuthStoreData(ntauthstoreData) + s.IngestBasicData(batch, converted) } case DataTypeCertTemplate: - if adcsFlag.Enabled { - var certtemplateData []ein.CertTemplate - if err := json.Unmarshal(wrapper.Payload, &certtemplateData); err != nil { - return err - } else { - converted := convertCertTemplateData(certtemplateData) - s.IngestBasicData(batch, converted) - } + var certtemplateData []ein.CertTemplate + if err := json.Unmarshal(wrapper.Payload, &certtemplateData); err != nil { + return err + } else { + converted := convertCertTemplateData(certtemplateData) + s.IngestBasicData(batch, converted) } case DataTypeAzure: diff --git a/cmd/api/src/model/appcfg/flag.go b/cmd/api/src/model/appcfg/flag.go index e828fadbc0..8d7369b336 100644 --- a/cmd/api/src/model/appcfg/flag.go +++ b/cmd/api/src/model/appcfg/flag.go @@ -79,7 +79,7 @@ func AvailableFlags() FeatureFlagSet { Name: "Enable collection and processing of Active Directory Certificate Services Data", Description: "Enables the ability to collect, analyze, and explore Active Directory Certificate Services data and previews new attack paths.", Enabled: false, - UserUpdatable: true, + UserUpdatable: false, }, } } diff --git a/cmd/api/src/test/integration/harnesses/harnessgen.py b/cmd/api/src/test/integration/harnesses/harnessgen.py index 83699e7a5e..5266437a37 100644 --- a/cmd/api/src/test/integration/harnesses/harnessgen.py +++ b/cmd/api/src/test/integration/harnesses/harnessgen.py @@ -156,7 +156,7 @@ def create_statement(self): nodes[id] = UserNode(name) elif 'Group' in name: nodes[id] = GroupNode(name) - elif 'Computer' in name: + elif 'Computer' or 'DC' in name: nodes[id] = ComputerNode(name) elif 'OU' in name: nodes[id] = OUNode(name) diff --git a/packages/go/analysis/ad/adcs.go b/packages/go/analysis/ad/adcs.go index e21807c529..90031494c4 100644 --- a/packages/go/analysis/ad/adcs.go +++ b/packages/go/analysis/ad/adcs.go @@ -35,10 +35,6 @@ var ( ) func PostADCS(ctx context.Context, db graph.Database, groupExpansions impact.PathAggregator, adcsEnabled bool) (*analysis.AtomicPostProcessingStats, error) { - if !adcsEnabled { - return &analysis.AtomicPostProcessingStats{}, nil - } - if enterpriseCertAuthorities, err := FetchNodesByKind(ctx, db, ad.EnterpriseCA); err != nil { return &analysis.AtomicPostProcessingStats{}, fmt.Errorf("failed fetching enterpriseCA nodes: %w", err) } else if rootCertAuthorities, err := FetchNodesByKind(ctx, db, ad.RootCA); err != nil { @@ -67,7 +63,7 @@ func PostADCS(ctx context.Context, db graph.Database, groupExpansions impact.Pat innerEnterpriseCA := enterpriseCA if cache.DoesCAChainProperlyToDomain(innerEnterpriseCA, innerDomain) { - processEnterpriseCAWithValidCertChainToDomain(innerEnterpriseCA, innerDomain, groupExpansions, cache, operation) + processEnterpriseCAWithValidCertChainToDomain(innerEnterpriseCA, innerDomain, groupExpansions, cache, operation, adcsEnabled) } } } @@ -113,7 +109,7 @@ func postADCSPreProcessStep2(ctx context.Context, db graph.Database, certTemplat } } -func processEnterpriseCAWithValidCertChainToDomain(enterpriseCA, domain *graph.Node, groupExpansions impact.PathAggregator, cache ADCSCache, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob]) { +func processEnterpriseCAWithValidCertChainToDomain(enterpriseCA, domain *graph.Node, groupExpansions impact.PathAggregator, cache ADCSCache, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], adcsEnabled bool) { operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { if err := PostGoldenCert(ctx, tx, outC, domain, enterpriseCA); err != nil { @@ -143,12 +139,14 @@ func processEnterpriseCAWithValidCertChainToDomain(enterpriseCA, domain *graph.N return nil }) - operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC6b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { - log.Errorf("failed post processing for %s: %v", ad.ADCSESC6a.String(), err) - } - return nil - }) + if adcsEnabled { + operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { + if err := PostADCSESC6b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + log.Errorf("failed post processing for %s: %v", ad.ADCSESC6a.String(), err) + } + return nil + }) + } operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { if err := PostADCSESC9a(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { @@ -157,12 +155,14 @@ func processEnterpriseCAWithValidCertChainToDomain(enterpriseCA, domain *graph.N return nil }) - operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC9b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { - log.Errorf("failed post processing for %s: %v", ad.ADCSESC9a.String(), err) - } - return nil - }) + if adcsEnabled { + operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { + if err := PostADCSESC9b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + log.Errorf("failed post processing for %s: %v", ad.ADCSESC9a.String(), err) + } + return nil + }) + } operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { if err := PostADCSESC10a(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { @@ -171,10 +171,12 @@ func processEnterpriseCAWithValidCertChainToDomain(enterpriseCA, domain *graph.N return nil }) - operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC10b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { - log.Errorf("failed post processing for %s: %v", ad.ADCSESC10b.String(), err) - } - return nil - }) + if adcsEnabled { + operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { + if err := PostADCSESC10b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + log.Errorf("failed post processing for %s: %v", ad.ADCSESC10b.String(), err) + } + return nil + }) + } }