Add lockout-safe password spraying controls - #1353
Conversation
Add timing and lockout-safety controls for password sprays: - --delay: fixed delay in seconds between every authentication attempt (additive with --jitter) - --spray-window / --spray-attempts: pace password rounds so the domain badPwdCount resets before accounts lock out - --abort-on-lockout: stop spraying a target as soon as an account lockout is detected (default remains warn-and-continue) Wire failed-login and lockout tracking through the SMB and LDAP login paths so lockouts feed the --gfail-limit/--fail-limit/--ufail-limit brakes and the abort flag consistently across plaintext, hash, and Kerberos authentication. Skip the delay/jitter sleep once a fail-limit has tripped, and skip the spray-window wait once a spray is aborted. Add e2e coverage for --delay, --spray-window, and --spray-attempts.
|
Hi and thanks for the PR, but this looks like it's mostly duplicate to |
|
I personally would find this pretty useful, currently have to use repos such as this one: https://github.com/GabrielDuschl/SmartSpray In order to password spray in production domains. Unless I'm missing something, |
|
Hey @NeffIsBack just removed --delay, is this still duplicate?
|
|
Okay so I have two thoughts about these kind of password spraying attacks. First, I think having a check on lockout responses would be really nice to prevent accidently locking entire AD environments. However, this is something that I personally would like to have as a config option so I can set it permanently to prevent such accidental lockouts. The second part, personally I rarely (or actually never) do actual bruteforce attacks live over the network in prod environments because of the risk of locking out accounts and at the same time low yield due to the low speed of network bruteforcing. Also, while you can check the default domain policy for lockout rules, as low privileged user you likely don't have read access to fggps so there is quite a risk of accidently assuming wrong lockout rules. So in my opinion, I don't think we should rely or implement such smart password spraying in NetExec itself. Authentication here is implemented to do basic checks e.g. password=username, but not for enhanced password spraying attacks. See also: #989 Tho, this is my opinion so to all other guys here, of course feel free to chime in if you disagree. |
|
I fully agree with your first point. On your second one, I think the 2 policy-based options still earn their keep in plenty of approved pentest environments, and are not automated since the user still needs to specify the window/count. At the same time I also get the concern of keeping nxc core to basic checks, maybe this is better implemented as a module? |
|
I've moved From my experience I’ve found password spraying to be very successful in prod environments. To clarify, this PR isn't "smart" spraying, NetExec isn't evaluating anything. It is completely up to the tester to specify the spray window, attempts per window, and the targetted users. NetExec simply follows the conditions given how light or heavy the spray is. Also, you make a good point with FGPPs. From my experience, FGPPs and pre-engagement password policy aside, lockouts will sometimes happen when you’re spraying. Sometimes it’s your fault and sometimes it’s theirs. You may incorrectly calculate your spray window/attempts (easier to miscalcualte with To @Mojo8898's point, would you prefer the password spraying handling as a module? @NeffIsBack |
A few thoughts:
Hmm okay, maybe I am just missing out on things.
Fair okay, I guess it doesn't hurt and people want to have this then let's integrate some spraying logic.
Oh no please not. If we integrate this then this must be implemented in the login logic itself so we don't build another login logic on-top of the existing one. That just gets really messy. |
|
Abort on lockout functionality is now implemented as an integer, where the default is As for ensuring that people don’t accidentally rely on other protocols for spraying that don’t support abort functionality yet, I would say that I rarely spray outside of SMB/LDAP if I’m targetting AD accounts. I can’t think of a clean solution to disclosing that the lockout is limited to SMB/LDAP, the alternative would be to implement the abort functionality as an option for SMB/LDAP, such as The abort functionality will (likely) not work for some protocols. For example, something like SSH, I don’t think it can tell the difference between a wrong password and a lockout. I’ll explore this further after the PR. I added a screenshot of what the abort looks like now (small 
|
|
Ah yeah what I was going for was lockout for Windows specific protocols. What I would like to prevent is that users think they now cannot lock out users with the config and then accidently try bruteforcing via WMI or WinRM. Extending the lockout to these protocols should be done if we somehow can detect a lockout based on the response of pywinrm.
I think that looks cool, although we should make it red so we make sure that the user understands the danger. Also, I am not sure if prompting multiple times might be confusing. Imo having one "Are you sure you know what you are doing??" should be the fallback. If the user still continues that is now their problem |
|
For WMI, there’s no real solution that I could find atm, but I implemented a workaround in my local branch by pairing it with Kerberos (and/or SMB), but it's funky and gets muddy fast. Holding off pushing to my fork until we brainstorm. WinRM's another issue: since it runs over HTTP, lockout status is unclear there too. What do you think about a warning message like "Lockouts not detected via WinRM" on a spray? Keeps each protocol handling as its own thing instead of hacking SMB/Kerberos/LDAP under WinRM/WMI and muddying the logic. Could also stick a warning at the end of Also, double-prompting "are you sure you know what you're doing?" on top of user confirmation feels off-brand for NetExec. One prompt already pushes it for usual behavior, two feels like too much. Curious what others think. @NeffIsBack @zblurx @Marshall-Hallenbeck @mpgn |
Do you know if it is impossible to detect, or if there just isn't support for proper error messages yet? At least from WinRM I would have expected some different response from the DC should a lockout happen. Same actually for wmi since this is just RPC at the end of the day from what I know. Maybe we need to adjust the current implementation to get that information.
Yeah worst case if we can't handle lockouts on that protocols we should warn somehow. At the bottom of the help page likely won't work since that is handled by argparse, but maybe somewhere else. We could even detect and perhaps prompt (not sure about that yet) when we have more than 1 user for authentication.
Agreed :) |
…inRM Kerberos auth support. Also warns when spraying passwords via WMI/WinRM without -k
|
From what I can tell, it’s not possible to implement this strictly via WMI/WinRM, and would likely muddy NetExec's core WMI/WinRM auth logic. I’ve also added a warning shown when a password list is used against WMI or WinRM: “ I’ve also implemented the same lockout logic into WMI/WinRM from SMB/LDAP. This can be done by tying the logic to Kerberos auth ( P.S., My most recent commit also properly implements Kerberos auth via WinRM. |
|
Okay I see. I think let's just not support the protocols at all before people get confused. Then we just have to be clear in the proto arg description and the wiki that only SMB and LDAP are supported protocols and hope that people can read. |
…+ full WinRM Kerberos auth support. Also warns when spraying passwords via WMI/WinRM without -k" This reverts commit 4857f08.
|
Sounds good to me! I've reverted my last commit. My most recent commit makes the warning message red on account lockouts, and also surfaces that lockouts aren’t detected via WMI/WinRM when a user runs FYI: I also removed the WinRM kerberos support in my most recent revert, I may open another PR after this one to implement the functionality. |
|
|
||
| def proto_args(parser, parents): | ||
| winrm_parser = parser.add_parser("winrm", help="own stuff using WINRM", parents=parents, formatter_class=DisplayDefaultsNotNone) | ||
| winrm_parser = parser.add_parser("winrm", help="own stuff using WINRM", parents=parents, formatter_class=DisplayDefaultsNotNone, epilog="[!] Account lockout is NOT detected over WINRM, so the lockout-abort safety will not trigger on this protocol. Use SMB or LDAP for lockout-aware password spraying.") |
There was a problem hiding this comment.
Let's not put this in the help page. Thought about it, either we implement a check on windows protos that do not support this and exit or we simply add it to each proto arg instead of the cli itself (might even be the better solution). Thoughts?
| @@ -1,5 +1,5 @@ | |||
| def proto_args(parser, parents): | |||
| wmi_parser = parser.add_parser("wmi", help="own stuff using WMI", conflict_handler="resolve", parents=parents) | |||
| wmi_parser = parser.add_parser("wmi", help="own stuff using WMI", conflict_handler="resolve", parents=parents, epilog="[!] Account lockout is NOT detected over WMI, so the lockout-abort safety will not trigger on this protocol. Use SMB or LDAP for lockout-aware password spraying.") | |||
| _abort_raw = nxc_config.get("nxc", "abort_on_lockout", fallback="3").strip().lower() | ||
| if _abort_raw in ("true", "yes"): | ||
| abort_on_lockout = 3 | ||
| elif _abort_raw in ("false", "no", ""): | ||
| abort_on_lockout = 0 | ||
| else: | ||
| try: | ||
| abort_on_lockout = max(0, int(_abort_raw)) | ||
| except ValueError: | ||
| abort_on_lockout = 3 |
There was a problem hiding this comment.
This should just be an integer. Everything else is much too confusing (and probably not necessary).
| def register_lockout(self, username): | ||
| global global_lockouts, spray_abort_all | ||
|
|
||
| if not abort_on_lockout: | ||
| return | ||
|
|
||
| with lockout_lock: | ||
| if spray_abort_all: | ||
| self.spray_aborted = True | ||
| return | ||
|
|
||
| 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 | ||
| self.spray_aborted = True | ||
| else: | ||
| global_lockouts = 0 | ||
|
|
There was a problem hiding this comment.
Let's just raise an exception when we want to quit. Then we don't need all of these "if ... False" statements and don't continue to iterate over all credentials (which can take some time). Additionally, we can still cleanly exit with proper disconnect calls from the proto flow.
There was a problem hiding this comment.
Looking at the code we should then probably move the disconnect call to the error handling of the proto flow instead of the normal execution so we always try to disconnect.
| owned[user_index] = True | ||
| if not self.args.continue_on_success: | ||
| return True | ||
| spray_attempts = max(1, self.args.spray_attempts) |
| if not self.args.continue_on_success: | ||
| return True | ||
| spray_attempts = max(1, self.args.spray_attempts) | ||
| if self.args.spray_window and not (self.spray_aborted or spray_abort_all) and secr_index < len(secret) - 1 and (secr_index + 1) % spray_attempts == 0: |
There was a problem hiding this comment.
This if statement is quite complicated, what is it trying to express? (Please add a comment and try to make it simpler)
| self.logger.info( | ||
| f"Completed {secr_index + 1} password round(s); sleeping {self.args.spray_window} " | ||
| f"second(s) so the lockout counter resets before the next round" | ||
| ) |
There was a problem hiding this comment.
Please only one line per logging statement
| if self.args.spray_window and not (self.spray_aborted or spray_abort_all) and user_index < len(username) - 1 and (user_index + 1) % spray_attempts == 0: | ||
| self.logger.info( | ||
| f"Completed {user_index + 1} attempt(s); sleeping {self.args.spray_window} " | ||
| f"second(s) so the lockout counter resets before the next attempt" | ||
| ) |
Description
Added timing and lockout-safety controls for password sprays:
Wired failed-login and lockout tracking through the SMB and LDAP login paths so changes don't break --gfail-limit/--fail-limit/--ufail-limit.
Claude Code was utilized to assist in the development of this PR.
Type of change
Insert an "x" inside the brackets for relevant items (do not delete options)
Setup guide for the review
Screenshots (if appropriate):
Checklist:
Insert an "x" inside the brackets for completed and relevant items (do not delete options)
poetry run ruff check ., use--fixto automatically fix what it can)tests/e2e_commands.txtfile if necessary (new modules or features are required to be added to the e2e tests)