-
Notifications
You must be signed in to change notification settings - Fork 64
Description
I've been working on rewriting some of the queries in Max's brain to work with the new database structure, and it's looking like it's going to require a rewrite of most of the queries. At this point, I'm trying to figure out how best to submit the changes here.
Couple of options I'm seeing:
- Drop support for OG bloodhound, only support CE
- Add some manner of specifying (or detecting?) which version of BH the database is based on
- This has the side effect of making max even bigger and harder to maintain, since it's basically two code bases in one
- Maintain a separate branch in the same repo (ie
mainfor legacy andmain-cefor bloodhound-ce support - Hard fork to a separate repo for CE support
- Complete rewrite to try and make better use of the new database schemas / features (rather than just replacing the existing queries with as much as a like-for-like as possible)
- Try to implement some kind of "make BH-CE compatible with Max.py" function, and keep the rest of the existing code base.
Looking around, doesn't seem to be a super clear "standard" approach. Posing this issue here to see what the feel is @knavesec
Technical details
Ok, so for some (I'm sure quite smart and not annoying at all) reason, BH no longer uses simple things like .high_value or .owned as clear attributes on a given node. In other words, doing queries like MATCH (u:User {owned:True}) no longer works. Instead, those attributes are shoved into a string delimited single field called system_tags. Basically, it maps like this:
n.high_value = True -> n.system_tags = "admin_tier_0"
n.owned = True -> n.system_tags = "owned"
n.owned = True, n.high_value = True -> n.system_tags = "admin_tier_0 owned"
This, of course, makes queries a lot harder. Using the built-in quires in BH-CE as a reference, it looks like n.system_tags isn't even guaranteed to exist, and since it's a string attribute, not a boolean attribute, just calling CONTAINS isn't enough. Therefore, the basic structure of a simple query changes. (Note the addition of COALESCE to ensure that there's an empty string if n.system_tags doesn't exist.
Old:
MATCH (u:User {owned=True}) RETURN uNew:
MATCH (u:User) WHERE COALESCE(u.system_tags, '') CONTAINS 'owned' RETURN uThis, of course, makes queries gross to write, and gets gross fast.
A more complex example, paths from owned to HVTs
Old:
MATCH shortestPath((n {owned:True})-[*1..]->(m {highvalue:True})) RETURN DISTINCT n.nameNew (This is directly from the BH examples):
MATCH p=shortestPath((s)-[:Owns|GenericAll|GenericWrite|WriteOwner|WriteDacl|MemberOf|ForceChangePassword|AllExtendedRights|AddMember|HasSession|GPLink|AllowedToDelegate|CoerceToTGT|AllowedToAct|AdminTo|CanPSRemote|CanRDP|ExecuteDCOM|HasSIDHistory|AddSelf|DCSync|ReadLAPSPassword|ReadGMSAPassword|DumpSMSAPassword|SQLAdmin|AddAllowedToAct|WriteSPN|AddKeyCredentialLink|SyncLAPSPassword|WriteAccountRestrictions|WriteGPLink|GoldenCert|ADCSESC1|ADCSESC3|ADCSESC4|ADCSESC6a|ADCSESC6b|ADCSESC9a|ADCSESC9b|ADCSESC10a|ADCSESC10b|ADCSESC13|SyncedToEntraUser|CoerceAndRelayNTLMToSMB|CoerceAndRelayNTLMToADCS|WriteOwnerLimitedRights|OwnsLimitedRights|CoerceAndRelayNTLMToLDAP|CoerceAndRelayNTLMToLDAPS|Contains|DCFor|TrustedBy*1..]->(t))
WHERE COALESCE(t.system_tags, '') CONTAINS 'admin_tier_0'
AND s<>t
AND COALESCE(s.system_tags, '') CONTAINS 'owned'
RETURN DISTINCT s.nameThis gets even grosser when we want to start modifying things. For example, to mark something as owned (as with --mark-owned), we can no longer just do
MATCH (u.User {username=JOHN.DOE@EXAMPLE.COM}) SET u.owned = TrueInstead, we have to do something like
MATCH (u:User {username=JOHN.DOE@EXAMPLE.COM})
SET u.system_tags=(
CASE WHEN COALESCE(n.system_tags, "") CONTAINS "owned"
THEN n.system_tags
ELSE trim(COALESCE(n.system_tags, "") + " owned" )
END
)To break that down:
- Match the user
- Use a
CASEto check if the user is already owned - If so, leave as is
- Otherwise:
COALESCEwith an empty string so we have a non-null- Append
ownedwith a space, in case something already exists
- This prevents `u.system_tags = "admin_tier_0owned" TRIMthe string, in case it doesn't already have some tag
- This preventsu.system_tags = " owned"
The point
The point of these examples is that it's not quite as simple as pointing max at a new DB. I'm still working on getting all the queries rewritten and optimized, but it feels like we'll have to make some change here to keep max useful, since bloodhound legacy isn't getting updates anymore.
Open to ideas, thoughts, whatever. Max is a good boy, I don't want to put him down because someone changed the DB schema