Skip to content

Commit

Permalink
PostgreSQL Related Query Path Fixes (#950)
Browse files Browse the repository at this point in the history
* fix: BED-4997 - post-processing constraint violations

* chore: BED-4997 - prepare for codereview

* fix: BED-5003 - correctly constrain expansion based on direction

* chore: BED-5003 - documentation update

* chore: BED-5003 - split out struct to avoid nil pointers
  • Loading branch information
zinic authored Nov 13, 2024
1 parent 9ef4057 commit e470eed
Show file tree
Hide file tree
Showing 17 changed files with 549 additions and 178 deletions.
2 changes: 1 addition & 1 deletion cmd/api/src/analysis/azure/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import (

func Post(ctx context.Context, db graph.Database) (*analysis.AtomicPostProcessingStats, error) {
aggregateStats := analysis.NewAtomicPostProcessingStats()
if stats, err := analysis.DeleteTransitEdges(ctx, db, graph.Kinds{ad.Entity, azure.Entity}, azureAnalysis.AzurePostProcessedRelationships()...); err != nil {
if stats, err := analysis.DeleteTransitEdges(ctx, db, graph.Kinds{ad.Entity, azure.Entity}, azureAnalysis.PostProcessedRelationships()...); err != nil {
return &aggregateStats, err
} else if userRoleStats, err := azureAnalysis.UserRoleAssignments(ctx, db); err != nil {
return &aggregateStats, err
Expand Down
8 changes: 4 additions & 4 deletions cmd/api/src/api/saml/saml.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,11 +350,11 @@ func (s ProviderResource) serveAssertionConsumerService(response http.ResponseWr
s.writeAPIErrorResponse(request, response, http.StatusBadRequest, "session assertion does not meet the requirements for user lookup")
} else {
s.authenticator.CreateSSOSession(request, response, principalName, model.SSOProvider{
Type: model.SessionAuthProviderSAML,
Name: s.serviceProvider.Config.Name,
Slug: s.serviceProvider.Config.Name,
Type: model.SessionAuthProviderSAML,
Name: s.serviceProvider.Config.Name,
Slug: s.serviceProvider.Config.Name,
SAMLProvider: &s.serviceProvider.Config,
Serial: model.Serial{ ID: s.serviceProvider.Config.SSOProviderID.Int32 },
Serial: model.Serial{ID: s.serviceProvider.Config.SSOProviderID.Int32},
})
}
}
Expand Down
16 changes: 8 additions & 8 deletions cmd/api/src/api/saml/saml_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ import (

func SSOProviderFromResource(resource ProviderResource) model.SSOProvider {
return model.SSOProvider{
Type: model.SessionAuthProviderSAML,
Name: resource.serviceProvider.Config.Name,
Slug: resource.serviceProvider.Config.Name,
SAMLProvider: &resource.serviceProvider.Config,
Serial: model.Serial{ ID: resource.serviceProvider.Config.SSOProviderID.Int32 },
}
Type: model.SessionAuthProviderSAML,
Name: resource.serviceProvider.Config.Name,
Slug: resource.serviceProvider.Config.Name,
SAMLProvider: &resource.serviceProvider.Config,
Serial: model.Serial{ID: resource.serviceProvider.Config.SSOProviderID.Int32},
}
}

func TestAuth_CreateSSOSession(t *testing.T) {
Expand All @@ -58,7 +58,7 @@ func TestAuth_CreateSSOSession(t *testing.T) {
SAMLProvider: &model.SAMLProvider{
Serial: model.Serial{ID: 1},
},
SSOProviderID: null.Int32From(1),
SSOProviderID: null.Int32From(1),
SAMLProviderID: null.Int32From(1),
}

Expand All @@ -71,7 +71,7 @@ func TestAuth_CreateSSOSession(t *testing.T) {
config.Configuration{RootURL: serde.MustParseURL("https://example.com")},
bhsaml.ServiceProvider{
Config: model.SAMLProvider{
Serial: model.Serial{ID: 1},
Serial: model.Serial{ID: 1},
SSOProviderID: null.Int32From(1),
},
},
Expand Down
4 changes: 2 additions & 2 deletions cmd/api/src/api/v2/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (s ManagementResource) OIDCLoginHandler(response http.ResponseWriter, reque
ClientID: ssoProvider.OIDCProvider.ClientID,
Endpoint: provider.Endpoint(),
RedirectURL: getRedirectURL(request, ssoProvider),
Scopes: []string{"openid", "profile", "email", "email_verified", "name", "given_name", "family_name"},
Scopes: []string{"openid", "profile", "email", "email_verified", "name", "given_name", "family_name"},
}

// use PKCE to protect against CSRF attacks
Expand Down Expand Up @@ -134,7 +134,7 @@ func (s ManagementResource) OIDCCallbackHandler(response http.ResponseWriter, re
// Extract custom claims
var claims struct {
Name string `json:"name"`
FamilyName string `json:"family_name"`
FamilyName string `json:"family_name"`
DisplayName string `json:"given_name"`
Email string `json:"email"`
Verified bool `json:"email_verified"`
Expand Down
2 changes: 2 additions & 0 deletions packages/go/analysis/ad/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ func PostProcessedRelationships() []graph.Kind {
ad.ADCSESC10a,
ad.ADCSESC10b,
ad.ADCSESC9a,
ad.ADCSESC9b,
ad.ADCSESC13,
ad.EnrollOnBehalfOf,
ad.SyncedToEntraUser,
ad.ExtendedByPolicy,
}
}

Expand Down
236 changes: 131 additions & 105 deletions packages/go/analysis/azure/post.go

Large diffs are not rendered by default.

54 changes: 54 additions & 0 deletions packages/go/analysis/post.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ package analysis
import (
"context"
"sort"
"sync"

"github.com/specterops/bloodhound/dawgs/cardinality"

"github.com/specterops/bloodhound/dawgs/graph"
"github.com/specterops/bloodhound/dawgs/ops"
Expand Down Expand Up @@ -120,6 +123,57 @@ type DeleteRelationshipJob struct {
ID graph.ID
}

// EdgeConstraintMap is a thread safe tracker for post-processed edges. It guarantees that only one edge of a given
// post-processed kind may exist. This is useful either to create a batch of post-processed edges to insert or to
// guard against double insertion of the same post-processed edge.
type EdgeConstraintMap struct {
lock *sync.Mutex
adjacent map[graph.ID]map[graph.Kind]cardinality.Duplex[uint64]
}

func NewEdgeConstraintMap() EdgeConstraintMap {
return EdgeConstraintMap{
lock: &sync.Mutex{},
adjacent: map[graph.ID]map[graph.Kind]cardinality.Duplex[uint64]{},
}
}

// TrackCreatePostRelationshipJob decomposes a CreatePostRelationshipJob type and returns the result of tracking it.
func (s EdgeConstraintMap) TrackCreatePostRelationshipJob(job CreatePostRelationshipJob) bool {
return s.Track(job.FromID, job.ToID, job.Kind)
}

// Track will attempt to track creation of the given relationship arguments. This function returns false
// if the given relationship has already been tracked; true otherwise.
func (s EdgeConstraintMap) Track(start, end graph.ID, edgeKind graph.Kind) bool {
s.lock.Lock()
defer s.lock.Unlock()

// Lookup what's adjacent outbound from the start ID
if startAdjacent, exists := s.adjacent[start]; !exists {
// If there's nothing adjacent for the start ID then this is the first outbound edge being created
// for it.
s.adjacent[start] = map[graph.Kind]cardinality.Duplex[uint64]{
edgeKind: cardinality.NewBitmap64With(end.Uint64()),
}
} else if kindAdjacent, exists := startAdjacent[edgeKind]; !exists {
// If there's no bitmap representing outbound edges over the given edge kind then create a new bitmap
// and track it.
startAdjacent[edgeKind] = cardinality.NewBitmap64With(end.Uint64())
} else if !kindAdjacent.CheckedAdd(end.Uint64()) {
// This Debugf statement is here to help engineers figure out where double-inserts are coming from in
// the post-processing logic.
log.Debugf("Duplicate post-processed edge: (%d)-[:%s]->(%d)", start, edgeKind, end)

// If the CheckedAdd function returns false, the outbound nodes already contains an entry for this end
// ID, meaning that the edge has already been created.
return false
}

// Getting here means we have added a new, unique edge.
return true
}

func DeleteTransitEdges(ctx context.Context, db graph.Database, baseKinds graph.Kinds, targetRelationships ...graph.Kind) (*AtomicPostProcessingStats, error) {
defer log.Measure(log.LevelInfo, "Finished deleting transit edges")()

Expand Down
2 changes: 2 additions & 0 deletions packages/go/cypher/models/pgsql/operators.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ const (
OperatorIn Operator = "in"
OperatorIs Operator = "is"
OperatorIsNot Operator = "is not"
OperatorSimilarTo Operator = "similar to"
OperatorRegexMatch Operator = "=~"
OperatorStartsWith Operator = "starts with"
OperatorContains Operator = "contains"
OperatorEndsWith Operator = "ends with"
Expand Down
20 changes: 16 additions & 4 deletions packages/go/cypher/models/pgsql/pgtypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,22 +294,34 @@ func ValueToDataType(value any) (DataType, error) {
case time.Duration:
return Interval, nil

// * uint8 is here since it can't fit in a signed byte and therefore must coerce into a higher sized type
case uint8, int8, int16:
return Int2, nil

// * uint8 is here since it can't fit in a signed byte and therefore must coerce into a higher sized type
case []uint8, []int8, []int16:
return Int2Array, nil

case uint16, int32, graph.ID:
// * uint16 is here since it can't fit in a signed 16-bit value and therefore must coerce into a higher sized type
case uint16, int32:
return Int4, nil

case []uint16, []int32, []graph.ID:
// * uint16 is here since it can't fit in a signed 16-bit value and therefore must coerce into a higher sized type
case []uint16, []int32:
return Int4Array, nil

case uint32, uint, uint64, int, int64:
// * uint32 is here since it can't fit in a signed 16-bit value and therefore must coerce into a higher sized type
// * uint is here because it is architecture dependent but expecting it to be an unsigned value between 32-bits and
// 64-bits is fine.
// * int is here for the same reasons as uint
case uint32, uint, uint64, int, int64, graph.ID:
return Int8, nil

case []uint32, []uint, []uint64, []int, []int64:
// * uint32 is here since it can't fit in a signed 16-bit value and therefore must coerce into a higher sized type
// * uint is here because it is architecture dependent but expecting it to be an unsigned value between 32-bits and
// 64-bits is fine.
// * int is here for the same reasons as uint
case []uint32, []uint, []uint64, []int, []int64, []graph.ID:
return Int8Array, nil

case float32:
Expand Down
Loading

0 comments on commit e470eed

Please sign in to comment.