fix: harden NTLM hash validation and improve cross-forest secretsdump#320
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #320 +/- ##
=======================================
Coverage 78.78% 78.78%
=======================================
Files 439 439
Lines 124638 124664 +26
=======================================
+ Hits 98190 98215 +25
- Misses 26448 26449 +1
🚀 New features to boost your workflow:
|
242c456 to
3457219
Compare
**Added:** - Validation to reject NTLM hashes that are not exactly 32 hex characters to prevent relay artifacts and partial captures from entering state - Tests to ensure malformed NTLM hashes are rejected and only valid hashes are accepted - Tests to verify non-NTLM hashes (e.g., AES256) of any length are accepted **Changed:** - Updated all NTLM hash test fixtures to use canonical 32-character values for consistency and to avoid confusion with validation logic
**Changed:** - Domain admin indicator detection now only trusts tool-output-backed evidence, specifically a krbtgt hash in the payload; agent self-reporting via `has_domain_admin: true` is ignored to prevent false positives from LLM or agent hallucination (`parsing.rs`, related tests updated) - `resolve_da_path` always returns a fixed path "secretsdump -> krbtgt hash", ignoring agent-provided path fields for safety and consistency; related tests updated to match new logic (`admin_checks.rs`, related tests updated) - Test formatting improved for clarity and consistency, including more compact `.push` and function call lines, and use of multi-line arguments for long values in test helpers (`laps.rs`, `credentials.rs`, `domains.rs`) - Removed unused test constant for NTLM hash B in credentials tests **Removed:** - Unused `NTLM_HASH_B` constant from credentials test module
3457219 to
9517867
Compare
**Changed:**
- Export S3_BUCKET in the test.sh script to ensure it is available to child
processes, improving environment setup reliability
- Reword comment in Rust parsing module for clarity ("derails" to "breaks")
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Key Changes:
Added:
to quickly exploit misconfigured trusts and avoid unnecessary delays
promotion via forest-root knowledge
Changed:
malformed hashes (relay artifacts, truncated, or non-hex) are rejected before
entering state or being dispatched to LAPS automation
with clear warnings and additional test coverage for edge cases
domains (e.g.,
child.contoso.local) to be auto-promoted if their parentdomain is already known, while preventing bare TLD promotion
crosson macOS andcargo-zigbuildon Linux for improved reliability with platform quirksRemoved:
processed and stored