diff --git a/packages/go/analysis/ad/ad.go b/packages/go/analysis/ad/ad.go index 8c73557bf..6538d389b 100644 --- a/packages/go/analysis/ad/ad.go +++ b/packages/go/analysis/ad/ad.go @@ -122,7 +122,8 @@ func FetchWellKnownTierZeroEntities(ctx context.Context, db graph.Database, doma func FixWellKnownNodeTypes(ctx context.Context, db graph.Database) error { defer measure.ContextMeasure(ctx, slog.LevelInfo, "Fix well known node types")() - groupSuffixes := []string{EnterpriseKeyAdminsGroupSIDSuffix, + groupSuffixes := []string{ + EnterpriseKeyAdminsGroupSIDSuffix, KeyAdminsGroupSIDSuffix, EnterpriseDomainControllersGroupSIDSuffix, DomainAdminsGroupSIDSuffix, @@ -190,7 +191,6 @@ func RunDomainAssociations(ctx context.Context, db graph.Database) error { // TODO: Reimplement unassociated node pruning if we decide that FOSS needs unassociated node pruning return nil }) - } func grabDomainInformation(tx graph.Transaction) (map[string]string, error) { @@ -274,14 +274,13 @@ func LinkWellKnownGroups(ctx context.Context, db graph.Database) error { } func getOrCreateWellKnownGroup(tx graph.Transaction, wellKnownSid string, domainSid, domainName, nodeName string) (*graph.Node, error) { + // Only filter by ObjectID, not by kind if wellKnownNode, err := tx.Nodes().Filterf(func() graph.Criteria { - return query.And( - query.Equals(query.NodeProperty(common.ObjectID.String()), wellKnownSid), - query.Kind(query.Node(), ad.Group), - ) + return query.Equals(query.NodeProperty(common.ObjectID.String()), wellKnownSid) }).First(); err != nil && !graph.IsErrNotFound(err) { return nil, err } else if graph.IsErrNotFound(err) { + // Only create a new node if no node with this ObjectID exists properties := graph.AsProperties(graph.PropertyMap{ common.Name: nodeName, common.ObjectID: wellKnownSid, @@ -291,6 +290,15 @@ func getOrCreateWellKnownGroup(tx graph.Transaction, wellKnownSid string, domain }) return tx.CreateNode(properties, ad.Entity, ad.Group) } else { + // If a node with this ObjectID exists (regardless of its kind), return it + // Optionally, we could add the ad.Group kind if it's missing + if !wellKnownNode.Kinds.ContainsOneOf(ad.Group) { + // Add the ad.Group kind if it's missing + wellKnownNode.AddKinds(ad.Group) + if err := tx.UpdateNode(wellKnownNode); err != nil { + return nil, fmt.Errorf("failed to update node with Group kind: %w", err) + } + } return wellKnownNode, nil } } @@ -342,7 +350,6 @@ func CalculateCrossProductNodeSets(tx graph.Transaction, groupExpansions impact. // Get the IDs of the Auth. Users and Everyone groups specialGroups, err := FetchAuthUsersAndEveryoneGroups(tx) - if err != nil { slog.Error(fmt.Sprintf("Could not fetch groups: %s", err.Error())) } diff --git a/packages/go/analysis/ad/ad_test.go b/packages/go/analysis/ad/ad_test.go new file mode 100644 index 000000000..48b181730 --- /dev/null +++ b/packages/go/analysis/ad/ad_test.go @@ -0,0 +1,276 @@ +package ad_test + +import ( + "context" + "fmt" + "testing" + + "github.com/specterops/bloodhound/dawgs/graph" + "github.com/specterops/bloodhound/dawgs/query" + "github.com/stretchr/testify/require" + + adAnalysis "github.com/specterops/bloodhound/analysis/ad" + + "github.com/specterops/bloodhound/graphschema/ad" + "github.com/specterops/bloodhound/graphschema/common" +) + +func TestLinkWellKnownGroups(t *testing.T) { + // Skip if running short tests + if testing.Short() { + t.Skip("Skipping test in short mode") + } + + // Define database drivers to test + dbDrivers := []struct { + name string + driver string + initFn func(t *testing.T, ctx context.Context) (graph.Database, func()) + }{ + { + name: "Neo4j", + driver: "neo4j", + initFn: initNeo4jDatabase, + }, + { + name: "PostgreSQL", + driver: "pg", + initFn: initPostgresDatabase, + }, + } + + // Define test cases + testCases := []struct { + name string + setupFunc func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error + expectedGroupExists map[string]bool // Map of well-known SID suffix to expected existence + expectedGroupKinds map[string][]graph.Kind // Map of well-known SID suffix to expected kinds + }{ + { + name: "Normal scenario - no pre-existing nodes", + setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error { + return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error { + // Just create the domain node + domainProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: domainName, + common.ObjectID: domainSid, + }) + _, err := tx.CreateNode(domainProperties, ad.Domain) + return err + }) + }, + expectedGroupExists: map[string]bool{ + "-512": true, // Domain Admins + "-513": true, // Domain Users + "-515": true, // Domain Computers + "-516": true, // Domain Controllers + }, + expectedGroupKinds: map[string][]graph.Kind{ + "-512": {ad.Entity, ad.Group}, + "-513": {ad.Entity, ad.Group}, + "-515": {ad.Entity, ad.Group}, + "-516": {ad.Entity, ad.Group}, + }, + }, + { + name: "Pre-existing node without Group kind", + setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error { + return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error { + // Create the domain node + domainProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: domainName, + common.ObjectID: domainSid, + ad.DomainSID: domainSid, + }) + _, err := tx.CreateNode(domainProperties, ad.Domain) + if err != nil { + return err + } + + // Create a node for Domain Users with the same SID but without Group kind + domainUsersSid := domainSid + "-513" + userProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: "Domain Users", + common.ObjectID: domainUsersSid, + }) + _, err = tx.CreateNode(userProperties, ad.Entity) // Note: No ad.Group kind + return err + }) + }, + expectedGroupExists: map[string]bool{ + "-512": true, // Domain Admins + "-513": true, // Domain Users (pre-existing) + "-515": true, // Domain Computers + "-516": true, // Domain Controllers + }, + expectedGroupKinds: map[string][]graph.Kind{ + "-512": {ad.Entity, ad.Group}, + "-513": {ad.Entity, ad.Group}, // Should have Group kind added + "-515": {ad.Entity, ad.Group}, + "-516": {ad.Entity, ad.Group}, + }, + }, + { + name: "Pre-existing node with different kind (constraint violation scenario)", + setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error { + return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error { + // Create the domain node + domainProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: domainName, + common.ObjectID: domainSid, + ad.DomainSID: domainSid, + }) + _, err := tx.CreateNode(domainProperties, ad.Domain) + if err != nil { + return err + } + + // Create a node with the same SID as Domain Computers but with a different kind + domainComputersSid := domainSid + "-515" + computerProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: "Some Other Entity", + common.ObjectID: domainComputersSid, + }) + _, err = tx.CreateNode(computerProperties, ad.Entity, ad.User) // Not a Group + return err + }) + }, + expectedGroupExists: map[string]bool{ + "-512": true, // Domain Admins + "-513": true, // Domain Users + "-515": true, // Domain Computers (pre-existing with different kind) + "-516": true, // Domain Controllers + }, + expectedGroupKinds: map[string][]graph.Kind{ + "-512": {ad.Entity, ad.Group}, + "-513": {ad.Entity, ad.Group}, + "-515": {ad.Entity, ad.Group, ad.User}, // Should have both User and Group kinds + "-516": {ad.Entity, ad.Group}, + }, + }, + { + name: "Pre-existing node with Group kind", + setupFunc: func(ctx context.Context, graphDB graph.Database, domainSid, domainName string) error { + return graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error { + // Create the domain node + domainProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: domainName, + common.ObjectID: domainSid, + ad.DomainSID: domainSid, + }) + _, err := tx.CreateNode(domainProperties, ad.Domain) + if err != nil { + return err + } + + // Create a node for Domain Admins with Group kind already + domainAdminsSid := domainSid + "-512" + adminProperties := graph.AsProperties(graph.PropertyMap{ + common.Name: "Domain Admins", + common.ObjectID: domainAdminsSid, + }) + _, err = tx.CreateNode(adminProperties, ad.Entity, ad.Group) + return err + }) + }, + expectedGroupExists: map[string]bool{ + "-512": true, // Domain Admins (pre-existing with Group kind) + "-513": true, // Domain Users + "-515": true, // Domain Computers + "-516": true, // Domain Controllers + }, + expectedGroupKinds: map[string][]graph.Kind{ + "-512": {ad.Entity, ad.Group}, + "-513": {ad.Entity, ad.Group}, + "-515": {ad.Entity, ad.Group}, + "-516": {ad.Entity, ad.Group}, + }, + }, + } + + // Run tests for each database driver + for _, dbDriver := range dbDrivers { + t.Run(dbDriver.name, func(t *testing.T) { + ctx := context.Background() + + // Initialize the database + graphDB, cleanup := dbDriver.initFn(t, ctx) + defer cleanup() + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Clean the database before each test + cleanDatabase(t, ctx, graphDB) + + // Set up the test case + domainSid := "S-1-5-21-123456789-123456789-123456789" + domainName := "TESTDOMAIN.LOCAL" + err := tc.setupFunc(ctx, graphDB, domainSid, domainName) + require.NoError(t, err) + + // Make sure the domain node has the required properties for LinkWellKnownGroups + err = graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error { + domain, err := tx.Nodes().Filterf(func() graph.Criteria { + return query.Kind(query.Node(), ad.Domain) + }).First() + if err != nil { + return err + } + + // Ensure domain has DomainSID property + if _, err := domain.Properties.Get(ad.DomainSID.String()).String(); err != nil { + domain.Properties.Set(ad.DomainSID.String(), domainSid) + if err := tx.UpdateNode(domain); err != nil { + return err + } + } + return nil + }) + require.NoError(t, err) + require.NoError(t, err) + + // Run LinkWellKnownGroups + err = adAnalysis.LinkWellKnownGroups(ctx, graphDB) + require.NoError(t, err) + + // Verify the results in a read transaction + err = graphDB.ReadTransaction(ctx, func(tx graph.Transaction) error { + for sidSuffix, shouldExist := range tc.expectedGroupExists { + wellKnownSid := domainSid + sidSuffix + node, err := tx.Nodes().Filterf(func() graph.Criteria { + return query.Equals(query.NodeProperty(common.ObjectID.String()), wellKnownSid) + }).First() + + if shouldExist { + if err != nil { + return fmt.Errorf("node with SID %s should exist: %w", wellKnownSid, err) + } + if node == nil { + return fmt.Errorf("node with SID %s should not be nil", wellKnownSid) + } + + // Verify the kinds + expectedKinds := tc.expectedGroupKinds[sidSuffix] + for _, kind := range expectedKinds { + if !node.Kinds.ContainsOneOf(kind) { + return fmt.Errorf( + "node with SID %s should have kind %s", + wellKnownSid, + kind, + ) + } + } + } else { + if err == nil || !graph.IsErrNotFound(err) { + return fmt.Errorf("node with SID %s should not exist", wellKnownSid) + } + } + } + return nil + }) + require.NoError(t, err) + }) + } + }) + } +} diff --git a/packages/go/analysis/ad/db_test.go b/packages/go/analysis/ad/db_test.go new file mode 100644 index 000000000..dce711c32 --- /dev/null +++ b/packages/go/analysis/ad/db_test.go @@ -0,0 +1,112 @@ +package ad_test + +import ( + "context" + "os" + "testing" + + "github.com/specterops/bloodhound/dawgs/graph" + "github.com/specterops/bloodhound/src/bootstrap" + "github.com/specterops/bloodhound/src/config" + "github.com/specterops/bloodhound/src/database" + "github.com/specterops/bloodhound/src/services" + "github.com/stretchr/testify/require" + + "github.com/specterops/bloodhound/graphschema/common" +) + +// Get database configuration from environment variables or use defaults +func getDatabaseConfig(t *testing.T, driver string) config.Configuration { + neo4jURI := os.Getenv("NEO4J_URI") + if neo4jURI == "" { + neo4jURI = "neo4j://neo4j:bloodhoundcommunityedition@localhost:7474/" // Default Neo4j URI + } + + postgresURI := os.Getenv("POSTGRES_URI") + if postgresURI == "" { + postgresURI = "postgres://bloodhound:bloodhoundcommunityedition@localhost:5432/bloodhound?sslmode=disable" // Default PostgreSQL URI + } + + cfg := config.Configuration{ + GraphDriver: driver, + Neo4J: config.DatabaseConfiguration{ + Connection: neo4jURI, + }, + Database: config.DatabaseConfiguration{ + Connection: postgresURI, + }, + } + + return cfg +} + +// Initialize database using the Initializer pattern +func initDatabase(t *testing.T, ctx context.Context, driver string) (graph.Database, func()) { + // Create configuration with database settings + cfg := getDatabaseConfig(t, driver) + require.NotEmpty(t, cfg) + + // Create an initializer with the database connector + initializer := bootstrap.Initializer[*database.BloodhoundDB, *graph.DatabaseSwitch]{ + Configuration: cfg, + DBConnector: services.ConnectDatabases, + } + require.NotNil(t, initializer) + + // Connect to the database using the initializer's DBConnector + connections, err := initializer.DBConnector(ctx, cfg) + require.NotNil(t, connections) + require.NoError(t, err, "Failed to connect to database") + + // Initialize the schema + schema := graph.Schema{ + DefaultGraph: graph.Graph{ + Name: "bloodhound", + NodeConstraints: []graph.Constraint{ + { + Name: "object_id_constraint", + Field: common.ObjectID.String(), + Type: graph.BTreeIndex, + }, + }, + }, + } + + // Assert schema on the graph database + err = connections.Graph.AssertSchema(ctx, schema) + require.NoError(t, err, "Failed to assert schema") + + // Return the graph database and a cleanup function + cleanup := func() { + t.Logf("closing database connections") + t.Logf("closing GraphDB") + if err := connections.Graph.Close(ctx); err != nil { + t.Logf("Error closing graph database: %v", err) + } + t.Logf("closing RDMS") + connections.RDMS.Close(ctx) + } + + return connections.Graph, cleanup +} + +// Initialize Neo4j database +func initNeo4jDatabase(t *testing.T, ctx context.Context) (graph.Database, func()) { + return initDatabase(t, ctx, "neo4j") +} + +// Initialize PostgreSQL database +func initPostgresDatabase(t *testing.T, ctx context.Context) (graph.Database, func()) { + return initDatabase(t, ctx, "pg") +} + +// Helper function to clean the database +func cleanDatabase(t *testing.T, ctx context.Context, graphDB graph.Database) { + err := graphDB.WriteTransaction(ctx, func(tx graph.Transaction) error { + // Delete all nodes and relationships + result := tx.Raw("MATCH (n) DETACH DELETE n", nil) + defer result.Close() + return result.Error() + }) + require.NoError(t, err, "Failed to clean the database") +}