Skip to content

fix(ntlm): preserve RC4 sealing state across mechListMIC - #753

Merged
Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
masterfrom
copilot/mechlistmic-rc4-sealing-reset
Sep 25, 2026
Merged

Marc-André Moreau (mamoreau-devolutions) merged 1 commit into
masterfrom
copilot/mechlistmic-rc4-sealing-reset

Conversation

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

CredSSP can wrap pubKeyAuth after the initiator sends its mechListMIC but before it verifies the acceptor's MIC. Resetting both NTLM RC4 handles during verification loses the advanced send state and breaks the next wrapped message.

Snapshot and restore only the sealing handle used for each MIC, including when verification fails. Remove the obsolete reset helper and add regression coverage for both directions, the CredSSP ordering, and invalid signatures. Sequence numbers remain unchanged.

Tests: cargo test -p sspi --lib ntlm::test:: --quiet (36 passed); cargo test -p sspi --features network_client,__test-data --test sspi ntlm --quiet (14 passed); cargo check -p sspi --quiet; cargo fmt --all --check.

Fixes: #752

Restore only the sealing handle used for MIC generation or verification so CredSSP wrapping between MIC exchanges stays in sync. Cover late verification and invalid signatures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 25, 2026 14:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation matches the protocol requirement and includes focused regression coverage for the reported failure modes.

Review effort: Balanced
Findings: None

What changed in this PR

Preserves NTLM RC4 sealing state across SPNEGO mechListMIC operations, fixing subsequent CredSSP message wrapping.

Changes:

  • Snapshot and restore only the MIC-related RC4 handle.
  • Remove obsolete role-based cipher resets.
  • Add regression coverage for ordering, direction, and invalid signatures.
File Description
src/​ntlm/​mod.rs Implements targeted sealing-state preservation.
src/​ntlm/​test.rs Adds RC4 state regression tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit 5b2b137 into master Sep 25, 2026
62 checks passed
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) deleted the copilot/mechlistmic-rc4-sealing-reset branch September 25, 2026 15:22
@jborean93

Copy link
Copy Markdown
Contributor

Thanks for the changes, I can confirm that the changes here fix the underlying problem I was having.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

mechListMIC handling resets both RC4 sealing handles in NTLM through Negotiate

4 participants