From 3f689529c7fda45ae0b8eaefbddcbc2c9050c9c8 Mon Sep 17 00:00:00 2001 From: John Coffey Date: Sun, 20 Sep 2026 15:46:46 -0700 Subject: [PATCH] A listener put back has to be bound, or it never comes up Found while setting up the live check, which is the only place it could have shown: every unit test passes without it. spawn_restored_listeners re-parsed the listeners and spawned them, but never bound their sockets. Binding is not part of parsing -- it happens in bind_and_drop_priv, once, at startup -- so listen() would have failed on an unbound socket and the port would have stayed shut while the policy recorded it as reopened. LP-5 would have been a promise the server did not keep, and the operator's only clue a log line. bind() is now split out of bind_and_drop_priv and called on its own here. It cannot be the whole of bind_and_drop_priv, because that also drops privileges, which must happen once at startup and never again. That split has a consequence worth stating: a listener on a port below 1024 cannot be bound again once privileges are gone. Ports 143 and 110 are the realistic cases. Rather than leave such a listener parsed, spawned and silently dead, the bind errors are read back and those listeners are reported as needing a restart -- which is the "cannot be recreated" case LP-5 already anticipated, and it stays saved for another try. Re-parsing is also narrowed to the listeners being restored, so putting one back cannot bind a port another listener already holds. --- crates/common/src/network/legacy.rs | 23 +++++++++++++++++++++++ crates/common/src/network/listen.rs | 15 +++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/crates/common/src/network/legacy.rs b/crates/common/src/network/legacy.rs index 5517a4f..f14aeb7 100644 --- a/crates/common/src/network/legacy.rs +++ b/crates/common/src/network/legacy.rs @@ -26,6 +26,7 @@ use inbuxa_features::security::{ listeners, protocol_policy::{self, ProtocolPolicy, SavedListener}, }; +use registry::types::{error::Error, id::ObjectId}; use store::registry::bootstrap::Bootstrap; /// What turning the switch actually did. @@ -162,7 +163,29 @@ impl Server { .parse_tcp_acceptors(&mut bootstrap, self.inner.clone()) .await; + // Only the wanted listeners, so re-parsing does not bind a port some + // other listener already holds. let wanted: Vec<&str> = restored.iter().map(|l| l.id.as_str()).collect(); + parsed + .servers + .retain(|listener| wanted.contains(&listener.id.as_str())); + + // Bind, but do not drop privileges again. A port below 1024 fails + // here once privileges are gone; that listener is reported as needing + // a restart rather than quietly left dead. + let errors_before = bootstrap.errors.len(); + parsed.bind(&mut bootstrap); + let unbindable: Vec = bootstrap.errors[errors_before..] + .iter() + .filter_map(|error| match error { + Error::Build { object_id, .. } => Some(*object_id), + _ => None, + }) + .collect(); + parsed + .servers + .retain(|listener| !unbindable.contains(&listener.registry_id)); + let mut spawned = Vec::new(); let mut acceptors = std::mem::take(&mut parsed.tcp_acceptors); diff --git a/crates/common/src/network/listen.rs b/crates/common/src/network/listen.rs index bbb2d01..65d1231 100644 --- a/crates/common/src/network/listen.rs +++ b/crates/common/src/network/listen.rs @@ -326,8 +326,14 @@ impl SocketOpts { } impl Listeners { - pub fn bind_and_drop_priv(&self, bp: &mut Bootstrap) { - // Bind as root + /// Binds every socket, reporting each failure against its listener. + /// + /// Split out of [`Listeners::bind_and_drop_priv`] so a listener can be + /// bound again at runtime, when the legacy-protocols switch puts one back + /// (LP-5), without dropping privileges a second time. A port below 1024 + /// will fail here once privileges are gone, which is one of the cases + /// LP-5 expects and reports rather than hides. + pub fn bind(&self, bp: &mut Bootstrap) { for server in &self.servers { for listener in &server.listeners { if let Err(err) = listener.socket.bind(listener.addr) { @@ -338,6 +344,11 @@ impl Listeners { } } } + } + + pub fn bind_and_drop_priv(&self, bp: &mut Bootstrap) { + // Bind as root + self.bind(bp); // Drop privileges #[cfg(not(target_env = "msvc"))]