Skip to content

Commit

Permalink
chore: simplify graph errors (#656)
Browse files Browse the repository at this point in the history
chore: clean up some unused parameters
  • Loading branch information
superlinkx committed Jun 18, 2024
1 parent b18e002 commit 20c6a80
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 59 deletions.
4 changes: 2 additions & 2 deletions packages/go/analysis/ad/adcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,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, adcsEnabled)
processEnterpriseCAWithValidCertChainToDomain(innerEnterpriseCA, innerDomain, groupExpansions, cache, operation)
}
}
}
Expand Down Expand Up @@ -112,7 +112,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], adcsEnabled bool) {
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 {
Expand Down
12 changes: 6 additions & 6 deletions packages/go/analysis/azure/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func AppRoleAssignments(ctx context.Context, db graph.Database) (*analysis.Atomi
return err
} else if err := createAZMGApplicationReadWriteAllEdges(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := createAZMGAppRoleAssignmentReadWriteAllEdges(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
} else if err := createAZMGAppRoleAssignmentReadWriteAllEdges(ctx, db, operation, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := createAZMGDirectoryReadWriteAllEdges(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
return err
Expand All @@ -223,13 +223,13 @@ func AppRoleAssignments(ctx context.Context, db graph.Database) (*analysis.Atomi
return err
} else if err := createAZMGRoleManagementReadWriteDirectoryEdgesPart2(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := createAZMGRoleManagementReadWriteDirectoryEdgesPart3(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
} else if err := createAZMGRoleManagementReadWriteDirectoryEdgesPart3(ctx, db, operation, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := createAZMGRoleManagementReadWriteDirectoryEdgesPart4(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := createAZMGRoleManagementReadWriteDirectoryEdgesPart5(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := createAZMGServicePrincipalEndpointReadWriteAllEdges(ctx, db, operation, tenant, tenantContainsServicePrincipalRelationships); err != nil {
} else if err := createAZMGServicePrincipalEndpointReadWriteAllEdges(ctx, db, operation, tenantContainsServicePrincipalRelationships); err != nil {
return err
} else if err := addSecret(ctx, db, operation, tenant); err != nil {
return err
Expand Down Expand Up @@ -289,7 +289,7 @@ func createAZMGApplicationReadWriteAllEdges(ctx context.Context, db graph.Databa
}
}

func createAZMGAppRoleAssignmentReadWriteAllEdges(ctx context.Context, db graph.Database, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], tenant *graph.Node, tenantContainsServicePrincipalRelationships []*graph.Relationship) error {
func createAZMGAppRoleAssignmentReadWriteAllEdges(ctx context.Context, db graph.Database, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], tenantContainsServicePrincipalRelationships []*graph.Relationship) error {
if err := db.ReadTransaction(ctx, func(tx graph.Transaction) error {
if sourceNodes, err := aggregateSourceReadWriteServicePrincipals(tx, tenantContainsServicePrincipalRelationships, azure.AppRoleAssignmentReadWriteAll); err != nil {
return err
Expand Down Expand Up @@ -500,7 +500,7 @@ func createAZMGRoleManagementReadWriteDirectoryEdgesPart2(ctx context.Context, d
}
}

func createAZMGRoleManagementReadWriteDirectoryEdgesPart3(ctx context.Context, db graph.Database, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], tenant *graph.Node, tenantContainsServicePrincipalRelationships []*graph.Relationship) error {
func createAZMGRoleManagementReadWriteDirectoryEdgesPart3(ctx context.Context, db graph.Database, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], tenantContainsServicePrincipalRelationships []*graph.Relationship) error {
if err := db.ReadTransaction(ctx, func(tx graph.Transaction) error {
if sourceNodes, err := aggregateSourceReadWriteServicePrincipals(tx, tenantContainsServicePrincipalRelationships, azure.RoleManagementReadWriteDirectory); err != nil {
return err
Expand Down Expand Up @@ -624,7 +624,7 @@ func createAZMGRoleManagementReadWriteDirectoryEdgesPart5(ctx context.Context, d
}
}

func createAZMGServicePrincipalEndpointReadWriteAllEdges(ctx context.Context, db graph.Database, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], tenant *graph.Node, tenantContainsServicePrincipalRelationships []*graph.Relationship) error {
func createAZMGServicePrincipalEndpointReadWriteAllEdges(ctx context.Context, db graph.Database, operation analysis.StatTrackedOperation[analysis.CreatePostRelationshipJob], tenantContainsServicePrincipalRelationships []*graph.Relationship) error {
if err := db.ReadTransaction(ctx, func(tx graph.Transaction) error {
if sourceNodes, err := aggregateSourceReadWriteServicePrincipals(tx, tenantContainsServicePrincipalRelationships, azure.ServicePrincipalEndpointReadWriteAll); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion packages/go/dawgs/drivers/neo4j/neo4j.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const (
defaultNeo4jTransactionTimeout = math.MinInt
)

func newNeo4jDB(ctx context.Context, cfg dawgs.Config) (graph.Database, error) {
func newNeo4jDB(_ context.Context, cfg dawgs.Config) (graph.Database, error) {
if connectionURLStr, typeOK := cfg.DriverCfg.(string); !typeOK {
return nil, fmt.Errorf("expected string for configuration type but got %T", cfg.DriverCfg)
} else if connectionURL, err := url.Parse(connectionURLStr); err != nil {
Expand Down
44 changes: 8 additions & 36 deletions packages/go/dawgs/graph/errors.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
// Copyright 2023 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

package graph
Expand Down Expand Up @@ -41,47 +41,19 @@ var (
ErrContextTimedOut = errors.New("context timed out")
)

func unwrappedIs(err, sentinel error) bool {
if typedErr, typeOK := err.(Error); typeOK {
return typedErr.DriverError == sentinel
}

return errors.Is(err, sentinel)
}

func IsErrNotFound(err error) bool {
return unwrappedIs(err, ErrNoResultsFound)
return errors.Is(err, ErrNoResultsFound)
}

func IsErrPropertyNotFound(err error) bool {
return unwrappedIs(err, ErrPropertyNotFound)
return errors.Is(err, ErrPropertyNotFound)
}

func IsMissingResultExpectation(err error) bool {
return unwrappedIs(err, ErrMissingResultExpectation)
}

// Error from the query package is a DAWGS recognized error type that can be used to output contextual information about
// a failed database action.
type Error struct {
// Query is the drive's best-effort string representation of the operation that failed.
Query string

// DriverError is the raw error type captured from the underlying driver. Access to this error type is provided in
// cases where direct type negotiation of the database error is required.
DriverError error
}

// Error satisfies the error interface by returning a formatted string containing all interesting contextual details
// about the Error.
func (s Error) Error() string {
return fmt.Sprintf("driver error: %s - query: %s", s.DriverError, s.Query)
return errors.Is(err, ErrMissingResultExpectation)
}

// NewError returns an error that contains the given query context elements.
func NewError(query string, driverErr error) error {
return Error{
Query: query,
DriverError: driverErr,
}
return fmt.Errorf("driver error: %w - query: %s", driverErr, query)
}
7 changes: 1 addition & 6 deletions packages/go/dawgs/util/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"sync"

"github.com/neo4j/neo4j-go-driver/v5/neo4j"
"github.com/specterops/bloodhound/dawgs/graph"
)

type ErrorCollector interface {
Expand Down Expand Up @@ -63,11 +62,7 @@ func IsNeoTimeoutError(err error) bool {
switch e := err.(type) {
case *neo4j.Neo4jError:
return strings.Contains(e.Code, "TransactionTimedOut")
case graph.Error:
return strings.Contains(e.Error(), "Neo.ClientError.Transaction.TransactionTimedOut")
case *graph.Error:
return strings.Contains(e.Error(), "Neo.ClientError.Transaction.TransactionTimedOut")
default:
return false
return strings.Contains(e.Error(), "Neo.ClientError.Transaction.TransactionTimedOut")
}
}
17 changes: 9 additions & 8 deletions packages/go/dawgs/util/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ func TestIsNeoTimeoutError(t *testing.T) {
}
require.False(t, util.IsNeoTimeoutError(&notTimeOutErr))

driverTimeOutErr := graph.Error{
Query: "match (u1:User {domain: \"ESC6.LOCAL\"}), (u2:User {domain: \"ESC3.LOCAL\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2",
DriverError: errors.New("Neo4jError: Neo.ClientError.Transaction.TransactionTimedOut (The transaction has been terminated. Retry your operation in a new transaction, and you should see a successful result. The transaction has not completed within the specified timeout (dbms.transaction.timeout). You may want to retry with a longer timeout. )"),
}
driverTimeOutQuery := "match (u1:User {domain: \"ESC6.LOCAL\"}), (u2:User {domain: \"ESC3.LOCAL\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2"
driverError := errors.New("Neo4jError: Neo.ClientError.Transaction.TransactionTimedOut (The transaction has been terminated. Retry your operation in a new transaction, and you should see a successful result. The transaction has not completed within the specified timeout (dbms.transaction.timeout). You may want to retry with a longer timeout. )")

driverTimeOutErr := graph.NewError(driverTimeOutQuery, driverError)
require.True(t, util.IsNeoTimeoutError(driverTimeOutErr))

notDriverTimeOutErr := graph.Error{
Query: "match (u1:User {domain: \"ESC6.LOCAL\"}), (u2:User {domain: \"ESC3.LOCAL\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2",
DriverError: errors.New("Some other error"),
}
notDriverTimeOutQuery := "match (u1:User {domain: \"ESC6.LOCAL\"}), (u2:User {domain: \"ESC3.LOCAL\"}) where u1.samaccountname <> \"krbtgt\" and u1.samaccountname = u2.samaccountname with u2 match p1 = (u2)-[*1..]->(g:Group) with p1 match p2 = (u2)-[*1..]->(g:Group) return p1, p2"
notDriverError := errors.New("Some other error")

notDriverTimeOutErr := graph.NewError(notDriverTimeOutQuery, notDriverError)

require.False(t, util.IsNeoTimeoutError(notDriverTimeOutErr))
}

0 comments on commit 20c6a80

Please sign in to comment.