From d2e4b00789a161aca770147db484d4ab11c97e98 Mon Sep 17 00:00:00 2001 From: archipelago Date: Tue, 4 Aug 2026 08:50:46 -0400 Subject: [PATCH] fix(security): gate classifies from the catalog overlay and releases withdrawn claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dev-box verification of the Tor/FIPS fixes caught a pre-existing split brain: the orchestrator publishes containers from the signed catalog's embedded manifests (origin-wins), but the gate classified ports from the stale disk manifests — so it externally bound nbxplorer 32838, a port the catalog declares auth: local and pins to loopback. Reachable behind a login, but reachable where it deliberately was not. - build_port_map now consults the catalog overlay first, via the same parse/validate/image-only filter the orchestrator uses (moved to app_catalog::catalog_manifest_overlay so the two cannot diverge again). - GatedPort carries . The gated set still includes undeclared Session-default ports for challenge/audit, but every action that REDIRECTS traffic — the torrc 127.0.0.2 repoint, the FIPS relay stand-down, the Tor-upstream bind — now keys on the declaration. - The sweep releases held claims whose port left the gated set, so a catalog refresh that withdraws a port (gated → local/none) takes effect without a daemon restart. Co-Authored-By: Claude Fable 5 --- core/archipelago/src/api/rpc/tor/mod.rs | 1 + core/archipelago/src/appgate/identity.rs | 291 ++++++++++++------ core/archipelago/src/appgate/listener.rs | 78 +++-- core/archipelago/src/appgate/mod.rs | 1 + core/archipelago/src/container/app_catalog.rs | 40 +++ .../src/container/prod_orchestrator.rs | 25 +- core/archipelago/src/server.rs | 1 + 7 files changed, 293 insertions(+), 144 deletions(-) diff --git a/core/archipelago/src/api/rpc/tor/mod.rs b/core/archipelago/src/api/rpc/tor/mod.rs index 7b3cc5af..6153a0b0 100644 --- a/core/archipelago/src/api/rpc/tor/mod.rs +++ b/core/archipelago/src/api/rpc/tor/mod.rs @@ -231,6 +231,7 @@ pub(in crate::api::rpc) async fn regenerate_torrc(config: &ServicesConfig) -> Re // not an instruction (the v1.7.121 incident rule). let gated_ports: std::collections::HashSet = crate::appgate::identity::build_port_map() .gated_ports() + .filter(|g| g.declared) .map(|g| g.port) .collect(); diff --git a/core/archipelago/src/appgate/identity.rs b/core/archipelago/src/appgate/identity.rs index 26ef272c..aeee0b49 100644 --- a/core/archipelago/src/appgate/identity.rs +++ b/core/archipelago/src/appgate/identity.rs @@ -26,6 +26,15 @@ pub struct GatedPort { pub app_name: String, /// Manifest-declared icon path (`metadata.icon`), when present. pub icon: Option, + /// True only when the manifest says `auth: gated` in so many words. + /// + /// The gated set deliberately also carries undeclared Session-default + /// ports (so the gate challenges them wherever it can already stand, and + /// the audit reports them). But everything that CHANGES where traffic + /// goes — the torrc repoint to 127.0.0.2, the FIPS relay stand-down, the + /// Tor-upstream bind — must key on this flag: acting on an undeclared + /// port is the v1.7.121 incident class, whatever the action. + pub declared: bool, } /// A port deliberately left unauthenticated, and the manifest's stated reason. @@ -101,13 +110,35 @@ fn manifest_icon(manifest: &AppManifest) -> Option { /// Classify every published port across all installed manifests. /// -/// The first directory that yields a manifest for an app id wins, so a node's -/// `/opt/archipelago/apps` copy shadows a repo checkout rather than merging -/// with it — otherwise a stale checked-out manifest could re-open a port the -/// installed one gates. +/// The signed catalog's embedded manifests are consulted FIRST, because they +/// are what the orchestrator actually publishes containers from +/// (origin-wins; see `app_catalog::catalog_manifest_overlay`). Classifying +/// from disk alone made the gate act on policy the node was no longer +/// running: the catalog declared nbxplorer `auth: local` and pinned it to +/// loopback, the stale disk manifest declared nothing, and the gate +/// externally bound a deliberately host-local port (archi-dev-box +/// 2026-08-04). +/// +/// After the catalog, the first directory that yields a manifest for an app +/// id wins, so a node's `/opt/archipelago/apps` copy shadows a repo checkout +/// rather than merging with it — otherwise a stale checked-out manifest could +/// re-open a port the installed one gates. pub fn build_port_map() -> PortMap { let mut map = PortMap::default(); - let mut seen_apps: HashMap = HashMap::new(); + let mut seen_apps: std::collections::HashSet = std::collections::HashSet::new(); + + for (app_id, value) in crate::container::app_catalog::catalog_manifest_values() { + let Some(manifest) = + crate::container::app_catalog::catalog_manifest_overlay(&app_id, value) + else { + // Unparseable/invalid/build-source → the orchestrator falls back + // to disk for this app, so classification must too. + continue; + }; + if seen_apps.insert(app_id) { + classify_manifest(&manifest, &mut map); + } + } for dir in apps_dirs() { let Ok(entries) = std::fs::read_dir(&dir) else { @@ -124,100 +155,8 @@ pub fn build_port_map() -> PortMap { // would have published. continue; }; - let app_id = manifest.app.id.clone(); - if seen_apps.contains_key(&app_id) { - continue; - } - seen_apps.insert(app_id.clone(), path); - - let icon = manifest_icon(&manifest); - let app_name = if manifest.app.name.trim().is_empty() { - app_id.clone() - } else { - manifest.app.name.clone() - }; - - for port in &manifest.app.ports { - let protocol = if port.protocol.is_empty() { - "tcp" - } else { - port.protocol.as_str() - }; - match port.auth_policy() { - PortAuth::None => map.exempt.push(ExemptPort { - port: port.host, - app_id: app_id.clone(), - rationale: port - .auth_rationale - .clone() - .unwrap_or_else(|| "(no rationale recorded)".to_string()), - protocol: protocol.to_string(), - }), - // Declared host-local. Not gated and not reported as - // exposed, because it is neither — see PortAuth::Local - // for why this cannot be inferred from `bind`. - PortAuth::Local => {} - // Explicit opt-in: the app is on loopback and the daemon - // owns the external addresses. This is the ONLY way a - // port gets bound by the gate, regardless of `bind`. - PortAuth::Gated => { - map.gated.insert( - port.host, - GatedPort { - port: port.host, - app_id: app_id.clone(), - app_name: app_name.clone(), - icon: icon.clone(), - }, - ); - } - PortAuth::Session => { - // UDP cannot carry an HTTP challenge. Such a port has - // no business defaulting into the gated set where it - // would look protected without being protectable — - // surface it as an unrationalised exemption instead, - // which is honest and shows up in the audit list. - if protocol != "tcp" { - map.exempt.push(ExemptPort { - port: port.host, - app_id: app_id.clone(), - rationale: format!( - "{protocol} cannot carry an HTTP challenge; declare auth: none \ - with a rationale to record why this is safe" - ), - protocol: protocol.to_string(), - }); - continue; - } - // A loopback publish is skipped, and this is the - // safety property of the whole module: the gate must - // never be the reason a port becomes reachable - // somewhere it was not. `session` is the DEFAULT, so - // it is what every un-migrated manifest carries — - // and a node's installed manifests always lag the - // repo. Binding those externally published Bitcoin - // RPC across the LAN within seconds of deploy - // (archi-dev-box 2026-08-03). Taking over a port is - // opt-in only: `auth: gated`, shipped in the same - // manifest edit as the loopback pin. - if port - .bind - .parse::() - .is_ok_and(|ip| ip.is_loopback()) - { - continue; - } - map.gated.insert( - port.host, - GatedPort { - port: port.host, - app_id: app_id.clone(), - app_name: app_name.clone(), - icon: icon.clone(), - }, - ); - } - } + if seen_apps.insert(manifest.app.id.clone()) { + classify_manifest(&manifest, &mut map); } } } @@ -226,6 +165,103 @@ pub fn build_port_map() -> PortMap { map } +/// Classify one manifest's ports into the map. Split from [`build_port_map`] +/// so the catalog-overlay pass and the disk pass cannot diverge. +fn classify_manifest(manifest: &AppManifest, map: &mut PortMap) { + let app_id = manifest.app.id.clone(); + let icon = manifest_icon(manifest); + let app_name = if manifest.app.name.trim().is_empty() { + app_id.clone() + } else { + manifest.app.name.clone() + }; + + for port in &manifest.app.ports { + let protocol = if port.protocol.is_empty() { + "tcp" + } else { + port.protocol.as_str() + }; + match port.auth_policy() { + PortAuth::None => map.exempt.push(ExemptPort { + port: port.host, + app_id: app_id.clone(), + rationale: port + .auth_rationale + .clone() + .unwrap_or_else(|| "(no rationale recorded)".to_string()), + protocol: protocol.to_string(), + }), + // Declared host-local. Not gated and not reported as + // exposed, because it is neither — see PortAuth::Local + // for why this cannot be inferred from `bind`. + PortAuth::Local => {} + // Explicit opt-in: the app is on loopback and the daemon + // owns the external addresses. This is the ONLY way a + // port gets bound by the gate, regardless of `bind`. + PortAuth::Gated => { + map.gated.insert( + port.host, + GatedPort { + port: port.host, + app_id: app_id.clone(), + app_name: app_name.clone(), + icon: icon.clone(), + declared: true, + }, + ); + } + PortAuth::Session => { + // UDP cannot carry an HTTP challenge. Such a port has + // no business defaulting into the gated set where it + // would look protected without being protectable — + // surface it as an unrationalised exemption instead, + // which is honest and shows up in the audit list. + if protocol != "tcp" { + map.exempt.push(ExemptPort { + port: port.host, + app_id: app_id.clone(), + rationale: format!( + "{protocol} cannot carry an HTTP challenge; declare auth: none \ + with a rationale to record why this is safe" + ), + protocol: protocol.to_string(), + }); + continue; + } + // A loopback publish is skipped, and this is the + // safety property of the whole module: the gate must + // never be the reason a port becomes reachable + // somewhere it was not. `session` is the DEFAULT, so + // it is what every un-migrated manifest carries — + // and a node's installed manifests always lag the + // repo. Binding those externally published Bitcoin + // RPC across the LAN within seconds of deploy + // (archi-dev-box 2026-08-03). Taking over a port is + // opt-in only: `auth: gated`, shipped in the same + // manifest edit as the loopback pin. + if port + .bind + .parse::() + .is_ok_and(|ip| ip.is_loopback()) + { + continue; + } + map.gated.insert( + port.host, + GatedPort { + port: port.host, + app_id: app_id.clone(), + app_name: app_name.clone(), + icon: icon.clone(), + declared: false, + }, + ); + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -257,6 +293,63 @@ mod tests { } } + fn manifest(yaml: &str) -> AppManifest { + AppManifest::parse(yaml).expect("test manifest must parse") + } + + const BASE: &str = r#" +app: + id: testapp + name: Test App + version: "1.0" + container: + image: example.org/testapp:1.0 +"#; + + /// `auth: gated` is the only classification allowed to redirect traffic — + /// torrc repoints, relay stand-down, and the 127.0.0.2 bind all key on + /// `declared`. An undeclared Session port is challenged and audited but + /// must never be `declared`. + #[test] + fn declared_tracks_the_manifest_not_the_default() { + let mut map = PortMap::default(); + classify_manifest( + &manifest(&format!( + "{BASE} ports:\n - host: 8090\n container: 7777\n protocol: tcp\n bind: 127.0.0.1\n auth: gated\n" + )), + &mut map, + ); + assert!(map.gated(8090).expect("gated").declared); + + let mut map = PortMap::default(); + classify_manifest( + &manifest(&format!( + "{BASE} ports:\n - host: 9100\n container: 9100\n protocol: tcp\n" + )), + &mut map, + ); + let undeclared = map.gated(9100).expect("session default is challenged"); + assert!( + !undeclared.declared, + "an absent auth field must never read as an instruction" + ); + } + + /// `auth: local` keeps the gate's hands off entirely — the port is + /// neither gated nor exempt-reported. + #[test] + fn local_ports_are_untouched() { + let mut map = PortMap::default(); + classify_manifest( + &manifest(&format!( + "{BASE} ports:\n - host: 32838\n container: 32838\n protocol: tcp\n bind: 127.0.0.1\n auth: local\n" + )), + &mut map, + ); + assert!(map.gated(32838).is_none()); + assert!(map.exempt_ports().is_empty()); + } + /// Protocol ports that wallets dial directly must never end up gated — /// this is the constraint that decided the design (Zeus and electrum /// clients keep working untouched). diff --git a/core/archipelago/src/appgate/listener.rs b/core/archipelago/src/appgate/listener.rs index 4e1a198d..0a7844da 100644 --- a/core/archipelago/src/appgate/listener.rs +++ b/core/archipelago/src/appgate/listener.rs @@ -151,8 +151,12 @@ pub async fn run( mut shutdown_rx: tokio::sync::watch::Receiver, ) { // (port, addr) pairs already served, so a sweep does not rebind what it - // already holds. - let mut held: HashMap<(u16, IpAddr), ()> = HashMap::new(); + // already holds. The accept-loop handle is kept so a claim can be + // RELEASED when its port leaves the gated set — a catalog refresh + // declaring a port `local`/`none` must make the gate let go without a + // daemon restart, or the stale bind keeps republishing a port the + // catalog just withdrew (nbxplorer 32838, archi-dev-box 2026-08-04). + let mut held: HashMap<(u16, IpAddr), tokio::task::JoinHandle<()>> = HashMap::new(); let mut interval = tokio::time::interval(SWEEP_INTERVAL); interval.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); @@ -169,7 +173,7 @@ pub async fn run( async fn sweep( gate: &Arc, status: &Arc>, - held: &mut HashMap<(u16, IpAddr), ()>, + held: &mut HashMap<(u16, IpAddr), tokio::task::JoinHandle<()>>, shutdown_rx: &tokio::sync::watch::Receiver, ) { // Re-read the manifests every sweep rather than trusting the map built @@ -179,6 +183,23 @@ async fn sweep( // enforced while serving a brand-new app to anyone who asked. gate.refresh().await; let port_map = gate.port_map().await; + + // Release claims whose port left the gated set (or whose Tor-upstream + // claim lost its declaration). Aborting the accept loop drops the + // listener, freeing the address for whoever now legitimately owns it — + // the app itself, or nobody. + held.retain(|(port, addr), handle| { + let keep = match port_map.gated(*port) { + None => false, + Some(app) => *addr != GATE_TOR_UPSTREAM || app.declared, + }; + if !keep { + handle.abort(); + info!(port, %addr, "app gate released a claim: port is no longer gated here"); + } + keep + }); + let addresses = host_addresses().await; if addresses.is_empty() { debug!("app gate: no external addresses yet"); @@ -214,34 +235,46 @@ async fn sweep( } match TcpListener::bind(SocketAddr::new(addr, app.port)).await { Ok(listener) => { - held.insert(key, ()); + let handle = + spawn_accept_loop(listener, gate.clone(), app.clone(), shutdown_rx.clone()); + held.insert(key, handle); claimed.push((app.port, addr.to_string())); claimed_any = true; info!( port = app.port, %addr, app = %app.app_id, "app gate claimed an app port" ); - spawn_accept_loop(listener, gate.clone(), app.clone(), shutdown_rx.clone()); } // Almost always the app itself holding 0.0.0.0:. Err(_) => blocked = true, } } - let tor_key = (app.port, GATE_TOR_UPSTREAM); - if held.contains_key(&tor_key) { - claimed.push((app.port, GATE_TOR_UPSTREAM.to_string())); - } else { - match TcpListener::bind(SocketAddr::new(GATE_TOR_UPSTREAM, app.port)).await { - Ok(listener) => { - held.insert(tor_key, ()); - claimed.push((app.port, GATE_TOR_UPSTREAM.to_string())); - info!( - port = app.port, app = %app.app_id, - "app gate claimed the Tor upstream (127.0.0.2)" - ); - spawn_accept_loop(listener, gate.clone(), app.clone(), shutdown_rx.clone()); + // The Tor upstream is bound for DECLARED gated ports only: torrc only + // repoints an onion at 127.0.0.2 for a declared port, and standing a + // challenge on an undeclared port's would-be upstream would change + // where its traffic goes on nothing but a default. + if app.declared { + let tor_key = (app.port, GATE_TOR_UPSTREAM); + if held.contains_key(&tor_key) { + claimed.push((app.port, GATE_TOR_UPSTREAM.to_string())); + } else { + match TcpListener::bind(SocketAddr::new(GATE_TOR_UPSTREAM, app.port)).await { + Ok(listener) => { + let handle = spawn_accept_loop( + listener, + gate.clone(), + app.clone(), + shutdown_rx.clone(), + ); + held.insert(tor_key, handle); + claimed.push((app.port, GATE_TOR_UPSTREAM.to_string())); + info!( + port = app.port, app = %app.app_id, + "app gate claimed the Tor upstream (127.0.0.2)" + ); + } + Err(_) => blocked = true, } - Err(_) => blocked = true, } } @@ -281,12 +314,15 @@ async fn app_is_listening(port: u16) -> bool { .is_some() } +/// Returns the accept-loop task handle so the sweep can release the claim +/// (abort → listener drops → address freed) when the port leaves the gated +/// set. In-flight connections finish on their own tasks. fn spawn_accept_loop( listener: TcpListener, gate: Arc, app: GatedPort, mut shutdown_rx: tokio::sync::watch::Receiver, -) { +) -> tokio::task::JoinHandle<()> { tokio::spawn(async move { loop { tokio::select! { @@ -317,7 +353,7 @@ fn spawn_accept_loop( _ = shutdown_rx.changed() => break, } } - }); + }) } #[cfg(test)] diff --git a/core/archipelago/src/appgate/mod.rs b/core/archipelago/src/appgate/mod.rs index 8ac1ef43..dee3e7e0 100644 --- a/core/archipelago/src/appgate/mod.rs +++ b/core/archipelago/src/appgate/mod.rs @@ -578,6 +578,7 @@ mod tests { app_id: "strfry".to_string(), app_name: "Strfry Relay".to_string(), icon: None, + declared: true, } } diff --git a/core/archipelago/src/container/app_catalog.rs b/core/archipelago/src/container/app_catalog.rs index fccb23ba..63b46b30 100644 --- a/core/archipelago/src/container/app_catalog.rs +++ b/core/archipelago/src/container/app_catalog.rs @@ -216,6 +216,46 @@ pub fn catalog_manifest_values() -> Vec<(String, serde_json::Value)> { .collect() } +/// A catalog-embedded manifest as the node actually applies it: parsed, +/// id-checked, validated, and image-only (build-source manifests defer to +/// disk). `None` = the caller must fall back to the disk manifest. +/// +/// Shared between the orchestrator's load overlay and the app gate's port +/// classification so both answer "which manifest governs this app?" from the +/// same origin. They diverged once — the orchestrator published containers +/// from the catalog while the gate classified from stale disk manifests, and +/// the gate externally bound a port the catalog had declared `auth: local` +/// (nbxplorer 32838, archi-dev-box 2026-08-04). +pub fn catalog_manifest_overlay( + app_id: &str, + value: serde_json::Value, +) -> Option { + let m: archipelago_container::manifest::AppManifest = match serde_json::from_value(value) { + Ok(m) => m, + Err(e) => { + tracing::warn!(app = %app_id, error = %e, + "skipping unparseable catalog manifest; using disk fallback"); + return None; + } + }; + if m.app.id != app_id { + tracing::warn!(catalog_id = %app_id, manifest_id = %m.app.id, + "skipping catalog manifest: embedded app id mismatches catalog key"); + return None; + } + if let Err(e) = m.validate() { + tracing::warn!(app = %app_id, error = %e, + "skipping invalid catalog manifest; using disk fallback"); + return None; + } + if m.app.container.build.is_some() { + tracing::debug!(app = %app_id, + "catalog manifest has a build source; deferring to disk (phase 1 = image-only)"); + return None; + } + Some(m) +} + /// The catalog's default/latest version string for an app (the top-level /// `version` field), if covered. Used to decide whether an install-time /// selection should pin (older) or track-latest (default). diff --git a/core/archipelago/src/container/prod_orchestrator.rs b/core/archipelago/src/container/prod_orchestrator.rs index 0a31df14..a68c605a 100644 --- a/core/archipelago/src/container/prod_orchestrator.rs +++ b/core/archipelago/src/container/prod_orchestrator.rs @@ -1183,30 +1183,7 @@ struct LoadedManifest { /// source (build contexts aren't registry-distributed yet — phase 1 is /// image-only). See `docs/registry-manifest-design.md`. fn catalog_manifest_to_overlay(app_id: &str, value: serde_json::Value) -> Option { - let m: AppManifest = match serde_json::from_value(value) { - Ok(m) => m, - Err(e) => { - tracing::warn!(app = %app_id, error = %e, - "skipping unparseable catalog manifest; using disk fallback"); - return None; - } - }; - if m.app.id != app_id { - tracing::warn!(catalog_id = %app_id, manifest_id = %m.app.id, - "skipping catalog manifest: embedded app id mismatches catalog key"); - return None; - } - if let Err(e) = m.validate() { - tracing::warn!(app = %app_id, error = %e, - "skipping invalid catalog manifest; using disk fallback"); - return None; - } - if m.app.container.build.is_some() { - tracing::debug!(app = %app_id, - "catalog manifest has a build source; deferring to disk (phase 1 = image-only)"); - return None; - } - Some(m) + crate::container::app_catalog::catalog_manifest_overlay(app_id, value) } struct OrchestratorState { diff --git a/core/archipelago/src/server.rs b/core/archipelago/src/server.rs index 8969b212..6ee7c73a 100644 --- a/core/archipelago/src/server.rs +++ b/core/archipelago/src/server.rs @@ -1162,6 +1162,7 @@ async fn app_port_v6_relay_loop(mut shutdown_rx: tokio::sync::watch::Receiver = crate::appgate::identity::build_port_map() .gated_ports() + .filter(|g| g.declared) .map(|g| g.port) .collect(); for &port in crate::fips::app_ports::APP_LAUNCH_PORTS {