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.
This commit is contained in:
@@ -26,6 +26,7 @@ use inbuxa_features::security::{
|
|||||||
listeners,
|
listeners,
|
||||||
protocol_policy::{self, ProtocolPolicy, SavedListener},
|
protocol_policy::{self, ProtocolPolicy, SavedListener},
|
||||||
};
|
};
|
||||||
|
use registry::types::{error::Error, id::ObjectId};
|
||||||
use store::registry::bootstrap::Bootstrap;
|
use store::registry::bootstrap::Bootstrap;
|
||||||
|
|
||||||
/// What turning the switch actually did.
|
/// What turning the switch actually did.
|
||||||
@@ -162,7 +163,29 @@ impl Server {
|
|||||||
.parse_tcp_acceptors(&mut bootstrap, self.inner.clone())
|
.parse_tcp_acceptors(&mut bootstrap, self.inner.clone())
|
||||||
.await;
|
.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();
|
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<ObjectId> = 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 spawned = Vec::new();
|
||||||
|
|
||||||
let mut acceptors = std::mem::take(&mut parsed.tcp_acceptors);
|
let mut acceptors = std::mem::take(&mut parsed.tcp_acceptors);
|
||||||
|
|||||||
@@ -326,8 +326,14 @@ impl SocketOpts {
|
|||||||
}
|
}
|
||||||
|
|
||||||
impl Listeners {
|
impl Listeners {
|
||||||
pub fn bind_and_drop_priv(&self, bp: &mut Bootstrap) {
|
/// Binds every socket, reporting each failure against its listener.
|
||||||
// Bind as root
|
///
|
||||||
|
/// 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 server in &self.servers {
|
||||||
for listener in &server.listeners {
|
for listener in &server.listeners {
|
||||||
if let Err(err) = listener.socket.bind(listener.addr) {
|
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
|
// Drop privileges
|
||||||
#[cfg(not(target_env = "msvc"))]
|
#[cfg(not(target_env = "msvc"))]
|
||||||
|
|||||||
Reference in New Issue
Block a user