From 84976b1fa2944118897c05fb3659c3d17e27699a Mon Sep 17 00:00:00 2001 From: Johnathan Corgan Date: Sun, 27 Sep 2026 19:35:48 +0000 Subject: [PATCH] fips-gateway: exit when the DNS listener cannot bind or stops The resolver ran as a detached task whose error was only logged, so a gateway whose DNS port was taken stayed up with .fips resolution dead and every health check passing. The listener is now bound before the address pool, NAT table and routes are created, and a bind failure exits with status 1. When the port is already in use, the error names the service most likely to hold it and how to find the holder with ss or netstat. A resolver task that stops while the gateway runs also ends the process, after NAT and routes are torn down, so systemd or procd restarts it or shows it failed. The resolver now forwards to the upstream address the startup probe resolved, so an upstream written as a hostname no longer passes the probe and then kills the resolver. The dns-resolver harness gains a check that a gateway configured on the daemon's DNS port exits with the hint before it creates the pool or the NAT table. The troubleshooting guide describes the new failure, and the exit-code table no longer lists a control-socket bind failure, which only warns. --- docs/how-to/troubleshoot-gateway.md | 61 ++++++++--- docs/reference/cli-fips-gateway.md | 2 +- src/bin/fips-gateway.rs | 81 ++++++++++----- src/config/gateway.rs | 6 ++ src/gateway/dns.rs | 155 +++++++++++++++++++++++++++- testing/dns-resolver/test.sh | 69 +++++++++++++ 6 files changed, 329 insertions(+), 45 deletions(-) diff --git a/docs/how-to/troubleshoot-gateway.md b/docs/how-to/troubleshoot-gateway.md index 6b532ded..a101a7d4 100644 --- a/docs/how-to/troubleshoot-gateway.md +++ b/docs/how-to/troubleshoot-gateway.md @@ -81,28 +81,57 @@ for the full flag list. ### Port conflict on the DNS listen port -Symptom: gateway fails to start with "address already in use" on -the configured `gateway.dns.listen` address. +Symptom: the gateway exits at startup, before it creates the address +pool or the NAT table, and the log carries an error such as this one, +wrapped here for reading: -The default `[::1]:5353` is loopback-only on an unprivileged port and -should not collide with any standard resolver. If you have overridden -`dns.listen` to bind port 53 (or a LAN-side address) and another DNS -server (systemd-resolved, dnsmasq, BIND) is already bound there, -identify it: - -```sh -sudo ss -tulnp | grep ':53' +```text +cannot bind the gateway DNS listener on [::]:53: Address already in +use (os error 98); another DNS server holds port 53: dnsmasq, +systemd-resolved's stub listener, unbound or BIND; find the holder +with `ss -ulpn 'sport = :53'` or `netstat -ulnp`, or set +gateway.dns.listen to a free port and point the resolver that +forwards .fips at it ``` -Two options: +Under systemd the unit restarts every five seconds and fails the same +way each time; under procd on OpenWrt the service stops respawning +after five failures within an hour. The gateway also exits, after +removing its NAT table and routes, when the DNS resolver stops while +the gateway is running; that log line reads "Gateway DNS resolver +stopped; exiting so the service manager restarts the gateway". -- **Stay on the loopback default.** Drop the override and let the - gateway use `[::1]:5353`. Configure the existing resolver to - forward `.fips` queries to it (the canonical OpenWrt deployment - works this way out of the box). +The error names the service most likely to hold the port: + +- **53**: another DNS server, such as dnsmasq, systemd-resolved's stub + listener, unbound or BIND. +- **5353**: mDNS. The fips daemon's LAN rendezvous + (`node.rendezvous.lan`), avahi-daemon or systemd-resolved's + MulticastDNS. +- **5354**: the fips daemon's own DNS responder. `gateway.dns.listen` + must not be the daemon's DNS port. +- **5355**: LLMNR, held by systemd-resolved unless `LLMNR=no`. +- **Any other port**: another process. + +Find the actual holder, replacing the port with your own: + +```sh +sudo ss -ulpn 'sport = :53' +# OpenWrt ships netstat but not ss: +netstat -ulnp +``` + +The default listen address, `[::1]:5353`, is loopback-only on an +unprivileged port. Two options: + +- **Move the gateway.** Set `gateway.dns.listen` to a free port and + point the resolver that forwards `.fips` at the same port. With the + loopback default, configure the existing resolver to forward `.fips` + queries to `[::1]:5353` (the canonical OpenWrt deployment works this + way out of the box). - **Relocate the conflicting resolver.** Move it to a different port - (or disable it if not needed) and let the gateway bind 53. + (or disable it if not needed) and let the gateway bind the port. Practical for systemd-resolved (set `DNSStubListener=no` in `/etc/systemd/resolved.conf`); rarely worth it for production resolvers. diff --git a/docs/reference/cli-fips-gateway.md b/docs/reference/cli-fips-gateway.md index 5a764fce..572ecee8 100644 --- a/docs/reference/cli-fips-gateway.md +++ b/docs/reference/cli-fips-gateway.md @@ -73,7 +73,7 @@ Linux host) and | Code | Meaning | | ---- | ------- | | `0` | Clean shutdown after `SIGINT` / `SIGTERM`. | -| `1` | Non-Linux platform, configuration load failure, missing or invalid `gateway:` block, NAT/network setup failure, or control-socket bind failure. The reason is printed to stderr or the log before exit. | +| `1` | Non-Linux platform, configuration load failure, missing or invalid `gateway:` block, the DNS listener could not bind or stopped while running, or NAT/network setup failure. The reason is printed to stderr or the log before exit. A control-socket bind failure is logged as a warning and the gateway continues without the socket. | ## Environment diff --git a/src/bin/fips-gateway.rs b/src/bin/fips-gateway.rs index fb2c6164..589deb6e 100644 --- a/src/bin/fips-gateway.rs +++ b/src/bin/fips-gateway.rs @@ -251,8 +251,9 @@ async fn main() { std::process::exit(1); } - // Check DNS upstream reachability (proves the FIPS daemon is running) - { + // Check DNS upstream reachability (proves the FIPS daemon is running). + // The resolver later forwards to the address this probe reached. + let upstream_addr = { let upstream = gw_config.dns.upstream(); info!(upstream = %upstream, "Checking DNS upstream reachability"); @@ -363,6 +364,24 @@ async fn main() { ); std::process::exit(1); } + upstream_addr + }; + + // --- Bind the DNS listener --- + // + // Before the pool, NAT table and routes exist, so a port that is already + // taken ends the gateway with nothing to tear down, and a service manager + // restarting it does not churn nftables. + let dns_socket = match dns::bind_listener(gw_config.dns.listen()).await { + Ok(socket) => socket, + Err(e) => { + error!("{e}"); + std::process::exit(1); + } + }; + match dns_socket.local_addr() { + Ok(addr) => info!(addr = %addr, "Gateway DNS resolver listening"), + Err(_) => info!(addr = %gw_config.dns.listen(), "Gateway DNS resolver listening"), } // --- Initialize components --- @@ -421,27 +440,16 @@ async fn main() { // --- Start DNS resolver task --- - let dns_pool = Arc::clone(&ip_pool); - let dns_event_tx = event_tx.clone(); - let dns_shutdown = shutdown_rx.clone(); - let dns_listen = gw_config.dns.listen().to_string(); - let dns_upstream = gw_config.dns.upstream().to_string(); - let dns_ttl = gw_config.dns.ttl(); - - let dns_task = tokio::spawn(async move { - if let Err(e) = dns::run_dns_resolver( - &dns_listen, - &dns_upstream, - dns_ttl, - dns_pool, - dns_event_tx, - dns_shutdown, - ) - .await - { - error!(error = %e, "DNS resolver error"); - } - }); + // Held in an Option because the main loop may see it complete, and a + // completed JoinHandle panics if it is polled again. + let mut dns_task = Some(tokio::spawn(dns::serve( + dns_socket, + upstream_addr, + gw_config.dns.ttl(), + Arc::clone(&ip_pool), + event_tx.clone(), + shutdown_rx.clone(), + ))); // --- Snapshot channel for control socket --- @@ -531,6 +539,7 @@ async fn main() { info!("fips-gateway running"); + let mut exit_code = 0; loop { tokio::select! { Some(event) = event_rx.recv() => { @@ -559,6 +568,25 @@ async fn main() { } } } + // The resolver ends only on shutdown, which has not been + // signalled while this loop runs, so any completion here means + // .fips resolution has stopped. Exit non-zero so systemd or procd + // restarts the gateway or shows it failed. + result = async { dns_task.as_mut().expect("guarded by the precondition").await }, + if dns_task.is_some() => { + dns_task = None; + let cause = match result { + Ok(Ok(())) => "the resolver returned without an error".to_string(), + Ok(Err(e)) => e.to_string(), + Err(e) => e.to_string(), + }; + error!( + cause = %cause, + "Gateway DNS resolver stopped; exiting so the service manager restarts the gateway" + ); + exit_code = 1; + break; + } _ = tokio::signal::ctrl_c() => { info!("Received SIGINT, shutting down"); break; @@ -582,7 +610,9 @@ async fn main() { task.abort(); let _ = task.await; } - let _ = dns_task.await; + if let Some(task) = dns_task { + let _ = task.await; + } let _ = tick_task.await; // Log final pool status @@ -606,4 +636,7 @@ async fn main() { } info!("fips-gateway shutdown complete"); + if exit_code != 0 { + std::process::exit(exit_code); + } } diff --git a/src/config/gateway.rs b/src/config/gateway.rs index b9a96330..f7d4cf00 100644 --- a/src/config/gateway.rs +++ b/src/config/gateway.rs @@ -160,6 +160,12 @@ impl GatewayDnsConfig { pub fn ttl(&self) -> u32 { self.ttl.unwrap_or(DEFAULT_DNS_TTL) } + + /// The port of a listen address: the digits after its last `:`, or + /// `None` when they do not form a port. Works on a hostname form too. + pub(crate) fn port_of(listen: &str) -> Option { + listen.rsplit_once(':')?.1.parse().ok() + } } /// Conntrack timeout overrides (`gateway.conntrack.*`). diff --git a/src/gateway/dns.rs b/src/gateway/dns.rs index 4d9a069e..97054a5c 100644 --- a/src/gateway/dns.rs +++ b/src/gateway/dns.rs @@ -17,6 +17,7 @@ use tracing::{debug, info, trace, warn}; use super::pool::{PoolEvent, VirtualIpPool}; use crate::NodeAddr; +use crate::config::GatewayDnsConfig; /// Timeout for upstream DNS queries. const UPSTREAM_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); @@ -156,25 +157,108 @@ fn build_aaaa_response(query: &Packet, virtual_ip: Ipv6Addr, ttl: u32) -> Option response.build_bytes_vec_compressed().ok() } +/// The gateway DNS listener could not be bound. +/// +/// The message names the listen address and, when the port is already in +/// use, the service most likely to hold it and how to find the holder. +#[derive(Debug, thiserror::Error)] +#[error("cannot bind the gateway DNS listener on {listen}: {source}{}", in_use_hint(.listen, .source))] +pub struct ListenError { + listen: String, + source: std::io::Error, +} + +impl ListenError { + /// The kind of the underlying bind error. + pub fn kind(&self) -> std::io::ErrorKind { + self.source.kind() + } +} + +/// The suffix `ListenError`'s message carries for an address-in-use error: +/// the likely holder of the port, and how to find the actual one. +fn in_use_hint(listen: &str, source: &std::io::Error) -> String { + if source.kind() != std::io::ErrorKind::AddrInUse { + return String::new(); + } + let (holder, port) = match GatewayDnsConfig::port_of(listen) { + Some(port) => (holder_hint(port), port.to_string()), + None => (holder_hint(0), "".to_string()), + }; + format!( + "; {holder}; find the holder with `ss -ulpn 'sport = :{port}'` or `netstat -ulnp`, \ + or set gateway.dns.listen to a free port and point the resolver that forwards .fips at it" + ) +} + +/// The service most likely to hold a DNS listen port that is already in use. +pub(crate) fn holder_hint(port: u16) -> &'static str { + match port { + 53 => { + "another DNS server holds port 53: dnsmasq, systemd-resolved's stub listener, unbound or BIND" + } + 5353 => { + "port 5353 is mDNS: the fips daemon's LAN rendezvous (node.rendezvous.lan), \ + avahi-daemon or systemd-resolved's MulticastDNS may hold it" + } + 5354 => { + "the fips daemon's own DNS responder listens on 5354 by default; \ + gateway.dns.listen must not be the daemon's DNS port" + } + 5355 => "port 5355 is LLMNR, held by systemd-resolved unless LLMNR=no", + 5365 => "another fips-gateway may already be running", + _ => "another process holds it", + } +} + +/// Bind the gateway DNS listener. +/// +/// Called before the gateway creates anything it would have to tear down, so +/// a port that is already taken stops the gateway before it starts. +pub async fn bind_listener(listen: &str) -> Result { + UdpSocket::bind(listen).await.map_err(|source| ListenError { + listen: listen.to_string(), + source, + }) +} + /// Run the gateway DNS resolver. /// -/// Listens for DNS queries, forwards `.fips` queries to the upstream -/// daemon resolver, allocates virtual IPs, and returns them to clients. +/// Binds `listen_addr`, then serves as [`serve`] does. The gateway binary +/// binds and serves separately so that a bind failure stops it at startup. pub async fn run_dns_resolver( listen_addr: &str, upstream_addr: &str, ttl: u32, pool: std::sync::Arc>, event_tx: tokio::sync::mpsc::Sender, - mut shutdown: watch::Receiver, + shutdown: watch::Receiver, ) -> Result<(), std::io::Error> { - let socket = UdpSocket::bind(listen_addr).await?; + let socket = bind_listener(listen_addr) + .await + .map_err(|e| std::io::Error::new(e.kind(), e))?; info!(addr = %listen_addr, "Gateway DNS resolver listening"); let upstream: SocketAddr = upstream_addr .parse() .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))?; + serve(socket, upstream, ttl, pool, event_tx, shutdown).await +} + +/// Serve DNS queries on a bound listener until shutdown. +/// +/// Forwards `.fips` queries to the upstream daemon resolver, allocates +/// virtual IPs, and returns them to clients. Returns `Ok` on shutdown and +/// `Err` when receiving from the listener fails. +pub async fn serve( + socket: UdpSocket, + upstream: SocketAddr, + ttl: u32, + pool: std::sync::Arc>, + event_tx: tokio::sync::mpsc::Sender, + mut shutdown: watch::Receiver, +) -> Result<(), std::io::Error> { let mut buf = vec![0u8; MAX_DNS_SIZE]; loop { @@ -807,6 +891,69 @@ mod tests { )); } + #[test] + fn an_in_use_hint_names_the_mdns_responders_for_5353() { + let hint = holder_hint(5353); + assert!(hint.contains("mDNS"), "{hint}"); + assert!(hint.contains("node.rendezvous.lan"), "{hint}"); + assert!(hint.contains("avahi-daemon"), "{hint}"); + } + + #[test] + fn an_in_use_hint_names_llmnr_for_5355() { + let hint = holder_hint(5355); + assert!(hint.contains("LLMNR"), "{hint}"); + assert!(!hint.contains("mDNS"), "{hint}"); + } + + #[test] + fn an_in_use_hint_names_the_daemon_for_5354() { + let hint = holder_hint(5354); + assert!(hint.contains("fips daemon's own DNS responder"), "{hint}"); + } + + #[test] + fn an_in_use_hint_names_a_dns_server_for_53() { + let hint = holder_hint(53); + assert!(hint.contains("another DNS server"), "{hint}"); + assert!(hint.contains("dnsmasq"), "{hint}"); + } + + #[test] + fn an_in_use_hint_names_another_gateway_for_the_default_port() { + let hint = holder_hint(5365); + assert!(hint.contains("another fips-gateway"), "{hint}"); + } + + #[test] + fn an_in_use_hint_names_another_process_for_an_unknown_port() { + assert_eq!(holder_hint(40000), "another process holds it"); + } + + #[tokio::test] + async fn binding_a_held_port_fails_with_addr_in_use_and_names_the_port_ss_and_netstat() { + let holder = UdpSocket::bind("[::1]:0").await.unwrap(); + let port = holder.local_addr().unwrap().port(); + let listen = format!("[::1]:{port}"); + + let err = bind_listener(&listen) + .await + .expect_err("binding a held port must fail"); + assert_eq!(err.kind(), std::io::ErrorKind::AddrInUse); + let message = err.to_string(); + assert!(message.contains(&listen), "{message}"); + assert!(message.contains(&format!("sport = :{port}")), "{message}"); + assert!(message.contains("ss -ulpn"), "{message}"); + assert!(message.contains("netstat -ulnp"), "{message}"); + assert!(message.contains(holder_hint(port)), "{message}"); + } + + #[tokio::test] + async fn binding_a_free_port_returns_a_bound_socket() { + let socket = bind_listener("[::1]:0").await.expect("bind a free port"); + assert_ne!(socket.local_addr().unwrap().port(), 0); + } + #[test] fn test_extract_fips_name() { // Build a simple AAAA query for test.fips diff --git a/testing/dns-resolver/test.sh b/testing/dns-resolver/test.sh index 038a1190..8aea1bb9 100755 --- a/testing/dns-resolver/test.sh +++ b/testing/dns-resolver/test.sh @@ -701,6 +701,73 @@ DOCKERFILE # prepare_binaries) and are copied into each per-distro runtime image. # ───────────────────────────────────────────────────────────────────── +# Print a fips-gateway log from the container with terminal colour codes +# removed, so structured fields can be matched as plain "key=value" text. +# Fails when the log cannot be read. +read_gateway_log() { + local name="$1" log="$2" + local text + text=$(docker exec "$name" cat "$log" 2>/dev/null) || return 1 + printf '%s\n' "$text" | sed 's/\x1b\[[0-9;]*m//g' + return 0 +} + +# The gateway exits at the DNS bind when its listen port is held. The +# daemon in this container holds [::1]:5354, so a gateway configured to +# listen there must exit non-zero with the hint naming the daemon, before +# it creates the address pool or the NAT table. Any build that gets past +# the bind logs one of the pool or NAT lines below, whichever way NAT goes +# in this container, so their absence shows the exit came first. +check_gateway_exits_on_held_port() { + local name="$1" + local log=/var/log/fips-gateway-held.log + local fail_before=$FAIL + docker exec "$name" bash -c 'cat > /tmp/gateway-held.yaml <$log 2>&1; echo \"EXIT=\$?\" >>$log" + + local text + if ! text=$(read_gateway_log "$name" "$log"); then + fail "could not read $log, so the held-port exit was not observed" + return + fi + local rc + rc=$(printf '%s\n' "$text" | sed -n 's/^EXIT=//p' | tail -n 1) + if [ -z "$rc" ]; then + fail "the held-port gateway run left no exit status in $log" + elif [ "$rc" = "0" ] || [ "$rc" = "124" ]; then + fail "fips-gateway on a held DNS port exited $rc (expected non-zero, not the timeout)" + else + pass "fips-gateway on a held DNS port exits $rc" + fi + if printf '%s\n' "$text" | grep -qF "the fips daemon's own DNS responder listens on 5354"; then + pass "the bind error names the daemon as the likely holder of 5354" + else + fail "the bind error does not carry the 5354 hint" + fi + local line + for line in "Failed to create virtual IP pool" "Failed to create nftables table" "Created nftables table"; do + if printf '%s\n' "$text" | grep -qF "$line"; then + fail "fips-gateway reached a step after the DNS bind: '$line'" + else + pass "fips-gateway stopped before '$line'" + fi + done + if [ "$FAIL" -gt "$fail_before" ]; then + echo " --- $log ---" + printf '%s\n' "$text" | tail -20 + fi +} + # Args: # distro_label: short tag for container/image names (e.g. "debian12") # docker_base_image: e.g. "debian:12", "ubuntu:26.04" @@ -918,6 +985,8 @@ EOF' # care that the upstream reachability step succeeded). docker exec "$name" pkill -f fips-gateway 2>/dev/null || true + check_gateway_exits_on_held_port "$name" + # Teardown via the script: backend config file must be removed # (path varies by backend selected above). local teardown_path