fix(security): two gate bugs that would have made the rollout a no-op

Both found while setting up the on-node test, and both fail silently in
the same direction — the gate reports success while protecting nothing,
which is the exact failure the module was written to prevent.

1. Loopback-pinned ports were skipped entirely.

`identity.rs` dropped any port whose manifest sets `bind: 127.0.0.1`,
reasoning that a loopback publish is not externally reachable. But
`listener.rs` requires loopback-pinning as the PRECONDITION for gating —
while an app holds 0.0.0.0:<port> the kernel will not let the gate bind
that port at all. So the two contradicted each other: pinning an app, the
one action that lets the gate take over, was also what removed it from
the gated set. Completing the entire migration would have gated nothing,
and GateStatus would have reported zero unprotected ports while doing it.

`bind` cannot carry this decision, because two unrelated intentions
produce an identical loopback publish: Bitcoin's RPC 8332 is pinned so
the LAN CANNOT reach it (fronting it would newly expose it on every host
address, behind a login but exposed where it deliberately was not),
whereas a migrated app is pinned precisely so the gate CAN. Inferring
from `bind` breaks one or the other, so the intent is now declared:
`PortAuth::Local` means the first case. The three ports that are
host-local by intent (bitcoin-core/knots 8332, aiui 5180 — all already
`bind: 127.0.0.1`) say so, and a loopback publish with `auth: session`
stays gated. A test pins that property.

2. The port map was never refreshed.

`AppGate::refresh()` existed, was documented as making catalog changes
apply without a restart, and was called by nothing. The map was built
once in `new()`, so an app installed while the daemon runs would never be
gated — and would never appear in `unprotected` either, so the node would
report itself fully enforced while serving a brand-new app to anyone who
asked. The sweep now refreshes before classifying.

Tests: 22/22 appgate, 73/73 archipelago-container.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
archipelago
2026-08-03 18:41:29 -04:00
co-authored by Claude Opus 5
parent cc9e19589c
commit 3716b6e9c3
6 changed files with 61 additions and 11 deletions
+1
View File
@@ -28,6 +28,7 @@ app:
container: 80
protocol: tcp
bind: 127.0.0.1 # Only accessible via nginx proxy, not externally
auth: local
health_check:
type: http
+1
View File
@@ -85,6 +85,7 @@ app:
container: 8332
protocol: tcp
bind: 127.0.0.1
auth: local
- host: 8333
container: 8333
protocol: tcp
+1
View File
@@ -85,6 +85,7 @@ app:
container: 8332
protocol: tcp
bind: 127.0.0.1
auth: local
- host: 8333
container: 8333
protocol: tcp
+35 -11
View File
@@ -153,6 +153,10 @@ pub fn build_port_map() -> PortMap {
.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 => {}
PortAuth::Session => {
// UDP cannot carry an HTTP challenge. Such a port has
// no business defaulting into the gated set where it
@@ -171,13 +175,12 @@ pub fn build_port_map() -> PortMap {
});
continue;
}
// A publish pinned to loopback is not externally
// reachable, so the gate has nothing to stand in
// front of. Gating it would mean binding a port the
// app already holds and breaking in-node clients.
if port.bind.parse::<std::net::IpAddr>().is_ok_and(|ip| ip.is_loopback()) {
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`.
map.gated.insert(
port.host,
GatedPort {
@@ -242,11 +245,32 @@ mod tests {
}
}
/// Bitcoin's RPC is loopback-pinned, so the gate must leave it alone
/// even though its manifest does not declare an exemption.
/// Bitcoin's RPC is host-local by intent (`auth: local`), so the gate
/// must neither gate it nor report it as exposed — fronting it would
/// newly publish it on every host address, behind a login but reachable
/// where it deliberately was not.
#[test]
fn loopback_pinned_ports_are_not_gated() {
assert!(build_port_map().gated(8332).is_none());
fn host_local_ports_are_neither_gated_nor_reported() {
let map = build_port_map();
assert!(map.gated(8332).is_none(), "bitcoin RPC must not be gated");
assert!(
!map.exempt_ports().iter().any(|e| e.port == 8332),
"a host-local port is not an unauthenticated exposure"
);
}
/// 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.
#[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";
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");
}
/// An app UI that was reachable with no credential in the 2026-08-03
+6
View File
@@ -163,6 +163,12 @@ async fn sweep(
held: &mut HashMap<(u16, IpAddr), ()>,
shutdown_rx: &tokio::sync::watch::Receiver<bool>,
) {
// Re-read the manifests every sweep rather than trusting the map built
// at construction. An app installed while the daemon is running would
// otherwise never be gated until the next restart — and it would not
// appear in `unprotected` either, so the node would report itself fully
// enforced while serving a brand-new app to anyone who asked.
gate.refresh().await;
let port_map = gate.port_map().await;
let addresses = host_addresses().await;
if addresses.is_empty() {
+17
View File
@@ -530,6 +530,23 @@ pub enum PortAuth {
/// (Bitcoin p2p gossip, mDNS). Requires `auth_rationale`: an exemption
/// nobody can explain is an exemption nobody reviewed.
None,
/// Host-local by intent — the gate must not bind this port at all.
///
/// This exists because `bind: 127.0.0.1` is ambiguous on its own, and
/// reading intent out of it would be wrong in both directions. Two
/// unrelated situations produce an identical loopback publish:
///
/// * Bitcoin's RPC 8332 is loopback-pinned so that the LAN *cannot*
/// reach it. Fronting it with the gate would newly expose it on every
/// host address — behind a login, but exposed where it deliberately
/// was not.
/// * A gated app is loopback-pinned precisely *so that* the gate can
/// take over its external addresses; that is the whole migration.
///
/// Inferring from `bind` would break one or the other, so the intent is
/// declared. `Local` means the first case: never externally reachable,
/// gate keeps its hands off.
Local,
}
#[derive(Debug, Clone, Serialize, Deserialize)]