feat(winrm): add ntds_shadow module - #1373
Conversation
|
Sorry for opening a new PR. Something went wrong while cleaning up the previous one. This PR contains the final version. Looking forward to your feedback! |
No worries about that :) Thanks for the PR |
|
So looking at the code it is much better than #1369, but there is still a lot of overhead that could be removed. E.g. what is the reason for the custom command execution implementation? I uploaded an AGENTS.md file so I will give codex reviews a try to see how well it can detect the overhead and how good it is at reviewing stuff. @codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7086dba6d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if parts[2].upper() == "ENABLED": | ||
| log.success("SeBackupPrivilege confirmed") | ||
| return True | ||
| log.fail("SeBackupPrivilege is present but disabled") | ||
| return False |
There was a problem hiding this comment.
Accept a present backup privilege even when disabled
For the intended Backup Operators scenario, whoami commonly reports SeBackupPrivilege as disabled because it remains in the token but is enabled by the operation that needs it. Rejecting that state aborts before DiskShadow or robocopy /b can run, so valid Backup Operators accounts cannot use the module; check that the privilege is present and enable it when necessary rather than requiring the whoami state to already be ENABLED.
Useful? React with 👍 / 👎.
| lines = [ | ||
| "set verbose on", | ||
| "set context persistent nowriters", | ||
| f"set metadata {metadata_path}", |
There was a problem hiding this comment.
Resolve the configured NTDS database volume
On domain controllers where the NTDS database has been relocated, this always snapshots C: and later looks only for \Windows\NTDS\ntds.dit, so the acquisition fails even though the account has the required privilege. Read the configured NTDS database path, snapshot its containing volume, and derive the path within that snapshot instead of assuming the default installation layout.
Useful? React with 👍 / 👎.
| ps = ( | ||
| "$p=[System.Diagnostics.Process]::new();" | ||
| "$p.StartInfo.FileName='cmd.exe';" | ||
| f"$p.StartInfo.Arguments='/c {safe}';" |
There was a problem hiding this comment.
Reuse the WinRM command execution primitive
Every simple command is unnecessarily wrapped in a PowerShell-created Process, asynchronous stream tasks, and JSON serialization even though the underlying WinRM client's execute_cmd already returns (stdout, stderr, returncode). This adds an extra remote process and serialization round trip to every operation and introduces bespoke quoting and error handling that diverges from the protocol implementation; call the existing command primitive directly.
Useful? React with 👍 / 👎.
|
Hmm okay that was underwhelming |
iirc this wrapper was already part of my initial implementation because direct execute_cmd returned a WSMan fault with code 5 for the Backup Operators session. So I used it again when simplifying the module. I’ll test it with execute_cmd sometime over the next couple of days |
|
To use Codex here, create a Codex account and connect to github. |
Description
This adds an
ntds_shadowmodule for WinRM.With Backup Operators privileges on a domain controller, NetExec’s
backup_operatormodule can retrieve hashes from the local SAM. To retrieve domain account hashes, we also needNTDS.ditand the SYSTEM hive.Because
NTDS.ditis locked while the domain controller is running, getting these files through DiskShadow normally requires several manual steps: SeBackupPrivilege manual steps.The module checks for
SeBackupPrivilege, creates and exposes a DiskShadow snapshot, and downloadsNTDS.ditand the SYSTEM hive. It checks if the snapshot was exposed correctly, verifies the downloaded files, removes the remote files and snapshot, and prints theimpacket-secretsdumpcommand needed to process them locally.Hash extraction is not part of this module. It only gets the files and shows the user the command needed for the next step.
The module does not require extra software, DLLs, registry changes or GPO changes on the target. The module only uses built-in Windows tools.
I tried to keep the module as small as possible by looking at existing modules such as
backup_operator.py,ntdsutil.pyandntds-dump-raw.pyand reusing parts of the same structure and patterns where possible.The snapshot parsing, exposure checks, download checks and cleanup are kept in this module because they are specific to this workflow. Reusing the other module logic here would mean losing some of the checks and cleanup handling.
Closes #1366
AI assistance
I used Codex with
gpt-5.6-solmainly for the unit tests and an extra review of the module. Claude Sonnet 4.6 helped with some smaller refactoring changes. I checked the changes myself, ran Ruff and the local tests and tested the full workflow in my own labs.Type of change
Setup guide for the review
I tested the module from Kali Linux against:
The target must be a domain controller with WinRM enabled. The account needs
SeBackupPrivilege, for example through membership of the Backup Operators group.No extra software, DLLs, registry changes or GPO changes are needed on the target.
Example:
A successful run downloads ntds.dit and SYSTEM.hive, checks both files, removes the remote files and snapshot, and prints the
impacket-secretsdumpcommand for offline processing.Local checks:
git diff --checkpassedI also tested the full workflow in the lab, including processing the files with
impacket-secretsdumpand checking that the remote cleanup worked.Cleanup only uses the exact snapshot GUID created during the run. The module does not use a broad command such as delete shadows all.
Relevant sources:
Screenshots
Checklist
tests/e2e_commands.txt