diff --git a/design/followups.md b/design/followups.md index c15965f..a67daa0 100644 --- a/design/followups.md +++ b/design/followups.md @@ -6,12 +6,6 @@ move to `## Resolved` with a date + commit hash. ## Open -### F1 — NTLM consumer-layer helpers (workstation default + from_env constructor) -**Severity:** P3 -**Source:** M2 wave 1, `crates/mxaccess-rpc/src/ntlm.rs` -**Why deferred:** The .NET reference's `Environment.MachineName` default for `workstation` and its `FromEnvironment()` constructor (`ManagedNtlmClientContext.cs:38`, `:41-49`) read host state and env vars — both side effects that don't belong in a pure codec module. The constructor takes `workstation: Option<&str>` so callers can wire either later. -**Resolves when:** M2 wave 2 transport (or the M2 example `connect-nmx.rs`) wires `NtlmClientContext::new(.., Some(hostname()?))` and provides a small `from_env` helper at the consumer layer. - ### F2 — NTLM verify_signature path + constant-time MAC compare (server-to-client direction) **Severity:** P2 **Source:** M2 wave 1, `crates/mxaccess-rpc/src/ntlm.rs` @@ -84,5 +78,8 @@ move to `## Resolved` with a date + commit hash. ### F13 — `NmxClient` high-level write/advise/subscribe wrappers **Resolved:** 2026-05-05. All seven wrappers landed in `crates/mxaccess-nmx/src/client.rs`: `write`, `write2`, `write_secured2`, `advise_supervisory`, `send_observed_pre_advise_metadata`, `register_reference`, `un_advise`. Each takes a `GalaxyTagMetadata` + a typed `WriteValue` (re-exported from `mxaccess-codec`), builds the inner NMX body via `mxaccess-codec` (`write_message::encode` / `encode_timestamped` / `secured_write::encode` / `NmxItemControlMessage` / `NmxMetadataQueryMessage` / `NmxReferenceRegistrationMessage`), wraps in `NmxTransferEnvelope`, and routes through `transfer_data`. The pure-codec `encode_*_transfer_body` helpers are extracted as `pub(crate) fn` for testability, mirroring the .NET reference's `internal static` shape. `un_advise` preserves the .NET reference's quirky `NmxTransferMessageKind::Write` envelope (not `ItemControl`) per `cs:457`. +### F1 — NTLM consumer-layer helpers (workstation default + from_env constructor) +**Resolved:** 2026-05-05. `NtlmClientContext::from_env()` reads `MX_RPC_USER` / `MX_RPC_PASSWORD` / `MX_RPC_DOMAIN` (mirrors `ManagedNtlmClientContext.FromEnvironment` at `cs:41-49`); empty `MX_RPC_DOMAIN` is permitted. `local_hostname()` checks `COMPUTERNAME` then `HOSTNAME` and returns the empty string when neither is set — same "unavailable" semantics as `Environment.MachineName` returning null. Lives in `mxaccess-rpc/src/ntlm.rs`; deliberately doesn't pull `gethostname` (no native-libc deps, no `unsafe` for hostname lookup). Added `NtlmError::MissingEnvVar { name }` for the env-var-unset case. Test mod gained an `EnvScope` + `ENV_LOCK` mutex pattern for serializing process-global env mutation across parallel tests. + ### F9 — `ObjectExporterClient.cs` ResolveOxid wrapper methods **Resolved:** 2026-05-05. Both portable methods land in `crates/mxaccess-rpc/src/object_exporter_client.rs`: `resolve_oxid_unauthenticated` (mirrors `cs:14-30`) and `resolve_oxid_with_managed_ntlm_packet_integrity` (mirrors `cs:66-81`). Each opens a TCP connection, binds to `IObjectExporter`, calls opnum 0 with the encoded request, and decodes the response — preferring `parse_resolve_oxid_result` then falling back to `parse_resolve_oxid_failure` for short stubs. The two SSPI flavours (`ResolveOxidWithNtlmConnect`, `ResolveOxidWithNtlmPacketIntegrity`) wrap .NET's `System.Net.Security.SspiClientContext` and are explicitly out of scope for the Rust port — that's a permanent skip, not a deferral. diff --git a/rust/crates/mxaccess-rpc/src/ntlm.rs b/rust/crates/mxaccess-rpc/src/ntlm.rs index 1a8c3fe..5dc4dad 100644 --- a/rust/crates/mxaccess-rpc/src/ntlm.rs +++ b/rust/crates/mxaccess-rpc/src/ntlm.rs @@ -205,6 +205,32 @@ pub enum NtlmError { /// expected bytes. #[error("NTLM signature mismatch")] SignatureMismatch, + + /// [`NtlmClientContext::from_env`] could not find a required env var. + /// Mirrors the .NET `ArgumentNullException` paths in + /// `ManagedNtlmClientContext.FromEnvironment()` (`cs:43-48`). + #[error("missing environment variable: {name}")] + MissingEnvVar { name: &'static str }, +} + +/// Look up the local machine name. Mirrors `Environment.MachineName` +/// (`ManagedNtlmClientContext.cs:38`). +/// +/// Cross-platform: checks `COMPUTERNAME` (Windows) then `HOSTNAME` +/// (POSIX). Returns the empty string when neither is set — same +/// "unavailable" semantics as `Environment.MachineName` returning +/// `null` on platforms where it can't read the host name. +/// +/// This deliberately doesn't call `gethostname(2)` to keep +/// `mxaccess-rpc` free of native-libc dependencies and `unsafe`. For +/// reliable hostname discovery on POSIX where `HOSTNAME` isn't +/// exported (the common case in Bash subshells), callers can pass +/// `Some(&hostname_string)` to [`NtlmClientContext::new`] explicitly. +#[must_use] +pub fn local_hostname() -> String { + std::env::var("COMPUTERNAME") + .or_else(|_| std::env::var("HOSTNAME")) + .unwrap_or_default() } /// Trait for supplying the random/clock inputs the .NET reference reads from @@ -339,6 +365,39 @@ impl NtlmClientContext { } } + /// Create an unauthenticated context from environment variables. + /// Mirrors `ManagedNtlmClientContext.FromEnvironment()` + /// (`ManagedNtlmClientContext.cs:41-49`). + /// + /// Required env vars (all three must be set; empty values are + /// permitted for `MX_RPC_DOMAIN`): + /// + /// - `MX_RPC_USER` — user name + /// - `MX_RPC_PASSWORD` — plaintext password + /// - `MX_RPC_DOMAIN` — NT domain (or workgroup name; empty is fine) + /// + /// `workstation` defaults to [`local_hostname`] (the .NET reference + /// uses `Environment.MachineName` per `cs:38`). When the env-var + /// reads succeed but the local hostname is unavailable on this + /// platform, `workstation` is the empty string — same as + /// `Environment.MachineName` returning `null` on .NET. + /// + /// # Errors + /// [`NtlmError::MissingEnvVar`] if any of the three env vars is unset. + pub fn from_env() -> Result { + let user = std::env::var("MX_RPC_USER").map_err(|_| NtlmError::MissingEnvVar { + name: "MX_RPC_USER", + })?; + let password = std::env::var("MX_RPC_PASSWORD").map_err(|_| NtlmError::MissingEnvVar { + name: "MX_RPC_PASSWORD", + })?; + let domain = std::env::var("MX_RPC_DOMAIN").map_err(|_| NtlmError::MissingEnvVar { + name: "MX_RPC_DOMAIN", + })?; + let hostname = local_hostname(); + Ok(Self::new(&user, &password, &domain, Some(&hostname))) + } + /// Build the 32-byte Type1 (NEGOTIATE) message. Mirrors `cs:51-70`. pub fn create_type1(&mut self) -> [u8; TYPE1_LEN] { self.flags = TYPE1_FLAGS; @@ -811,7 +870,12 @@ const _: () = assert!(SIGNATURE_LEN == 16); const _: () = assert!(NTLMSSP_SIGNATURE[7] == 0); #[cfg(test)] -#[allow(clippy::unwrap_used, clippy::expect_used, clippy::indexing_slicing)] +#[allow( + clippy::unwrap_used, + clippy::expect_used, + clippy::indexing_slicing, + clippy::panic +)] mod tests { use super::*; @@ -1167,4 +1231,181 @@ mod tests { assert_eq!(&msg[16..19], &[0xaa, 0xbb, 0xcc]); assert_eq!(off, 19); } + + // ---- F1: from_env + local_hostname ------------------------------ + + /// All env-mutating tests in this mod serialize on this mutex. + /// `#[test]`s run in parallel by default, but `std::env::set_var` + /// touches process-global state, so concurrent env-var tests race. + /// Holding `ENV_LOCK` for the duration of each `EnvScope` makes + /// snapshot-and-restore safe. + static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + + /// Snapshot the listed env vars on creation, restore them on Drop. + /// Holds [`ENV_LOCK`] for the scope's lifetime so tests don't race + /// on process-global env state. + struct EnvScope { + snapshots: Vec<(&'static str, Option)>, + _guard: std::sync::MutexGuard<'static, ()>, + } + + impl EnvScope { + fn capture(names: &[&'static str]) -> Self { + // Acquire before snapshotting so a concurrent test can't + // mutate vars between our snapshot and restore. + let guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let snapshots = names + .iter() + .map(|n| (*n, std::env::var(n).ok())) + .collect(); + Self { + snapshots, + _guard: guard, + } + } + } + + impl Drop for EnvScope { + fn drop(&mut self) { + for (name, prev) in &self.snapshots { + // SAFETY: ENV_LOCK serializes all env-mutating tests in + // this mod, so concurrent set_var/remove_var calls from + // tests-in-this-file are not possible. Other Cargo test + // binaries run in separate processes. + unsafe { + match prev { + Some(v) => std::env::set_var(name, v), + None => std::env::remove_var(name), + } + } + } + } + } + + fn set_env(name: &str, value: &str) { + // SAFETY: see EnvScope::drop note. Caller must hold ENV_LOCK + // (acquired via EnvScope::capture). + unsafe { + std::env::set_var(name, value); + } + } + + fn unset_env(name: &str) { + // SAFETY: see EnvScope::drop note. + unsafe { + std::env::remove_var(name); + } + } + + #[test] + fn from_env_reads_three_mx_rpc_vars() { + let _scope = EnvScope::capture(&[ + "MX_RPC_USER", + "MX_RPC_PASSWORD", + "MX_RPC_DOMAIN", + "COMPUTERNAME", + "HOSTNAME", + ]); + set_env("MX_RPC_USER", "alice"); + set_env("MX_RPC_PASSWORD", "secret"); + set_env("MX_RPC_DOMAIN", "TESTDOMAIN"); + set_env("COMPUTERNAME", "TESTHOST"); + unset_env("HOSTNAME"); + + let ctx = NtlmClientContext::from_env().unwrap(); + // Use the Debug projection (which exposes user/domain/workstation + // but redacts the password) to assert the fields landed. + let dbg = format!("{ctx:?}"); + assert!(dbg.contains("user: \"alice\"")); + assert!(dbg.contains("domain: \"TESTDOMAIN\"")); + assert!(dbg.contains("workstation: \"TESTHOST\"")); + } + + #[test] + fn from_env_missing_user_errors() { + let _scope = + EnvScope::capture(&["MX_RPC_USER", "MX_RPC_PASSWORD", "MX_RPC_DOMAIN"]); + unset_env("MX_RPC_USER"); + set_env("MX_RPC_PASSWORD", "p"); + set_env("MX_RPC_DOMAIN", "d"); + let err = NtlmClientContext::from_env().unwrap_err(); + match err { + NtlmError::MissingEnvVar { name } => assert_eq!(name, "MX_RPC_USER"), + other => panic!("expected MissingEnvVar(MX_RPC_USER), got {other:?}"), + } + } + + #[test] + fn from_env_missing_password_errors() { + let _scope = + EnvScope::capture(&["MX_RPC_USER", "MX_RPC_PASSWORD", "MX_RPC_DOMAIN"]); + set_env("MX_RPC_USER", "u"); + unset_env("MX_RPC_PASSWORD"); + set_env("MX_RPC_DOMAIN", "d"); + let err = NtlmClientContext::from_env().unwrap_err(); + assert!(matches!( + err, + NtlmError::MissingEnvVar { + name: "MX_RPC_PASSWORD" + } + )); + } + + #[test] + fn from_env_missing_domain_errors() { + let _scope = + EnvScope::capture(&["MX_RPC_USER", "MX_RPC_PASSWORD", "MX_RPC_DOMAIN"]); + set_env("MX_RPC_USER", "u"); + set_env("MX_RPC_PASSWORD", "p"); + unset_env("MX_RPC_DOMAIN"); + let err = NtlmClientContext::from_env().unwrap_err(); + assert!(matches!( + err, + NtlmError::MissingEnvVar { + name: "MX_RPC_DOMAIN" + } + )); + } + + #[test] + fn from_env_accepts_empty_domain() { + let _scope = EnvScope::capture(&[ + "MX_RPC_USER", + "MX_RPC_PASSWORD", + "MX_RPC_DOMAIN", + "COMPUTERNAME", + "HOSTNAME", + ]); + set_env("MX_RPC_USER", "u"); + set_env("MX_RPC_PASSWORD", "p"); + set_env("MX_RPC_DOMAIN", ""); + set_env("COMPUTERNAME", "X"); + let ctx = NtlmClientContext::from_env().unwrap(); + let dbg = format!("{ctx:?}"); + assert!(dbg.contains("domain: \"\"")); + } + + #[test] + fn local_hostname_prefers_computername_over_hostname() { + let _scope = EnvScope::capture(&["COMPUTERNAME", "HOSTNAME"]); + set_env("COMPUTERNAME", "WIN_HOST"); + set_env("HOSTNAME", "POSIX_HOST"); + assert_eq!(local_hostname(), "WIN_HOST"); + } + + #[test] + fn local_hostname_falls_back_to_hostname_when_computername_unset() { + let _scope = EnvScope::capture(&["COMPUTERNAME", "HOSTNAME"]); + unset_env("COMPUTERNAME"); + set_env("HOSTNAME", "POSIX_HOST"); + assert_eq!(local_hostname(), "POSIX_HOST"); + } + + #[test] + fn local_hostname_returns_empty_when_neither_set() { + let _scope = EnvScope::capture(&["COMPUTERNAME", "HOSTNAME"]); + unset_env("COMPUTERNAME"); + unset_env("HOSTNAME"); + assert_eq!(local_hostname(), ""); + } }