fix(security): silence is not consent — undeclared ports are never acted on
Two live incidents on archi-dev-box today, one bug. Both times a safety
decision read an ABSENT manifest field as if it were a value, and a
node's installed manifests always lag the binary — so "absent" is the
state of essentially every port on every node.
1. Gating any `session` port regardless of `bind` published Bitcoin's
loopback-only RPC 8332 on the LAN, Tailscale and IPv6 within seconds
of deploy.
2. The `bind`-keyed replacement looked safe because it protected
`bind: 127.0.0.1` ports — but LND's gRPC 10009 and REST 18080 carry
an EMPTY bind, so they fell through. One container recreate from
pinning them to loopback and breaking Zeus and every remote wallet.
`auth` is now `Option<PortAuth>`, separating two questions that were
conflated:
* `auth_policy()` — what to CLASSIFY the port as. Undeclared reports as
Session, i.e. shows in the audit as something that should be behind
the gate. Reporting is always safe.
* `auth_is_declared()` — whether the daemon may ACT. Only an explicit
declaration authorises changing how a port is published.
Also reverts the daemon-side publish rewriting entirely. The node proved
it wrong twice over: the recreate path that actually ran was in
package::install, not podman_client, so the pin never fired; and even
`bind: 127.0.0.1` written directly into the node's manifest was
overridden by the signed catalog. Publishes are built in several places
and all of them already honour `bind`, so the migration belongs in the
catalog as data — not in daemon-side inference that can only ever cover
one path and guess wrong on the rest.
Tests: 75/75 container, incl. the LND wallet-port shape (`host: 10009`,
empty bind, no auth) asserted to be non-actionable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
edc9a172e9
commit
ab2c8b6e96
@@ -143,7 +143,7 @@ pub fn build_port_map() -> PortMap {
|
||||
} else {
|
||||
port.protocol.as_str()
|
||||
};
|
||||
match port.auth {
|
||||
match port.auth_policy() {
|
||||
PortAuth::None => map.exempt.push(ExemptPort {
|
||||
port: port.host,
|
||||
app_id: app_id.clone(),
|
||||
@@ -157,6 +157,20 @@ pub fn build_port_map() -> PortMap {
|
||||
// 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
|
||||
@@ -175,12 +189,24 @@ pub fn build_port_map() -> PortMap {
|
||||
});
|
||||
continue;
|
||||
}
|
||||
// NOTE: a loopback `bind` is deliberately NOT skipped
|
||||
// here. Pinning an app to loopback is exactly what
|
||||
// frees its external addresses for the gate to claim
|
||||
// — skipping those would mean nothing is gated once
|
||||
// the migration is done. Ports that must never be
|
||||
// externally reachable say so with `auth: local`.
|
||||
// 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::<std::net::IpAddr>()
|
||||
.is_ok_and(|ip| ip.is_loopback())
|
||||
{
|
||||
continue;
|
||||
}
|
||||
map.gated.insert(
|
||||
port.host,
|
||||
GatedPort {
|
||||
@@ -259,18 +285,37 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The migration property, and the one a `bind`-sniffing heuristic got
|
||||
/// backwards: pinning an app to loopback is what frees its external
|
||||
/// addresses for the gate, so such a port must STILL be gated. If this
|
||||
/// regresses, completing the rollout would silently gate nothing.
|
||||
/// THE safety property. A `session` port pinned to loopback must NOT be
|
||||
/// gated, because gating means binding external addresses — the one
|
||||
/// action that can make a port reachable where it was not.
|
||||
///
|
||||
/// This is not hypothetical. `session` is the default, so it is what
|
||||
/// every un-migrated manifest carries, and a node's installed manifests
|
||||
/// always lag the repo. An earlier revision gated these regardless of
|
||||
/// `bind`, and within seconds of deploying to archi-dev-box the daemon
|
||||
/// had published Bitcoin's loopback-only RPC 8332 on the LAN, Tailscale
|
||||
/// and IPv6 addresses. Taking over a port must be opt-in.
|
||||
#[test]
|
||||
fn a_loopback_pinned_session_port_is_still_gated() {
|
||||
use archipelago_container::manifest::AppManifest;
|
||||
let yaml = "app:\n id: pinned\n name: Pinned\n version: 1.0.0\n container:\n image: x:y\n ports:\n - host: 9911\n container: 80\n bind: 127.0.0.1\n";
|
||||
fn a_loopback_pinned_session_port_is_never_gated() {
|
||||
let map = build_port_map();
|
||||
// aiui and bitcoin RPC are both loopback-pinned in the shipped tree.
|
||||
for port in [5180, 8332] {
|
||||
assert!(
|
||||
map.gated(port).is_none(),
|
||||
"port {port} is loopback-pinned; gating it would newly expose it"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The migration end state: `auth: gated` opts a loopback-pinned port
|
||||
/// into daemon ownership. Without this the rollout could never complete.
|
||||
#[test]
|
||||
fn an_explicitly_gated_loopback_port_is_gated() {
|
||||
use archipelago_container::manifest::{AppManifest, PortAuth as PA};
|
||||
let yaml = "app:\n id: pinned\n name: Pinned\n version: 1.0.0\n container:\n image: x:y\n ports:\n - host: 9911\n container: 80\n bind: 127.0.0.1\n auth: gated\n";
|
||||
let m = AppManifest::parse(yaml).expect("parses");
|
||||
let port = &m.app.ports[0];
|
||||
assert_eq!(port.auth, PortAuth::Session);
|
||||
assert_eq!(port.bind, "127.0.0.1");
|
||||
assert_eq!(m.app.ports[0].auth, Some(PA::Gated));
|
||||
assert_eq!(m.app.ports[0].bind, "127.0.0.1");
|
||||
}
|
||||
|
||||
/// An app UI that was reachable with no credential in the 2026-08-03
|
||||
|
||||
@@ -4435,7 +4435,7 @@ mod tests {
|
||||
container,
|
||||
protocol: "tcp".to_string(),
|
||||
bind: String::new(),
|
||||
auth: archipelago_container::manifest::PortAuth::Session,
|
||||
auth: None,
|
||||
auth_rationale: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -547,6 +547,20 @@ pub enum PortAuth {
|
||||
/// declared. `Local` means the first case: never externally reachable,
|
||||
/// gate keeps its hands off.
|
||||
Local,
|
||||
/// The app publishes on loopback ONLY, and the daemon owns this port's
|
||||
/// external addresses — bind them and authenticate every connection.
|
||||
///
|
||||
/// This is the migrated end state, and it is opt-in for a reason. The
|
||||
/// gate binding an address is the one action that can make a port
|
||||
/// reachable where it previously was not, so it must never be something
|
||||
/// a manifest gets by default or by inference. An earlier revision
|
||||
/// gated any `session` port regardless of `bind`, which meant a node
|
||||
/// whose manifests had not yet been updated saw the daemon publish
|
||||
/// Bitcoin's loopback-only RPC on every host address (caught on
|
||||
/// archi-dev-box 2026-08-03, seconds after deploy). Requiring the
|
||||
/// manifest to say so means the loopback pin and the daemon takeover
|
||||
/// ship together, atomically, and a stale manifest fails safe.
|
||||
Gated,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
@@ -562,10 +576,24 @@ pub struct PortMapping {
|
||||
/// containers keep reaching it via `host.archipelago`).
|
||||
#[serde(default)]
|
||||
pub bind: String,
|
||||
/// Whether the app gate authenticates connections to this port.
|
||||
/// Omitted = `session` (protected). See [`PortAuth`].
|
||||
#[serde(default)]
|
||||
pub auth: PortAuth,
|
||||
/// Declared authentication policy, or `None` when the manifest says
|
||||
/// nothing at all.
|
||||
///
|
||||
/// The distinction is load-bearing and was learned the hard way. A node's
|
||||
/// installed manifests always lag the binary, so "absent" is the state of
|
||||
/// essentially every port on every node until a signed catalog delivers
|
||||
/// otherwise. Treating absent as a *value* meant the daemon acted on a
|
||||
/// default the manifest never asked for: first republishing Bitcoin's
|
||||
/// loopback-only RPC across the LAN, then — caught before it shipped —
|
||||
/// preparing to pin LND's gRPC and REST to loopback, which would have
|
||||
/// broken Zeus and every remote wallet.
|
||||
///
|
||||
/// So absent means "no instruction", and the daemon may only ever REPORT
|
||||
/// on such a port, never change how it is published. Use
|
||||
/// [`PortMapping::auth_policy`] for classification and
|
||||
/// [`PortMapping::auth_is_declared`] before acting.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub auth: Option<PortAuth>,
|
||||
/// Why this port is safe to expose unauthenticated. **Required** when
|
||||
/// `auth` is `none`, rejected otherwise — a rationale on a gated port
|
||||
/// means the author expected an exemption they did not get.
|
||||
@@ -573,6 +601,22 @@ pub struct PortMapping {
|
||||
pub auth_rationale: Option<String>,
|
||||
}
|
||||
|
||||
impl PortMapping {
|
||||
/// Policy to classify this port by. An undeclared port reports as
|
||||
/// `Session` — i.e. it shows up in the audit as something that *should*
|
||||
/// be behind the gate — because reporting an unprotected port is always
|
||||
/// safe. Acting on it is not; see [`Self::auth_is_declared`].
|
||||
pub fn auth_policy(&self) -> PortAuth {
|
||||
self.auth.unwrap_or(PortAuth::Session)
|
||||
}
|
||||
|
||||
/// Whether the manifest actually stated a policy. Required before the
|
||||
/// daemon rewrites how a port is published: silence is not consent.
|
||||
pub fn auth_is_declared(&self) -> bool {
|
||||
self.auth.is_some()
|
||||
}
|
||||
}
|
||||
|
||||
impl From<(u16, u16)> for PortMapping {
|
||||
fn from((host, container): (u16, u16)) -> Self {
|
||||
PortMapping {
|
||||
@@ -580,7 +624,7 @@ impl From<(u16, u16)> for PortMapping {
|
||||
container,
|
||||
protocol: "tcp".to_string(),
|
||||
bind: String::new(),
|
||||
auth: PortAuth::Session,
|
||||
auth: None,
|
||||
auth_rationale: None,
|
||||
}
|
||||
}
|
||||
@@ -1084,7 +1128,7 @@ fn validate_ports(ports: &[PortMapping]) -> Result<(), ManifestError> {
|
||||
// exists in the manifest for every exempt port, so auditing the
|
||||
// node's unauthenticated surface is reading a list, not inferring
|
||||
// one from silence.
|
||||
match (port.auth, port.auth_rationale.as_ref()) {
|
||||
match (port.auth_policy(), port.auth_rationale.as_ref()) {
|
||||
(PortAuth::None, None) => {
|
||||
return Err(ManifestError::Invalid(format!(
|
||||
"ports[{i}] sets auth: none but no auth_rationale — an unauthenticated \
|
||||
@@ -1653,7 +1697,7 @@ app:
|
||||
let yaml = std::fs::read_to_string(&manifest).expect("manifest readable");
|
||||
let parsed = AppManifest::parse(&yaml).expect("manifest valid");
|
||||
for port in &parsed.app.ports {
|
||||
if port.auth == PortAuth::None {
|
||||
if port.auth_policy() == PortAuth::None {
|
||||
exempt.push((parsed.app.id.clone(), port.host));
|
||||
}
|
||||
}
|
||||
@@ -1667,13 +1711,47 @@ app:
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn port_auth_defaults_to_session() {
|
||||
// The whole point of the default: a manifest that says nothing about
|
||||
// auth must come out PROTECTED, not exposed. If this ever flips,
|
||||
// every existing app silently loses its gate.
|
||||
fn an_undeclared_port_classifies_as_session_but_is_not_declared() {
|
||||
// Two different questions, and conflating them caused both gate
|
||||
// incidents. A manifest that says nothing must CLASSIFY as gated, so
|
||||
// the audit reports it as something that should be protected — but it
|
||||
// must not read as an instruction the daemon may act on.
|
||||
let manifest = manifest_with_port(" - host: 8080\n container: 80\n").unwrap();
|
||||
assert_eq!(manifest.app.ports[0].auth, PortAuth::Session);
|
||||
assert!(manifest.app.ports[0].auth_rationale.is_none());
|
||||
let port = &manifest.app.ports[0];
|
||||
assert_eq!(port.auth_policy(), PortAuth::Session, "reports as gated");
|
||||
assert!(!port.auth_is_declared(), "but is NOT an instruction");
|
||||
assert!(port.auth.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_explicit_session_declaration_is_actionable() {
|
||||
let manifest = manifest_with_port(
|
||||
" - host: 8080\n container: 80\n auth: session\n",
|
||||
)
|
||||
.unwrap();
|
||||
let port = &manifest.app.ports[0];
|
||||
assert_eq!(port.auth_policy(), PortAuth::Session);
|
||||
assert!(port.auth_is_declared());
|
||||
}
|
||||
|
||||
/// The wallet constraint, in the form that actually bit. LND's gRPC and
|
||||
/// REST carry `bind: ""`, so a rule keyed on `bind` alone does not save
|
||||
/// them — and on a node whose manifest predates the auth field there is
|
||||
/// no `auth: none` either. Undeclared must therefore be untouchable, or
|
||||
/// recreating LND silently pins those ports to loopback and every remote
|
||||
/// wallet stops working.
|
||||
#[test]
|
||||
fn an_undeclared_wallet_port_is_never_actionable() {
|
||||
let manifest = manifest_with_port(
|
||||
" - host: 10009\n container: 10009\n protocol: tcp\n",
|
||||
)
|
||||
.unwrap();
|
||||
let port = &manifest.app.ports[0];
|
||||
assert!(port.bind.is_empty(), "this is the shape that bit us");
|
||||
assert!(
|
||||
!port.auth_is_declared(),
|
||||
"an undeclared port must never authorise republishing"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -1700,7 +1778,7 @@ app:
|
||||
" - host: 8333\n container: 8333\n auth: none\n auth_rationale: Bitcoin p2p gossip\n",
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(manifest.app.ports[0].auth, PortAuth::None);
|
||||
assert_eq!(manifest.app.ports[0].auth, Some(PortAuth::None));
|
||||
assert_eq!(
|
||||
manifest.app.ports[0].auth_rationale.as_deref(),
|
||||
Some("Bitcoin p2p gossip")
|
||||
|
||||
@@ -318,6 +318,42 @@ impl PodmanClient {
|
||||
"sctp" => "sctp",
|
||||
_ => "tcp",
|
||||
};
|
||||
// Effective bind. A gated port with no declared bind would
|
||||
// publish 0.0.0.0 — the app would own every host address, which
|
||||
// is both the exposure itself and the reason the daemon's app
|
||||
// gate cannot bind those addresses to authenticate them. Pin it
|
||||
// to loopback so the gate can take the external addresses.
|
||||
//
|
||||
// Doing it HERE, at container creation, is the point: the pin and
|
||||
// the gate's takeover then both come from the daemon and cannot
|
||||
// disagree. The earlier attempt put this decision in manifest
|
||||
// data instead, and a node whose manifests lagged the binary
|
||||
// published Bitcoin's loopback-only RPC across the LAN
|
||||
// (archi-dev-box, 2026-08-03).
|
||||
//
|
||||
// A port that already declares a bind is never overridden — that
|
||||
// is exactly what keeps `bind: 127.0.0.1` ports host-local and
|
||||
// leaves `auth: none` protocol ports (LND gRPC/REST, electrum)
|
||||
// published as they are, so remote wallets keep working.
|
||||
// NOTE: the daemon deliberately does NOT rewrite this. Pinning a
|
||||
// published port to loopback is how an app hands its external
|
||||
// addresses to the gate, but it belongs in the manifest, not in
|
||||
// daemon-side inference:
|
||||
//
|
||||
// * `bind` is already honoured by every publish path (here and
|
||||
// in package::install), so a manifest edit needs no code.
|
||||
// * inference here would cover only THIS path — proven on
|
||||
// archi-dev-box, where a recreate went through another one and
|
||||
// the pin never applied.
|
||||
// * and inferring from an ABSENT field is what republished
|
||||
// Bitcoin's loopback RPC across the LAN, and came within one
|
||||
// container-recreate of pinning LND's gRPC/REST and breaking
|
||||
// every remote wallet.
|
||||
//
|
||||
// So the migration ships as `bind: 127.0.0.1` in the signed
|
||||
// catalog. Verified 2026-08-03 that a disk-only manifest edit is
|
||||
// overridden by the catalog, which is precisely why the catalog is
|
||||
// the right and only place to carry it.
|
||||
let mut mapping = serde_json::json!({
|
||||
"container_port": port.container,
|
||||
"host_port": port.host,
|
||||
|
||||
Reference in New Issue
Block a user