-
Notifications
You must be signed in to change notification settings - Fork 756
Add lockout-safe password spraying controls #1353
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
base: main
Are you sure you want to change the base?
Changes from 10 commits
b136578
8bd2a66
4fa1a97
14ff6bb
9320782
4857f08
11d253c
9e2a9d6
65fcdb6
f83e711
b5544ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,10 +12,11 @@ | |
| from dns import resolver, rdatatype | ||
| from socket import AF_UNSPEC, SOCK_DGRAM, IPPROTO_IP, AI_CANONNAME, getaddrinfo | ||
|
|
||
| from nxc.config import pwned_label | ||
| from nxc.config import pwned_label, abort_on_lockout | ||
| from nxc.helpers.logger import highlight | ||
| from nxc.loaders.moduleloader import ModuleLoader | ||
| from nxc.logger import nxc_logger, NXCAdapter | ||
| from nxc.console import nxc_console | ||
| from nxc.context import Context | ||
| from nxc.paths import NXC_PATH | ||
| from nxc.protocols.ldap.laps import laps_search | ||
|
|
@@ -28,6 +29,13 @@ | |
| fail_lock = Lock() | ||
| global_failed_logins = 0 | ||
| user_failed_logins = {} | ||
| lockout_lock = Lock() | ||
| global_lockouts = 0 | ||
| spray_abort_all = False | ||
|
|
||
|
|
||
| class SprayAbort(Exception): | ||
| pass | ||
|
|
||
|
|
||
| def get_host_addr_info(target, force_ipv6, dns_server, dns_tcp, dns_timeout): | ||
|
|
@@ -181,6 +189,8 @@ def __init__(self, args, db, target): | |
| self.proto_flow() | ||
| except FileNotFoundError as e: | ||
| self.logger.error(f"File not found error on target {target}: {e}") | ||
| except SprayAbort: | ||
| self.logger.debug("Spray aborted on lockout") | ||
| except Exception as e: | ||
| if "ERROR_DEPENDENT_SERVICES_RUNNING" in str(e): | ||
| self.logger.error(f"Exception while calling proto_flow() on target {target}: {e}") | ||
|
|
@@ -255,14 +265,16 @@ def proto_flow(self): | |
| self.output_filename = os.path.join(base_log_dir, filename_pattern) | ||
|
|
||
| self.print_host_info() | ||
| if self.login() or (self.username == "" and self.password == "" and self.protocol != "mssql"): | ||
| self.logger.debug("Calling command arguments") | ||
| self.call_cmd_args() | ||
| if self.args.module: | ||
| self.load_modules() | ||
| self.logger.debug("Calling modules") | ||
| self.call_modules() | ||
| self.disconnect() | ||
| try: | ||
| if self.login() or (self.username == "" and self.password == "" and self.protocol != "mssql"): | ||
| self.logger.debug("Calling command arguments") | ||
| self.call_cmd_args() | ||
| if self.args.module: | ||
| self.load_modules() | ||
| self.logger.debug("Calling modules") | ||
| self.call_modules() | ||
| finally: | ||
| self.disconnect() | ||
|
|
||
| def call_cmd_args(self): | ||
| """Calls all the methods specified by the command line arguments | ||
|
|
@@ -340,6 +352,26 @@ def over_fail_limit(self, username): | |
|
|
||
| return False | ||
|
|
||
| def register_lockout(self, username): | ||
| global global_lockouts, spray_abort_all | ||
|
|
||
| if not abort_on_lockout: | ||
| return | ||
|
|
||
| with lockout_lock: | ||
| if spray_abort_all: | ||
| raise SprayAbort | ||
|
|
||
| global_lockouts += 1 | ||
| if global_lockouts < abort_on_lockout: | ||
| return | ||
|
|
||
| answer = nxc_console.input(f"[bold red]\\[!] {global_lockouts} lockout responses detected, would you like to quit? \\[Y/n] [/]") | ||
| if answer.strip().lower() in ("y", "yes", ""): | ||
| spray_abort_all = True | ||
| raise SprayAbort | ||
| global_lockouts = 0 | ||
|
|
||
|
NeffIsBack marked this conversation as resolved.
|
||
| def query_db_creds(self): | ||
| """Queries the database for credentials to be used for authentication. | ||
|
|
||
|
|
@@ -488,6 +520,9 @@ def try_credentials(self, domain, username, owned, secret, cred_type, data=None) | |
| if self.args.continue_on_success and owned: | ||
| return False | ||
|
|
||
| if self.over_fail_limit(username): | ||
| return False | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's duplicate, see below. Or what was the reason for adding that here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, that's redundant with the check inside |
||
| if self.args.jitter: | ||
| jitter = self.args.jitter | ||
| if "-" in jitter: | ||
|
|
@@ -578,22 +613,44 @@ def login(self): | |
| if not (username[0] or secret[0] or domain[0]): | ||
| return False | ||
|
|
||
| # Spray pacing is only exposed by protocols that support it (SMB/LDAP) | ||
| spray_window = getattr(self.args, "spray_window", 0) | ||
| spray_attempts = getattr(self.args, "spray_attempts", 1) | ||
|
|
||
| if not self.args.no_bruteforce: | ||
| for secr_index, secr in enumerate(secret): | ||
| if spray_abort_all: | ||
| raise SprayAbort | ||
| for user_index, user in enumerate(username): | ||
| if spray_abort_all: | ||
| raise SprayAbort | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's overkill (especially two times lol). Just check in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, fair (and lol at the double check 🤣) Removed all the loop-level checks and moved a single |
||
| if self.try_credentials(domain[user_index], user, owned[user_index], secr, cred_type[secr_index], data[secr_index]): | ||
| owned[user_index] = True | ||
| if not self.args.continue_on_success: | ||
| return True | ||
| # Pause between rounds so badPwdCount resets before the next batch can trip a lockout | ||
| completed_batch = (secr_index + 1) % spray_attempts == 0 | ||
| is_last_round = secr_index == len(secret) - 1 | ||
| if spray_window and completed_batch and not is_last_round: | ||
| self.logger.info(f"Completed {secr_index + 1} password round(s); sleeping {spray_window} second(s) so the lockout counter resets before the next round") | ||
| sleep(spray_window) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the upper for loop iterates over secrets (e.g. passwords). Therefore we can just move this between the two loops and just wait every round. That should eliminate the need for checking for "last batch" if we just skip secret index 0.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Additionally, since we are bruteforcing potentially hundreds of users per round we should measure the timing since the last check and sleep the remaining time (since we might already have exceeded the throttle window once we tried the last user) Although we have to make sure to not undercut the window. Thoughts? Maybe check a time per window if that doesn't take too much time?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I moved the sleep to the top of the outer loop and to skip index 0, which drops the last-batch check. It's more timing-aware now: We stamp Kept |
||
| else: | ||
| if len(username) != len(secret): | ||
| self.logger.error("Number provided of usernames and passwords/hashes do not match!") | ||
| return False | ||
| for user_index, user in enumerate(username): | ||
| if spray_abort_all: | ||
| raise SprayAbort | ||
| if self.try_credentials(domain[user_index], user, owned[user_index], secret[user_index], cred_type[user_index], data[user_index]) and not self.args.continue_on_success: | ||
| owned[user_index] = True | ||
| if not self.args.continue_on_success: | ||
| return True | ||
| # Pause between attempts so badPwdCount resets before the next batch can trip a lockout | ||
| completed_batch = (user_index + 1) % spray_attempts == 0 | ||
| is_last_attempt = user_index == len(username) - 1 | ||
| if spray_window and completed_batch and not is_last_attempt: | ||
| self.logger.info(f"Completed {user_index + 1} attempt(s); sleeping {spray_window} second(s) so the lockout counter resets before the next attempt") | ||
| sleep(spray_window) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have only one round of usernames (since we do username=password) I don't see why we would need this here anyway. Or am I missing something?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, no-bruteforce is one password per user, so there are no rounds to pace and |
||
|
|
||
| def mark_pwned(self): | ||
| return highlight(f"({pwned_label})" if self.admin_privs else "") | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -375,6 +375,11 @@ def kerberos_login(self, domain, username, password="", ntlm_hash="", aesKey="", | |
| f"{self.domain}\\{self.username}{used_ccache} {error!s}", | ||
| color="magenta" if error in ldap_error_status else "red", | ||
| ) | ||
| if error == "KDC_ERR_CLIENT_REVOKED": | ||
| self.inc_failed_login(self.username) | ||
| self.register_lockout(self.username) | ||
| elif error not in ldap_error_status: | ||
| self.inc_failed_login(self.username) | ||
| return False | ||
| except (KeyError, KerberosException, OSError) as e: | ||
| self.logger.fail( | ||
|
|
@@ -422,20 +427,33 @@ def kerberos_login(self, domain, username, password="", ntlm_hash="", aesKey="", | |
| f"{self.domain}\\{self.username}{' from ccache' if useCache else f':{process_secret(kerb_pass)}'} {error!s}", | ||
| color="magenta" if error in ldap_error_status else "red", | ||
| ) | ||
| if error == "KDC_ERR_CLIENT_REVOKED": | ||
| self.inc_failed_login(self.username) | ||
| self.register_lockout(self.username) | ||
| elif error not in ldap_error_status: | ||
| self.inc_failed_login(self.username) | ||
| return False | ||
| except Exception as e: | ||
| error_code = str(e).split()[-2][:-1] | ||
| self.logger.fail( | ||
| f"{self.domain}\\{self.username}:{process_secret(self.password)} {ldap_error_status.get(error_code, '')}", | ||
| color="magenta" if error_code in ldap_error_status else "red", | ||
| ) | ||
| if error_code in ("775", "KDC_ERR_CLIENT_REVOKED"): | ||
| self.inc_failed_login(self.username) | ||
| self.register_lockout(self.username) | ||
| elif error_code not in ldap_error_status: | ||
| self.inc_failed_login(self.username) | ||
| return False | ||
| else: | ||
| error_code = str(e).split()[-2][:-1] | ||
| self.logger.fail( | ||
| f"{self.domain}\\{self.username}{' from ccache' if useCache else f':{process_secret(kerb_pass)}'} {error_code!s}", | ||
| color="magenta" if error_code in ldap_error_status else "red", | ||
| ) | ||
| self.inc_failed_login(self.username) | ||
| if error_code in ("775", "KDC_ERR_CLIENT_REVOKED"): | ||
| self.register_lockout(self.username) | ||
| return False | ||
|
|
||
| def plaintext_login(self, domain, username, password): | ||
|
|
@@ -513,6 +531,9 @@ def plaintext_login(self, domain, username, password): | |
| f"{self.domain}\\{self.username}:{process_secret(self.password)} {ldap_error_status.get(error_code, '')}", | ||
| color="magenta" if (error_code in ldap_error_status and error_code != 1) else "red", | ||
| ) | ||
| self.inc_failed_login(self.username) | ||
| if error_code in ("775", "KDC_ERR_CLIENT_REVOKED"): | ||
| self.register_lockout(self.username) | ||
|
Comment on lines
+534
to
+536
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would NTLM return a "KDC_ERR_CLIENT_REVOKED"?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It wouldn't, good catch. That branch parses a numeric LDAP code, so the Kerberos string was dead; the real Kerberos path is handled separately via |
||
| return False | ||
| except OSError as e: | ||
| self.logger.fail(f"{self.domain}\\{self.username}:{process_secret(self.password)} {'Error connecting to the domain, are you sure LDAP service is running on the target?'} \nError: {e}") | ||
|
|
@@ -607,6 +628,9 @@ def hash_login(self, domain, username, ntlm_hash): | |
| f"{self.domain}\\{self.username}:{process_secret(nthash)} {ldap_error_status.get(error_code, '')}", | ||
| color="magenta" if (error_code in ldap_error_status and error_code != 1) else "red", | ||
| ) | ||
| self.inc_failed_login(self.username) | ||
| if error_code in ("775", "KDC_ERR_CLIENT_REVOKED"): | ||
| self.register_lockout(self.username) | ||
|
Comment on lines
+631
to
+633
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same fix here as above. Also spotted the identical copy-paste in two Kerberos-fallback branches (442/455) that parse numeric codes too, so I cleaned those for consistency.. all four numeric sites are now just |
||
| return False | ||
| except OSError as e: | ||
| self.logger.fail(f"{self.domain}\\{self.username}:{process_secret(self.password)} {'Error connecting to the domain, are you sure LDAP service is running on the target?'} \nError: {e}") | ||
|
|
||
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.
You can actually make arguments depend on each other. Take a look at how it's done with
--ntdsand its logic in the smb arg logic.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.
Switched to the native
make_requiredpattern like--ntds, so--spray-attemptsnow requires--spray-windowat the argparse level. Kept it one-way on purpose so--spray-windowalone is the common case (attempts defaults to 1 = wait after every round), so it works standalone;--spray-attemptsdoesn’t have anything to gate without a window to pair with it.