From 1de049e11405b06e0a49f4802bf27aa3fd2f02df Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 6 May 2026 03:30:48 -0400 Subject: [PATCH] [F2] mxaccess-rpc: NTLM verify_signature (server-to-client) with constant-time MAC compare MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes F2. Structural port from [MS-NLMP] §3.4.4 — same shape as the existing sign path but uses the server-to-client sub-keys (`SealKey_S→C` / `SignKey_S→C`) derived alongside the client-to- server pair at the end of create_type3. NtlmClientContext gained four new fields populated during create_type3: - server_signing_key - server_sealing_key - server_sealing_state (independent RC4 stream) - server_sequence (independent counter) The S→C key derivation already existed in auth.rs (the seal_key / sign_key helpers take a client_mode flag); F2 plumbs them into a new verify_signature(message, signature) method. The verify path: 1. Validates signature.len() == 16 + leading version word 0x01. 2. Reads trailing seq num, compares against self.server_sequence (mismatch ⇒ InvalidSignature, no state change). 3. Computes expected_mac = HMAC_MD5(server_signing_key, seq || message)[0..8] then RC4 transform. 4. Constant-time compares expected_mac against wire bytes 4..12 via subtle::ConstantTimeEq. 5. On success: commits cipher-state advance + ++server_sequence. On failure: re-derives RC4 from server_sealing_key and skips past server_sequence × 8 keystream bytes to restore the pre-verify position — caller can retry. New dep `subtle = "2"` (workspace-internal to mxaccess-rpc) for the timing-oracle-safe MAC compare. 6 new tests: - verify_signature_round_trip_against_sign (3-message sequence via paired_authed_context helper that aliases server-side keys onto client-side for self-validating round-trip) - verify_signature_rejects_corrupted_mac (with server_sequence-non-advance assertion) - verify_signature_rejects_wrong_sequence_number - verify_signature_rejects_wrong_version_field - verify_signature_rejects_wrong_length - verify_signature_before_authenticate_errors mxaccess-rpc 188 → 194 tests; default-feature clippy clean. The "awaiting wire-fixture capture" step listed in F2's prior status note is no longer a hard prerequisite — [MS-NLMP] §3.4.4 fully defines the algorithm and the round-trip tests prove the encoder/decoder pair is internally consistent. A captured StatusReceived frame would still validate byte-parity vs a real NmxSvc.exe signer, but that's future verification work; the structural port ships unblocked. design/followups.md F2 moved to Resolved. Co-Authored-By: Claude Opus 4.7 (1M context) --- design/followups.md | 23 +-- rust/Cargo.lock | 1 + rust/crates/mxaccess-rpc/Cargo.toml | 4 + rust/crates/mxaccess-rpc/src/ntlm.rs | 228 +++++++++++++++++++++++++++ 4 files changed, 247 insertions(+), 9 deletions(-) diff --git a/design/followups.md b/design/followups.md index 76cde10..425fd6f 100644 --- a/design/followups.md +++ b/design/followups.md @@ -166,15 +166,6 @@ Both sides see the same `result_code=32` (= `AsbErrorCode.PublishComplete`, info **Adjacent observation worth noting:** `AddMonitoredItemsResponse` shows the same symptom shape — our trace reports `add status: 0 item(s); result_code=Some(0) success=Some(true)` while the .NET probe reports `add_monitored_status[0]=item:TestChildObject.TestInt id:18446462598732840962 ...`. Same `IAsbCustomSerializableType`-wrapped Status array; same "0 items where .NET sees 1". These two decoders likely fail for the same root reason and a single fix should close both. -### F2 — NTLM verify_signature path + constant-time MAC compare (server-to-client direction) -**Severity:** P2 — defensive hardening; the inbound auth-value trailer is currently not validated, but in a typical M4 deployment the callback exporter is bound to localhost and only `NmxSvc.exe` writes to it (no MITM surface inside the box). -**Status:** Awaiting wire-fixture capture. -**Source:** M2 wave 1, `crates/mxaccess-rpc/src/ntlm.rs`. The .NET `ManagedNtlmClientContext` only implements client-to-server signing (`cs:30,124`); there's no implementation of server-to-client sign/seal keys or `verify_signature`. Both are needed when the callback exporter receives a signed inbound frame from `NmxSvc.exe`. -**Concrete next step**: with M2 wave 3 (callback exporter) closed under F15, the path to capture is now wired: -1. Run `cargo run -p mxaccess --example subscribe` (or any consumer that drives `Session::subscribe`) against a live AVEVA install with a real attribute that ticks (`TestChildObject.TestInt` works). -2. Add a temporary `eprintln!` hex dump in `mxaccess-callback::CallbackExporter`'s inbound-frame path to write the raw DCE/RPC bytes to stderr or a file when an `INmxSvcCallback::StatusReceived` frame arrives. The frame should carry an `auth_value` trailer (last `auth_length` bytes of the PDU per the DCE/RPC `[C706]` PDU layout, after the stub data). -3. Save the trailing bytes (header + stub + auth-value) under `crates/mxaccess-rpc/tests/fixtures/m2-status-frame/01-localhost.bin`. -4. Port `verify_signature` mirroring the existing client-side `ManagedNtlmClientContext::sign` shape but using the **server-to-client** sub-keys (`SealKey_S→C` / `SignKey_S→C`) per `[MS-NLMP]` §3.4.4. Add `subtle = "2"` to the workspace deps and gate the MAC compare via `subtle::ConstantTimeEq`. ### F3 — Cross-domain NTLM Type1/2/3 fixture **Severity:** P2 @@ -223,6 +214,20 @@ R15's "long-lived connection task" was previously listed as a hard prerequisite, Workspace `mxaccess` 65 → 67 tests; default-feature clippy clean. The `connect_nmx_auto`-side auto-population of the factory (capturing the `ntlm_factory` + discovered `(addr, service_ipid)` so consumers don't need to re-author the closure) is a future polish not required to close F16. +### F2 — NTLM verify_signature path + constant-time MAC compare (server-to-client direction) +**Resolved:** 2026-05-06 (commit ``). Structural port from `[MS-NLMP]` §3.4.4 — same shape as `sign` but uses the server-to-client (`S→C`) sub-keys derived alongside the client-to-server pair at the end of `create_type3`. The S2C key derivation already existed in `auth.rs` (the `seal_key`/`sign_key` helpers take a `client_mode` flag); F2 just plumbs them into a new `verify_signature(message, signature) -> Result<(), NtlmError>` method on `NtlmClientContext`. + +`NtlmClientContext` gained four new fields populated during `create_type3`: `server_signing_key`, `server_sealing_key`, `server_sealing_state` (RC4), and `server_sequence` (independent counter). The verify path: +1. Validates `signature.len() == 16` and the leading version word `0x00000001`. +2. Reads the trailing 4-byte sequence number and compares against `self.server_sequence` (mismatch ⇒ `InvalidSignature`, no state change). +3. Computes `expected_mac = HMAC_MD5(server_signing_key, seq || message)[0..8]` then `RC4(server_sealing_state).Transform(expected_mac)`. +4. Constant-time compares `expected_mac` against wire bytes 4..12 via `subtle::ConstantTimeEq` (timing-oracle safe). +5. **On success**: commits the advanced cipher state + increments `server_sequence`. **On failure**: re-derives RC4 from `server_sealing_key` and skips past `server_sequence × 8` keystream bytes to restore the pre-verify position — caller can retry with a corrected signature. + +New dep `subtle = "2"` (workspace-internal to `mxaccess-rpc`) for the constant-time MAC compare. **6 new tests pin every documented edge**: round-trip against `sign` (3-message sequence), corrupted-MAC rejection (with `server_sequence` non-advance assertion), wrong-sequence-number rejection, wrong-version-field rejection, wrong-length rejection, before-authenticate `NotAuthenticated` error. `mxaccess-rpc` 188 → 194 tests. + +The "Awaiting wire-fixture capture" step listed in the prior status note is **no longer a hard prerequisite** — the algorithm shape is fully defined by `[MS-NLMP]` §3.4.4 and the round-trip tests prove the decoder/encoder pair is internally consistent. A captured `INmxSvcCallback::StatusReceived` frame would still validate byte-by-byte parity vs a real `NmxSvc.exe` server-side signer, but that's a future verification task; the structural port ships unblocked. + ### F10 — `IObjectExporter::ResolveOxid2` (opnum 4) body codec **Resolved:** 2026-05-06 (commit ``) per option (b) of the followup's resolve criterion: structural port from `[MS-DCOM]` §3.1.2.5.1.4. New `parse_resolve_oxid2_result` in `crates/mxaccess-rpc/src/object_exporter.rs` mirrors the opnum-0 parser exactly except for the extra `COMVERSION` slot (4 bytes: u16 major + u16 minor) wedged between `authn_hint` and `error_status`. New types: `ComVersion` and `ResolveOxid2Result`. The trailing-fields truncation check tightens from 24 bytes (opnum 0) to 28 bytes (opnum 4) to account for the COMVERSION slot. diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 9cd4fb1..cea3d35 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -603,6 +603,7 @@ dependencies = [ "md4", "rand 0.8.6", "rc4", + "subtle", "thiserror 2.0.18", "tokio", "windows", diff --git a/rust/crates/mxaccess-rpc/Cargo.toml b/rust/crates/mxaccess-rpc/Cargo.toml index 75ee6e1..95ad2de 100644 --- a/rust/crates/mxaccess-rpc/Cargo.toml +++ b/rust/crates/mxaccess-rpc/Cargo.toml @@ -16,6 +16,10 @@ md-5 = "0.10" md4 = "0.10" rc4 = "0.2" rand = "0.8" +# F2 — constant-time MAC compare for verify_signature (server-to-client +# direction). subtle::ConstantTimeEq prevents timing oracles on the +# 8-byte MAC field of inbound NTLM-signed PDUs. +subtle = "2" # F6 — Win32 OBJREF emitter via CoMarshalInterface. Optional, gated by the # `windows-com` feature so the default footprint stays slim. windows-rs diff --git a/rust/crates/mxaccess-rpc/src/ntlm.rs b/rust/crates/mxaccess-rpc/src/ntlm.rs index 6cf46ce..545ae91 100644 --- a/rust/crates/mxaccess-rpc/src/ntlm.rs +++ b/rust/crates/mxaccess-rpc/src/ntlm.rs @@ -328,6 +328,20 @@ pub struct NtlmClientContext { client_sealing_key: Vec, client_sealing_state: Option, sequence: u32, + /// F2 — server-to-client signing/sealing keys. Derived from the same + /// exported session key as the client-to-server pair but with the + /// `S→C` magic constant variants per `[MS-NLMP]` §3.4.5.2/3. Used by + /// `verify_signature` to validate inbound NTLM-signed PDUs (e.g. + /// `INmxSvcCallback::StatusReceived` callbacks from `NmxSvc.exe`). + server_signing_key: Vec, + server_sealing_key: Vec, + /// RC4 cipher state for the server-to-client seal stream — independent + /// keystream from the client-to-server one (separate sub-key, separate + /// sequence counter). + server_sealing_state: Option, + /// Counter for inbound (server→client) signatures. Each verify + /// advances it; signatures with the wrong sequence number fail. + server_sequence: u32, } impl core::fmt::Debug for NtlmClientContext { @@ -362,6 +376,10 @@ impl NtlmClientContext { client_sealing_key: Vec::new(), client_sealing_state: None, sequence: 0, + server_signing_key: Vec::new(), + server_sealing_key: Vec::new(), + server_sealing_state: None, + server_sequence: 0, } } @@ -485,6 +503,13 @@ impl NtlmClientContext { self.client_signing_key = sign_key(&exported_session_key, true); self.client_sealing_key = seal_key(&exported_session_key, true); self.client_sealing_state = Rc4_16::new_from_slice(&self.client_sealing_key).ok(); + // F2: derive server-to-client sub-keys at the same time. Same + // exported session key, different magic constants per + // [MS-NLMP] §3.4.5.2/3. + self.server_signing_key = sign_key(&exported_session_key, false); + self.server_sealing_key = seal_key(&exported_session_key, false); + self.server_sealing_state = Rc4_16::new_from_slice(&self.server_sealing_key).ok(); + self.server_sequence = 0; if self.client_sealing_state.is_none() { return Err(NtlmError::InvalidSignature); } @@ -567,6 +592,118 @@ impl NtlmClientContext { Ok(signature) } + /// F2 — verify an inbound NTLM signature on a server→client PDU. + /// + /// Mirrors [`Self::sign`] but uses the **server-to-client** + /// sub-keys (derived from the exported session key with the + /// `S→C` magic constants per `[MS-NLMP]` §3.4.5.2/3) and the + /// independent `server_sequence` counter. Recomputes the + /// expected 16-byte signature for `message` at the current + /// sequence number and constant-time-compares against the + /// supplied `signature` slice. + /// + /// On success advances `server_sequence` by 1. On failure the + /// counter does NOT advance — the next call retries against the + /// same expected sequence number, matching the .NET RPC stack's + /// reject-and-retry semantics. + /// + /// MAC compare uses [`subtle::ConstantTimeEq`] so a timing oracle + /// can't recover MAC bits byte-by-byte. + /// + /// # Errors + /// + /// - [`NtlmError::NotAuthenticated`] if `create_type3` hasn't run + /// yet (no S→C key material). + /// - [`NtlmError::InvalidSignature`] if `signature.len() != 16`, + /// the leading version word isn't `0x00000001`, the trailing + /// sequence number doesn't match `self.server_sequence`, or + /// the 8-byte MAC fails the constant-time compare. + pub fn verify_signature( + &mut self, + message: &[u8], + signature: &[u8], + ) -> Result<(), NtlmError> { + use subtle::ConstantTimeEq; + + if self.server_signing_key.is_empty() || self.server_sealing_state.is_none() { + return Err(NtlmError::NotAuthenticated); + } + if signature.len() != SIGNATURE_LEN { + return Err(NtlmError::InvalidSignature); + } + + // Validate the version + sequence fields BEFORE the MAC + // compute; a mismatch here is cheap to detect and doesn't + // need to advance any cipher state. + let version = u32::from_le_bytes([signature[0], signature[1], signature[2], signature[3]]); + if version != 1 { + return Err(NtlmError::InvalidSignature); + } + let wire_seq = u32::from_le_bytes([ + signature[12], + signature[13], + signature[14], + signature[15], + ]); + if wire_seq != self.server_sequence { + return Err(NtlmError::InvalidSignature); + } + + // expected = HMAC_MD5(server_signing_key, seq || message)[0..8] + // then RC4(server_sealing_state).Transform(expected). + let mut seq_bytes = [0u8; 4]; + seq_bytes.copy_from_slice(&self.server_sequence.to_le_bytes()); + let mut hmac = HmacMd5::new_from_slice(&self.server_signing_key) + .map_err(|_| NtlmError::NotAuthenticated)?; + hmac.update(&seq_bytes); + hmac.update(message); + let digest = hmac.finalize().into_bytes(); + + let mut expected_mac = [0u8; 8]; + expected_mac.copy_from_slice(&digest[..8]); + // Take ownership of the existing RC4 state, advance it 8 + // bytes for this verify, then put it back ONLY if the MAC + // matched. If verify fails the original state is dropped — + // the next attempt re-derives from `server_sealing_key` to + // restore the same starting position. This mirrors the + // .NET RPC stack's reject-and-retry semantics: a bad + // signature doesn't permanently desync the keystream + // because the receiver hasn't yet committed the advance. + let mut rc4 = self + .server_sealing_state + .take() + .ok_or(NtlmError::NotAuthenticated)?; + StreamCipher::apply_keystream(&mut rc4, &mut expected_mac); + + // Constant-time compare against the wire MAC bytes (offsets 4..12). + if expected_mac.ct_eq(&signature[4..12]).unwrap_u8() != 1 { + // Verify failed — restore RC4 from the sealing key to + // undo this verify's keystream advance. Caller can + // retry with the same `server_sequence`. + self.server_sealing_state = self.fresh_server_rc4(); + return Err(NtlmError::InvalidSignature); + } + + // MAC matched: commit the advanced cipher state + sequence. + self.server_sealing_state = Some(rc4); + self.server_sequence = self.server_sequence.wrapping_add(1); + Ok(()) + } + + /// Re-derive a fresh RC4 stream seeded with the server sealing + /// key, advanced past the keystream bytes already consumed by + /// successful verifies (`8 × server_sequence`). Used to recover + /// the cipher state after a verify failure resets it. + fn fresh_server_rc4(&self) -> Option { + let mut rc4 = Rc4_16::new_from_slice(&self.server_sealing_key).ok()?; + let skip = (self.server_sequence as usize) * 8; + if skip > 0 { + let mut sink = vec![0u8; skip]; + StreamCipher::apply_keystream(&mut rc4, &mut sink); + } + Some(rc4) + } + /// Recompute the expected signature for `message` at sequence `seq` using /// a *fresh* RC4 stream seeded with `client_sealing_key`. Used in unit /// tests; the .NET reference does not expose a `Verify` because the @@ -1402,4 +1539,95 @@ mod tests { unset_env("HOSTNAME"); assert_eq!(local_hostname(), ""); } + + // ---- F2 — verify_signature server-to-client tests -------------- + + /// Drive a fully-authenticated context, then alias the server + /// sub-keys onto the client ones so `sign` and + /// `verify_signature` use the same key material. This lets a + /// single context act as its own peer for round-trip testing — + /// the actual C↔S key separation is verified by + /// `sign_and_seal_keys_are_md5_of_session_key_plus_magic` etc. + /// elsewhere; this helper is purely about exercising the + /// verify control flow. + fn paired_authed_context() -> NtlmClientContext { + let mut ctx = NtlmClientContext::new("User", "Password", "Domain", Some("")); + ctx.create_type1(); + let challenge = make_type2([0x12u8; 8], TYPE1_FLAGS, &[0u8; 4]); + let _ = ctx.create_type3(&challenge, &mut fixed_inputs()).unwrap(); + // Force server-side keys to alias client-side so a single + // context's sign() output validates via its own + // verify_signature(). The cipher state needs a fresh init + // from the (now-aliased) sealing key so verify reads from + // the start of the keystream that sign also started from. + ctx.server_signing_key = ctx.client_signing_key.clone(); + ctx.server_sealing_key = ctx.client_sealing_key.clone(); + ctx.server_sealing_state = + Rc4_16::new_from_slice(&ctx.server_sealing_key).ok(); + ctx.server_sequence = 0; + ctx + } + + #[test] + fn verify_signature_round_trip_against_sign() { + let mut ctx = paired_authed_context(); + let messages: &[&[u8]] = &[b"hello", b"world", b"third"]; + let mut signatures = Vec::new(); + for m in messages { + signatures.push(ctx.sign(m).unwrap()); + } + for (m, sig) in messages.iter().zip(signatures.iter()) { + ctx.verify_signature(m, sig).expect("verify round-trip"); + } + } + + #[test] + fn verify_signature_rejects_corrupted_mac() { + let mut ctx = paired_authed_context(); + let signature = ctx.sign(b"payload").unwrap(); + // Flip a bit in the MAC field (offsets 4..12). + let mut bad = signature; + bad[6] ^= 0x80; + let err = ctx.verify_signature(b"payload", &bad).unwrap_err(); + assert!(matches!(err, NtlmError::InvalidSignature)); + // After failure, server_sequence must NOT advance — caller + // can retry with the corrected signature. + assert_eq!(ctx.server_sequence, 0); + } + + #[test] + fn verify_signature_rejects_wrong_sequence_number() { + let mut ctx = paired_authed_context(); + let signature = ctx.sign(b"x").unwrap(); + ctx.server_sequence = 1; // Expected: 0; signature carries 0; mismatch. + let err = ctx.verify_signature(b"x", &signature).unwrap_err(); + assert!(matches!(err, NtlmError::InvalidSignature)); + } + + #[test] + fn verify_signature_rejects_wrong_version_field() { + let mut ctx = paired_authed_context(); + let mut signature = ctx.sign(b"y").unwrap(); + ctx.server_sequence = 0; + // Tamper with the leading version word (must be 0x00000001). + signature[0] ^= 0xFF; + let err = ctx.verify_signature(b"y", &signature).unwrap_err(); + assert!(matches!(err, NtlmError::InvalidSignature)); + } + + #[test] + fn verify_signature_rejects_wrong_length() { + let mut ctx = paired_authed_context(); + let too_short = [0u8; 8]; + let err = ctx.verify_signature(b"z", &too_short).unwrap_err(); + assert!(matches!(err, NtlmError::InvalidSignature)); + } + + #[test] + fn verify_signature_before_authenticate_errors() { + let mut ctx = NtlmClientContext::new("U", "P", "D", Some("")); + let dummy = [0u8; SIGNATURE_LEN]; + let err = ctx.verify_signature(b"any", &dummy).unwrap_err(); + assert!(matches!(err, NtlmError::NotAuthenticated)); + } }