diff --git a/Cargo.lock b/Cargo.lock index 283964a..e2e3a69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2027,7 +2027,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.38.1" +version = "0.38.2" dependencies = [ "axum", "axum-server", diff --git a/Cargo.toml b/Cargo.toml index 5900abc..f607c4a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -31,7 +31,7 @@ edition = "2021" # the ROOT manifest (`[workspace.package].version`), so it MUST be set here for a # release to fire (§3.6). The library crates (dig-node-core/dig-runtime/dig-wallet) # keep their own independent versions — only the released binary tracks the workspace version. -version = "0.38.1" +version = "0.38.2" # Release hardening, matching digstore: keep integer-overflow checks ON in release. # The node parses untrusted serialized input and does offset/length arithmetic over diff --git a/SPEC.md b/SPEC.md index f2097e5..c04ddf9 100644 --- a/SPEC.md +++ b/SPEC.md @@ -894,6 +894,11 @@ token is NOT accepted for them. - **The daemon MAY create the token (write); an operator CLI MUST NOT mint one.** The CLI (`dig-node pair` / any control tool) reads the token READ-ONLY; if it is missing or unreadable it MUST fail with a precise remedy (§7.3a) rather than write a fresh token the daemon does not trust. +- **The operator control client MUST connect DIRECT to loopback, ignoring proxy environment.** The + HTTP client that carries the master control token to the node's loopback address MUST be pinned to + a direct connection (`no_proxy`), so an `HTTP_PROXY`/`HTTPS_PROXY` in the operator's environment + can NEVER route the token-bearing `control.*` POST through an interposed proxy. (The default HTTP + proxy behaviour has no automatic loopback bypass.) - **Trust a pre-existing token file ONLY when it is owned by a TRUSTED principal (owner verification).** Before the daemon loads and trusts the bytes of an EXISTING `control-token`, it MUST verify the file's OWNER; a foreign-owned token is DELETED and REGENERATED, never returned. diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 7045a3f..a8b31e6 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -117,6 +117,47 @@ pub const CONTROL_METHODS: &[&str] = &[ "control.listSubscriptions", ]; +/// The control methods this shell HANDLES ITSELF, in [`dispatch_control`]'s owned arms — the +/// ROUTING source of truth: [`dispatch_control`] delegates any method NOT in this set to the +/// embedded node. Adding an owned `match` arm without listing it here leaves that arm +/// unreachable (silently delegated); the lockstep test +/// (`control_methods_partition_into_owned_and_delegated`) forces this set + [`CONTROL_METHODS`] +/// to agree, so a shell-owned method can never be dispatched without also being declared. +pub const OWNED_CONTROL_METHODS: &[&str] = &[ + "control.status", + "control.config.get", + "control.config.setUpstream", + "control.log.setLevel", + "control.cache.get", + "control.cache.setCap", + "control.cache.clear", + "control.hostedStores.list", + "control.hostedStores.pin", + "control.hostedStores.unpin", + "control.hostedStores.status", + "control.sync.status", + "control.sync.trigger", + "control.updater.status", + "control.updater.setChannel", + "control.updater.pause", + "control.updater.resume", + "control.updater.checkNow", + "control.pairing.list", + "control.pairing.approve", + "control.pairing.revoke", +]; + +/// The control methods [`dispatch_control`] DELEGATES to the embedded node's own control surface +/// (`dig_node_core::handle_rpc`) — the node-internal subscription set + peer-status snapshot. +/// Together with [`OWNED_CONTROL_METHODS`] this partitions [`CONTROL_METHODS`] exactly (asserted +/// by the lockstep test): the two disjoint sets union to the full control surface. +pub const DELEGATED_CONTROL_METHODS: &[&str] = &[ + "control.peerStatus", + "control.subscribe", + "control.unsubscribe", + "control.listSubscriptions", +]; + /// Is this a PAIRING-ADMINISTRATION control method (#280)? PURE. /// /// These manage the pairing lifecycle — list pending requests, approve one (minting @@ -602,6 +643,27 @@ pub struct ControlCtx { /// auth gate ([`is_authorized`]); this performs the operation and returns the /// JSON-RPC response Value. Unknown `control.*` methods → METHOD_NOT_FOUND. pub async fn dispatch_control(ctx: &ControlCtx, id: Value, method: &str, params: &Value) -> Value { + // Route by the SINGLE source of truth: methods this shell owns go to `dispatch_owned`; + // everything else (the delegated set + any genuinely-unknown `control.*`) falls through to + // the embedded node's own control surface, which resolves it or returns -32601. + if OWNED_CONTROL_METHODS.contains(&method) { + return dispatch_owned(ctx, id, method, params).await; + } + // Control methods the shell does not own are delegated to the NODE's own control surface + // (`control.peerStatus` / `control.subscribe` / `control.unsubscribe` / + // `control.listSubscriptions` — the node's persisted subscription set + peer-status + // snapshot). The shell forwards them so the whole control surface is reachable through one + // loopback endpoint. A genuinely unknown control method falls through the node too and + // returns -32601. + let req = json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params }); + dig_node_core::handle_rpc(&ctx.node, req).await +} + +/// Handle a control method OWNED by this shell (guaranteed by [`dispatch_control`] to be a member +/// of [`OWNED_CONTROL_METHODS`]). The `_` arm is [`unreachable`] BY CONSTRUCTION: it fires only if +/// [`OWNED_CONTROL_METHODS`] lists a method with no arm here (or vice-versa), i.e. the routing +/// const and the arms drifted — the lockstep test exercises this correspondence. +async fn dispatch_owned(ctx: &ControlCtx, id: Value, method: &str, params: &Value) -> Value { match method { "control.status" => control_ok(id, status(ctx).await), "control.config.get" => control_ok(id, config_get(ctx)), @@ -634,17 +696,12 @@ pub async fn dispatch_control(ctx: &ControlCtx, id: Value, method: &str, params: crate::pairing::approve(&ctx.pairings, &ctx.state_dir, id, params) } "control.pairing.revoke" => crate::pairing::revoke(&ctx.state_dir, id, params), - // Control methods the shell does not own are delegated to the NODE's own - // control surface (`control.peerStatus` / `control.subscribe` / - // `control.unsubscribe` / `control.listSubscriptions` — the node's persisted - // subscription set + peer-status snapshot). These are node-internal control - // methods dig_node_core::handle_rpc resolves; the shell forwards them so the whole - // control surface is reachable through one loopback endpoint. A genuinely - // unknown control method falls through the node too and returns -32601. - _ => { - let req = json!({ "jsonrpc": "2.0", "id": id, "method": method, "params": params }); - dig_node_core::handle_rpc(&ctx.node, req).await - } + // Unreachable: `dispatch_control` only routes here for `OWNED_CONTROL_METHODS` members. + // Reaching this arm means the routing const and these arms have drifted. + _ => unreachable!( + "dispatch_owned reached for non-owned control method {method:?}: \ + OWNED_CONTROL_METHODS and dispatch_owned's arms have drifted" + ), } } @@ -1064,6 +1121,53 @@ mod tests { use super::*; use serde_json::json; + /// LOCKSTEP GATE (#711): [`dispatch_control`] resolves EXACTLY [`CONTROL_METHODS`] — the + /// owned set it routes to `dispatch_owned` ([`OWNED_CONTROL_METHODS`]) plus the set it + /// delegates to the node ([`DELEGATED_CONTROL_METHODS`]) — the two disjoint, and their union + /// equal to the declared surface. This closes the shell-owned-method drift gap the CLI-parity + /// test (`cli_covers_every_node_control_method`) leaves open: a `dispatch_owned` arm added + /// without declaring it (in `OWNED_CONTROL_METHODS` + `CONTROL_METHODS`) fails HERE, and a + /// declared owned method with no arm makes `dispatch_owned`'s `unreachable!` fire. + #[test] + fn control_methods_partition_into_owned_and_delegated() { + use std::collections::BTreeSet; + let listed: BTreeSet<&str> = CONTROL_METHODS.iter().copied().collect(); + let owned: BTreeSet<&str> = OWNED_CONTROL_METHODS.iter().copied().collect(); + let delegated: BTreeSet<&str> = DELEGATED_CONTROL_METHODS.iter().copied().collect(); + + // Each list is duplicate-free. + assert_eq!( + owned.len(), + OWNED_CONTROL_METHODS.len(), + "OWNED has duplicates" + ); + assert_eq!( + delegated.len(), + DELEGATED_CONTROL_METHODS.len(), + "DELEGATED has duplicates" + ); + assert_eq!( + listed.len(), + CONTROL_METHODS.len(), + "CONTROL_METHODS has duplicates" + ); + + // Owned and delegated are disjoint — no method is both handled and forwarded. + let both: Vec<&&str> = owned.intersection(&delegated).collect(); + assert!( + both.is_empty(), + "methods both owned AND delegated: {both:?}" + ); + + // The union is EXACTLY the declared surface — neither an undeclared handler nor a + // declared-but-unhandled method can slip through. + let union: BTreeSet<&str> = owned.union(&delegated).copied().collect(); + assert_eq!( + listed, union, + "CONTROL_METHODS drifted from dispatch_control's owned+delegated set" + ); + } + #[test] fn is_control_method_only_matches_control_namespace() { assert!(is_control_method("control.status")); diff --git a/crates/dig-node-service/src/control_client.rs b/crates/dig-node-service/src/control_client.rs index 8000ca7..1b3853d 100644 --- a/crates/dig-node-service/src/control_client.rs +++ b/crates/dig-node-service/src/control_client.rs @@ -21,6 +21,18 @@ use serde_json::{json, Value}; use crate::config::Config; use crate::control; +/// Build the reqwest client for the loopback control plane, PINNED to a DIRECT connection. +/// +/// `.no_proxy()` disables reqwest's default env-proxy behaviour (`HTTP_PROXY`/`HTTPS_PROXY`/ +/// `http_proxy`/…), which has NO automatic loopback bypass. Without it, a hostile operator-env +/// proxy could route the token-bearing `control.*` POST — carrying the master control token — to +/// `127.0.0.1`/`::1` through an attacker-controlled proxy. The control plane is loopback-only and +/// token-gated (#501/#553); pinning the transport DIRECT keeps the token on the wire it was +/// minted for and never hands it to an interposed proxy. +fn build_control_client() -> reqwest::Result { + reqwest::Client::builder().no_proxy().build() +} + /// Call one `control.*` method on the running node and return its `result` object. /// /// Reads the master control token read-only, builds a single-shot current-thread runtime, @@ -46,9 +58,7 @@ async fn call_async( method: &str, params: Value, ) -> std::io::Result { - let client = reqwest::Client::builder() - .build() - .map_err(std::io::Error::other)?; + let client = build_control_client().map_err(std::io::Error::other)?; let url = format!("http://{addr}/"); let body = json!({ "jsonrpc": "2.0", "id": 1, "method": method, "params": params }); let resp = client @@ -76,3 +86,57 @@ async fn call_async( } Ok(v.get("result").cloned().unwrap_or_else(|| json!({}))) } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Read, Write}; + use std::net::TcpListener; + + /// SECURITY (#711): the control client MUST reach loopback DIRECTLY, ignoring a hostile + /// `HTTP_PROXY` in the environment — otherwise the token-bearing control POST could be routed + /// through an attacker-controlled proxy. We stand up a one-shot loopback HTTP server, point + /// `HTTP_PROXY` at a DEAD address, and assert the request still lands on the server. Without + /// `.no_proxy()`, reqwest would dial the dead proxy instead and the request would fail. + #[tokio::test] + async fn control_client_ignores_http_proxy_and_connects_direct() { + let server = TcpListener::bind("127.0.0.1:0").unwrap(); + let addr = server.local_addr().unwrap(); + + // A one-shot server thread: accept a single connection, reply 200, signal it was hit. + let hit = std::thread::spawn(move || { + let (mut stream, _) = server.accept().unwrap(); + let mut buf = [0u8; 1024]; + let _ = stream.read(&mut buf); + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 2\r\n\r\n{}") + .unwrap(); + true + }); + + // A dead proxy: nothing listens on this port. If the client honoured it, the send fails. + let dead_proxy = "http://127.0.0.1:9"; + std::env::set_var("HTTP_PROXY", dead_proxy); + std::env::set_var("HTTPS_PROXY", dead_proxy); + + let client = build_control_client().unwrap(); + let resp = client + .post(format!("http://{addr}/")) + .body("{}") + .send() + .await; + + std::env::remove_var("HTTP_PROXY"); + std::env::remove_var("HTTPS_PROXY"); + + assert!( + resp.is_ok(), + "control client must connect DIRECT to loopback despite HTTP_PROXY: {resp:?}" + ); + assert_eq!(resp.unwrap().status(), 200); + assert!( + hit.join().unwrap(), + "the direct loopback server was reached" + ); + } +}