SnaffCon / Snaffler

a tool for pentesters to help find delicious candy, by @l0ss and @Sh3r4 ( Twitter: @/mikeloss and @/sh3r4_hax )
GNU General Public License v3.0
2.04k stars 208 forks source link

DomainUserRules don't fire if ComputerTargets is specified #73

Closed hkelley closed 2 years ago

hkelley commented 2 years ago

I would like to use the DomainUsers discovery but this means I either need to:

a) Split the disco routine for computers/DFS out from users - and then create separate config options for each. This would allow ComputerTargets to be defined but dynamically discover usernames. or b) add an LDAP filter to the computer search so that I can use DomainDisco but select targets

I use option B today, in a roundabout way. Before I scan, I search AD for computers based on an LDAP filter. Then I edit the TOML file before I scan. This filtering is important (to me only?) because scanning servers is useful. Scanning workstations has a much higher noise:signal ratio.

Two questions: 1) Which approach do you think is more generally useful? 2) Should I bundle any change in the PR I'm already doing for denied share logging, or break it up into two?

l0ss commented 2 years ago
  1. To my mind, A is preferable - seems cleaner to me.
  2. I'm OK with either approach, up to you.
hkelley commented 2 years ago

A it is. The changes ended up being more extensive than I had expected so I will run this version for a few days before I create the PR.

Feel free to comment in the meantime.

hkelley:log_denied_shares

l0ss commented 2 years ago

looks good so far! I especially like how you've made the computer target LDAP filter an option so folks can set it in the config file.

While you're working on related stuff, can you: a) add a thing to the username enumeration thing that checks user accounts for SPNs and adds them to the target list if they have one? b) put in something like the inverse of the MyOptions.DomainUserMatchStrings thing that is on line 237 - but make it do equals (case-insensitive) on a user-defined list, rather than .Contains()? - basically i want a skip-list in addition to the existing match-list. c) add the following items to the defaults for DomainUserMatchStrings: MSOL adsync thycotic secretserver cyberark sccm configmgr

As you can see from my thinking in b), i'm concerned about the risk of this list becoming too long and creating a bunch of false positives and a bad performance hit, but I can see so much potential in this feature and it deserves to be extended.

I presume what you're working towards here is leveraging the username enumeration stuff with the new forking rules capability? If so, I'm very keen to see it - I think it's gonna be killer.

hkelley commented 2 years ago

Will do on the above.

Yes, I hadn't used the DomainUsers feature at all until about a week ago, but it does seem to be a natural fit for the RelayTargets mode. I ended up creating a "dummy" rule at the back of the relay list to hold the username regexes. This keeps my output a bit easier to organize/filter in Excel.


[[ClassifierRules]]
EnumerationScope = "FileEnumeration"
RuleName = "ConfigContentByExt"
MatchAction = "Relay"
RelayTargets = ["KeepConfigRegexRed", "KeepSqlStringYellow", "KeepSqlStringRed","KeepUsernameRegexRed"]
Description = "Files with these extensions will be subjected to a generic search for keys and such."
MatchLocation = "FileExtension"
WordListType = "Exact"
MatchLength = 0
WordList = ["\\.yaml", "\\.yml", "\\.toml", "\\.xml", "\\.json", "\\.config", "\\.ini", "\\.inf", "\\.cnf", "\\.conf", "\\.properties", "\\.env", "\\.dist", "\\.txt", "\\.sql", "\\.log", "\\.sqlite", "\\.sqlite3", "\\.fdb"]
Triage = "Green"

[[ClassifierRules]]
EnumerationScope = "ContentsEnumeration"
RuleName = "KeepConfigRegexRed"
MatchAction = "Snaffle"
Description = "A description of what a rule does."
MatchLocation = "FileContentAsString"
WordListType = "Regex"
MatchLength = 0
WordList = ["sqlconnectionstring\\s*=\\s*[\\'\\\"][^\\'\\\"]....",  "validationkey\\s*=\\s*[\\'\\\"][^\\'\\\"]....", "decryptionkey\\s*=\\s*[\\'\\\"][^\\'\\\"]....", "passwo?r?d\\s*=\\s*[\\'\\\"][^\\'\\\"]....", "CREATE (USER|LOGIN) .{0,200} (IDENTIFIED BY|WITH PASSWORD)", "(xox[pboa]-[0-9]{12}-[0-9]{12}-[0-9]{12}-[a-z0-9]{32})", "https://hooks.slack.com/services/T[a-zA-Z0-9_]{8}/B[a-zA-Z0-9_]{8}/[a-zA-Z0-9_]{24}", "aws[_\\-\\.]?key", "[_\\-\\.]?api[_\\-\\.]?key", "[_\\-\\.]oauth\\s*=", "client_secret", "secret[_\\-\\.]?(key)?\\s*=", "-----BEGIN( RSA| OPENSSH| DSA| EC| PGP)? PRIVATE KEY( BLOCK)?-----", "(\\s|\\'|\\\"|\\^|=)(A3T[A-Z0-9]|AKIA|AGPA|AROA|AIPA|ANPA|ANVA|ASIA)[A-Z0-9]{16}(\\s|\\'|\\\"|$)", "NVRAM config last updated", "enable password \\.", "simple-bind authenticated encrypt", "CredFileType="]
Triage = "Red"

[[ClassifierRules]]
EnumerationScope = "ContentsEnumeration"
RuleName = "KeepSqlStringYellow"
MatchAction = "Snaffle"
Description = "Match SQL connection strings that appear to use integrated security (so no passwords)."
MatchLocation = "FileContentAsString"
WordListType = "Regex"
MatchLength = 0
WordList = ["Data Source=.+Integrated Security=true"]
Triage = "Yellow"

[[ClassifierRules]]
EnumerationScope = "ContentsEnumeration"
RuleName = "KeepSqlStringRed"
MatchAction = "Snaffle"
Description = "Match SQL connection strings that appear to have a password."
MatchLocation = "FileContentAsString"
WordListType = "Regex"
MatchLength = 0
WordList = ["Data Source=.+Password=.+"]
Triage = "Red"

[[ClassifierRules]]
EnumerationScope = "ContentsEnumeration"
RuleName = "KeepUsernameRegexRed"
MatchAction = "Snaffle"
Description = "Matches on potential service account names."
MatchLocation = "FileContentAsString"
WordListType = "Regex"
MatchLength = 0
WordList = []
Triage = "Red"
l0ss commented 2 years ago

this looks awesome. please feel free to add your rules to the defaults if you've tested them out in a couple of real envs.

just had a thought that if we made a rule where it sticks "DOMAIN\" and "domain.fqdn\" in front of each account, we could have a higher level of confidence in the match?

hkelley commented 2 years ago

Yes, I wondered about that domain specifier too. Maybe we make that an option.

I think this is one of those red vs. blue differences in perspective. As an attacker (who only needs one good match from the tool), that noise reduction would be good. As a defender, I'd want to find every single text file a developer left, which probably won't be neatly formatted enough to match the fully-qualified.

If I were to do implement the feature, I'd probably add a bool option like "DomainUserMatchSimpleFormat". If true, just the samaccountname, if false, NETBIOS-style and UPN-style names would go into the regex.

l0ss commented 2 years ago

Even if we don't add the domain to every single username produced by this function we'd probably want to add it to the "Administrator" user, and maybe some other false-positive-prone entries if they exist - i'm thinking about orgs that have renamed Administrator to 'admin', or have highly privileged users with names like 'desktop' or 'support' or 'helpdesk' that will turn up all over the place when the account isn't actually being referenced.

hkelley commented 2 years ago

Agreed. I added two more bits to the PR: 1) a name format enum and config array: NetBIOS, sAMAccountName, UPN 2) changed the DomainUsers exclude/ignore list to DomainUserStrictStrings. These common usernames will still be included in the regexes but only with NetBIOS or UPN formats, not the simple SAM name (even if SAM name was the only format requested)

l0ss commented 2 years ago

sounds good! let me know when you're ready for some extra testing/review.

hkelley commented 2 years ago

Should be fixed in #79