Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support export logging #386

Merged
merged 49 commits into from
Feb 6, 2024
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
0e1d356
feat: Initial migration and model updates for AuditLog improvements
juggernot325 Jan 5, 2024
d0c6173
feat: populate actor_email for existing audit_logs
juggernot325 Jan 8, 2024
1d08960
chore: Fix audit log docs to show the actual response object instead …
juggernot325 Jan 9, 2024
6019c99
feat: Log Requester IP for source and actor email with audit logs
juggernot325 Jan 11, 2024
0f0d55a
wip: createUser audit logging via middleware
juggernot325 Jan 12, 2024
3d65a85
chore: refactor audit logging to trigger from LoggingMiddleware based…
juggernot325 Jan 19, 2024
86b77a3
wip: Successfully creating audit logs from middleware. Capturing fail…
juggernot325 Jan 23, 2024
22c3e8c
feat: Capture DB errors in audit logs
juggernot325 Jan 23, 2024
cf4b854
chore: remove debug logging
juggernot325 Jan 23, 2024
985d580
chore: Refactor setting of status to receiver function
juggernot325 Jan 23, 2024
29673a9
wip: All "simple" audit context endpoints updated for middeware
juggernot325 Jan 24, 2024
a05ba51
fix: Fix a few bugs found during testing
juggernot325 Jan 24, 2024
cd78c04
fix: Remove doc for non-existent POST /api/v2/saml endpoint
juggernot325 Jan 24, 2024
9cb8a3d
wip: Refactor to simplify setting of AuditCtx in endpoints
juggernot325 Jan 25, 2024
e1d81bd
fix: registration issue after rebase
superlinkx Jan 25, 2024
3ac13c1
tasks 6 and 7 for #345 (#357)
irshadaj Jan 26, 2024
fd32507
Merge remote-tracking branch 'origin/main' into populate-audit-log-fi…
superlinkx Jan 29, 2024
2a14ff5
redo IP parsing for changed requirements (#362)
irshadaj Jan 29, 2024
602d04b
feat: audit log steel thread (#364)
superlinkx Jan 30, 2024
6fe1f94
chore: steel thread cleanup (#365)
superlinkx Jan 30, 2024
6810bd3
add eula_accepted to user audit data (#371)
irshadaj Jan 30, 2024
0482b61
audit logs for auth tokens and secrets (#372)
irshadaj Jan 31, 2024
af01314
Merge branch 'main' into populate-audit-log-fields
superlinkx Jan 31, 2024
9ff9831
Merge remote-tracking branch 'origin/main' into populate-audit-log-fi…
superlinkx Jan 31, 2024
3902523
feat: UpdateAssetGroup, UpdateAssetGroupSelector, DeleteAssetGroupSel…
superlinkx Jan 31, 2024
d744c31
more auth handlers for audit log (#373)
irshadaj Jan 31, 2024
061dc91
chore: update documentation for our new commit_id field and `intent` …
superlinkx Jan 31, 2024
4cd2264
Generate audit log entries on unauthorized access attempts (#375)
juggernot325 Jan 31, 2024
13f4160
chore: change source column to TEXT
juggernot325 Feb 1, 2024
2a2c839
rename audit log source column, add indices
irshadaj Feb 1, 2024
cc16a11
Revert "rename audit log source column, add indices"
juggernot325 Feb 1, 2024
2624969
chore: rename audit_log.source as 'source' is a reserved keyword in p…
juggernot325 Feb 1, 2024
c36c609
chore: add additional fields to user.AuditData
juggernot325 Feb 1, 2024
eb5e741
Refactored unauthorized access audit logging: (#381)
sircodemane Feb 2, 2024
9c3b76b
chore: add RequestedPath to BHCtx to ensure we can reflect on the URL…
superlinkx Feb 2, 2024
0330679
Merge remote-tracking branch 'origin/main' into support-export-logging
superlinkx Feb 2, 2024
de6bc2e
feat: MaybeAuditableTransaction added to support methods that are som…
superlinkx Feb 2, 2024
0719913
chore: cleanup before review
juggernot325 Feb 2, 2024
847c92c
Merge branch 'main' into populate-audit-log-fields
juggernot325 Feb 2, 2024
ed025f5
chore: final redesign for RequestedURL to support Auditable interface
superlinkx Feb 2, 2024
8ec67ab
Merge branch 'populate-audit-log-fields' into support-export-logging
superlinkx Feb 2, 2024
5ff9e33
chore: move our toolapi trace handler to a proper pprof compatible se…
superlinkx Feb 5, 2024
b39f476
fix: changes from @zinic to clean up ResolveAllGroupMemberships, avoi…
superlinkx Feb 5, 2024
0185b1e
Merge remote-tracking branch 'origin/main' into support-export-logging
superlinkx Feb 5, 2024
7e15c49
chore: remove commented out code again
superlinkx Feb 5, 2024
c5b2fd3
chore: remove unused code
superlinkx Feb 5, 2024
9874e26
Merge remote-tracking branch 'origin/main' into support-export-logging
superlinkx Feb 5, 2024
deafa44
revert: bring back the old tracing endpoint, since it'll require upda…
superlinkx Feb 5, 2024
6666105
Merge remote-tracking branch 'origin/main' into support-export-logging
superlinkx Feb 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion cmd/api/src/api/middleware/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/specterops/bloodhound/src/config"
"github.com/specterops/bloodhound/src/ctx"
"github.com/specterops/bloodhound/src/database"
"github.com/specterops/bloodhound/src/model"
"github.com/specterops/bloodhound/src/utils"
"github.com/unrolled/secure"
)
Expand Down Expand Up @@ -139,7 +140,8 @@ func ContextMiddleware(next http.Handler) http.Handler {
Scheme: getScheme(request),
Host: request.Host,
},
RequestIP: parseUserIP(request),
RequestedURL: model.AuditableURL(request.URL.String()),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull the request URL for auditing purposes

RequestIP: parseUserIP(request),
})

// Route the request with the embedded context
Expand Down
81 changes: 0 additions & 81 deletions cmd/api/src/api/tools/trace.go

This file was deleted.

14 changes: 8 additions & 6 deletions cmd/api/src/ctx/ctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"time"

"github.com/specterops/bloodhound/src/auth"
"github.com/specterops/bloodhound/src/model"
)

// Use our own type rather than a primitive to avoid collisions
Expand All @@ -39,12 +40,13 @@ type RequestedWaitDuration struct {

// Context holds contextual data that is passed around to functions. This is an extension to Golang's built in context.
type Context struct {
StartTime time.Time
Timeout RequestedWaitDuration
RequestID string
AuthCtx auth.Context
Host *url.URL
RequestIP string
StartTime time.Time
Timeout RequestedWaitDuration
RequestID string
AuthCtx auth.Context
Host *url.URL
RequestedURL model.AuditableURL
RequestIP string
}

func (s *Context) ConstructGoContext() context.Context {
Expand Down
16 changes: 13 additions & 3 deletions cmd/api/src/daemons/api/toolapi/api.go
Copy link
Contributor Author

@superlinkx superlinkx Feb 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace our bespoke trace endpoint in the toolapi with the proper pprof compatible tools for easier use.

EDIT: added the old trace endpoint back due to how Acumen interacts with it. Will update acumen later and then properly remove this endpoint

Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ package toolapi

import (
"context"
"net/http"
"net/http/pprof"
"time"

"github.com/specterops/bloodhound/dawgs/graph"
"github.com/specterops/bloodhound/src/bootstrap"
"github.com/specterops/bloodhound/src/database"
"net/http"
"time"

"github.com/go-chi/chi/v5"
"github.com/prometheus/client_golang/prometheus/promhttp"
Expand All @@ -47,7 +49,15 @@ func NewDaemon[DBType database.Database](ctx context.Context, connections bootst

router.Mount("/metrics", promhttp.Handler())

router.Get("/trace", tools.NewTraceHandler())
router.Mount("/debug/pprof/allocs", pprof.Handler("allocs"))
router.Mount("/debug/pprof/block", pprof.Handler("block"))
router.Mount("/debug/pprof/goroutine", pprof.Handler("goroutine"))
router.Mount("/debug/pprof/heap", pprof.Handler("heap"))
router.Mount("/debug/pprof/mutex", pprof.Handler("mutex"))
router.Mount("/debug/pprof/threadcreate", pprof.Handler("threadcreate"))
router.Get("/debug/pprof/profile", pprof.Profile)
router.Get("/debug/pprof/trace", pprof.Trace)

router.Put("/graph-db/switch/pg", pgMigrator.SwitchPostgreSQL)
router.Put("/graph-db/switch/neo4j", pgMigrator.SwitchNeo4j)
router.Put("/pg-migration/start", pgMigrator.MigrationStart)
Expand Down
8 changes: 8 additions & 0 deletions cmd/api/src/database/audit.go
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kind of a dumb name, but allows us to add conditional audit logging a bit easier

Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,14 @@ func (s *BloodhoundDB) ListAuditLogs(before, after time.Time, offset, limit int,
return auditLogs, int(count), CheckError(result)
}

func (s *BloodhoundDB) MaybeAuditableTransaction(ctx context.Context, auditDisabled bool, auditEntry model.AuditEntry, f func(tx *gorm.DB) error, opts ...*sql.TxOptions) error {
superlinkx marked this conversation as resolved.
Show resolved Hide resolved
if auditDisabled {
return s.db.Transaction(f, opts...)
} else {
return s.AuditableTransaction(ctx, auditEntry, f, opts...)
}
}

func (s *BloodhoundDB) AuditableTransaction(ctx context.Context, auditEntry model.AuditEntry, f func(tx *gorm.DB) error, opts ...*sql.TxOptions) error {
var (
commitID, err = uuid.NewV4()
Expand Down
8 changes: 8 additions & 0 deletions cmd/api/src/model/audit.go
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AuditableURL was needed to support passing the request url to the audit log logic that requires an Auditable interface

Original file line number Diff line number Diff line change
Expand Up @@ -155,3 +155,11 @@ type AuditEntry struct {
Status AuditEntryStatus
ErrorMsg string
}

type AuditableURL string

func (s AuditableURL) AuditData() AuditData {
return AuditData{
"request_url": s,
}
}
74 changes: 28 additions & 46 deletions packages/go/analysis/ad/membership.go
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Work by @zinic to plug a deadlock condition I hit locally. This cleans up the logic of this function in general, but should be reviewed carefully to ensure nothing was missed. Tests fine locally now

Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func ResolveAllGroupMemberships(ctx context.Context, db graph.Database, addition
adGroupIDs []graph.ID

searchCriteria = []graph.Criteria{query.KindIn(query.Relationship(), ad.MemberOf, ad.MemberOfLocalGroup)}
coordC = make(chan struct{}, analysis.MaximumDatabaseParallelWorkers)
traversalMap = cardinality.ThreadSafeDuplex(cardinality.NewBitmap32())
traversalInst = traversal.NewIDTraversal(db, analysis.MaximumDatabaseParallelWorkers)
memberships = impact.NewThreadSafeAggregator(impact.NewIDA(func() cardinality.Provider[uint32] {
return cardinality.NewBitmap32()
}))
Expand All @@ -64,64 +64,46 @@ func ResolveAllGroupMemberships(ctx context.Context, db graph.Database, addition

log.Infof("Collected %d groups to resolve", len(adGroupIDs))

for i := 0; i < analysis.MaximumDatabaseParallelWorkers; i++ {
coordC <- struct{}{}
}

for _, adGroupID := range adGroupIDs {
if traversalMap.Contains(adGroupID.Uint32()) {
continue
}

<-coordC

go func(adGroupID graph.ID) {
if err := traversal.NewIDTraversal(db, analysis.MaximumDatabaseParallelWorkers).BreadthFirst(ctx, traversal.IDPlan{
Root: adGroupID,
Delegate: func(ctx context.Context, tx graph.Transaction, segment *graph.IDSegment) ([]*graph.IDSegment, error) {
if nextQuery, err := newTraversalQuery(tx, segment, graph.DirectionInbound, searchCriteria...); err != nil {
return nil, err
} else {
var nextSegments []*graph.IDSegment

if err := nextQuery.FetchTriples(func(cursor graph.Cursor[graph.RelationshipTripleResult]) error {
for nextTriple := range cursor.Chan() {
if traversalMap.CheckedAdd(nextTriple.StartID.Uint32()) {
nextSegments = append(nextSegments, segment.Descend(nextTriple.StartID, nextTriple.ID))
} else {
memberships.AddShortcut(segment.Descend(nextTriple.StartID, nextTriple.ID))
}
if err := traversalInst.BreadthFirst(ctx, traversal.IDPlan{
Root: adGroupID,
Delegate: func(ctx context.Context, tx graph.Transaction, segment *graph.IDSegment) ([]*graph.IDSegment, error) {
if nextQuery, err := newTraversalQuery(tx, segment, graph.DirectionInbound, searchCriteria...); err != nil {
return nil, err
} else {
var nextSegments []*graph.IDSegment

if err := nextQuery.FetchTriples(func(cursor graph.Cursor[graph.RelationshipTripleResult]) error {
for nextTriple := range cursor.Chan() {
if traversalMap.CheckedAdd(nextTriple.StartID.Uint32()) {
nextSegments = append(nextSegments, segment.Descend(nextTriple.StartID, nextTriple.ID))
} else {
memberships.AddShortcut(segment.Descend(nextTriple.StartID, nextTriple.ID))
}

return cursor.Error()
}); err != nil {
return nil, err
}

// Is this path terminal?
if len(nextSegments) == 0 {
memberships.AddPath(segment)
}

return nextSegments, nil
return cursor.Error()
}); err != nil {
return nil, err
}
},
}); err != nil {
log.Errorf("Error during traversal: %v", err)
}

coordC <- struct{}{}
}(adGroupID)
}

log.Infof("Finished submitting all groups to be traversed. Waiting...")
// Is this path terminal?
if len(nextSegments) == 0 {
memberships.AddPath(segment)
}

for finished := 0; finished < analysis.MaximumDatabaseParallelWorkers; {
<-coordC
finished++
return nextSegments, nil
}
},
}); err != nil {
return nil, err
}
}

close(coordC)
return memberships, nil
}

Expand Down
11 changes: 0 additions & 11 deletions packages/go/slicesext/slicesext_test.go
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove dead code

Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,6 @@ import (
"github.com/stretchr/testify/require"
)

var (
listEmpty = []int{}
listSingle = []int{0}
listDuo = []int{0, 1}
reversedListDuo = []int{1, 0}
listEven = []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}
reversedListEven = []int{15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}
listOdd = []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14}
reversedListOdd = []int{14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}
)

func TestFilter(t *testing.T) {
require.Equal(t, []int{1, 3}, slicesext.Filter([]int{1, 2, 3, 4}, isOdd))
require.Equal(t, []string{"bbbbbb", "cccccccc"}, slicesext.Filter([]string{"aaaa", "bbbbbb", "cccccccc", "dd"}, isLong))
Expand Down
Loading