diff --git a/Cargo.lock b/Cargo.lock index f66bd066..a6a4adc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3031,7 +3031,7 @@ dependencies = [ [[package]] name = "dig-node-service" -version = "0.252.1" +version = "0.252.2" dependencies = [ "async-trait", "axum", diff --git a/Cargo.toml b/Cargo.toml index cffa98de..a4177251 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,7 +32,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.252.1" +version = "0.252.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 ab897632..a3c5291f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -1539,7 +1539,14 @@ mutation or custody method is ever open. ### 7.3a. The daemon state dir — location, ACL, threat model (#501) The state dir holds ONLY the control/auth state — the control token (§7.3) and the paired-token -store (`paired-tokens.json`, §7.11). The bulk per-user `.dig` cache and `config.json` (§3.5–3.6) do +store (`paired-tokens.json`, §7.11). The CLIENT-side paired token (§7.11a) is NOT in this dir: it +lives at `/DigNode/client-token`, owned by the invoking user, so an unprivileged +client can hold a token without any machine-wide path being widened. `` is the +platform per-user base — `$HOME` on Unix/macOS, `%LOCALAPPDATA%` on Windows. It MUST be resolved +directly from that base and MUST NOT be resolved through the cache resolver, whose unwritable-dir +fallback is a PID-keyed directory under the system temp dir: a world-writable temp directory is +never an acceptable location for a bearer credential. When no per-user base can be resolved, the +client MUST REFUSE to store the token rather than degrade to the current working directory. The bulk per-user `.dig` cache and `config.json` (§3.5–3.6) do NOT move; they stay per-user (shared with the browser/digstore, #96). **Resolution order** (the daemon and every operator CLI MUST resolve this identically, so it MUST @@ -2294,6 +2301,62 @@ and REVOCABLE. All token comparisons are constant-time. client_name, created_ms }] }`, restricted (dir ACL), atomic writes. The auth gate accepts the master token OR any token in this store (except for the pairing-administration methods). +### 7.11a. Client-side pairing and the CLI token ladder (#403) + +An MV3 extension is not the only client that cannot read `/control-token`. On a `.deb` +install that file is `0600 root:root` and its directory `0700 root:root` (§7.3a), so an ORDINARY OS +USER driving the CLI is in exactly the extension's position: the node is running, the user is on the +machine, and every token-gated `control.*` verb is out of reach. The remedy MUST NOT be to widen the +mode — the master token is the master capability, and it authorizes pairing administration and +chain authority over the wallet replica (§7.11, §18.16). + +The unreadable-master-token remedy (§7.3) is read ONLY by a caller holding neither token, i.e. by +definition an unprivileged one. On Unix it MUST therefore name `dign pair connect` — the verb THAT +reader can run, unelevated — in addition to the operator's `sudo dign pair approve `. +Naming only the elevated half directs the one audience that sees the message to a command it cannot +execute. + +**`dig-node pair connect [--client-name NAME]`** is the CLIENT half of the §7.11 handshake, and the +one `pair` verb that requires NO token. It MUST: + +1. call the OPEN `pairing.request { client_name }` (never the gated client — a requester holds no + token by definition, and routing an open method through the gated path fails with an elevation + remedy for a question the node answers to anyone); +2. DISPLAY the returned `pairing_code` and name the operator's command + (`sudo dign pair approve `), so the compare-codes consent step of §7.11 is performed; +3. poll the OPEN `pairing.poll { pairing_id }` to a TERMINAL state, bounded by the server's own + `expires_ms` rather than by a local retry count, and terminate on expiry rather than spin; +4. on `status: "approved"`, persist the delivered token to the INVOKING USER's own per-user base + (`/DigNode/client-token`, §7.3a). The file MUST be created EXCLUSIVELY and + owner-only in a single step (`O_CREAT|O_EXCL` at mode `0600` on Unix), never written first and + restricted afterwards: a write-then-chmod leaves the token readable under the process umask for + a window, and a plain write FOLLOWS a symlink planted at that path — which discloses the token, + or clobbers an arbitrary file, as the invoking user. Replacing this user's OWN existing store on + a re-pair is permitted and MUST unlink it rather than write through it. + +`client_name` is bounded by the same 64-character REFUSAL bound the node applies (§7.11): an +over-long name MUST be refused, never shortened. A name this client shortened is a name the client +partly wrote, and the operator approves what they are shown. + +**The token ladder.** Every CLI `control.*` call selects its token in this fixed order: + +1. the MASTER control token, when this account can read it; +2. otherwise this user's paired token from `/DigNode/client-token`, when present (an unresolvable per-user base counts as "no paired token", not an error); +3. otherwise the master read's own error, VERBATIM — its kind (which sets the CLI exit code) and its + remedy text (§7.3) MUST both survive unchanged, because that sentence is what tells the user how + to become able to act. + +Rung 1 MUST NOT consult the per-user store: a user who can read the master token never reads a +paired one. The selection MUST be expressed as a function of the two read OUTCOMES rather than as a +compile-time platform branch, because the unprivileged case cannot be reproduced by a test process +that is privileged, and a `cfg!` branch is exercised only on the platform that compiles it. + +**What pairing a CLI client does NOT grant.** The paired token remains SCOPED and REVOCABLE +(§7.11): it cannot drive `control.pairing.*` and it cannot drive `control.chiaPeers.add`/`.remove`. +Pairing a user therefore never confers pairing administration or chain authority, and no file mode +changes anywhere in the flow. Revocation is unchanged: `sudo dign pair revoke ` invalidates +it on the very next request. + ### 7.12. Paired-token authorization for wallet methods (#370) The pairing framework (§7.11) authorizes `control.*` mutations. The thin-client model (epic #365) diff --git a/crates/dig-node-service/src/control.rs b/crates/dig-node-service/src/control.rs index 9623fc8d..c2322ee9 100644 --- a/crates/dig-node-service/src/control.rs +++ b/crates/dig-node-service/src/control.rs @@ -552,7 +552,7 @@ fn remedy_for_unreadable_token(path: &Path, dir: &str, unix: bool) -> String { ); if unix { format!( - "{elevated}. For a program that must keep running as an ordinary user (the dig-app Agent on a server), do NOT widen the mode on this file — it is the master capability. Pair a scoped, revocable token for that client instead: `sudo dign pair` LISTS the pending requests and `sudo dign pair approve ` approves one -- the bare verb only lists, it approves nothing -- and the token the client receives cannot mint or revoke pairings and cannot grant chain authority. Revoke it any time with `sudo dign pair revoke `." + "{elevated}. If you cannot elevate, you do not have to: run `dign pair connect` as THIS account to request a scoped token of your own, then ask the operator to approve it. Do NOT widen the mode on this file — it is the master capability. On the operator's side: `sudo dign pair` LISTS the pending requests and `sudo dign pair approve ` approves one -- the bare verb only lists, it approves nothing -- and the token the client receives cannot mint or revoke pairings and cannot grant chain authority. Revoke it any time with `sudo dign pair revoke `." ) } else { format!( @@ -6669,6 +6669,21 @@ mod tests { unix.contains("revoke"), "a grant with no stated revocation is a permanent one: {unix}" ); + // dig-node#403 -- this message is rung 3 of the ladder, so the ONLY reader who ever sees + // it is one holding neither token: an unprivileged user who by construction cannot run a + // `sudo` verb. Naming only the operator's half sends that reader to find an + // administrator for a step they can perform themselves. The verb is asserted UNPREFIXED + // (`contains("sudo dign pair connect")` would be the same dead end wearing the right + // words), and the operator half is asserted above and below rather than replaced -- a fix + // that swapped one audience's guidance for the other's is not a fix. + assert!( + unix.contains("run `dign pair connect`"), + "the unprivileged reader must be told the verb THEY can run: {unix}" + ); + assert!( + !unix.contains("sudo dign pair connect"), + "`pair connect` needs no elevation; prefixing it recreates the dead end: {unix}" + ); assert!( !unix.contains("uninstall"), "reinstalling does not grant read access on Unix; advising it is the dead end: {unix}" diff --git a/crates/dig-node-service/src/control_client.rs b/crates/dig-node-service/src/control_client.rs index c107a142..148c33dc 100644 --- a/crates/dig-node-service/src/control_client.rs +++ b/crates/dig-node-service/src/control_client.rs @@ -15,6 +15,16 @@ //! mutating CLI control is gated by the same capability as the WS, not an unauthenticated side //! door. A node running as a service under another OS account surfaces the precise //! service-vs-user remedy from `load_token_readonly` (elevate / grant read ACL / start the node). +//! +//! # The ladder (#403) -- a paired token when the master token is out of reach +//! +//! On a `.deb` install the master token is `0600 root:root` (#501), so an ordinary user's read is +//! DENIED. Rather than widen that mode -- it is the master capability -- the client falls through +//! to the scoped token an operator approved for this account (`dign pair connect`, then +//! `sudo dign pair approve `; see [`crate::paired_client`]). That token is strictly +//! less powerful: it cannot administer pairings and carries no chain authority over the wallet +//! replica. With NEITHER token the master read's rich remedy is returned unchanged, because it is +//! the message that tells the user what to do next. use serde_json::{json, Value}; @@ -43,7 +53,16 @@ fn build_control_client() -> reqwest::Result { /// ("is the node running?"), a JSON-RPC `error` → `Other` (the node's own message). pub fn call_control(config: &Config, method: &str, params: Value) -> std::io::Result { let addr = config.bind_addr(); - let token = control::load_token_readonly()?; + // The #403 ladder: the master token when this account can read it, else this user's paired + // token, else the master read's own remedy verbatim. Rung 2 is a thunk, so a user who can read + // the master token never touches the per-user store. + let token = crate::paired_client::select_token(control::load_token_readonly(), || { + // An unresolvable per-user base is "this account has no paired token", not an error of + // its own: rung 3's master remedy is the message the user needs either way. + crate::paired_client::paired_token_path() + .ok() + .and_then(|p| crate::paired_client::load_paired_token(&p)) + })?; let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build()?; diff --git a/crates/dig-node-service/src/entrypoint.rs b/crates/dig-node-service/src/entrypoint.rs index f1943d4c..d58334de 100644 --- a/crates/dig-node-service/src/entrypoint.rs +++ b/crates/dig-node-service/src/entrypoint.rs @@ -759,6 +759,14 @@ enum PairCommand { /// The token id from `dig-node pair`. token_id: String, }, + /// Ask this node for a scoped control token for YOUR account, then wait for the operator + /// to approve it. Needs no elevation and no master token. + Connect { + /// The label the operator sees when approving. Defaults to `dign CLI ()`. + /// Refused, never shortened, above 64 characters. + #[arg(long)] + client_name: Option, + }, } impl Command { @@ -876,6 +884,7 @@ pub fn run() -> std::process::ExitCode { None | Some(PairCommand::List) => PairAction::List, Some(PairCommand::Approve { pairing_id }) => PairAction::Approve { pairing_id }, Some(PairCommand::Revoke { token_id }) => PairAction::Revoke { token_id }, + Some(PairCommand::Connect { client_name }) => PairAction::Connect { client_name }, }; render(pair::run(&config, pair_action), action, json) } @@ -1467,6 +1476,7 @@ mod tests { None | Some(PairCommand::List) => PairAction::List, Some(PairCommand::Approve { pairing_id }) => PairAction::Approve { pairing_id }, Some(PairCommand::Revoke { token_id }) => PairAction::Revoke { token_id }, + Some(PairCommand::Connect { client_name }) => PairAction::Connect { client_name }, }, _ => panic!("expected a pair command from {argv:?}"), }; @@ -1482,6 +1492,22 @@ mod tests { ), "`pair approve ` must approve, and must carry the id through" ); + // #403: `connect` is a DISTINCT verb, and it must never be reachable by accident from the + // bare noun -- the bare noun is the operator's read-only listing. + assert!( + matches!( + pair_action(&["dig-node", "pair", "connect"]), + PairAction::Connect { client_name: None } + ), + "`pair connect` with no flag must default its own label" + ); + assert!( + matches!( + pair_action(&["dig-node", "pair", "connect", "--client-name", "Agent"]), + PairAction::Connect { client_name: Some(ref n) } if n == "Agent" + ), + "`--client-name` must reach the action verbatim -- the operator approves what they see" + ); } /// **`dign mirror bond-states --after` sends the cursor to the node.** diff --git a/crates/dig-node-service/src/lib.rs b/crates/dig-node-service/src/lib.rs index c03a863a..0860ebb6 100644 --- a/crates/dig-node-service/src/lib.rs +++ b/crates/dig-node-service/src/lib.rs @@ -87,6 +87,10 @@ pub mod network_info; /// handler argument, then opens the user's default browser at the resolving URL. See [`open`]. pub mod open; pub mod pair; +/// The CLIENT half of the #280 pairing handshake (#403): the token ladder + the per-user +/// paired-token store, so an unprivileged user can drive `control.*` without widening a mode. +/// See [`paired_client`]. +pub mod paired_client; pub mod pairing; /// `control.peers.ping` (dig_ecosystem#1985): the connection-ladder diagnostic — dial one peer a /// tier at a time and report WHICH tier reached it. See [`peer_ping`]. diff --git a/crates/dig-node-service/src/pair.rs b/crates/dig-node-service/src/pair.rs index ba2b0a79..f049138a 100644 --- a/crates/dig-node-service/src/pair.rs +++ b/crates/dig-node-service/src/pair.rs @@ -12,6 +12,11 @@ //! The operator FIRST confirms the printed `pairing_code` matches what the //! extension shows (compare-codes consent), then approves. //! * `dig-node pair revoke ` — revoke an issued controller token. +//! * `dig-node pair connect [--client-name NAME]` — the CLIENT side (#403), and the only verb +//! here that needs NO master token: it asks for a token, prints the code for the operator to +//! compare, waits for approval, and stores the granted token in the invoking user's own state +//! dir. This is how an ordinary OS user drives `control.*` against a root-owned service +//! without any file mode being widened. //! //! Everything here reaches the node over `POST /` on its loopback address with the //! `X-Dig-Control-Token` header — the same authorized surface the DIG Browser uses. @@ -22,7 +27,10 @@ use crate::untrusted_text::render_untrusted; use crate::cli::Outcome; use crate::config::Config; -use crate::control_client::call_control; +use crate::control_client::{call_control, call_open}; +use crate::paired_client::{ + self, default_client_name, next_poll_step, validate_client_name, PollStep, POLL_INTERVAL, +}; /// The operator action, clap-agnostic (mapped from the CLI subcommand in `main.rs`). pub enum PairAction { @@ -32,6 +40,9 @@ pub enum PairAction { Approve { pairing_id: String }, /// Revoke an issued controller token by id. Revoke { token_id: String }, + /// CLIENT side (#403): ask this node for a scoped token, wait for the operator to approve, + /// and persist the result in THIS user's own state dir. + Connect { client_name: Option }, } /// Run a `pair` subcommand: read the master token, call the node's `control.pairing.*`, @@ -64,6 +75,7 @@ pub fn run(config: &Config, action: PairAction) -> std::io::Result { result, )) } + PairAction::Connect { client_name } => connect(config, client_name), PairAction::Revoke { token_id } => { let result = call_control( config, @@ -81,6 +93,114 @@ pub fn run(config: &Config, action: PairAction) -> std::io::Result { } } +/// Current unix time in milliseconds (0 on a clock error — only affects the poll deadline, and a +/// zero clock reads as "not yet expired", so the server's own sweep remains the real bound). +fn now_ms() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as u64) + .unwrap_or(0) +} + +/// `dig-node pair connect` — the CLIENT half of the handshake (#403). +/// +/// Uses [`call_open`], never [`call_control`]: the requester by definition holds no token yet, and +/// routing an OPEN method through the gated client would fail with an elevation remedy for a +/// question the node answers to anyone. Progress goes to STDERR so `--json` stdout stays a single +/// machine-readable object. +fn connect(config: &Config, client_name: Option) -> std::io::Result { + let name = client_name.unwrap_or_else(default_client_name); + validate_client_name(&name)?; + + let requested = call_open(config, "pairing.request", json!({ "client_name": name }))?; + let pairing_id = requested["pairing_id"] + .as_str() + .ok_or_else(|| std::io::Error::other("dig-node: pairing.request returned no pairing_id"))? + .to_string(); + let code = requested["pairing_code"].as_str().unwrap_or("??????"); + let expires_ms = requested["expires_ms"].as_u64().unwrap_or(0); + + eprintln!("{}", waiting_banner(code, &pairing_id)); + + loop { + let polled = call_open(config, "pairing.poll", json!({ "pairing_id": pairing_id }))?; + match polled["status"].as_str().unwrap_or("unknown") { + "approved" => { + let token = polled["token"].as_str().ok_or_else(|| { + std::io::Error::other("dig-node: approved pairing carried no token") + })?; + let path = paired_client::paired_token_path()?; + paired_client::store_paired_token(&path, token)?; + return Ok(Outcome::new( + paired_message(&path), + json!({ "status": "approved", "token_path": path.display().to_string() }), + )); + } + "pending" => match next_poll_step(now_ms(), expires_ms, POLL_INTERVAL) { + PollStep::Wait(d) => std::thread::sleep(d), + PollStep::Expired => { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + EXPIRED_BEFORE_APPROVAL, + )) + } + }, + // `expired` and `unknown` are both terminal: the node has dropped the pending entry, + // so no amount of further polling can change the answer. + other => { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + terminal_status_message(other), + )) + } + } + } +} + +// --------------------------------------------------------------------------- +// The four user-facing strings of `pair connect`, lifted out of the poll loop. +// +// They are named functions rather than inline literals for one reason: the loop that emitted them +// cannot be driven from a unit test (it dials a node and sleeps), so every one of these strings +// shipped with NOTHING asserting on it. Three of them were corrupted — a lost `\` line +// continuation left a ~26-space run mid-sentence, which compiles, passes clippy, and is visible +// only to a reader. Lifting them out is what makes +// [`tests::the_user_facing_pair_strings_have_no_lost_line_continuation`] able to see them at all. +// --------------------------------------------------------------------------- + +/// What the client prints while it waits for the operator to approve. +fn waiting_banner(code: &str, pairing_id: &str) -> String { + format!( + "dig-node: pairing code {code}\n\ + Ask the machine's operator to CONFIRM this code, then run:\n\n \ + sudo dign pair approve {pairing_id}\n\n\ + Waiting for approval..." + ) +} + +/// The SUCCESS message of `dig-node pair connect`. +fn paired_message(path: &std::path::Path) -> String { + format!( + "dig-node: paired. The scoped token is stored for your account at {}. \ + `dign` control commands now work as this user, with no elevation. The operator can \ + revoke it any time with `sudo dign pair revoke `.", + path.display() + ) +} + +/// The first terminal failure: the request aged out while still pending. +const EXPIRED_BEFORE_APPROVAL: &str = + "dig-node: the pairing request expired before it was approved. Run `dign pair connect` \ + again and have the operator approve it within five minutes."; + +/// The second terminal failure: the node no longer holds the pending entry at all. +fn terminal_status_message(status: &str) -> String { + format!( + "dig-node: the pairing request is {status} — it was never approved, or the node \ + restarted. Run `dign pair connect` again." + ) +} + /// The display budget, in terminal columns, for an attacker-supplied `client_name`. /// /// It matches `pairing::MAX_CLIENT_NAME`, so a name this node ACCEPTED renders unmarked and the @@ -139,6 +259,75 @@ fn format_list(result: &Value) -> String { mod tests { use super::*; + /// **Proves (dig-node#403):** none of `pair connect`'s user-facing strings carries the + /// signature of a lost `\` line continuation. + /// + /// A multi-line Rust string literal without the trailing backslash keeps the source's own + /// indentation, so the sentence ships with a ~26-space run in the middle of it. Three of these + /// four strings shipped that way: the SUCCESS message of the verb this ticket added, and the + /// only guidance on BOTH terminal failure paths. It compiles, clippy is clean, and no test + /// asserted on them — the defect was reachable only by a human reading the source. + /// + /// The property asserted is the SIGNATURE, not the specific wording: any run of three or more + /// consecutive spaces that is not the deliberate indent of the banner's command line. Wording + /// changes freely; a lost continuation is caught mechanically the next time it happens. + #[test] + fn the_user_facing_pair_strings_have_no_lost_line_continuation() { + let banner = waiting_banner("123456", "aabbccdd"); + let paired = paired_message(std::path::Path::new("/home/u/DigNode/client-token")); + let terminal = terminal_status_message("expired"); + + // The banner is the ONE string with legitimate leading whitespace: it indents a + // copy-pasteable command by exactly four spaces so the reader can select it. Every other + // string is prose and must carry none, because a lost continuation shows up as leading + // indentation just as readily as it shows up mid-line -- `"a\n b"` prints as + // raggedly as `"a b"`, and a check that trims the start cannot see the first one. + const BANNER_INDENT: &str = " "; + for (what, text, indent_allowed) in [ + ("waiting banner", banner.as_str(), true), + ("paired message", paired.as_str(), false), + ("expired-before-approval", EXPIRED_BEFORE_APPROVAL, false), + ("terminal status", terminal.as_str(), false), + ] { + for line in text.lines() { + let interior = line.trim_start(); + let leading = &line[..line.len() - interior.len()]; + assert!( + !interior.contains(" "), + "{what}: a run of 3+ spaces mid-line is the signature of a lost `\\` line \ + continuation in a multi-line literal: {line:?}" + ); + if indent_allowed { + assert!( + leading.is_empty() || leading == BANNER_INDENT, + "{what}: the only legitimate indent is the command line's four spaces; \ + anything else is source indentation that leaked into the output: \ + {line:?}" + ); + } else { + assert!( + leading.is_empty(), + "{what}: prose must not be indented -- leading whitespace here is the \ + source's own indentation, which is what a lost `\\` continuation \ + emits: {line:?}" + ); + } + } + } + } + + /// **Proves:** the message an unprivileged reader gets on both terminal paths tells them the + /// verb THEY can run, not only the one the operator runs. + /// + /// Asserted here as well as in `control.rs` because these two strings are the entire guidance + /// on the failure paths of the new verb; a user who reaches them has already discovered + /// `pair connect` and needs to be told to retry it rather than to find an administrator. + #[test] + fn both_terminal_failures_name_the_verb_the_reader_can_rerun() { + assert!(EXPIRED_BEFORE_APPROVAL.contains("`dign pair connect`")); + assert!(terminal_status_message("unknown").contains("`dign pair connect`")); + } + /// **Proves (dig-node#346):** an attacker-supplied `client_name` cannot forge a line of the /// operator's approval prompt. /// diff --git a/crates/dig-node-service/src/paired_client.rs b/crates/dig-node-service/src/paired_client.rs new file mode 100644 index 00000000..ea70f020 --- /dev/null +++ b/crates/dig-node-service/src/paired_client.rs @@ -0,0 +1,536 @@ +//! The CLIENT half of the #280 control-token pairing handshake (dig-node#403). +//! +//! `pairing.rs` implements the node's side of the three steps; `pair.rs` implements the +//! OPERATOR's side (list / approve / revoke). Neither gives an ordinary, unprivileged OS user a +//! way to ASK for a token and KEEP it — so on a `.deb` install, where the master control token is +//! `0600 root:root` (#501), every `control.*` CLI verb was reachable only under `sudo`. +//! +//! This module closes that gap WITHOUT widening a single file mode: +//! +//! * [`select_token`] is the token LADDER — master token when readable, else the per-user paired +//! token, else the master read's own rich remedy error, unchanged. +//! * [`paired_token_path`] / [`store_paired_token`] / [`load_paired_token`] own the per-user +//! store, which lives in the invoking user's own state dir and is `0600` on Unix. +//! * [`validate_client_name`] mirrors the server's REFUSAL bound, and [`next_poll_step`] bounds +//! the approval wait against the server's own `expires_ms`. +//! +//! # Why the ladder is a pure function and not a `cfg!` branch +//! +//! A `cfg!(unix)` branch is only ever exercised on the half of the fleet that compiles it, so the +//! behaviour that matters most on Ubuntu would be untested on the machine most likely to run these +//! tests. More sharply: the unprivileged case CANNOT be reproduced in a test that runs as root, +//! and a unit test cannot drop privileges. Taking both read OUTCOMES as arguments makes the +//! decision assertable on every platform, under any account — the pattern #458 established for +//! [`crate::control`]'s remedy text. +//! +//! # What this does NOT change +//! +//! The paired token is strictly LESS powerful than the master token: `pairing.rs` records that +//! master authorizes pairing administration (`control.pairing.approve` / `.revoke`) and +//! `chiaPeers.add`/`.remove`. A user holding a paired token gains the scoped control surface an +//! operator explicitly approved for them, and nothing else. Nothing here reads, writes, chmods or +//! relocates the master token. + +use std::io; +use std::path::{Path, PathBuf}; +use std::time::Duration; + +/// The per-user file holding the scoped token this account was granted. +/// +/// It lives under the invoking user's own home/known-folder base (`$HOME/DigNode` / +/// `%LOCALAPPDATA%\DigNode`), NOT in the machine-wide state dir: the whole point is that an +/// ordinary user can own it without anyone touching `/var/lib/dig-node`. +pub const PAIRED_CLIENT_TOKEN_FILE: &str = "client-token"; + +/// The folder name the ecosystem uses under the per-user base, byte-identical to the one +/// [`dig_node_core::config_path`] resolves under. Named here rather than reused from the cache +/// resolver on purpose — see [`paired_store_dir`]. +const PAIRED_STORE_FOLDER: &str = "DigNode"; + +/// The default interval between `pairing.poll` calls while waiting for the operator to approve. +/// +/// Short enough that approval feels immediate, long enough that a 5-minute wait is ~100 requests +/// rather than a spin. The wait is bounded by the SERVER's `expires_ms`, never by a local count. +pub const POLL_INTERVAL: Duration = Duration::from_secs(3); + +/// The directory THIS user's paired token lives in, or a REFUSAL when no per-user base exists. +/// +/// # Why this does not go through the state/cache resolver +/// +/// The obvious spelling — [`crate::state::legacy_state_dir`], the parent of +/// [`dig_node_core::config_path`] — resolves through `resolve_cache_dir`, which falls back to +/// `std::env::temp_dir()/DigNode-/cache` whenever the canonical dir is unwritable. That +/// fallback is a reasonable degraded mode for a CACHE and an unacceptable one for a BEARER +/// CREDENTIAL: on Unix `/tmp` is mode `1777`, the `` component is enumerable from `/proc`, +/// and a directory another user pre-created is accepted by `create_dir_all` without complaint. +/// Resolving from [`dig_node_core::platform_user_base`] directly makes that path STRUCTURALLY +/// unreachable for this file rather than merely unlikely — there is no branch left to sanitise. +/// +/// # Why an unresolvable base REFUSES instead of degrading +/// +/// [`dig_node_core::platform_user_base`] itself ends in `PathBuf::from(".")` when the OS knows no +/// home and neither `LOCALAPPDATA` nor `HOME` is set. Writing a bearer token into the process's +/// current working directory — frequently a shared or world-writable path, and one that moves with +/// every invocation — is a worse outcome than having no paired token at all, and the ladder +/// already has a correct answer for "no paired token": rung 3's master remedy. So this refuses, +/// the same refuse-rather-than-degrade discipline [`validate_client_name`] applies to a name. +pub fn paired_store_dir() -> io::Result { + paired_store_dir_from(dig_node_core::platform_user_base()) +} + +/// The pure core of [`paired_store_dir`], with the resolved base as an ARGUMENT. +/// +/// The no-home case cannot be produced from a test process — it needs an account the OS knows no +/// home for and an environment with neither `HOME` nor `LOCALAPPDATA` — so taking the base in is +/// what makes the REFUSAL assertable at all. Same reason [`select_token`] takes the master read's +/// outcome rather than performing it (the pattern #458 established). +fn paired_store_dir_from(base: PathBuf) -> io::Result { + if base.as_os_str().is_empty() || base == Path::new(".") { + return Err(io::Error::new( + io::ErrorKind::NotFound, + "dig-node: cannot store a paired token because no per-user directory could be \ + resolved for this account (no home directory, and neither HOME nor LOCALAPPDATA is \ + set). Run this command as an account with a home directory; the token is refused \ + rather than written to the current working directory, which is not a private place \ + to keep a credential.", + )); + } + Ok(base.join(PAIRED_STORE_FOLDER)) +} + +/// Where THIS user's paired token lives, or the [`paired_store_dir`] refusal. +pub fn paired_token_path() -> io::Result { + Ok(paired_token_path_in(&paired_store_dir()?)) +} + +/// [`paired_token_path`] for an explicit directory, so tests use a temp dir and never touch a +/// real one. +pub fn paired_token_path_in(dir: &Path) -> PathBuf { + dir.join(PAIRED_CLIENT_TOKEN_FILE) +} + +/// Read this user's paired token, or `None` when there is not one. +/// +/// Every failure is `None`: an absent, blank, or unreadable store means "this account has no +/// paired token", which is exactly the ladder's second rung failing. It is NEVER an error in its +/// own right, because the master read's remedy is the message the user needs. +pub fn load_paired_token(path: &Path) -> Option { + let s = std::fs::read_to_string(path).ok()?; + let t = s.trim(); + (!t.is_empty()).then(|| t.to_string()) +} + +/// Persist a freshly-approved token for this user, owner-only from the moment it exists. +/// +/// The file is created by the INVOKING user, so it is owned by them; this function never runs +/// elevated and never touches a machine-wide path. +/// +/// # Why this is not `write` followed by a chmod +/// +/// `std::fs::write` creates the file under the process umask — typically `0644` — and any later +/// `set_permissions` narrows it only AFTER the bytes are on disk, so the token is world-readable +/// for a window. Worse, `write` follows symlinks: a pre-planted `client-token -> ~/.ssh/authorized_keys` +/// in a directory another user can create makes this an arbitrary-file clobber running as the +/// victim, with no race to win. Creating the file EXCLUSIVELY at `0600` closes both — `create_new` +/// fails outright on an existing path, symlink included, and the mode is applied at `open(2)` +/// time rather than afterwards. This mirrors [`crate::state::ensure_dir_restricted`] + +/// `pairing::save_paired_tokens`, one module over. +/// +/// A pre-existing OWN store is removed first so that re-pairing still works; that removal deletes +/// the link rather than following it, so it cannot be turned into a write through a symlink. +pub fn store_paired_token(path: &Path, token: &str) -> io::Result<()> { + if let Some(dir) = path.parent() { + crate::state::ensure_dir_restricted(dir)?; + } + // `create_new` refuses an existing path; a re-pair legitimately replaces its own store. + match std::fs::remove_file(path) { + Ok(()) => {} + Err(e) if e.kind() == io::ErrorKind::NotFound => {} + Err(e) => return Err(e), + } + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let mut f = opts.open(path)?; + io::Write::write_all(&mut f, token.as_bytes())?; + f.sync_all()?; + Ok(()) +} + +/// The token LADDER, as a pure function over the two read OUTCOMES. +/// +/// 1. the master control token when this account can read it (the pre-#403 behaviour, unchanged); +/// 2. else this user's paired token; +/// 3. else the master read's own error, VERBATIM — its remedy text is platform-correct and names +/// the pairing flow (#458), so degrading it would replace the one message that tells the user +/// what to do next. +/// +/// `paired` is a THUNK rather than a value so that rung 1 provably never consults the store: with +/// an `Option` argument the caller has already read the file before the decision is made, and no +/// test could tell a correct implementation from one that reads it every time. +pub fn select_token(master: io::Result, paired: F) -> io::Result +where + F: FnOnce() -> Option, +{ + match master { + Ok(token) => Ok(token), + Err(master_err) => match paired() { + Some(token) => Ok(token), + None => Err(master_err), + }, + } +} + +/// Validate a `client_name` against the server's bound BEFORE spending a round trip. +/// +/// The bound is `pairing::MAX_CLIENT_NAME` itself, so the client cannot drift from the server, and +/// an over-long name is REFUSED rather than shortened — a name this program shortened is a name +/// this program partly wrote, which is precisely the forgery `pairing.rs` refuses to perform. +pub fn validate_client_name(name: &str) -> io::Result<&str> { + let max = crate::pairing::MAX_CLIENT_NAME; + if name.chars().count() > max { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "--client-name must be at most {max} characters; this name is refused rather \ + than shortened, because a name shortened here is a name you did not write and \ + the operator approves what they see" + ), + )); + } + Ok(name) +} + +/// A sensible default label for this client, always within the bound. +/// +/// Names the program and the account, because that is what the operator needs in order to decide +/// whether to approve. If the account name is long enough to blow the budget we fall back to the +/// bare program name rather than clipping — the same refusal-not-truncation rule, applied to a +/// value we chose ourselves. +pub fn default_client_name() -> String { + let user = std::env::var("USER") + .or_else(|_| std::env::var("USERNAME")) + .unwrap_or_default(); + let candidate = if user.trim().is_empty() { + "dign CLI".to_string() + } else { + format!("dign CLI ({})", user.trim()) + }; + if candidate.chars().count() > crate::pairing::MAX_CLIENT_NAME { + "dign CLI".to_string() + } else { + candidate + } +} + +/// What the poll loop should do next, given the clock and the server's own deadline. +#[derive(Debug, PartialEq, Eq)] +pub enum PollStep { + /// Sleep this long, then poll again. + Wait(Duration), + /// The server's `expires_ms` has passed — stop, and tell the user to start over. + Expired, +} + +/// Bound the approval wait by the SERVER's deadline, not by a local retry count. +/// +/// `expires_ms` comes from `pairing.request` and is the same value the node's own TTL sweep uses, +/// so the client stops asking at exactly the moment the node stops answering. The final wait is +/// clamped to the remaining time so the loop can never sleep past the deadline and then report a +/// stale state. +pub fn next_poll_step(now_ms: u64, expires_ms: u64, interval: Duration) -> PollStep { + if now_ms >= expires_ms { + return PollStep::Expired; + } + let remaining = Duration::from_millis(expires_ms - now_ms); + PollStep::Wait(interval.min(remaining)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::Cell; + + fn denied() -> io::Error { + io::Error::new( + io::ErrorKind::PermissionDenied, + "the node's control token exists but is NOT readable by your account — \ + `sudo dign pair approve `", + ) + } + + /// **Proves (dig-node#403):** an unprivileged account whose master-token read was DENIED + /// reaches the control plane with its paired token. + /// + /// This is the decision the whole ticket is about, and it cannot be exercised any other way: + /// the test process here runs with whatever privilege CI grants it, a root run cannot observe + /// the denial, and a unit test cannot drop privileges. Passing the master read's OUTCOME in is + /// what makes the unprivileged case reachable from a privileged process. + #[test] + fn a_denied_master_read_falls_back_to_the_paired_token() { + let chosen = select_token(Err(denied()), || Some("paired-token".into())) + .expect("the paired token is the second rung"); + assert_eq!(chosen, "paired-token"); + } + + /// **Proves:** with no paired token, the master read's rich remedy survives INTACT. + /// + /// The remedy text landed in #458 and is the only thing that tells the user how to get + /// unstuck. Both the KIND and the message are asserted, because the CLI maps the kind to its + /// exit code — a ladder that replaced the error with a generic one would still "fail", and a + /// test asserting only `is_err()` would not see the regression. + #[test] + fn with_no_paired_token_the_master_remedy_is_returned_unchanged() { + let original = denied(); + let kind = original.kind(); + let text = original.to_string(); + + let err = select_token(Err(original), || None).expect_err("no rung can succeed"); + + assert_eq!(err.kind(), kind, "the exit-code-bearing kind must survive"); + assert_eq!(err.to_string(), text, "the remedy must not be degraded"); + } + + /// **Proves:** a readable master token wins AND the paired store is never even consulted. + /// + /// The second half is the load-bearing half. A ladder that reads the paired file on every + /// call would satisfy "master wins" identically, so the observable that distinguishes them is + /// whether the thunk RAN — which is why the parameter is a thunk and not an `Option`. + #[test] + fn a_readable_master_token_wins_without_consulting_the_paired_store() { + let consulted = Cell::new(false); + let chosen = select_token(Ok("master-token".into()), || { + consulted.set(true); + Some("paired-token".into()) + }) + .expect("the master token is the first rung"); + + assert_eq!(chosen, "master-token"); + assert!( + !consulted.get(), + "rung 1 must not read the per-user store at all" + ); + } + + /// **Proves:** the per-user store round-trips, and is created owner-only on Unix. + /// + /// The mode assertion is one-sided on purpose: `0600` exactly, not "no world bit", because a + /// group-readable store would be a quieter version of the very widening this ticket refuses to + /// perform. + #[test] + fn the_per_user_store_round_trips_and_is_owner_only() { + let dir = tempfile::tempdir().unwrap(); + let path = paired_token_path_in(dir.path()); + + assert_eq!(load_paired_token(&path), None, "absent store reads as None"); + + store_paired_token(&path, "scoped-token").unwrap(); + assert_eq!(load_paired_token(&path).as_deref(), Some("scoped-token")); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "the paired store must be owner-only"); + } + } + + /// **Proves (dig-node#403):** the store resolves from the per-user base, so it can NEVER land + /// under `std::env::temp_dir()`. + /// + /// The path it used to take — `state::legacy_state_dir()`, the parent of + /// `dig_node_core::config_path()` — runs through `resolve_cache_dir`, whose fallback is + /// `temp_dir()/DigNode-/cache` whenever the canonical dir is unwritable. On Unix that is + /// a `1777` directory with a `/proc`-enumerable name, which is where a bearer credential must + /// never be written. + /// + /// Asserted structurally (the dir IS `platform_user_base()/DigNode`) as well as negatively: + /// the negative alone would pass on any host whose home simply does not sit under the temp + /// dir, which is every host, so it cannot tell a fixed resolver from the broken one. + #[test] + fn the_store_resolves_from_the_per_user_base_and_never_the_temp_fallback() { + let dir = paired_store_dir().expect("this test account has a home directory"); + assert_eq!( + dir, + dig_node_core::platform_user_base().join("DigNode"), + "the store must come from the per-user base, not the cache resolver" + ); + assert!( + !dir.starts_with(std::env::temp_dir()), + "a bearer token must never be stored under the world-writable temp dir: {}", + dir.display() + ); + // The PID-keyed fallback's own shape, spelled out: no component of the resolved path may + // look like the private cache fallback, on any host. + assert!( + !dir.components() + .any(|c| c.as_os_str().to_string_lossy().starts_with("DigNode-")), + "resolved a PID-keyed private fallback dir: {}", + dir.display() + ); + } + + /// **Proves:** with no per-user base resolvable, the store REFUSES rather than degrading to + /// the current working directory. + /// + /// `platform_user_base()` ends in `PathBuf::from(".")` when the OS knows no home and neither + /// `HOME` nor `LOCALAPPDATA` is set. A token written to `.` is a credential in whatever + /// directory the user happened to be standing in — frequently shared, and different on every + /// invocation. Refusing is correct because the ladder already has an answer for "no paired + /// token": rung 3's master remedy. + #[test] + fn an_unresolvable_per_user_base_refuses_rather_than_writing_to_the_cwd() { + for base in [PathBuf::from("."), PathBuf::new()] { + let err = paired_store_dir_from(base.clone()) + .expect_err("no per-user base must not resolve to a path"); + assert_eq!(err.kind(), io::ErrorKind::NotFound); + assert!( + err.to_string().contains("no per-user directory"), + "the refusal must say WHY: {err}" + ); + } + // The truthful control: a real base still resolves, so the refusal above is a property of + // the unresolvable input and not of the function refusing everything. + assert_eq!( + paired_store_dir_from(PathBuf::from("/home/u")).unwrap(), + PathBuf::from("/home/u").join("DigNode") + ); + } + + /// **Proves (dig-node#403):** the store is created EXCLUSIVELY and never follows a symlink an + /// attacker planted at its path. + /// + /// This is the revert-proof for the safe-create. The previous implementation was + /// `fs::write` + a later chmod; `write` opens with `O_TRUNC` and FOLLOWS symlinks, so a + /// `client-token -> ` planted in a directory the attacker can create hands them the + /// token — or, pointed at `~/.ssh/authorized_keys`, clobbers an arbitrary file as the victim, + /// with no race to win. Restoring `fs::write` fails this test: the victim's contents become + /// the token. + /// + /// The transient-mode window (the file existing at `0644` between `write` and the chmod) is + /// NOT separately asserted, and deliberately not faked: observing it requires racing a + /// single `write` call from another thread, which is inherently flaky and would assert timing + /// rather than the property. `create_new` + `mode(0o600)` removes the window structurally — + /// the mode is applied at `open(2)` time, so no wider mode ever exists to observe. + #[cfg(unix)] + #[test] + fn the_store_is_created_exclusively_and_never_follows_a_planted_symlink() { + use std::os::unix::fs::PermissionsExt; + + let dir = tempfile::tempdir().unwrap(); + let victim = dir.path().join("victim"); + std::fs::write(&victim, "PRECIOUS").unwrap(); + + let store = dir.path().join("store"); + std::fs::create_dir(&store).unwrap(); + let path = paired_token_path_in(&store); + std::os::unix::fs::symlink(&victim, &path).expect("plant the symlink"); + + store_paired_token(&path, "scoped-token").unwrap(); + + assert_eq!( + std::fs::read_to_string(&victim).unwrap(), + "PRECIOUS", + "the token was written THROUGH the symlink into the victim file" + ); + assert!( + !std::fs::symlink_metadata(&path).unwrap().is_symlink(), + "the store must be a regular file the invoking user created" + ); + assert_eq!(load_paired_token(&path).as_deref(), Some("scoped-token")); + let mode = std::fs::metadata(&path).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o600, "created owner-only, not chmodded afterwards"); + } + + /// **Proves:** re-pairing REPLACES this user's own store rather than failing on `create_new`. + /// + /// The exclusive create is what closes the symlink hole, and the obvious way to get it wrong + /// is to make a second `pair connect` fail with `AlreadyExists`. Both halves are needed: the + /// test above would pass on an implementation that never overwrites anything. + #[test] + fn re_pairing_replaces_this_users_own_store() { + let dir = tempfile::tempdir().unwrap(); + let path = paired_token_path_in(dir.path()); + store_paired_token(&path, "first").unwrap(); + store_paired_token(&path, "second").unwrap(); + assert_eq!(load_paired_token(&path).as_deref(), Some("second")); + } + + /// **Proves:** a blank store is not a token — it reads as "this account has no paired token" + /// so the ladder falls through to the master remedy instead of presenting an empty header. + #[test] + fn a_blank_store_is_not_a_token() { + let dir = tempfile::tempdir().unwrap(); + let path = paired_token_path_in(dir.path()); + std::fs::write(&path, " \n").unwrap(); + assert_eq!(load_paired_token(&path), None); + } + + /// **Proves:** an over-long `--client-name` is REFUSED, not truncated. + /// + /// Pinned from BOTH sides: exactly at the bound must pass, one character over must fail. A + /// bound tested only from below can only confirm itself, and the failing side is the one that + /// matters — a client that clipped locally would send a short, trusted-looking name for the + /// operator to approve. + #[test] + fn an_over_long_client_name_is_refused_not_truncated() { + let at_bound = "n".repeat(crate::pairing::MAX_CLIENT_NAME); + assert_eq!( + validate_client_name(&at_bound).unwrap(), + at_bound, + "a name AT the bound is accepted verbatim" + ); + + let too_long = "n".repeat(crate::pairing::MAX_CLIENT_NAME + 1); + let err = validate_client_name(&too_long).expect_err("one over the bound is refused"); + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert!( + !err.to_string().contains(&"n".repeat(8)), + "the refusal must not echo a shortened form of the name back as if it were usable" + ); + } + + /// **Proves:** the default label never trips the bound the client just promised to respect. + #[test] + fn the_default_client_name_is_within_the_bound() { + let name = default_client_name(); + assert!(validate_client_name(&name).is_ok(), "default was {name:?}"); + assert!(!name.is_empty()); + } + + /// **Proves:** polling TERMINATES on the server's deadline rather than spinning. + /// + /// Time is pinned explicitly rather than read from the wall clock — a fixture that passed a + /// small literal through a real-clock API would be ~1.8 billion seconds expired and would + /// exercise only the expiry arm while claiming to test both. + #[test] + fn polling_waits_until_the_servers_deadline_and_then_stops() { + const NOW: u64 = 1_700_000_000_000; + let interval = Duration::from_secs(3); + + assert_eq!( + next_poll_step(NOW, NOW + 60_000, interval), + PollStep::Wait(interval), + "well inside the window, poll at the ordinary interval" + ); + assert_eq!( + next_poll_step(NOW, NOW + 1_000, interval), + PollStep::Wait(Duration::from_millis(1_000)), + "the last wait is clamped so the loop cannot sleep PAST the deadline" + ); + assert_eq!( + next_poll_step(NOW, NOW, interval), + PollStep::Expired, + "at the deadline the node has already stopped answering" + ); + assert_eq!( + next_poll_step(NOW + 1, NOW, interval), + PollStep::Expired, + "past the deadline, stop" + ); + } +} diff --git a/crates/dig-node-service/src/pairing.rs b/crates/dig-node-service/src/pairing.rs index 9388ea85..b77fb8d9 100644 --- a/crates/dig-node-service/src/pairing.rs +++ b/crates/dig-node-service/src/pairing.rs @@ -86,7 +86,7 @@ const MAX_PENDING: usize = 32; /// /// The stored value stays BYTE-VERBATIM; neutralisation happens at render time /// ([`crate::untrusted_text::render_untrusted`]), because only the display is a lie surface. -const MAX_CLIENT_NAME: usize = 64; +pub const MAX_CLIENT_NAME: usize = 64; /// Current unix time in milliseconds (0 on a clock error — only affects TTL math). fn now_ms() -> u64 {