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

added ADCS #45

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

added ADCS #45

wants to merge 7 commits into from

Conversation

ajm4n
Copy link

@ajm4n ajm4n commented Sep 15, 2024

this works now :)

max.py Outdated
@@ -1519,8 +1728,7 @@ def main():
addspw = switch.add_parser("add-spw",help="Create 'SharesPasswordWith' relationships with targets from a file. Adds edge indicating two objects share a password (repeated local administrator)")
dpat = switch.add_parser("dpat",help="BloodHound Domain Password Audit Tool, run cracked user-password analysis tied with BloodHound through a Hashcat potfile & NTDS")
petmax = switch.add_parser("pet-max",help="Pet max, hes a good boy (pet me again, I say different things)")

# GETINFO function parameters
adcs = switch.add_parser("adcs",help="adcs")
Copy link
Owner

Choose a reason for hiding this comment

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

This is instantiated but never used. Use it and stay in line with the current code structure. Use adcs as a new module and structure it the same as get-info rather than having them combined

max.py Outdated
@@ -1555,7 +1763,8 @@ def main():
getinfo_switch.add_argument("--hvt-paths",dest="hvtpaths",default="",help="Return all paths from the input node to HVTs")
getinfo_switch.add_argument("--owned-paths",dest="ownedpaths",default=False,action="store_true",help="Return all paths from owned objects to HVTs")
getinfo_switch.add_argument("--owned-admins", dest="ownedadmins",default=False,action="store_true",help="Return all computers owned users are admins to")

getinfo_switch.add_argument("--adcs", dest="adcs", default=False, action="store_true", help="Perform AD CS ESC attack detection queries")
Copy link
Owner

Choose a reason for hiding this comment

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

Similar to the comment above, separate get-info and adcs modules. Made the adcs module more granular with specific flags to be able to select one specific attack path required

max.py Outdated
@@ -243,6 +243,214 @@ def get_info(args):
}
}


if args.adcs:
Copy link
Owner

Choose a reason for hiding this comment

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

Put all these in a new function/module similar to get info. Provide the ability for the user to select just one query rather than requiring all of them to be run

Copy link
Owner

Choose a reason for hiding this comment

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

it can literally be as simple as --esc1 --esc2 --esc3

max.py Outdated
cols = []
data_format = "row"

if args.adcs:
Copy link
Owner

Choose a reason for hiding this comment

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

As I said above, provide the ability to make this more granular. Provide an --all flag if you want to be able to pull everything at once

max.py Outdated
data_format = "row"

if args.adcs:
for query_name, query_info in queries.items():
Copy link
Owner

Choose a reason for hiding this comment

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

In general the coding here is very inefficient. You add your ADCS queries to the full list, then just iterate through the full list and identify the ADCS related ones. Again, speaks to separating it out to a new module. The arg parser is a mutually exclusive (mutex) grouping so if you select ADCS then you can't get any other datapoints, so there's no reason to add to the full queries then parse them back out

max.py Outdated
@@ -900,31 +1108,31 @@ def dpat_func(args):
"label" : "Enabled User Accounts Cracked"
},
{
'query' : "match p = (k:Group)<-[:MemberOf*1..]-(m) where k.highvalue = true WITH [ n in nodes(p) WHERE n:User] as ulist UNWIND (ulist) as u RETURN DISTINCT u.enabled,u.ntds_uname,u.password,u.nt_hash",
'query' : "MATCH p=(u:User {cracked:true})-[r:MemberOf*1..]->(g:Group {highvalue:true}) RETURN DISTINCT u.enabled,u.ntds_uname,u.password,u.nt_hash",
Copy link
Owner

Choose a reason for hiding this comment

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

I like these additions, but please separate them out into a new PR if desired. Too much going on in this PR

max.py Outdated
@@ -210,7 +210,7 @@ def get_info(args):
"columns" : ["ObjectName","SID","DomainName","ForeignObjectName"]
},
"unsupos" : {
"query" : "MATCH (c:Computer) WHERE toLower(c.operatingsystem) =~ '.*(2000|2003|2008|xp|vista| 7 |me).*' RETURN c.name,c.operatingsystem",
"query" : "MATCH (c:Computer) WHERE c.operatingsystem =~ '.*(2000|2003|2008|xp|vista| 7 |me).*' RETURN c.name,c.operatingsystem",
Copy link
Owner

Choose a reason for hiding this comment

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

this needs to be lowered, undo this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants