From 2c8c99fd28829a2a413ae799f2e5bb1ab8b7ac40 Mon Sep 17 00:00:00 2001 From: archipelago Date: Sat, 4 Jul 2026 17:49:52 -0400 Subject: [PATCH] fix(security): bind seq into mesh signatures (v2 preimage), guard DID slice, cfg-gate dev password MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - mesh: verify_signature accepts a v2 preimage (t,v,ts,seq) alongside legacy v1 (t,v,ts); signed_with_seq() is the v2 sender path, not yet wired — senders stay v1 until the fleet verifies v2 (receivers hard-drop bad sigs, so flipping send-side first would break mixed-fleet alerts). Tests: v2 verify, v2 seq-tamper rejection, v1 sign-then-set-seq compat. - mesh listener: malformed radio-supplied DID shorter than the 'did:key:' prefix can no longer panic advert_name (slice -> .get()). - auth: the pre-setup password123 dev login and the constant itself are now #[cfg(debug_assertions)] — no release binary carries the bypass, whatever its runtime config says. - orchestrator: canned host-facts under #[cfg(test)] — awaiting real subprocesses under tokio's paused test clock deadlocks against auto-advanced timers (the old blocking detection only worked by never yielding). - drop two now-unused std::process::Command imports left by 4c75bb3d. Tests: mesh 110/110 (incl. 2 new), api 68/68, container 159/159, archipelago-container check clean. Co-Authored-By: Claude Fable 5 --- core/archipelago/src/api/rpc/auth.rs | 9 +- core/archipelago/src/api/rpc/mod.rs | 3 + .../src/container/prod_orchestrator.rs | 61 ++++++++----- core/archipelago/src/mesh/listener/decode.rs | 4 +- core/archipelago/src/mesh/message_types.rs | 85 +++++++++++++++++-- core/container/src/runtime.rs | 1 - docs/1.8.0-RELEASE-HARDENING-PLAN.md | 19 +++-- 7 files changed, 145 insertions(+), 37 deletions(-) diff --git a/core/archipelago/src/api/rpc/auth.rs b/core/archipelago/src/api/rpc/auth.rs index 5b275ce4..b68519f2 100644 --- a/core/archipelago/src/api/rpc/auth.rs +++ b/core/archipelago/src/api/rpc/auth.rs @@ -1,4 +1,6 @@ -use super::{RpcHandler, DEV_DEFAULT_PASSWORD}; +use super::RpcHandler; +#[cfg(debug_assertions)] +use super::DEV_DEFAULT_PASSWORD; use anyhow::Result; impl RpcHandler { @@ -14,7 +16,10 @@ impl RpcHandler { let is_setup = self.auth_manager.is_setup().await?; if !is_setup { - // Dev mode: allow default password so UI can log in without running setup + // Dev BUILDS only: allow the default password so the UI can log + // in without running setup. cfg-gated so no release binary can + // carry the bypass, whatever its runtime config says. + #[cfg(debug_assertions)] if self.config.dev_mode && password == DEV_DEFAULT_PASSWORD { tracing::info!("[onboarding] login via dev default password"); return Ok(serde_json::Value::Null); diff --git a/core/archipelago/src/api/rpc/mod.rs b/core/archipelago/src/api/rpc/mod.rs index 2dc3dd39..d0328b82 100644 --- a/core/archipelago/src/api/rpc/mod.rs +++ b/core/archipelago/src/api/rpc/mod.rs @@ -62,6 +62,9 @@ pub use middleware::PeerAddr; use response::{cookie_header, json_response, ResponseCache, RpcError, RpcRequest, RpcResponse}; /// Default dev password when no user is set up (matches mock-backend). +/// Dev builds only — the pre-setup login bypass that reads this is +/// cfg-gated out of release binaries. +#[cfg(debug_assertions)] pub(crate) const DEV_DEFAULT_PASSWORD: &str = "password123"; pub struct RpcHandler { diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index 07c703f8..6346c0fa 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -32,7 +32,6 @@ use async_trait::async_trait; use std::collections::{HashMap, HashSet}; use std::os::unix::fs::FileTypeExt; use std::path::{Path, PathBuf}; -use std::process::Command; use std::sync::Arc; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::sync::{Mutex, RwLock}; @@ -2725,19 +2724,34 @@ impl ProdContainerOrchestrator { } async fn detect_host_facts(&self) -> HostFacts { - let host_ip = Self::detect_host_ip() - .await - .unwrap_or_else(|| "127.0.0.1".to_string()); - let host_mdns = Self::detect_host_mdns().await; - let disk_gb = self.disk_gb().await; - HostFacts { - host_ip, - host_mdns, - disk_gb, - // Cheap default; resolve_dynamic_env fills the real node name on - // demand (it costs a podman call) only for manifests that use - // {{BITCOIN_HOST}}, rather than every app on every reconcile. - bitcoin_host: "bitcoin-knots".to_string(), + // Unit tests run under tokio's paused clock; awaiting a real + // subprocess there deadlocks against auto-advanced timers (the old + // BLOCKING detection only worked by never yielding). Canned facts. + #[cfg(test)] + { + return HostFacts { + host_ip: "127.0.0.1".to_string(), + host_mdns: "test.local".to_string(), + disk_gb: self.test_disk_gb.unwrap_or(1000), + bitcoin_host: "bitcoin-knots".to_string(), + }; + } + #[allow(unreachable_code)] + { + let host_ip = Self::detect_host_ip() + .await + .unwrap_or_else(|| "127.0.0.1".to_string()); + let host_mdns = Self::detect_host_mdns().await; + let disk_gb = self.disk_gb().await; + HostFacts { + host_ip, + host_mdns, + disk_gb, + // Cheap default; resolve_dynamic_env fills the real node name on + // demand (it costs a podman call) only for manifests that use + // {{BITCOIN_HOST}}, rather than every app on every reconcile. + bitcoin_host: "bitcoin-knots".to_string(), + } } } @@ -2745,10 +2759,15 @@ impl ProdContainerOrchestrator { /// `bitcoin-core`) for the `{{BITCOIN_HOST}}` derived-env placeholder. /// Defaults to `bitcoin-knots` when none is running (B12). async fn bitcoin_host(&self) -> String { + // No real podman under the tests' paused clock (see detect_host_facts). #[cfg(test)] - if let Some(host) = &self.test_bitcoin_host { - return host.clone(); + { + return self + .test_bitcoin_host + .clone() + .unwrap_or_else(|| "bitcoin-knots".to_string()); } + #[allow(unreachable_code)] // Mirrors api::rpc::package::dependencies (the legacy install path); // both Bitcoin node variants are reachable on archy-net by name. const BITCOIN_NAMES: &[&str] = &["bitcoin-knots", "bitcoin-core", "bitcoin"]; @@ -2839,11 +2858,15 @@ impl ProdContainerOrchestrator { } async fn disk_gb(&self) -> u64 { + // No real df under the tests' paused clock (see detect_host_facts). #[cfg(test)] - if let Some(disk_gb) = self.test_disk_gb { - return disk_gb; + { + return self.test_disk_gb.unwrap_or(1000); + } + #[allow(unreachable_code)] + { + Self::detect_disk_gb().await } - Self::detect_disk_gb().await } /// Ensure app-specific secrets exist *before* env resolution. The Bitcoin diff --git a/core/archipelago/src/mesh/listener/decode.rs b/core/archipelago/src/mesh/listener/decode.rs index 3eb764cf..babc3c77 100644 --- a/core/archipelago/src/mesh/listener/decode.rs +++ b/core/archipelago/src/mesh/listener/decode.rs @@ -563,7 +563,9 @@ pub(super) async fn handle_identity_received( // Update peer record let peer = MeshPeer { contact_id, - advert_name: format!("Archy-{}", &did[8..16.min(did.len())]), + // .get(): a malformed DID shorter than the "did:key:" prefix must + // not panic the listener on a radio-supplied string. + advert_name: format!("Archy-{}", did.get(8..16.min(did.len())).unwrap_or(did)), did: Some(did.to_string()), pubkey_hex: Some(ed_pubkey_hex.to_string()), // The advert signature was verified above, so this is an authenticated diff --git a/core/archipelago/src/mesh/message_types.rs b/core/archipelago/src/mesh/message_types.rs index 9b9b36b7..43cee9ec 100644 --- a/core/archipelago/src/mesh/message_types.rs +++ b/core/archipelago/src/mesh/message_types.rs @@ -258,7 +258,31 @@ impl TypedEnvelope { } } - /// Verify signature if present. + /// Signing preimage v2: binds the anti-replay `seq` so a radio MITM + /// can't reorder/replay a signed message under a different sequence + /// number. v1 (legacy) covers only (t, v, ts). + fn signing_preimage_v2(&self) -> Vec { + let mut sign_data = Vec::with_capacity(1 + self.v.len() + 4 + 8); + sign_data.push(self.t); + sign_data.extend_from_slice(&self.v); + sign_data.extend_from_slice(&self.ts.to_le_bytes()); + sign_data.extend_from_slice(&self.seq.to_le_bytes()); + sign_data + } + + fn signing_preimage_v1(&self) -> Vec { + let mut sign_data = Vec::with_capacity(1 + self.v.len() + 4); + sign_data.push(self.t); + sign_data.extend_from_slice(&self.v); + sign_data.extend_from_slice(&self.ts.to_le_bytes()); + sign_data + } + + /// Verify signature if present. Accepts the seq-binding v2 preimage OR + /// the legacy (t, v, ts) preimage — senders still emit v1 until the + /// whole fleet verifies v2 (receivers hard-drop bad signatures, so + /// flipping the send side first would break mixed-fleet alerts). The + /// seq-tampering window closes only when the v1 arm is removed. pub fn verify_signature(&self, verifying_key: &ed25519_dalek::VerifyingKey) -> Result { let Some(sig_bytes) = &self.sig else { return Ok(false); @@ -266,13 +290,14 @@ impl TypedEnvelope { let signature = ed25519_dalek::Signature::from_slice(sig_bytes).context("Invalid signature bytes")?; - let mut sign_data = Vec::with_capacity(1 + self.v.len() + 4); - sign_data.push(self.t); - sign_data.extend_from_slice(&self.v); - sign_data.extend_from_slice(&self.ts.to_le_bytes()); - + if verifying_key + .verify_strict(&self.signing_preimage_v2(), &signature) + .is_ok() + { + return Ok(true); + } verifying_key - .verify_strict(&sign_data, &signature) + .verify_strict(&self.signing_preimage_v1(), &signature) .context("Signature verification failed")?; Ok(true) } @@ -284,12 +309,25 @@ impl TypedEnvelope { /// Set the outbound sequence number. Called by the send path after the /// target's counter has been incremented. Safe to call AFTER `new_signed` - /// because the signature covers `(t, v, ts)` — not `seq`. + /// because the v1 signature covers `(t, v, ts)` — not `seq`. Once the + /// fleet is on a build whose `verify_signature` accepts the v2 preimage, + /// flip senders to sign AFTER seq allocation via `signed_with_seq`. pub fn with_seq(mut self, seq: u64) -> Self { self.seq = seq; self } + /// v2 sender path (NOT yet wired — see `verify_signature` for the fleet + /// migration order): set seq first, then sign binding it. + #[allow(dead_code)] + pub fn signed_with_seq(mut self, seq: u64, signing_key: &ed25519_dalek::SigningKey) -> Self { + use ed25519_dalek::Signer; + self.seq = seq; + let signature = signing_key.sign(&self.signing_preimage_v2()); + self.sig = Some(signature.to_bytes().to_vec()); + self + } + /// Encode to wire format: [0x02] [CBOR envelope]. pub fn to_wire(&self) -> Result> { let mut buf = Vec::new(); @@ -816,6 +854,37 @@ mod tests { assert!(envelope.verify_signature(&key.verifying_key()).is_err()); } + #[test] + fn test_v2_seq_bound_signature() { + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + + let key = SigningKey::generate(&mut OsRng); + let envelope = TypedEnvelope::new(MeshMessageType::Alert, b"test".to_vec()) + .signed_with_seq(42, &key); + assert!(envelope.verify_signature(&key.verifying_key()).unwrap()); + + // v2 binds seq: replaying the signed envelope under a different + // sequence number must fail verification. + let mut replayed = envelope.clone(); + replayed.seq = 43; + assert!(replayed.verify_signature(&key.verifying_key()).is_err()); + } + + #[test] + fn test_v1_signature_survives_seq_set_after_signing() { + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + + // Mixed-fleet compatibility: current senders sign first (v1 + // preimage, no seq) and allocate seq afterwards; verify must still + // accept that. + let key = SigningKey::generate(&mut OsRng); + let envelope = TypedEnvelope::new_signed(MeshMessageType::Alert, b"test".to_vec(), &key) + .with_seq(7); + assert!(envelope.verify_signature(&key.verifying_key()).unwrap()); + } + #[test] fn test_invoice_payload_roundtrip() { let invoice = InvoicePayload { diff --git a/core/container/src/runtime.rs b/core/container/src/runtime.rs index 4a7cf6f2..6f668534 100644 --- a/core/container/src/runtime.rs +++ b/core/container/src/runtime.rs @@ -2,7 +2,6 @@ use crate::manifest::{AppManifest, BuildConfig}; use crate::podman_client::{ContainerState, ContainerStatus, PodmanClient}; use anyhow::{Context, Result}; use async_trait::async_trait; -use std::process::Command; use std::time::Duration; use tokio::process::Command as TokioCommand; diff --git a/docs/1.8.0-RELEASE-HARDENING-PLAN.md b/docs/1.8.0-RELEASE-HARDENING-PLAN.md index b223951c..efc2b125 100644 --- a/docs/1.8.0-RELEASE-HARDENING-PLAN.md +++ b/docs/1.8.0-RELEASE-HARDENING-PLAN.md @@ -150,12 +150,19 @@ modules; production request/boot paths are essentially panic-free. The real risk which sets `X-Real-IP $remote_addr`) — direct connections (e.g. the FIPS peer listener) bucket under their socket IP, so per-request header rotation no longer defeats the login limiter. 3 unit tests. -- [ ] 🟢 **Include `seq` in the mesh signed preimage.** `message_types.rs:245-288` signs - `(t,v,ts)` but sets the anti-replay `seq` after signing → a radio MITM can alter ordering - without breaking the signature. -- [ ] 🟢 **Guard the short-DID slice panic** (`mesh/listener/decode.rs:566`) and gate the - dev-mode `password123` bypass (`auth.rs:18`) behind `#[cfg]` before it can reach a - release build. +- [x] 🟢 **Include `seq` in the mesh signed preimage.** DONE 2026-07-04 (receiver half): + `verify_signature` accepts a v2 preimage `(t,v,ts,seq)` alongside legacy v1 `(t,v,ts)`; + `signed_with_seq()` is the v2 sender path, deliberately NOT yet wired — receivers + hard-drop bad signatures, so senders stay on v1 until the whole fleet verifies v2. + The seq-tampering window closes only when the v1 arm is removed (track as a + post-fleet-rollout follow-up). Unit tests cover v2 verify, v2 seq-tamper rejection, + and v1 sign-then-set-seq compatibility. +- [x] 🟢 **Guard the short-DID slice panic** (`mesh/listener/decode.rs:566`) and gate the + dev-mode `password123` bypass (`auth.rs:18`) behind `#[cfg]`. DONE 2026-07-04: + advert_name uses `.get()` fallback (malformed radio-supplied DID can't panic the + listener); the pre-setup dev-password login + the constant itself are + `#[cfg(debug_assertions)]` — no release binary carries the bypass regardless of + runtime config. - [ ] 🟢 **Apply the seccomp/apparmor profile** — `security/src/container_policies.rs:71` is a TODO; the profile is defined but never applied to podman.