Skip to content

fix: harden NTLM hash validation and improve cross-forest secretsdump#320

Merged
l50 merged 3 commits into
mainfrom
feat/cross-forest-ticket-fallbacks
May 15, 2026
Merged

fix: harden NTLM hash validation and improve cross-forest secretsdump#320
l50 merged 3 commits into
mainfrom
feat/cross-forest-ticket-fallbacks

Conversation

@l50
Copy link
Copy Markdown
Contributor

@l50 l50 commented May 15, 2026

Key Changes:

  • Enforced strict NTLM hash validation (32 hex chars) at publish and dispatch
  • Added immediate cross-forest secretsdump after Kerberos ticket forge
  • Improved domain promotion logic for child domains based on forest knowledge
  • Expanded tests for malformed hash rejection and domain inference

Added:

  • Immediate cross-forest secretsdump dispatch after inter-realm ticket creation
    to quickly exploit misconfigured trusts and avoid unnecessary delays
  • Additional tests for NTLM hash length/format validation and for domain
    promotion via forest-root knowledge

Changed:

  • NTLM hash handling now strictly requires exactly 32 hexadecimal characters;
    malformed hashes (relay artifacts, truncated, or non-hex) are rejected before
    entering state or being dispatched to LAPS automation
  • LAPS hash sweep and hash publication paths enforce stricter NTLM validation,
    with clear warnings and additional test coverage for edge cases
  • Domain candidate promotion (hostname inference) now allows multi-label child
    domains (e.g., child.contoso.local) to be auto-promoted if their parent
    domain is already known, while preventing bare TLD promotion
  • Taskfile cross-compilation logic now prefers cross on macOS and
    cargo-zigbuild on Linux for improved reliability with platform quirks

Removed:

  • Acceptance of malformed NTLM hashes in state, ensuring only valid hashes are
    processed and stored

@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

❌ Patch coverage is 98.52941% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.78%. Comparing base (5282fbe) to head (3754110).

Files with missing lines Patch % Lines
...i/src/orchestrator/state/publishing/credentials.rs 97.91% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #320   +/-   ##
=======================================
  Coverage   78.78%   78.78%           
=======================================
  Files         439      439           
  Lines      124638   124664   +26     
=======================================
+ Hits        98190    98215   +25     
- Misses      26448    26449    +1     
Files with missing lines Coverage Δ
ares-cli/src/orchestrator/automation/laps.rs 92.62% <100.00%> (-0.04%) ⬇️
...src/orchestrator/result_processing/admin_checks.rs 57.11% <100.00%> (-0.28%) ⬇️
...-cli/src/orchestrator/result_processing/parsing.rs 98.76% <100.00%> (-0.02%) ⬇️
...es-cli/src/orchestrator/result_processing/tests.rs 100.00% <100.00%> (ø)
...s-cli/src/orchestrator/state/publishing/domains.rs 99.10% <100.00%> (-0.02%) ⬇️
...i/src/orchestrator/state/publishing/credentials.rs 89.69% <97.91%> (+0.46%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@l50 l50 force-pushed the feat/cross-forest-ticket-fallbacks branch from 242c456 to 3457219 Compare May 15, 2026 20:24
l50 added 2 commits May 15, 2026 14:36
**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
@l50 l50 force-pushed the feat/cross-forest-ticket-fallbacks branch from 3457219 to 9517867 Compare May 15, 2026 20:36
**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")
@l50 l50 merged commit 3b4d36e into main May 15, 2026
12 checks passed
@l50 l50 deleted the feat/cross-forest-ticket-fallbacks branch May 15, 2026 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant