From ab521ab4fd1eec3b8485c42dc3479d0f1ad98ece Mon Sep 17 00:00:00 2001 From: stephanieslamb Date: Wed, 25 Sep 2024 09:16:54 -0500 Subject: [PATCH] Bed 4752 (#873) * chore: updated log errors to warns * missed comment * ran prepare-for-codereview * updates from peer review * updated file to use errors package instead of bloodhounf/errors * reran just prepare for code review * prepare for review --- cmd/api/src/api/saml/saml.go | 3 +- cmd/api/src/api/v2/audit_test.go | 208 +++++++++--------- cmd/api/src/api/v2/helpers.go | 18 +- cmd/api/src/bootstrap/initializer.go | 2 +- cmd/api/src/cmd/bhapi/main.go | 6 +- .../src/daemons/datapipe/azure_convertors.go | 9 +- cmd/api/src/daemons/datapipe/jobs.go | 10 +- cmd/api/src/queries/graph.go | 10 +- packages/go/analysis/ad/adcs.go | 44 +++- packages/go/analysis/ad/esc13.go | 7 +- packages/go/analysis/ad/esc3.go | 27 ++- packages/go/analysis/azure/filters.go | 3 +- packages/go/analysis/azure/post.go | 2 +- packages/go/ein/azure.go | 19 +- packages/go/graphschema/ad/ad.go | 1 + packages/go/graphschema/azure/azure.go | 1 + packages/go/graphschema/common/common.go | 1 + 17 files changed, 217 insertions(+), 154 deletions(-) diff --git a/cmd/api/src/api/saml/saml.go b/cmd/api/src/api/saml/saml.go index ad43299d35..6fd8dc0878 100644 --- a/cmd/api/src/api/saml/saml.go +++ b/cmd/api/src/api/saml/saml.go @@ -200,8 +200,7 @@ func assertionFindString(assertion *saml.Assertion, names ...string) (string, er return value.Value, nil } } - - log.Errorf("[SAML] Found attribute values for attribute %s however none of the values have an XML type of %s. Choosing the first value.", bhsaml.ObjectIDAttributeNameFormat, bhsaml.XMLTypeString) + log.Warnf("[SAML] Found attribute values for attribute %s however none of the values have an XML type of %s. Choosing the first value.", bhsaml.ObjectIDAttributeNameFormat, bhsaml.XMLTypeString) return attribute.Values[0].Value, nil } } diff --git a/cmd/api/src/api/v2/audit_test.go b/cmd/api/src/api/v2/audit_test.go index bb060835f9..58eb0051b9 100644 --- a/cmd/api/src/api/v2/audit_test.go +++ b/cmd/api/src/api/v2/audit_test.go @@ -243,122 +243,122 @@ func TestResources_ListAuditLogs_Filtered(t *testing.T) { } func TestResources_ListAuditLogs_SkipAndOffset(t *testing.T) { - var ( - mockCtrl = gomock.NewController(t) - mockDB = mocks.NewMockDatabase(mockCtrl) - resources = v2.Resources{DB: mockDB} - ) - defer mockCtrl.Finish() - - mockDB.EXPECT().ListAuditLogs(gomock.Any(), gomock.Any(), gomock.Any(), 10, gomock.Any(), "", model.SQLFilter{}).Return(model.AuditLogs{}, 1000, nil) - - endpoint := "/api/v2/audit" - - if req, err := http.NewRequest("GET", endpoint, nil); err != nil { - t.Fatal(err) - } else { - q := url.Values{} - q.Add("skip", "10") - q.Add("offset", "20") - - req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) - req.URL.RawQuery = q.Encode() - - router := mux.NewRouter() - router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") - - response := httptest.NewRecorder() - router.ServeHTTP(response, req) - require.Equal(t, http.StatusOK, response.Code) - } + var ( + mockCtrl = gomock.NewController(t) + mockDB = mocks.NewMockDatabase(mockCtrl) + resources = v2.Resources{DB: mockDB} + ) + defer mockCtrl.Finish() + + mockDB.EXPECT().ListAuditLogs(gomock.Any(), gomock.Any(), gomock.Any(), 10, gomock.Any(), "", model.SQLFilter{}).Return(model.AuditLogs{}, 1000, nil) + + endpoint := "/api/v2/audit" + + if req, err := http.NewRequest("GET", endpoint, nil); err != nil { + t.Fatal(err) + } else { + q := url.Values{} + q.Add("skip", "10") + q.Add("offset", "20") + + req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) + req.URL.RawQuery = q.Encode() + + router := mux.NewRouter() + router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") + + response := httptest.NewRecorder() + router.ServeHTTP(response, req) + require.Equal(t, http.StatusOK, response.Code) + } } func TestResources_ListAuditLogs_OnlyOffset(t *testing.T) { - var ( - mockCtrl = gomock.NewController(t) - mockDB = mocks.NewMockDatabase(mockCtrl) - resources = v2.Resources{DB: mockDB} - ) - defer mockCtrl.Finish() + var ( + mockCtrl = gomock.NewController(t) + mockDB = mocks.NewMockDatabase(mockCtrl) + resources = v2.Resources{DB: mockDB} + ) + defer mockCtrl.Finish() - mockDB.EXPECT().ListAuditLogs(gomock.Any(), gomock.Any(), gomock.Any(), 20, gomock.Any(), "", model.SQLFilter{}).Return(model.AuditLogs{}, 1000, nil) + mockDB.EXPECT().ListAuditLogs(gomock.Any(), gomock.Any(), gomock.Any(), 20, gomock.Any(), "", model.SQLFilter{}).Return(model.AuditLogs{}, 1000, nil) - endpoint := "/api/v2/audit" + endpoint := "/api/v2/audit" - if req, err := http.NewRequest("GET", endpoint, nil); err != nil { - t.Fatal(err) - } else { - q := url.Values{} - q.Add("offset", "20") + if req, err := http.NewRequest("GET", endpoint, nil); err != nil { + t.Fatal(err) + } else { + q := url.Values{} + q.Add("offset", "20") - req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) - req.URL.RawQuery = q.Encode() + req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) + req.URL.RawQuery = q.Encode() - router := mux.NewRouter() - router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") + router := mux.NewRouter() + router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") - response := httptest.NewRecorder() - router.ServeHTTP(response, req) - require.Equal(t, http.StatusOK, response.Code) - } + response := httptest.NewRecorder() + router.ServeHTTP(response, req) + require.Equal(t, http.StatusOK, response.Code) + } } func TestResources_ListAuditLogs_OnlySkip(t *testing.T) { - var ( - mockCtrl = gomock.NewController(t) - mockDB = mocks.NewMockDatabase(mockCtrl) - resources = v2.Resources{DB: mockDB} - ) - defer mockCtrl.Finish() - - // Expect skip to be 5 (from "skip" parameter) - mockDB.EXPECT().ListAuditLogs(gomock.Any(), gomock.Any(), gomock.Any(), 5, gomock.Any(), gomock.Any(), gomock.Any()).Return(model.AuditLogs{}, 1000, nil) - - endpoint := "/api/v2/audit" - - if req, err := http.NewRequest("GET", endpoint, nil); err != nil { - t.Fatal(err) - } else { - q := url.Values{} - q.Add("skip", "5") - - req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) - req.URL.RawQuery = q.Encode() - - router := mux.NewRouter() - router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") - - response := httptest.NewRecorder() - router.ServeHTTP(response, req) - require.Equal(t, http.StatusOK, response.Code) - } + var ( + mockCtrl = gomock.NewController(t) + mockDB = mocks.NewMockDatabase(mockCtrl) + resources = v2.Resources{DB: mockDB} + ) + defer mockCtrl.Finish() + + // Expect skip to be 5 (from "skip" parameter) + mockDB.EXPECT().ListAuditLogs(gomock.Any(), gomock.Any(), gomock.Any(), 5, gomock.Any(), gomock.Any(), gomock.Any()).Return(model.AuditLogs{}, 1000, nil) + + endpoint := "/api/v2/audit" + + if req, err := http.NewRequest("GET", endpoint, nil); err != nil { + t.Fatal(err) + } else { + q := url.Values{} + q.Add("skip", "5") + + req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) + req.URL.RawQuery = q.Encode() + + router := mux.NewRouter() + router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") + + response := httptest.NewRecorder() + router.ServeHTTP(response, req) + require.Equal(t, http.StatusOK, response.Code) + } } func TestResources_ListAuditLogs_InvalidSkip(t *testing.T) { - var ( - mockCtrl = gomock.NewController(t) - mockDB = mocks.NewMockDatabase(mockCtrl) - resources = v2.Resources{DB: mockDB} - ) - defer mockCtrl.Finish() - - endpoint := "/api/v2/audit" - - if req, err := http.NewRequest("GET", endpoint, nil); err != nil { - t.Fatal(err) - } else { - q := url.Values{} - q.Add("skip", "invalid") - - req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) - req.URL.RawQuery = q.Encode() - - router := mux.NewRouter() - router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") - - response := httptest.NewRecorder() - router.ServeHTTP(response, req) - require.Equal(t, http.StatusBadRequest, response.Code) - require.Contains(t, response.Body.String(), "query parameter \\\"skip\\\" is malformed") - } -} \ No newline at end of file + var ( + mockCtrl = gomock.NewController(t) + mockDB = mocks.NewMockDatabase(mockCtrl) + resources = v2.Resources{DB: mockDB} + ) + defer mockCtrl.Finish() + + endpoint := "/api/v2/audit" + + if req, err := http.NewRequest("GET", endpoint, nil); err != nil { + t.Fatal(err) + } else { + q := url.Values{} + q.Add("skip", "invalid") + + req.Header.Set(headers.ContentType.String(), mediatypes.ApplicationJson.String()) + req.URL.RawQuery = q.Encode() + + router := mux.NewRouter() + router.HandleFunc(endpoint, resources.ListAuditLogs).Methods("GET") + + response := httptest.NewRecorder() + router.ServeHTTP(response, req) + require.Equal(t, http.StatusBadRequest, response.Code) + require.Contains(t, response.Body.String(), "query parameter \\\"skip\\\" is malformed") + } +} diff --git a/cmd/api/src/api/v2/helpers.go b/cmd/api/src/api/v2/helpers.go index bd5daec725..360f702b89 100644 --- a/cmd/api/src/api/v2/helpers.go +++ b/cmd/api/src/api/v2/helpers.go @@ -58,15 +58,15 @@ func ParseSkipQueryParameter(params url.Values, defaultValue int) (int, error) { } func ParseSkipQueryParameterWithKey(params url.Values, key string, defaultValue int) (int, error) { - if param := params.Get(key); param == "" { - return defaultValue, nil - } else if skip, err := strconv.Atoi(param); err != nil { - return 0, fmt.Errorf("error converting %s value %v to int: %v", key, param, err) - } else if skip < 0 { - return 0, fmt.Errorf(utils.ErrorInvalidSkip, skip) - } else { - return skip, nil - } + if param := params.Get(key); param == "" { + return defaultValue, nil + } else if skip, err := strconv.Atoi(param); err != nil { + return 0, fmt.Errorf("error converting %s value %v to int: %v", key, param, err) + } else if skip < 0 { + return 0, fmt.Errorf(utils.ErrorInvalidSkip, skip) + } else { + return skip, nil + } } func ParseLimitQueryParameter(params url.Values, defaultValue int) (int, error) { diff --git a/cmd/api/src/bootstrap/initializer.go b/cmd/api/src/bootstrap/initializer.go index bc3c8e94e9..47f669d3b2 100644 --- a/cmd/api/src/bootstrap/initializer.go +++ b/cmd/api/src/bootstrap/initializer.go @@ -39,7 +39,7 @@ type Initializer[DBType database.Database, GraphType graph.Database] struct { Configuration config.Configuration PreMigrationDaemons InitializerLogic[DBType, GraphType] Entrypoint InitializerLogic[DBType, GraphType] - DBConnector DatabaseConstructor[DBType, GraphType] + DBConnector DatabaseConstructor[DBType, GraphType] } func (s Initializer[DBType, GraphType]) Launch(parentCtx context.Context, handleSignals bool) error { diff --git a/cmd/api/src/cmd/bhapi/main.go b/cmd/api/src/cmd/bhapi/main.go index b69f7e4979..cd67a8e191 100644 --- a/cmd/api/src/cmd/bhapi/main.go +++ b/cmd/api/src/cmd/bhapi/main.go @@ -64,10 +64,10 @@ func main() { log.Fatalf("Unable to read configuration %s: %v", configFilePath, err) } else { initializer := bootstrap.Initializer[*database.BloodhoundDB, *graph.DatabaseSwitch]{ - Configuration: cfg, - DBConnector: services.ConnectDatabases, + Configuration: cfg, + DBConnector: services.ConnectDatabases, PreMigrationDaemons: services.PreMigrationDaemons, - Entrypoint: services.Entrypoint, + Entrypoint: services.Entrypoint, } if err := initializer.Launch(context.Background(), true); err != nil { diff --git a/cmd/api/src/daemons/datapipe/azure_convertors.go b/cmd/api/src/daemons/datapipe/azure_convertors.go index 5df3ec47e0..e84f60f8d0 100644 --- a/cmd/api/src/daemons/datapipe/azure_convertors.go +++ b/cmd/api/src/daemons/datapipe/azure_convertors.go @@ -18,6 +18,7 @@ package datapipe import ( "encoding/json" + "errors" "fmt" "strings" @@ -190,7 +191,9 @@ func convertAzureAppOwner(raw json.RawMessage, converted *ConvertedAzureData) { ) if err := json.Unmarshal(raw.Owner, &owner); err != nil { log.Errorf(SerialError, "app owner", err) - } else if ownerType, err := ein.ExtractTypeFromDirectoryObject(owner); err != nil { + } else if ownerType, err := ein.ExtractTypeFromDirectoryObject(owner); errors.Is(err, ein.InvalidTypeErr) { + log.Warnf(ExtractError, err) + } else if err != nil { log.Errorf(ExtractError, err) } else { converted.RelProps = append(converted.RelProps, ein.ConvertAzureOwnerToRel(owner, ownerType, azure.App, data.AppId)) @@ -235,7 +238,9 @@ func convertAzureDeviceOwner(raw json.RawMessage, converted *ConvertedAzureData) ) if err := json.Unmarshal(raw.Owner, &owner); err != nil { log.Errorf(SerialError, "device owner", err) - } else if ownerType, err := ein.ExtractTypeFromDirectoryObject(owner); err != nil { + } else if ownerType, err := ein.ExtractTypeFromDirectoryObject(owner); errors.Is(err, ein.InvalidTypeErr) { + log.Warnf(ExtractError, err) + } else if err != nil { log.Errorf(ExtractError, err) } else { converted.RelProps = append(converted.RelProps, ein.ConvertAzureOwnerToRel(owner, ownerType, azure.Device, data.DeviceId)) diff --git a/cmd/api/src/daemons/datapipe/jobs.go b/cmd/api/src/daemons/datapipe/jobs.go index 942e2a438f..a368b1d3a0 100644 --- a/cmd/api/src/daemons/datapipe/jobs.go +++ b/cmd/api/src/daemons/datapipe/jobs.go @@ -19,8 +19,10 @@ package datapipe import ( "archive/zip" "context" + "errors" "fmt" "io" + "io/fs" "os" "github.com/specterops/bloodhound/bomenc" @@ -218,7 +220,9 @@ func (s *Daemon) processIngestFile(ctx context.Context, path string, fileType mo if err := file.Close(); err != nil { log.Errorf("Error closing ingest file %s: %v", filePath, err) - } else if err := os.Remove(filePath); err != nil { + } else if err := os.Remove(filePath); errors.Is(err, fs.ErrNotExist) { + log.Warnf("Removing ingest file %s: %w", filePath, err) + } else if err != nil { log.Errorf("Error removing ingest file %s: %v", filePath, err) } } @@ -253,7 +257,9 @@ func (s *Daemon) processIngestTasks(ctx context.Context, ingestTasks model.Inges log.Errorf("Failed to fetch job for ingest task %d: %v", ingestTask.ID, err) } total, failed, err := s.processIngestFile(ctx, ingestTask.FileName, ingestTask.FileType) - if err != nil { + if errors.Is(err, fs.ErrNotExist) { + log.Warnf("Did not process ingest task %d with file %s: %v", ingestTask.ID, ingestTask.FileName, err) + } else if err != nil { log.Errorf("Failed processing ingest task %d with file %s: %v", ingestTask.ID, ingestTask.FileName, err) } diff --git a/cmd/api/src/queries/graph.go b/cmd/api/src/queries/graph.go index fc8b94cd43..f525ce747d 100644 --- a/cmd/api/src/queries/graph.go +++ b/cmd/api/src/queries/graph.go @@ -21,6 +21,7 @@ package queries import ( "bytes" "context" + "errors" "fmt" "net/http" "net/url" @@ -46,7 +47,6 @@ import ( "github.com/specterops/bloodhound/dawgs/graph" "github.com/specterops/bloodhound/dawgs/ops" "github.com/specterops/bloodhound/dawgs/query" - "github.com/specterops/bloodhound/errors" "github.com/specterops/bloodhound/graphschema/ad" "github.com/specterops/bloodhound/graphschema/azure" "github.com/specterops/bloodhound/graphschema/common" @@ -83,7 +83,7 @@ type EntityQueryParameters struct { func GetEntityObjectIDFromRequestPath(request *http.Request) (string, error) { if id, hasID := mux.Vars(request)["object_id"]; !hasID { - return "", errors.Error("no object ID found in request") + return "", errors.New("no object ID found in request") } else { return id, nil } @@ -494,7 +494,7 @@ func (s *GraphQuery) RawCypherQuery(ctx context.Context, pQuery PreparedQuery, i timeoutLog.Str("query cost", fmt.Sprintf("%d", pQuery.complexity.Weight)) timeoutLog.Msg("Neo4j timed out while executing cypher query") } else { - log.Errorf("RawCypherQuery failed: %v", err) + log.Warnf("RawCypherQuery failed: %v", err) } return graphResponse, err } @@ -636,7 +636,9 @@ func (s *GraphQuery) GetEntityCountResults(ctx context.Context, node *graph.Node go func(delegateKey string, delegate any) { defer waitGroup.Done() - if result, err := runEntityQuery(ctx, s.Graph, delegate, node, 0, 0); err != nil { + if result, err := runEntityQuery(ctx, s.Graph, delegate, node, 0, 0); errors.Is(err, graph.ErrContextTimedOut) { + log.Warnf("Running entity query for key %s: %v", delegateKey, err) + } else if err != nil { log.Errorf("Error running entity query for key %s: %v", delegateKey, err) data.Store(delegateKey, 0) } else { diff --git a/packages/go/analysis/ad/adcs.go b/packages/go/analysis/ad/adcs.go index 04f2dd0fca..75faf1d9a8 100644 --- a/packages/go/analysis/ad/adcs.go +++ b/packages/go/analysis/ad/adcs.go @@ -111,77 +111,99 @@ 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]) { 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 { + if err := PostGoldenCert(ctx, tx, outC, domain, enterpriseCA); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.GoldenCert.String(), err) } return nil }) operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC1(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC1(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC1.String(), err) } return nil }) operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC3(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC3(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC3.String(), err) } return nil }) operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC4(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC4(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC4.String(), err) } return nil }) operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC6a(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC6a(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if 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 := PostADCSESC6b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC6b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC6b.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 { + if err := PostADCSESC9a(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if 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 := PostADCSESC9b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC9b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC9b.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 { + if err := PostADCSESC10a(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC10a.String(), err) } 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 { + if err := PostADCSESC10b(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC10b.String(), err) } return nil }) operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { - if err := PostADCSESC13(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); err != nil { + if err := PostADCSESC13(ctx, tx, outC, groupExpansions, enterpriseCA, domain, cache); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Post processing for %s: %v", ad.GoldenCert.String(), err) + } else if err != nil { log.Errorf("Failed post processing for %s: %v", ad.ADCSESC13.String(), err) } return nil diff --git a/packages/go/analysis/ad/esc13.go b/packages/go/analysis/ad/esc13.go index aafab52202..cfd72994d7 100644 --- a/packages/go/analysis/ad/esc13.go +++ b/packages/go/analysis/ad/esc13.go @@ -18,6 +18,7 @@ package ad import ( "context" + "errors" "sync" "github.com/RoaringBitmap/roaring" @@ -42,8 +43,10 @@ func PostADCSESC13(ctx context.Context, tx graph.Transaction, outC chan<- analys } else { ecaEnrollers := cache.GetEnterpriseCAEnrollers(eca.ID) for _, template := range publishedCertTemplates { - if isValid, err := isCertTemplateValidForESC13(template); err != nil { - log.Errorf("Error checking esc13 cert template: %v", err) + if isValid, err := isCertTemplateValidForESC13(template); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Checking esc13 cert template PostADCSESC13: %v", err) + } else if err != nil { + log.Errorf("Error checking esc13 cert template PostADCSESC13: %v", err) } else if !isValid { continue } else if groupNodes, err := getCertTemplateGroupLinks(template, tx); err != nil { diff --git a/packages/go/analysis/ad/esc3.go b/packages/go/analysis/ad/esc3.go index 0a2a8f2313..626ad3744c 100644 --- a/packages/go/analysis/ad/esc3.go +++ b/packages/go/analysis/ad/esc3.go @@ -18,6 +18,7 @@ package ad import ( "context" + "errors" "fmt" "slices" "sync" @@ -146,7 +147,9 @@ func PostEnrollOnBehalfOf(certTemplates []*graph.Node, operation analysis.StatTr versionTwoTemplates := make([]*graph.Node, 0) for _, node := range certTemplates { - if version, err := node.Properties.Get(ad.SchemaVersion.String()).Float64(); err != nil { + if version, err := node.Properties.Get(ad.SchemaVersion.String()).Float64(); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Did not get schema version for cert template %d: %v", node.ID, err) + } else if err != nil { log.Errorf("Error getting schema version for cert template %d: %v", node.ID, err) } else { if version == 1 { @@ -193,11 +196,15 @@ func PostEnrollOnBehalfOf(certTemplates []*graph.Node, operation analysis.StatTr func EnrollOnBehalfOfVersionTwo(tx graph.Transaction, versionTwoCertTemplates, allCertTemplates []*graph.Node) ([]analysis.CreatePostRelationshipJob, error) { results := make([]analysis.CreatePostRelationshipJob, 0) for _, certTemplateOne := range allCertTemplates { - if hasBadEku, err := certTemplateHasEku(certTemplateOne, EkuAnyPurpose); err != nil { + if hasBadEku, err := certTemplateHasEku(certTemplateOne, EkuAnyPurpose); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Did not get EffectiveEKUs for cert template %d: %v", certTemplateOne.ID, err) + } else if err != nil { log.Errorf("Error getting EffectiveEKUs for cert template %d: %v", certTemplateOne.ID, err) } else if hasBadEku { continue - } else if hasEku, err := certTemplateHasEku(certTemplateOne, EkuCertRequestAgent); err != nil { + } else if hasEku, err := certTemplateHasEku(certTemplateOne, EkuCertRequestAgent); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Did not get EffectiveEKUs for cert template %d: %v", certTemplateOne.ID, err) + } else if err != nil { log.Errorf("Error getting EffectiveEKUs for cert template %d: %v", certTemplateOne.ID, err) } else if !hasEku { continue @@ -258,7 +265,9 @@ func EnrollOnBehalfOfVersionOne(tx graph.Transaction, versionOneCertTemplates [] for _, certTemplateOne := range allCertTemplates { //prefilter as much as we can first - if hasEku, err := certTemplateHasEkuOrAll(certTemplateOne, EkuCertRequestAgent, EkuAnyPurpose); err != nil { + if hasEku, err := certTemplateHasEkuOrAll(certTemplateOne, EkuCertRequestAgent, EkuAnyPurpose); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Error checking ekus for certtemplate %d: %v", certTemplateOne.ID, err) + } else if err != nil { log.Errorf("Error checking ekus for certtemplate %d: %v", certTemplateOne.ID, err) } else if !hasEku { continue @@ -311,12 +320,18 @@ func isStartCertTemplateValidESC3(template *graph.Node) bool { } func isEndCertTemplateValidESC3(template *graph.Node) bool { - if authEnabled, err := template.Properties.Get(ad.AuthenticationEnabled.String()).Bool(); err != nil { + if authEnabled, err := template.Properties.Get(ad.AuthenticationEnabled.String()).Bool(); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Did not getting authenabled for cert template %d: %v", template.ID, err) + return false + } else if err != nil { log.Errorf("Error getting authenabled for cert template %d: %v", template.ID, err) return false } else if !authEnabled { return false - } else if reqManagerApproval, err := template.Properties.Get(ad.RequiresManagerApproval.String()).Bool(); err != nil { + } else if reqManagerApproval, err := template.Properties.Get(ad.RequiresManagerApproval.String()).Bool(); errors.Is(err, graph.ErrPropertyNotFound) { + log.Warnf("Did not getting reqManagerApproval for cert template %d: %v", template.ID, err) + return false + } else if err != nil { log.Errorf("Error getting reqManagerApproval for cert template %d: %v", template.ID, err) return false } else if reqManagerApproval { diff --git a/packages/go/analysis/azure/filters.go b/packages/go/analysis/azure/filters.go index 4926397fd2..82d73a870c 100644 --- a/packages/go/analysis/azure/filters.go +++ b/packages/go/analysis/azure/filters.go @@ -94,9 +94,8 @@ func roleDescentFilter(ctx *ops.TraversalContext, segment *graph.PathSegment) bo // If the group does not allow role inheritance then we do not inherit the terminal role if isRoleAssignable, err := end.Properties.Get(azure.IsAssignableToRole.String()).Bool(); err != nil || !isRoleAssignable { if graph.IsErrPropertyNotFound(err) { - log.Errorf("Node %d is missing property %s", end.ID, azure.IsAssignableToRole) + log.Warnf("Node %d is missing property %s", end.ID, azure.IsAssignableToRole) } - acceptDescendent = false return false } diff --git a/packages/go/analysis/azure/post.go b/packages/go/analysis/azure/post.go index a5869ba5f7..3d662d58a0 100644 --- a/packages/go/analysis/azure/post.go +++ b/packages/go/analysis/azure/post.go @@ -874,7 +874,7 @@ func addMembers(roleAssignments RoleAssignments, operation analysis.StatTrackedO operation.Operation.SubmitReader(func(ctx context.Context, tx graph.Transaction, outC chan<- analysis.CreatePostRelationshipJob) error { if isRoleAssignable, err := innerGroup.Properties.Get(azure.IsAssignableToRole.String()).Bool(); err != nil { if graph.IsErrPropertyNotFound(err) { - log.Errorf("Node %d is missing property %s", innerGroup.ID, azure.IsAssignableToRole) + log.Warnf("Node %d is missing property %s", innerGroup.ID, azure.IsAssignableToRole) } else { return err } diff --git a/packages/go/ein/azure.go b/packages/go/ein/azure.go index 05d5247fdc..8fb60a3ee6 100644 --- a/packages/go/ein/azure.go +++ b/packages/go/ein/azure.go @@ -41,7 +41,10 @@ const ( KeyVaultPermissionGet string = "Get" ) -var resourceGroupLevel = regexp.MustCompile(`^[\\w\\d\\-\\/]*/resourceGroups/[0-9a-zA-Z]+$`) +var ( + resourceGroupLevel = regexp.MustCompile(`^[\\w\\d\\-\\/]*/resourceGroups/[0-9a-zA-Z]+$`) + InvalidTypeErr = errors.New("invalid type returned from directory object") +) func ConvertAZAppToNode(app models.App) IngestibleNode { return IngestibleNode{ @@ -451,7 +454,9 @@ func ConvertAzureGroupMembersToRels(data models.GroupMembers) []IngestibleRelati ) if err := json.Unmarshal(raw.Member, &member); err != nil { log.Errorf(SerialError, "azure group member", err) - } else if memberType, err := ExtractTypeFromDirectoryObject(member); err != nil { + } else if memberType, err := ExtractTypeFromDirectoryObject(member); errors.Is(err, InvalidTypeErr) { + log.Warnf(ExtractError, err) + } else if err != nil { log.Errorf(ExtractError, err) } else { relationships = append(relationships, NewIngestibleRelationship( @@ -483,7 +488,9 @@ func ConvertAzureGroupOwnerToRels(data models.GroupOwners) []IngestibleRelations ) if err := json.Unmarshal(raw.Owner, &owner); err != nil { log.Errorf(SerialError, "azure group owner", err) - } else if ownerType, err := ExtractTypeFromDirectoryObject(owner); err != nil { + } else if ownerType, err := ExtractTypeFromDirectoryObject(owner); errors.Is(err, InvalidTypeErr) { + log.Warnf(ExtractError, err) + } else if err != nil { log.Errorf(ExtractError, err) } else { relationships = append(relationships, NewIngestibleRelationship( @@ -1067,7 +1074,9 @@ func ConvertAzureServicePrincipalOwnerToRels(data models.ServicePrincipalOwners) if err := json.Unmarshal(raw.Owner, &owner); err != nil { log.Errorf(SerialError, "azure service principal owner", err) - } else if ownerType, err := ExtractTypeFromDirectoryObject(owner); err != nil { + } else if ownerType, err := ExtractTypeFromDirectoryObject(owner); errors.Is(err, InvalidTypeErr) { + log.Warnf(ExtractError, err) + } else if err != nil { log.Errorf(ExtractError, err) } else { relationships = append(relationships, NewIngestibleRelationship( @@ -1862,7 +1871,7 @@ func ExtractTypeFromDirectoryObject(directoryObject azure2.DirectoryObject) (obj case enums.EntityDevice: return azure.Device, nil default: - return nil, errors.New(fmt.Sprintf("invalid type returned from directory object: %s", directoryObject.Type)) + return nil, fmt.Errorf("%w: %s", InvalidTypeErr, directoryObject.Type) } } diff --git a/packages/go/graphschema/ad/ad.go b/packages/go/graphschema/ad/ad.go index 27e38f0da3..cef7bba525 100644 --- a/packages/go/graphschema/ad/ad.go +++ b/packages/go/graphschema/ad/ad.go @@ -21,6 +21,7 @@ package ad import ( "errors" + graph "github.com/specterops/bloodhound/dawgs/graph" ) diff --git a/packages/go/graphschema/azure/azure.go b/packages/go/graphschema/azure/azure.go index 00b20f190f..787ee392e6 100644 --- a/packages/go/graphschema/azure/azure.go +++ b/packages/go/graphschema/azure/azure.go @@ -21,6 +21,7 @@ package azure import ( "errors" + graph "github.com/specterops/bloodhound/dawgs/graph" ) diff --git a/packages/go/graphschema/common/common.go b/packages/go/graphschema/common/common.go index 6fd161585e..9320bb8d29 100644 --- a/packages/go/graphschema/common/common.go +++ b/packages/go/graphschema/common/common.go @@ -21,6 +21,7 @@ package common import ( "errors" + graph "github.com/specterops/bloodhound/dawgs/graph" )