feat(security): self-heal legacy containers on declared bind drift
Legacy pre-quadlet containers kept publishing 0.0.0.0 after the catalog pinned their app to loopback, because host_port_bindings_drifted only compared host PORT numbers — closing them needed a manual package.update per app per node. The drift check now also compares the bind ADDRESS, but only when the manifest declares one: an empty bind never fires, since recreating a loopback-published container to wildcard on silence is exactly the v1.7.121 Bitcoin-RPC incident. With this, every node recreates its legacy containers to the declared state on its own after the OTA. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
f08ed79b8a
commit
e46af8cfe5
@@ -595,10 +595,20 @@ async fn wait_for_manifest_host_ports(
|
||||
/// `podman inspect --format '{{json .HostConfig.PortBindings}}'` emits, e.g.
|
||||
/// `{"8080/tcp":[{"HostIp":"","HostPort":"18080"}]}`. Returns true only when a
|
||||
/// manifest container-port is positively published to a *different* host port
|
||||
/// than the manifest now asks for. Absence of a binding is deliberately NOT
|
||||
/// treated as drift here — that case is handled by the host-port repair/restart
|
||||
/// path and by host-networked apps that publish nothing — so we never trigger a
|
||||
/// destructive recreate on a false positive.
|
||||
/// than the manifest now asks for — or, when the manifest DECLARES a bind
|
||||
/// address, to a different host address. Absence of a binding is deliberately
|
||||
/// NOT treated as drift here — that case is handled by the host-port
|
||||
/// repair/restart path and by host-networked apps that publish nothing — so we
|
||||
/// never trigger a destructive recreate on a false positive.
|
||||
///
|
||||
/// The bind comparison is what lets a node self-heal after a catalog refresh
|
||||
/// pins an app to loopback for the app gate: a legacy (pre-quadlet) container
|
||||
/// still publishing `0.0.0.0:P` against a manifest that now declares
|
||||
/// `bind: 127.0.0.1` is recreated to the declared state, exactly as
|
||||
/// `package.update` would. An EMPTY manifest bind means "no instruction" and
|
||||
/// never fires this — recreating a loopback-published container to wildcard on
|
||||
/// silence is precisely the v1.7.121 incident class (Bitcoin RPC republished
|
||||
/// on the LAN).
|
||||
fn host_port_bindings_drifted(
|
||||
port_bindings_json: &str,
|
||||
manifest_ports: &[archipelago_container::manifest::PortMapping],
|
||||
@@ -626,10 +636,26 @@ fn host_port_bindings_drifted(
|
||||
}
|
||||
let expected = port.host.to_string();
|
||||
let matches_expected = bindings.iter().any(|b| {
|
||||
b.get("HostPort")
|
||||
let host_port_ok = b
|
||||
.get("HostPort")
|
||||
.and_then(|h| h.as_str())
|
||||
.map(|h| h == expected)
|
||||
.unwrap_or(false)
|
||||
.unwrap_or(false);
|
||||
if !host_port_ok {
|
||||
return false;
|
||||
}
|
||||
// Only a DECLARED bind participates; podman reports a wildcard
|
||||
// publish as "" or "0.0.0.0".
|
||||
if port.bind.is_empty() {
|
||||
return true;
|
||||
}
|
||||
let actual_ip = b.get("HostIp").and_then(|h| h.as_str()).unwrap_or("");
|
||||
let actual = if actual_ip.is_empty() {
|
||||
"0.0.0.0"
|
||||
} else {
|
||||
actual_ip
|
||||
};
|
||||
actual == port.bind
|
||||
});
|
||||
if !matches_expected {
|
||||
return true;
|
||||
@@ -4569,6 +4595,76 @@ mod tests {
|
||||
));
|
||||
}
|
||||
|
||||
fn bound_port(
|
||||
host: u16,
|
||||
container: u16,
|
||||
bind: &str,
|
||||
) -> archipelago_container::manifest::PortMapping {
|
||||
archipelago_container::manifest::PortMapping {
|
||||
bind: bind.to_string(),
|
||||
..port(host, container)
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bind_drift_detected_when_declared_loopback_but_published_wildcard() {
|
||||
// The legacy-container case: a pre-quadlet container still publishes
|
||||
// 0.0.0.0 while the catalog-delivered manifest pins the app to
|
||||
// loopback for the app gate. Must recreate, or the port stays open on
|
||||
// every interface and the gate can never claim it.
|
||||
for wildcard in [r#""""#, r#""0.0.0.0""#] {
|
||||
let bindings = format!(r#"{{"80/tcp":[{{"HostIp":{wildcard},"HostPort":"8082"}}]}}"#);
|
||||
assert!(host_port_bindings_drifted(
|
||||
&bindings,
|
||||
&[bound_port(8082, 80, "127.0.0.1")]
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_bind_drift_when_declared_loopback_and_published_loopback() {
|
||||
let bindings = r#"{"80/tcp":[{"HostIp":"127.0.0.1","HostPort":"8082"}]}"#;
|
||||
assert!(!host_port_bindings_drifted(
|
||||
bindings,
|
||||
&[bound_port(8082, 80, "127.0.0.1")]
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_bind_drift_on_undeclared_bind() {
|
||||
// Silence is not consent (v1.7.121 incident class): an EMPTY manifest
|
||||
// bind must never recreate a loopback-published container to
|
||||
// wildcard — that is how Bitcoin's RPC got republished on the LAN.
|
||||
let bindings = r#"{"8332/tcp":[{"HostIp":"127.0.0.1","HostPort":"8332"}]}"#;
|
||||
assert!(!host_port_bindings_drifted(bindings, &[port(8332, 8332)]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn multi_bind_publish_satisfies_each_declared_entry() {
|
||||
// Same host/container pair listed twice (loopback + archy-net
|
||||
// gateway): both declared binds are present in the actual publish.
|
||||
let bindings = r#"{"8332/tcp":[
|
||||
{"HostIp":"127.0.0.1","HostPort":"8332"},
|
||||
{"HostIp":"10.89.0.1","HostPort":"8332"}
|
||||
]}"#;
|
||||
assert!(!host_port_bindings_drifted(
|
||||
bindings,
|
||||
&[
|
||||
bound_port(8332, 8332, "127.0.0.1"),
|
||||
bound_port(8332, 8332, "10.89.0.1")
|
||||
]
|
||||
));
|
||||
// And a wildcard-only publish drifts BOTH declared entries.
|
||||
let wildcard = r#"{"8332/tcp":[{"HostIp":"","HostPort":"8332"}]}"#;
|
||||
assert!(host_port_bindings_drifted(
|
||||
wildcard,
|
||||
&[
|
||||
bound_port(8332, 8332, "127.0.0.1"),
|
||||
bound_port(8332, 8332, "10.89.0.1")
|
||||
]
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn missing_secret_error_names_the_secret() {
|
||||
use archipelago_container::manifest::SecretsProvider;
|
||||
|
||||
Reference in New Issue
Block a user