-
Notifications
You must be signed in to change notification settings - Fork 127
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
BED-4965 - Owns/Owner Rework #993
base: main
Are you sure you want to change the base?
Conversation
* Added inheritance condition for WriteOwner abuse, added comments to Owns/WriteOwner logic * Saving changes to post Owns/WriteOwner
# Conflicts: # cmd/api/src/analysis/ad/post.go # packages/go/graphschema/ad/ad.go
…ing OWNER RIGHTS are present
… computer object ownership and WriteOwner permissions
…Hound versions prior to change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a commit to the changeset that highlights a few requested fixes via FIXME
comments. Additionally we'd ask that the following changes also be made:
Please adhere to the standard outlined here for your conditional statements: https://specterops.atlassian.net/wiki/spaces/DEVRES/pages/1507363/Golang+Coding+and+Formatting+Standards#GolangCodingandFormattingStandards-InlineallInitializerStatements
Description
This change was made to account for cases where the OWNER RIGHTS SID (S-1-3-4) is explicitly granted permissions on AD objects, which in some cases renders implicit owner rights non-abusable.
Please refer to https://specterops.atlassian.net/wiki/spaces/BE/pages/750157858/Owns+WriteOwner for details.
Motivation and Context
This PR addresses: https://specterops.atlassian.net/browse/BED-4965
How Has This Been Tested?
Integration tests were written for post-processed edges. Ingest edges were tested manually.
A test environment including test cases for OWNER RIGHTS permissions was configured using a PowerShell script included in the linked Confluence page. After generating test data for two domains, one where implicit owner rights were blocked and another where they were not, tests included:
... and confirming that expected ingest and post-processed edges were created or removed, where appropriate.
Also ran and passed:
This change does not account for inheritance hashes for ACEs but will require an update after that initiative is complete.
Screenshots (optional):
Types of changes
Checklist: