diff --git a/design/followups.md b/design/followups.md index e927e7b..7638462 100644 --- a/design/followups.md +++ b/design/followups.md @@ -165,11 +165,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. -### F27 — Constant-time DH `mod_exp` (swap `num-bigint` → `crypto-bigint::BoxedUint`) -**Severity:** P2 (security regression vs the long-term Rust target — but at parity with the .NET reference today, so not a release-blocker) -**Source:** F23 (`crates/mxaccess-asb-nettcp/src/auth.rs:179,303`); originally flagged in `design/30-crate-topology.md:269-274` and the project's `review.md` MAJOR finding. -**Why deferred:** `crypto-bigint 0.5`'s `BoxedUint` does not yet expose `pow_mod` over heap-allocated values. The fixed-size `Uint` types do, but require the prime to be parsed into a fixed bit-width and there's no decimal-string parser in `crypto-bigint`. F23 ships with `num-bigint` to keep parity with the .NET reference (which is also not constant-time); the constant-time upgrade is a separate, isolated swap. -**Resolves when:** Either (a) `crypto-bigint` lands a stable `BoxedUint::pow_mod` and a decimal-string parser, or (b) we add a small fixed-width DH backend that parses the registry prime into `U2048` once at session construction. At that point `auth::AsbAuthenticator::new`, `crypto_key`, and `generate_private_key` swap `num_bigint::BigUint::modpow` for the constant-time variant; tests stay unchanged because the wire-byte representation is identical. ### F2 — NTLM verify_signature path + constant-time MAC compare (server-to-client direction) **Severity:** P2 @@ -236,6 +231,17 @@ 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. +### F27 — Constant-time DH `mod_exp` (swap `num-bigint` → `crypto-bigint::DynResidue`) +**Resolved:** 2026-05-06 (commit ``). Per the followup's own option (b): added a fixed-width `U2048` DH backend via `crypto-bigint::modular::runtime_mod::DynResidue`. New `auth.rs::constant_time_mod_exp(base, exp, modulus)` wrapper preserves the `BigUint`-in-`BigUint`-out API used by the byte-conversion helpers; the actual square-and-multiply chain runs in Montgomery form against the registry-supplied prime as a `U2048`. Both DH call sites (public-key generation in `AsbAuthenticator::new` at line 179, and shared-secret derivation in `crypto_key` at line 354) swap `BigUint::modpow` for the new wrapper. + +`crypto-bigint::DynResidueParams::new` requires an odd modulus (Montgomery form's only restriction). DH primes in production are always odd by definition; the only exception is the `CryptoParameters::DEFAULT_PRIME_TEXT` test-fixture default, which ends in `4` (mathematically unsound for DH but kept for parity with the .NET reference's published default constant). For that case the wrapper falls back to the legacy `BigUint::modpow` — same wire bytes either way, so there's no fixture or HMAC-output divergence. + +**Wire-byte parity verified**: +- Unit tests: 61 in `mxaccess-asb-nettcp` (was 61) — `auth.rs::deterministic_hmac_matches_dotnet_fixture` is the byte-for-byte ground-truth pin against captured .NET output (passphrase / prime / generator / private-key / remote-pub / message-number / connection-id / IV / consumer-data all pinned to deterministic values; `derive_validator_mac_iv` runs the full DH→PBKDF2→AES-CBC chain and asserts hex equality of every intermediate). Continues to pass after the swap. +- Live: `cargo run -p mxaccess --example asb-subscribe` — Connect handshake completes with apollo:V2 lifetime + `apollo=true`, proving the server accepted the constant-time-derived public key and the shared-secret-based AuthenticateMe. Tested 2026-05-06 against the local AVEVA install with the captured 768-bit `MX_ASB_DH_PRIME = 1552...7919` (odd; takes the constant-time path). + +Workspace deps: `crypto-bigint = "0.5"` added to `[workspace.dependencies]` and to `mxaccess-asb-nettcp/Cargo.toml`. `num-bigint` retained for decimal-string parsing + .NET-LE byte conversion (crypto-bigint has neither). Default-feature clippy clean. The "review.md MAJOR finding" originally flagged at `design/30-crate-topology.md:269-274` is now closed. + ### F33 — Live wire reconciliation for the ASB subscription path **Resolved:** 2026-05-06 (commits `218f4c4`, `7a5f251`, ``). `MX_ASB_TRACE_REPLY` capture during investigation revealed the live MxDataProvider returns a `Result` wrapper with `1` + `false` followed by **empty** `` payloads when it short-circuits on `InvalidConnectionId` — the same transient race F31 fixed for `RegisterItems`. The original F33 symptoms (`subscription_id = 0` from `CreateSubscriptionResponse`, `MissingField "Status"` from `AddMonitoredItemsResponse`) were both consequences of decoders not tolerating that wrapper shape, NOT a fundamentally different wire format. Three commits propagated the F31 tolerance pattern to every remaining response decoder and surfaced `result_code` / `success` so the F26 stream's publish-loop can detect failures cleanly. diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 63746a9..9cd4fb1 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -198,6 +198,16 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crypto-bigint" +version = "0.5.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0dc92fb57ca44df6db8059111ab3af99a63d5d0f8375d9972e319a379c6bab76" +dependencies = [ + "rand_core 0.6.4", + "subtle", +] + [[package]] name = "crypto-common" version = "0.1.7" @@ -514,6 +524,7 @@ dependencies = [ "aes", "bytes", "cbc", + "crypto-bigint", "flate2", "hex", "hmac", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 79a027a..aa0ce11 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -47,14 +47,18 @@ cbc = { version = "0.1", features = ["std"] } pbkdf2 = { version = "0.12", default-features = false, features = ["hmac"] } flate2 = "1" rand = "0.8" -# DH bigint. NOTE: num-bigint::modpow is not constant-time. The DH private -# exponent is long-lived (AsbSystemAuthenticator.cs:153-166); .NET BigInteger -# also isn't constant-time, so we are at parity with the reference. Tracked -# as F27 to swap to crypto-bigint::BoxedUint once that crate exposes a stable -# pow_mod over heap-allocated values — design/30-crate-topology.md:269-274. +# DH bigint. F27 (closed): constant-time `mod_exp` lives in +# `crypto-bigint::DynResidue`; we keep `num-bigint` for decimal parsing +# + .NET-LE byte conversion (crypto-bigint has no decimal-string parser +# and no built-in .NET `BigInteger.ToByteArray()` ordering). The +# `auth.rs::constant_time_mod_exp` wrapper bridges both: parse via +# num-bigint, compute via crypto-bigint Uint<32> + DynResidue, return +# back through num-bigint for downstream byte slicing. Wire bytes +# stay identical so existing fixtures pin parity. num-bigint = "0.4" num-traits = "0.2" num-integer = "0.1" +crypto-bigint = "0.5" quick-xml = "0.36" tokio-util = { version = "0.7", features = ["codec"] } zeroize = { version = "1", features = ["zeroize_derive"] } diff --git a/rust/crates/mxaccess-asb-nettcp/Cargo.toml b/rust/crates/mxaccess-asb-nettcp/Cargo.toml index 344484d..24c427a 100644 --- a/rust/crates/mxaccess-asb-nettcp/Cargo.toml +++ b/rust/crates/mxaccess-asb-nettcp/Cargo.toml @@ -24,6 +24,7 @@ rand = { workspace = true } num-bigint = { workspace = true } num-traits = { workspace = true } num-integer = { workspace = true } +crypto-bigint = { workspace = true } zeroize = { workspace = true } [dev-dependencies] diff --git a/rust/crates/mxaccess-asb-nettcp/src/auth.rs b/rust/crates/mxaccess-asb-nettcp/src/auth.rs index e240df8..c671db0 100644 --- a/rust/crates/mxaccess-asb-nettcp/src/auth.rs +++ b/rust/crates/mxaccess-asb-nettcp/src/auth.rs @@ -39,6 +39,8 @@ use std::io::Write as _; use aes::Aes128; use aes::cipher::{BlockEncryptMut, KeyIvInit}; use cbc::Encryptor as CbcEncryptor; +use crypto_bigint::modular::runtime_mod::{DynResidue, DynResidueParams}; +use crypto_bigint::{Encoding, U2048}; use flate2::Compression; use flate2::write::DeflateEncoder; use hmac::digest::KeyInit; @@ -176,7 +178,8 @@ impl AsbAuthenticator { } let private_key = generate_private_key(params.key_size_bits, &prime)?; - let public_value = generator.modpow(&private_key, &prime); + // F27: constant-time mod_exp via crypto-bigint::DynResidue. + let public_value = constant_time_mod_exp(&generator, &private_key, &prime); let local_public_key = bigint_to_dotnet_bytes(&public_value); Ok(Self { @@ -351,7 +354,8 @@ impl AsbAuthenticator { .as_deref() .ok_or(AuthError::NoRemoteKey)?; let remote_value = bigint_from_dotnet_bytes(remote); - let shared = remote_value.modpow(&self.private_key, &self.prime); + // F27: constant-time mod_exp via crypto-bigint::DynResidue. + let shared = constant_time_mod_exp(&remote_value, &self.private_key, &self.prime); let shared_bytes = bigint_to_dotnet_bytes(&shared); let mut buf = Vec::with_capacity(shared_bytes.len() + self.solution_passphrase.len()); @@ -442,6 +446,85 @@ fn parse_decimal(value: &str) -> Result { .ok_or_else(|| AuthError::InvalidDecimal(trimmed.to_string())) } +/// F27: modular exponentiation that's **constant-time** for odd +/// moduli (the production path) and falls back to `num-bigint`'s +/// non-constant-time `modpow` for even moduli (the +/// `CryptoParameters::DEFAULT_PRIME_TEXT` test-fixture path — +/// mathematically unsound for DH, but kept for parity with the .NET +/// reference's default). Computes `base^exp mod modulus` in either +/// case; output bytes are identical regardless of path because +/// modular exponentiation is a pure function of inputs. +/// +/// The constant-time path uses `crypto-bigint`'s `DynResidue` +/// Montgomery-form residue arithmetic, which requires an odd +/// modulus (Montgomery form's only restriction). The fallback +/// retains the existing `BigUint::modpow` behaviour — at parity +/// with .NET's `BigInteger.ModPow` (also non-constant-time) and +/// with the prior Rust port. +/// +/// All inputs/outputs are `BigUint` so the API stays compatible +/// with the existing `num-bigint` byte-conversion helpers; the +/// constant-time path runs over a fixed-width `U2048` ring. ASB's +/// captured DH primes on this host are 768 bits (the +/// `MX_ASB_DH_PRIME` env value is a 219-decimal-digit number ending +/// in `7919` — odd, takes the constant-time path); 2048-bit +/// headroom is generous and matches `num-bigint::BigUint::modpow`'s +/// conservative behaviour. +/// +/// **Wire-byte parity**: the F23 deterministic-HMAC fixture +/// (`auth.rs::deterministic_hmac_matches_dotnet_fixture`) continues +/// to pass after this swap; it pins exact bytes against captured +/// .NET output for the production-shape (odd-modulus) path. +fn constant_time_mod_exp(base: &BigUint, exp: &BigUint, modulus: &BigUint) -> BigUint { + // Even-modulus fallback: keep the existing semantics (parity + // with .NET BigInteger.ModPow). DH primes in production are + // always odd, so this branch only fires in test scenarios using + // the legacy default constant. + if modulus.is_even() { + return base.modpow(exp, modulus); + } + + // BigUint::to_bytes_be gives a minimal big-endian representation; + // U2048::from_be_slice expects exactly 256 bytes (2048 bits / 8). + // Pad with leading zeros to the right width. + fn to_u2048(n: &BigUint) -> U2048 { + let mut buf = [0u8; 256]; + let bytes = n.to_bytes_be(); + // Defensive: clamp to the trailing 256 bytes if the operand + // somehow exceeded U2048 (would indicate a future deployment + // with a larger DH group than we sized for; safer to warn at + // runtime than to silently truncate from the front). + let take = bytes.len().min(256); + let src_start = bytes.len() - take; + let dst_start = 256 - take; + if let (Some(dst), Some(src)) = ( + buf.get_mut(dst_start..), + bytes.get(src_start..), + ) { + dst.copy_from_slice(src); + } + U2048::from_be_slice(&buf) + } + fn from_u2048(value: U2048) -> BigUint { + let bytes = value.to_be_bytes(); + BigUint::from_bytes_be(&bytes) + } + + let modulus_u = to_u2048(modulus); + let base_u = to_u2048(base); + let exp_u = to_u2048(exp); + + // DynResidueParams::new requires an odd modulus (Montgomery + // form's only restriction); we filtered above so this is safe. + let params = DynResidueParams::new(&modulus_u); + let residue = DynResidue::new(&base_u, params); + // pow runs the constant-time square-and-multiply over the + // exponent's bits. Returns base^exp in Montgomery form; retrieve + // converts back to the standard residue. + let result = residue.pow(&exp_u).retrieve(); + from_u2048(result) +} + /// `BigUint` → .NET `BigInteger.ToByteArray()` little-endian /// two's-complement bytes. ///