From 672d828ef58c9106f81ccccb53e7812ff9164306 Mon Sep 17 00:00:00 2001 From: Arjen <18398758+Origami74@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:28:22 +0100 Subject: [PATCH 1/2] feat(node): publish the DNS responder's bound address for embedders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An embedder that owns the TUN fd has no system DNS socket to point at the built-in `.fips` responder. On Android specifically, `VpnService.Builder` exposes `addDnsServer(address)` with no port — the OS resolver always uses 53, which an unprivileged app UID cannot bind — and it aims the resolver *into* the tunnel, so `.fips` queries surface as IPv6/UDP packets on the app's own fd rather than at any socket FIPS holds. The app can still use the responder rather than reimplementing resolution: lift the DNS payload out of the packet it read, send it to the responder over an ordinary UDP socket of its own, and splice the answer back into a reply packet. Nothing in the responder's start-up is desktop-specific — `bind_dns_socket` is plain socket2, `lookup_mesh_ifindex` returns None with no system TUN so the mesh filter self-disables, and `HostMapReloader` on an absent hosts file settles at a no-op stat. What was missing is the address to dial and whether anything is listening at it. `dns_local_addr()` answers both, as a one-shot read taken after `start()` returns and before the node is moved into a background task. That is the only window in which an embedder running `run_rx_loop` holds a `&Node` to call it on, and the value is settled by then: the responder is either up for the rest of the node's life or it never came up. It reports the address read back off the bound socket, so a `dns.port = 0` config yields the port the kernel assigned rather than 0. Config alone cannot answer the second question — `dns.enabled` with a failed bind leaves `bind_addr` naming a plausible target nothing is listening on, and a bind failure only warns rather than failing node start. It is deliberately not a liveness feed. Watching a responder that dies later needs a way to read live node state from a backgrounded `run_rx_loop`, which is a general gap and not one an accessor should try to close. Routing through the responder rather than resolving in the app is what keeps route warming intact. Answering a `.fips` query is what puts that peer's public key in the node's identity cache, and a FipsAddress is SHA-256(pubkey) truncated twice: the key cannot be recovered from the IPv6 address. With no cache entry the first packet to a freshly-resolved name is rejected with ICMPv6 "No route" — a failure that direct neighbours mask entirely, since their identity arrives with the Noise handshake and never needed resolving. The address is retracted on `stop()`. `retract_child_publications` also handles a responder that exits on its own at runtime, where the FSM's `ChildExited` handling republishes node health but touches no per-child handles. That consumer is dormant as written and documented as such: `run_dns_responder` is an unconditional loop whose every failure arm continues, so it never returns and the `Child::Dns` send after it is unreachable. It lands here so a producer fix does not have to rediscover the consuming side. A panicking responder is not covered either way, since the unwind goes past the send rather than through it — true of every child producer, not just this one. The IPv6-adapter design doc gains an App-Owned DNS Path section beside the App-Owned TUN one it mirrors, plus implementation-status rows for both. Three tests. `dns_responder_serves_a_proxying_embedder` is the load-bearing one: port-0 read-back, a proxied query answered with the right AAAA, and the resolved identity arriving on the channel `run_rx_loop` drains into `register_identity`. `dns_local_addr_stays_none_when_the_bind_fails` forces `EADDRINUSE` against a socket the test holds open — `bind_dns_socket` sets neither `SO_REUSEADDR` nor `SO_REUSEPORT`, so that is deterministic, where naming a non-local address is not: `net.ipv4.ip_nonlocal_bind = 1` is ordinary on hosts running keepalived or HAProxy and makes the bind succeed. `retract_child_publications_clears_the_dns_address` is scoped and named for the helper rather than the scenario, because deleting the `run_rx_loop` call site leaves it green; that wiring is covered by nothing. Full suite 1672 passed, 0 failed. fmt, `clippy --all-targets -D warnings` and the Android `cargo ndk clippy --lib -D warnings` gate are clean. The changelog entry was added at merge rather than in the pull request: the app-owned TUN seam it mirrors gained an Unreleased entry in the master-only sweep, so this one would otherwise recreate that debt. --- CHANGELOG.md | 8 ++ docs/design/fips-ipv6-adapter.md | 43 +++++++ src/node/dataplane/rx_loop.rs | 5 + src/node/lifecycle/mod.rs | 41 ++++++- src/node/lifecycle/supervisor.rs | 6 + src/node/mod.rs | 50 ++++++++ src/node/tests/unit.rs | 193 +++++++++++++++++++++++++++++++ 7 files changed, 345 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b82fb51..bc6c0681 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `fd00::/8`-destined packets and clamp TCP MSS on outbound SYNs. Desktop builds are unchanged and no Cargo features are introduced. +- `Node::dns_local_addr()`, the DNS companion to the app-owned TUN seam above. + An embedder whose resolver is pointed into the tunnel has no system socket + aimed at the built-in `.fips` responder, so the accessor reports the address + read back off the bound socket: `dns.port = 0` therefore yields the + kernel-assigned port, and it returns `Some` only while the responder is up. + Read it once, after `start()` returns and before the node is moved into a + background task; it is not a liveness feed (#136). + - A bounded graceful-shutdown drain phase, controlled by the new `node.drain_timeout_secs` (default 2s). On the shutdown signal the node broadcasts Disconnect to all peers and then keeps serving for that window, diff --git a/docs/design/fips-ipv6-adapter.md b/docs/design/fips-ipv6-adapter.md index 06ad35a6..f86cbae9 100644 --- a/docs/design/fips-ipv6-adapter.md +++ b/docs/design/fips-ipv6-adapter.md @@ -330,6 +330,47 @@ and the [TUN-Side TCP MSS Clamping](#tun-side-tcp-mss-clamping). The embedder is therefore responsible for routing only `fd00::/8` to its TUN (so only mesh-bound packets arrive) and for clamping TCP MSS on outbound SYNs. +### App-Owned DNS Path (embedded hosts) + +An embedded host that owns the TUN fd generally has no system DNS socket to aim +at the responder either. On Android, `VpnService.Builder.addDnsServer()` takes an +address with no port — the resolver always uses 53, which an unprivileged app UID +cannot bind — and it points the resolver *into* the tunnel, so `.fips` queries +surface as IPv6/UDP packets on the app's own fd rather than at any socket FIPS +holds. The [DNS responder](#dns-integration) itself needs no changes for this: +its bind is a plain UDP socket, and with no system TUN the +[mesh-interface filter](#mesh-interface-query-filter) self-disables because the +interface name does not resolve. + +`Node::dns_local_addr()` closes the gap. It reports the address read back off the +bound socket — so a `dns.port = 0` config yields the port the kernel assigned — +and is `None` when no responder came up. The embedder lifts the DNS payload out +of the packet it read, sends it to that address over an ordinary UDP socket of +its own, and splices the answer back into a reply packet. + +It is a one-shot read taken after `start()` returns and before the node is moved +into a background task, because `run_rx_loop` then borrows the node exclusively +for its whole lifetime and no `&Node` remains to call it on. By that point the +value is settled: the responder is either up for the rest of the node's life or +it never came up. + +Proxying to the responder rather than resolving in the app is what keeps the +[identity cache](#identity-cache) warm. Answering a `.fips` query is what +registers that peer's public key, and a FIPS address is a truncated hash of a +hash of the pubkey — the key cannot be recovered from the IPv6 address alone. An +app that resolves the AAAA itself leaves the cache empty, and the first packet to +the resolved name is rejected with ICMPv6 Destination Unreachable. Direct +neighbours mask the omission, since their identity arrives with the Noise +handshake and never needed resolving. + +The address is retracted on `stop()`. A retraction hook for a responder that +exits on its own at runtime is wired on the consuming side, but is dormant: +`run_dns_responder` never returns, so nothing produces the `Child::Dns` exit +event it consumes. Watching a responder that dies mid-run is therefore not +something an embedder can do today, and would in any case need a way to read +live node state from a backgrounded `run_rx_loop` — a general gap rather than a +DNS-specific one. + ## Implementation Status | Feature | Status | @@ -343,6 +384,8 @@ packets arrive) and for clamping TCP MSS on outbound SYNs. | TCP MSS clamping (SYN + SYN-ACK) | **Implemented** | | DNS service (.fips domain) | **Implemented** | | DNS responder mesh-interface filter | **Implemented** | +| App-owned TUN (`Node::enable_app_owned_tun`) | **Implemented** | +| App-owned DNS path (`Node::dns_local_addr`) | **Implemented** | | Port-based service multiplexing (port 256) | **Implemented** | | IPv6 header compression (format 0x00) | **Implemented** | | Per-destination route MTU (netlink) | Planned | diff --git a/src/node/dataplane/rx_loop.rs b/src/node/dataplane/rx_loop.rs index 0a9bdd28..75ec0797 100644 --- a/src/node/dataplane/rx_loop.rs +++ b/src/node/dataplane/rx_loop.rs @@ -271,6 +271,11 @@ impl Node { // `PublishState`; other variants are ignored defensively. maybe_child = child_exit_rx.recv() => { if let Some(child) = maybe_child { + // Drop anything the dead child published for embedders + // (e.g. the DNS responder's bound address) before + // republishing health, so nothing outside the node can + // observe an address the listener no longer answers on. + self.retract_child_publications(child); let actions = self .supervisor .fsm diff --git a/src/node/lifecycle/mod.rs b/src/node/lifecycle/mod.rs index ef30bccf..831bef32 100644 --- a/src/node/lifecycle/mod.rs +++ b/src/node/lifecycle/mod.rs @@ -1747,6 +1747,12 @@ impl Node { let bind = std::net::SocketAddr::new(ip, self.config().dns.port()); match Self::bind_dns_socket(bind) { Ok(socket) => { + // Read the bound address back off the socket + // rather than reusing `bind`: a port-0 config + // resolves to the kernel-assigned port here, + // and this is the address an embedder that + // proxies queries to us has to dial. + let local_addr = socket.local_addr().unwrap_or(bind); let dns_channel_size = self.config().node.buffers.dns_channel; let (identity_tx, identity_rx) = tokio::sync::mpsc::channel(dns_channel_size); @@ -1771,7 +1777,7 @@ impl Node { let mesh_ifindex = Self::lookup_mesh_ifindex(self.config().tun.name()); info!( - bind = %bind, + bind = %local_addr, hosts = reloader.hosts().len(), mesh_ifindex = ?mesh_ifindex, "DNS responder started for .fips domain (auto-reload enabled)" @@ -1797,6 +1803,7 @@ impl Node { }); self.supervisor.dns_identity_rx = Some(identity_rx); self.supervisor.dns_task = Some(handle); + self.supervisor.dns_local_addr = Some(local_addr); Event::SubstrateUp { child } } Err(e) => { @@ -2057,6 +2064,10 @@ impl Node { handle.abort(); debug!("DNS responder stopped"); } + // Retract the published address in the same step that kills + // the listener, so an embedder polling `dns_local_addr()` + // never dials a socket that is already gone. + self.supervisor.dns_local_addr.take(); } Child::Nostr => { // Stop Nostr overlay discovery background work and withdraw @@ -2153,6 +2164,34 @@ impl Node { } } + /// Retract anything a child published about itself, after it exited on its + /// own at runtime (as opposed to being torn down by [`Self::stop`]). + /// + /// The FSM's `ChildExited` handling only republishes node health; it does + /// not touch per-child handles. That is fine for state nobody outside the + /// node reads, but not for an address an embedder dials: a stale + /// [`Node::dns_local_addr`] would have the app proxying `.fips` queries + /// into a socket that is gone, and the only symptom would be resolution + /// quietly timing out. + /// + /// Deliberately narrow — it clears published facts, not handles. + /// `dns_task` is left alone because [`Self::reconstruct_supervised_up`] + /// reads it to rebuild the teardown set, and aborting an already-finished + /// handle there is harmless. + /// + /// **Dormant for `Dns` as written.** `run_dns_responder` is an unconditional + /// loop whose every failure arm continues, so it never returns and the + /// `Child::Dns` send that follows it is unreachable — nothing produces the + /// event this consumes. The consumer side is correct and lands here so the + /// producer fix does not have to rediscover it. A responder that *panics* is + /// not covered either way, since the unwind goes past the send rather than + /// through it; that is true of every child producer, not just this one. + pub(in crate::node) fn retract_child_publications(&mut self, child: Child) { + if matches!(child, Child::Dns) { + self.supervisor.dns_local_addr.take(); + } + } + /// Reconstruct the supervised up-set from observed runtime presence, so the /// FSM authors the teardown order regardless of how the node reached /// `Running`. Worker pools are deliberately excluded: today's teardown never diff --git a/src/node/lifecycle/supervisor.rs b/src/node/lifecycle/supervisor.rs index 9fc1913f..0f3c39cf 100644 --- a/src/node/lifecycle/supervisor.rs +++ b/src/node/lifecycle/supervisor.rs @@ -692,6 +692,11 @@ pub(crate) struct Supervisor { pub(in crate::node) dns_identity_rx: Option, /// DNS responder task handle. pub(in crate::node) dns_task: Option>, + /// Address the DNS responder actually bound, read back from the socket + /// after `bind` so a port-0 config resolves to the assigned port. `Some` + /// only while the responder is up; published to embedders through + /// [`Node::dns_local_addr`](crate::Node::dns_local_addr). + pub(in crate::node) dns_local_addr: Option, /// Node-side driver state for the Nostr overlay peer-rendezvous /// subsystem: the engine handle, its startup timestamp, the one-shot @@ -737,6 +742,7 @@ impl Supervisor { tun_shutdown_fd: None, dns_identity_rx: None, dns_task: None, + dns_local_addr: None, nostr_rendezvous: crate::nostr::RendezvousDriver::default(), lan_rendezvous: None, #[cfg(unix)] diff --git a/src/node/mod.rs b/src/node/mod.rs index 20bda80c..f395b4fd 100644 --- a/src/node/mod.rs +++ b/src/node/mod.rs @@ -2874,6 +2874,56 @@ impl Node { (outbound_tx, tun_rx) } + /// Address the built-in `.fips` DNS responder is listening on, or `None` + /// when it is not running (`dns.enabled = false`, the bind failed, or the + /// node is stopped). + /// + /// This is the companion to [`Self::enable_app_owned_tun`] for embedders + /// that own the TUN fd. On a platform with no system DNS socket to point + /// at us — an Android `VpnService`, whose `addDnsServer()` takes an address + /// with no port and aims the OS resolver *into* the tunnel — `.fips` + /// queries arrive as IPv6/UDP packets on the app's own fd. The app can + /// forward the DNS payload here and splice the answer back into a reply + /// packet, rather than reimplementing resolution: + /// + /// ```no_run + /// # async fn f(node: &fips::Node, query: &[u8]) -> std::io::Result<()> { + /// let Some(dns) = node.dns_local_addr() else { return Ok(()) }; + /// let sock = tokio::net::UdpSocket::bind("[::1]:0").await?; + /// sock.send_to(query, dns).await?; // payload only, no IP/UDP header + /// let mut answer = [0u8; 512]; + /// let (n, _) = sock.recv_from(&mut answer).await?; + /// # let _ = n; Ok(()) + /// # } + /// ``` + /// + /// Going through the responder rather than resolving in the app is what + /// keeps route warming working: answering a `.fips` query is what + /// populates the node's identity cache with that peer's public key, and a + /// `FipsAddress` is a truncated hash — the pubkey cannot be recovered from + /// the IPv6 address alone. Without a cache entry the first packet to a + /// freshly-resolved name is rejected with ICMPv6 "No route". Direct + /// neighbours mask this, since their identity comes from the Noise + /// handshake and never needed resolving. + /// + /// Read this **once, after [`Self::start`] returns and before the node is + /// moved into a background task** — that is the only window in which an + /// embedder running [`Self::run_rx_loop`] holds a `&Node` to call it on, and + /// the value is fixed by then: the responder is either up for the rest of + /// the node's life or it never came up. The address is read back off the + /// bound socket, so a `dns.port = 0` config reports the port the kernel + /// actually assigned. + /// + /// This is a one-shot read, not a liveness feed. `None` distinguishes "no + /// responder" from "responder at this address" at that moment; it is not a + /// signal an embedder can watch for a responder that dies later, because + /// `run_rx_loop` borrows the node exclusively for its whole lifetime. + /// Reading live node state from a backgrounded loop is a general gap, not + /// one this accessor tries to close. + pub fn dns_local_addr(&self) -> Option { + self.supervisor.dns_local_addr + } + // === Sending === /// Encrypt and send a link-layer message to an authenticated peer. diff --git a/src/node/tests/unit.rs b/src/node/tests/unit.rs index 1af6c590..a223b130 100644 --- a/src/node/tests/unit.rs +++ b/src/node/tests/unit.rs @@ -2798,6 +2798,199 @@ async fn start_skips_system_tun_when_app_owned() { node.stop().await.unwrap(); } +/// The embedder-facing DNS contract, end to end. +/// +/// An embedder that owns the TUN fd (Android `VpnService`) has no system DNS +/// socket to point at us, so it proxies `.fips` query payloads it lifts out of +/// its own tunnel to the built-in responder. That requires three things to +/// hold, and this pins all three: +/// +/// 1. `dns_local_addr()` publishes where to send — read back off the bound +/// socket, so a `port = 0` config reports the assigned port, not 0. +/// 2. The responder answers a proxied query with the right AAAA. +/// 3. The resolved identity reaches `dns_identity_rx` — the channel +/// `run_rx_loop` drains into `register_identity`. This is the leg that +/// populates the identity cache, without which the first packet to a +/// freshly-resolved `.fips` is rejected with ICMPv6 "No route". +#[tokio::test] +async fn dns_responder_serves_a_proxying_embedder() { + let mut config = crate::Config::new(); + config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }); + config.dns.enabled = true; + config.dns.bind_addr = Some("::1".to_string()); + // Port 0: proves the address is read back off the socket rather than + // echoed from config — an embedder dialling 0 would reach nothing. + config.dns.port = Some(0); + // The TUN is app-owned, as it is on the platform this seam serves. + let mut node = make_node_with(config); + let (_outbound_tx, _tun_rx) = node.enable_app_owned_tun(); + + assert!( + node.dns_local_addr().is_none(), + "no responder before start()", + ); + + node.start().await.unwrap(); + + let dns_addr = node + .dns_local_addr() + .expect("responder is up, so its address is published"); + assert_ne!(dns_addr.port(), 0, "must report the kernel-assigned port"); + + // Proxy a query the way the embedder would: payload only, no IP/UDP header + // (it strips those off the packet it read from its own TUN fd). + let peer = Identity::generate(); + let query = { + use simple_dns::{CLASS, Name, Packet, QCLASS, QTYPE, Question, TYPE}; + let mut packet = Packet::new_query(0x1234); + packet.questions.push(Question::new( + Name::new_unchecked(&format!("{}.fips", peer.npub())).into_owned(), + QTYPE::TYPE(TYPE::AAAA), + QCLASS::CLASS(CLASS::IN), + false, + )); + packet.build_bytes_vec().unwrap() + }; + let client = tokio::net::UdpSocket::bind("[::1]:0").await.unwrap(); + client.send_to(&query, dns_addr).await.unwrap(); + + let mut buf = [0u8; 512]; + let (len, _) = tokio::time::timeout( + std::time::Duration::from_secs(2), + client.recv_from(&mut buf), + ) + .await + .expect("responder answered within the timeout") + .unwrap(); + + let answer = simple_dns::Packet::parse(&buf[..len]).expect("well-formed DNS response"); + let rdata = &answer.answers.first().expect("one AAAA answer").rdata; + let simple_dns::rdata::RData::AAAA(aaaa) = rdata else { + panic!("expected an AAAA record, got {rdata:?}"); + }; + assert_eq!( + std::net::Ipv6Addr::from(aaaa.address), + peer.address().to_ipv6(), + "AAAA must be the peer's FipsAddress", + ); + + // The identity leg. `run_rx_loop` owns the node for its whole life, so the + // embedder cannot register identities itself — the responder publishes them + // on this channel instead. Drain and register exactly as the rx-loop arm in + // `dataplane/rx_loop.rs` does, then assert the cache is populated. + let identity = tokio::time::timeout( + std::time::Duration::from_secs(2), + node.supervisor + .dns_identity_rx + .as_mut() + .expect("responder installed the identity receiver") + .recv(), + ) + .await + .expect("identity published within the timeout") + .expect("channel is open"); + + assert_eq!(identity.node_addr, *peer.node_addr()); + node.register_identity(identity.node_addr, identity.pubkey); + assert!( + node.has_cached_identity(peer.node_addr()), + "resolving a name must warm the identity cache, or the first packet \ + to that address is rejected with ICMPv6 \"No route\"", + ); + + node.stop().await.unwrap(); + assert!( + node.dns_local_addr().is_none(), + "the published address must be retracted with the listener", + ); +} + +/// `retract_child_publications(Dns)` clears the published address. +/// +/// Scoped to the helper deliberately, and named for that rather than for the +/// scenario: no responder dies here, and deleting the `run_rx_loop` call site +/// leaves this green. Driving a real exit through the loop needs the node moved +/// into a task, which puts `dns_local_addr()` out of reach — and the producer +/// side cannot deliver `Child::Dns` today regardless, since `run_dns_responder` +/// never returns. +/// +/// What it does pin is the behavior the eventual wiring depends on: the FSM's +/// `ChildExited` handling only republishes node health, so without this +/// retraction `dns_local_addr()` would keep naming a socket nobody is listening +/// on, and a proxying embedder would see `.fips` queries silently time out +/// rather than any error it could act on. +#[tokio::test] +async fn retract_child_publications_clears_the_dns_address() { + let mut config = crate::Config::new(); + config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }); + config.dns.enabled = true; + config.dns.bind_addr = Some("::1".to_string()); + config.dns.port = Some(0); + let mut node = make_node_with(config); + + node.start().await.unwrap(); + assert!(node.dns_local_addr().is_some(), "responder came up"); + + // What `run_rx_loop` does when the DNS task self-reports its exit. + node.retract_child_publications(crate::node::lifecycle::supervisor::Child::Dns); + + assert!( + node.dns_local_addr().is_none(), + "a dead responder must not keep publishing an address to dial", + ); + + node.stop().await.unwrap(); +} + +/// `dns.enabled` with a bind that fails must report `None`, not an address. +/// +/// This is the third state an embedder has to tell apart, and the one that +/// would otherwise be indistinguishable from a healthy responder by reading +/// config alone: DNS is switched on, so `config.dns.bind_addr()` names a +/// plausible target, but nothing is listening there. A bind failure is only +/// warned about and leaves the node running, so config is not evidence — +/// `dns_local_addr()` is. +/// +/// The failure is forced with `EADDRINUSE` against a socket this test holds +/// open, rather than by naming an address the host has no interface for. +/// `bind_dns_socket` sets neither `SO_REUSEADDR` nor `SO_REUSEPORT`, so the +/// collision is deterministic on Linux and macOS. A non-local address is not: +/// `net.ipv4.ip_nonlocal_bind = 1` is ordinary on hosts running keepalived or +/// HAProxy and makes the bind succeed, which reds the test on a developer +/// machine while CI — at the default `0` — stays green. +#[tokio::test] +async fn dns_local_addr_stays_none_when_the_bind_fails() { + // Hold the port for the whole test so the responder's bind collides. + let squatter = tokio::net::UdpSocket::bind("[::1]:0").await.unwrap(); + let taken = squatter.local_addr().unwrap(); + + let mut config = crate::Config::new(); + config.transports.udp = crate::config::TransportInstances::Single(crate::config::UdpConfig { + bind_addr: Some("127.0.0.1:0".to_string()), + ..Default::default() + }); + config.dns.enabled = true; + config.dns.bind_addr = Some("::1".to_string()); + config.dns.port = Some(taken.port()); + let mut node = make_node_with(config); + + node.start().await.unwrap(); + + assert!( + node.dns_local_addr().is_none(), + "an unbound responder must not publish an address", + ); + + node.stop().await.unwrap(); + drop(squatter); +} + /// A connection whose handshake failed is retained with BOTH Noise handles /// empty, and the stale-connection sweep depends on that: presence of the /// pending connection — not presence of a handle — is what marks a machine as From 5e3892edb82740aac6f65282e10aa5939ea3c389 Mon Sep 17 00:00:00 2001 From: Jeff Gardner <202880+erskingardner@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:01:30 +0000 Subject: [PATCH 2/2] Secure the control socket's runtime directory, and stop chowning /tmp On any Unix host that falls through to the last-resort /tmp/fips-control.sock path, bind chowned the socket's parent unconditionally, and that parent is /tmp itself. A root daemon on such a host changed the group ownership of /tmp to fips at every start. The mode was left alone, so nothing lost access, but the ownership was ours to take and never ours to keep. macOS has no /run, so the resolver's /var/run/fips arm only fired when the directory already existed, and nothing on macOS creates it: /var/run is cleared at boot and the shipped LaunchDaemon has no equivalent of the FreeBSD rc.d fips_precmd. The packaged macOS daemon has therefore been landing on /tmp/fips-control.sock every boot. A privileged macOS process now selects /var/run/fips before its leaf exists, so that bind creates it, and the clients follow once it is there. The two halves are the same change: the bootstrap only works if bind may create and secure that directory, and the /tmp chown had to go before bind could be trusted to. Which parent bind may secure is keyed on the directory's identity rather than on which call created it. is_managed_socket_parent matches only the resolver's own candidates: /run/fips, /var/run/fips where the platform policy consults it, and $XDG_RUNTIME_DIR/fips. Keying it on creation alone was tried first and regressed Linux, because systemd removes RuntimeDirectory=fips when the unit stops and recreates it as root:root on the next start, while the tmpfiles fragment that sets the fips group runs only at install and boot. The daemon's own chown was what repaired that at every bind, so a fips-group operator lost fipsctl after the first restart following a boot. Matching on identity restores it and still leaves /tmp, and any operator-configured directory, alone. The resolver is split into a pure core taking the policy, the XDG_RUNTIME_DIR value and an is_dir predicate, so the macOS and Linux policies are both exercised deterministically on a Linux runner with no environment mutation. The deb-install suite gains the end-to-end half: after a service restart it asserts /run/fips is 750 root:fips and that a real non-root fips-group user can reach the socket, which is the property an operator actually has. Also corrects a configuration.md paragraph claiming the daemon and the clients use different fallback orders, which stopped being true when the resolver order disagreement was resolved and the prose was never updated. --- CHANGELOG.md | 10 ++ docs/reference/cli-fipsctl.md | 2 +- docs/reference/configuration.md | 6 +- docs/reference/control-socket.md | 22 ++- src/config/mod.rs | 266 +++++++++++++++++++++++++++---- src/config/node.rs | 10 +- src/control/mod.rs | 177 ++++++++++++++++++-- testing/deb-install/test.sh | 20 +++ 8 files changed, 448 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc6c0681..54760add 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -246,6 +246,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- The packaged macOS daemon now recreates and binds its control socket at + `/var/run/fips/control.sock` instead of falling through to the shared + `/tmp/fips-control.sock` path after boot. The previous fallback also made the + root daemon attempt to chown `/tmp` itself to the `fips` group because socket + setup unconditionally changed the parent directory. A privileged macOS + process now selects the private runtime path before its leaf exists, clients + follow it once created, and socket setup changes ownership and mode only for + a private parent directory it creates or recognizes as a canonical FIPS + runtime directory. + - Nostr NAT traversal signals are now sent only to relays the client pool actually holds. A signal is addressed to the merge of the peer's NIP-17 inbox relays, the relays its advert nominates for signaling, and our own DM relays, diff --git a/docs/reference/cli-fipsctl.md b/docs/reference/cli-fipsctl.md index 3593117f..c9631cd0 100644 --- a/docs/reference/cli-fipsctl.md +++ b/docs/reference/cli-fipsctl.md @@ -187,7 +187,7 @@ so; `profile tick status` then reports `stopped_by_cap` until the next | Path | Purpose | | ---- | ------- | | `/etc/fips/hosts` | Maps hostnames to npubs for the `connect`, `disconnect`, and `--peer` arguments. See [configuration.md](configuration.md). | -| Control socket (default) | Same resolution as the daemon: `/run/fips/control.sock` if present, else `$XDG_RUNTIME_DIR/fips/control.sock`, else `/tmp/fips-control.sock` (Unix); TCP `localhost:21210` (Windows). | +| Control socket (default) | Same resolution as the daemon: `/run/fips/control.sock` if present; then `/var/run/fips/control.sock` on macOS/FreeBSD if present; then `$XDG_RUNTIME_DIR/fips/control.sock`; finally `/tmp/fips-control.sock` (Unix). A privileged macOS daemon bootstraps the private `/var/run/fips` directory. Windows uses TCP `localhost:21210`. | If you get `Permission denied` connecting to the socket on Linux, add your user to the `fips` group (`sudo usermod -aG fips $USER`) diff --git a/docs/reference/configuration.md b/docs/reference/configuration.md index ce60ebbc..b9c3987f 100644 --- a/docs/reference/configuration.md +++ b/docs/reference/configuration.md @@ -54,7 +54,7 @@ peers: # Static peer list | Parameter | Type | Default | Description | |-----------|------|---------|-------------| | `node.control.enabled` | bool | `true` | Enable the control socket | -| `node.control.socket_path` | string | *(auto)* | **Linux:** Socket file path. Resolved at daemon startup: `$XDG_RUNTIME_DIR/fips/control.sock` if `XDG_RUNTIME_DIR` is set, else `/run/fips/control.sock` if `/run/fips` can be created (typical when running under the shipped systemd unit), else `/tmp/fips-control.sock`. (Note: the `fipsctl` / `fipstop` clients use a different fallback order — `/run/fips` first if it already exists, then `XDG_RUNTIME_DIR`, then `/tmp` — so when both schemes apply, set this field explicitly to avoid mismatch.) **Windows:** TCP port number (default: `21210`); the control socket listens on `127.0.0.1` at this port. | +| `node.control.socket_path` | string | *(auto)* | **Unix:** Socket file path. Resolution is shared by daemon and clients: `/run/fips/control.sock` when `/run/fips` exists; then `/var/run/fips/control.sock` on macOS/FreeBSD when its private directory exists; then `$XDG_RUNTIME_DIR/fips/control.sock`; finally `/tmp/fips-control.sock`. A privileged macOS daemon selects `/var/run/fips/control.sock` even when the private directory must be created after boot. **Windows:** TCP port number (default: `21210`); the control socket listens on `127.0.0.1` at this port. | The control socket provides access to node state and runtime management via the `fipsctl` command-line tool. In addition to read-only status @@ -62,7 +62,7 @@ queries, `fipsctl connect` and `fipsctl disconnect` enable runtime peer management. See the [`fipsctl` reference](cli-fipsctl.md) for the command list. -On Linux, the control socket is a Unix domain socket with filesystem +On Unix, the control socket is a Unix domain socket with filesystem permissions (mode 0770, group `fips`). On Windows, it is a TCP listener on localhost. TCP does not provide filesystem-level ACLs, so any local user can connect to the control port. @@ -985,7 +985,7 @@ node: after_messages: 65536 # rekey after N messages sent control: enabled: true - socket_path: null # null = auto ($XDG_RUNTIME_DIR → /run/fips → /tmp fallback) + socket_path: null # null = auto (platform runtime dir → XDG → /tmp) buffers: packet_channel: 1024 tun_channel: 1024 diff --git a/docs/reference/control-socket.md b/docs/reference/control-socket.md index 2422704f..181f2ccf 100644 --- a/docs/reference/control-socket.md +++ b/docs/reference/control-socket.md @@ -8,20 +8,28 @@ length-bounded JSON over a stream socket. ## Connection -### Linux / macOS +### Unix A Unix domain socket. The default path is resolved in this order: 1. `/run/fips/control.sock` (or `/run/fips/gateway.sock` for the gateway), if `/run/fips` exists. This is what the `fips.service` systemd unit creates. -2. `$XDG_RUNTIME_DIR/fips/control.sock` otherwise. -3. `/tmp/fips-control.sock` if neither of the above is available. +2. On macOS and FreeBSD, `/var/run/fips/control.sock` if its private + directory exists. A privileged macOS daemon selects this path before the + directory exists and creates it at bind time, so the packaged LaunchDaemon + recreates its runtime state after every boot. The FreeBSD rc.d service + creates the directory before starting FIPS. +3. `$XDG_RUNTIME_DIR/fips/control.sock` otherwise. +4. `/tmp/fips-control.sock` if none of the above is available. -The daemon `chown`s the socket file and its parent directory to the -`fips` group at bind time and sets mode `0770`. Members of the `fips` -group can therefore connect without root. Add a user with -`sudo usermod -aG fips $USER` (re-login required). +The daemon sets the socket to group `fips`, mode `0770`. It sets a private +parent directory to group `fips`, mode `0750`, both when it creates that +directory and when a service manager pre-creates a canonical runtime directory +(`/run/fips`, `/var/run/fips`, or `$XDG_RUNTIME_DIR/fips`). Existing shared or +custom parents remain unchanged, so fallback locations such as `/tmp` retain +their system ownership and mode. Members of the `fips` group can connect +without root. The path can be overridden at the daemon side via `node.control.socket_path` in the YAML config, and at the client side diff --git a/src/config/mod.rs b/src/config/mod.rs index 9c3a9c78..0eb4b0f5 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -138,18 +138,119 @@ pub fn pub_file_path(config_path: &Path) -> PathBuf { .join(PUB_FILENAME) } -/// Resolve a default Unix-socket path under the canonical order: -/// `/run/fips/` → `$XDG_RUNTIME_DIR/fips/` → `/tmp/fips-`. +/// How `/var/run/fips` participates in Unix control-socket resolution. +#[cfg(unix)] +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct VarRunPolicy { + /// Whether an existing `/var/run/fips` directory participates in resolution. + consult_existing: bool, + /// Whether to select `/var/run/fips` before its private leaf exists. + create_private_dir: bool, +} + +#[cfg(target_os = "macos")] +fn default_var_run_policy() -> VarRunPolicy { + // LaunchDaemons run as root unless their plist declares another user. + // Selecting the private runtime path before it exists lets ControlSocket + // create it at every boot; non-root development runs retain XDG and /tmp + // fallbacks until a packaged daemon has created /var/run/fips. + VarRunPolicy { + consult_existing: true, + create_private_dir: unsafe { libc::geteuid() } == 0, + } +} + +#[cfg(target_os = "freebsd")] +fn default_var_run_policy() -> VarRunPolicy { + // The rc.d service creates /var/run/fips before starting the daemon. + VarRunPolicy { + consult_existing: true, + create_private_dir: false, + } +} + +#[cfg(all(unix, not(any(target_os = "macos", target_os = "freebsd"))))] +fn default_var_run_policy() -> VarRunPolicy { + VarRunPolicy { + consult_existing: false, + create_private_dir: false, + } +} + +/// Pure path-selection core used by the host resolver and deterministic tests. +#[cfg(unix)] +fn resolve_default_socket_with( + filename: &str, + var_run_policy: VarRunPolicy, + xdg_runtime_dir: Option<&Path>, + is_dir: impl Fn(&Path) -> bool, +) -> String { + if is_dir(Path::new("/run/fips")) { + return format!("/run/fips/{filename}"); + } + + if var_run_policy.consult_existing { + let private_var_run = Path::new("/var/run/fips"); + let may_create_private_dir = + var_run_policy.create_private_dir && is_dir(Path::new("/var/run")); + if is_dir(private_var_run) || may_create_private_dir { + return format!("/var/run/fips/{filename}"); + } + } + + if let Some(xdg) = xdg_runtime_dir + && is_dir(xdg) + { + return xdg + .join("fips") + .join(filename) + .to_string_lossy() + .into_owned(); + } + + format!("/tmp/fips-{filename}") +} + +/// Return whether `parent` is one of the private runtime directories used by +/// the default Unix socket resolver. /// -/// `/run/fips` is the packaged convention (`root:fips 0770` directory -/// created by the daemon at bind time, or by the postinst script). -/// `XDG_RUNTIME_DIR` covers dev runs where `/run/fips` does not exist. -/// `/tmp` is the last-resort fallback. +/// This is intentionally stricter than matching any leaf named `fips`: an +/// explicitly configured existing directory remains operator-owned unless it +/// is also a canonical resolver candidate. +#[cfg(unix)] +fn is_managed_socket_parent_with( + parent: &Path, + var_run_policy: VarRunPolicy, + xdg_runtime_dir: Option<&Path>, +) -> bool { + parent == Path::new("/run/fips") + || (var_run_policy.consult_existing && parent == Path::new("/var/run/fips")) + || xdg_runtime_dir.is_some_and(|xdg| parent == xdg.join("fips")) +} + +/// Return whether `parent` is a private runtime directory managed by the +/// default Unix socket resolver on this host. +#[cfg(unix)] +pub(crate) fn is_managed_socket_parent(parent: &Path) -> bool { + let xdg_runtime_dir = std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from); + is_managed_socket_parent_with(parent, default_var_run_policy(), xdg_runtime_dir.as_deref()) +} + +/// Resolve a default Unix-socket path under the canonical order: +/// `/run/fips/` → `/var/run/fips/` on macOS/FreeBSD → +/// `$XDG_RUNTIME_DIR/fips/` → `/tmp/fips-`. +/// +/// `/run/fips` is the packaged Linux convention. FreeBSD's rc.d service +/// creates `/var/run/fips` before starting the daemon. A privileged macOS +/// daemon selects `/var/run/fips` even when the private leaf does not exist so +/// it can be recreated at bind time after every boot; non-root macOS clients +/// select it once the daemon has created it. `XDG_RUNTIME_DIR` covers dev runs, +/// and `/tmp` is the last-resort fallback. /// /// Selection is by *existence*, not writability. A fips-group member /// whose shell session has not picked up the supplementary group (no /// re-login after `usermod -aG fips`) cannot tempfile-probe a -/// `root:fips 0770` directory but can still connect to a socket inside +/// `root:fips 0750` directory but can still connect to a socket inside /// it once the kernel checks the actual group at `connect(2)` time — /// and even where the user genuinely cannot connect, surfacing an /// `EACCES` from the socket call is clearer than silently steering @@ -163,37 +264,20 @@ pub fn pub_file_path(config_path: &Path) -> PathBuf { /// is treated as missing. #[cfg(unix)] pub(crate) fn resolve_default_socket(filename: &str) -> String { - // 1. /run/fips — preferred whenever the directory exists. - if Path::new("/run/fips").is_dir() { - return format!("/run/fips/{filename}"); - } - - // 1b. /var/run/fips — macOS and FreeBSD have no /run; the FreeBSD - // rc.d script creates this directory at service start. - #[cfg(any(target_os = "macos", target_os = "freebsd"))] - if Path::new("/var/run/fips").is_dir() { - return format!("/var/run/fips/{filename}"); - } - - // 2. $XDG_RUNTIME_DIR/fips/ — only if the variable points at an existing - // directory. - if let Ok(xdg) = std::env::var("XDG_RUNTIME_DIR") { - let xdg_path = Path::new(&xdg); - if xdg_path.is_dir() { - return format!("{xdg}/fips/{filename}"); - } - } - - // 3. Last resort: /tmp with a name-mangled prefix so multiple users - // don't collide. - format!("/tmp/fips-{filename}") + let xdg_runtime_dir = std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from); + resolve_default_socket_with( + filename, + default_var_run_policy(), + xdg_runtime_dir.as_deref(), + Path::is_dir, + ) } /// Default control socket path for fipsctl / fipstop. /// /// On Unix, delegates to [`resolve_default_socket`] for the canonical -/// `/run/fips` → `XDG_RUNTIME_DIR` → `/tmp` order. On Windows, returns the -/// default TCP port ("21210"). +/// platform runtime directory → `XDG_RUNTIME_DIR` → `/tmp` order. On Windows, +/// returns the default TCP port ("21210"). pub fn default_control_path() -> PathBuf { #[cfg(unix)] { @@ -207,7 +291,7 @@ pub fn default_control_path() -> PathBuf { /// Default gateway control socket path. /// -/// On Unix, delegates to [`resolve_default_socket`] (same canonical order as +/// On Unix, delegates to [`resolve_default_socket`] (the same platform order as /// the main control socket). The gateway daemon itself uses a hardcoded /// `/run/fips/gateway.sock` since gateway operation requires root for /// NAT/conntrack management; this client-side resolver falls through @@ -2386,6 +2470,120 @@ node: assert!(cfg.accept_connections()); } + #[cfg(unix)] + #[test] + fn test_privileged_macos_bootstraps_private_var_run_path() { + let path = resolve_default_socket_with( + "control.sock", + VarRunPolicy { + consult_existing: true, + create_private_dir: true, + }, + Some(Path::new("/valid/xdg")), + |candidate| matches!(candidate.to_str(), Some("/var/run" | "/valid/xdg")), + ); + + assert_eq!(path, "/var/run/fips/control.sock"); + } + + #[cfg(unix)] + #[test] + fn test_non_privileged_macos_uses_xdg_before_private_var_run_exists() { + let path = resolve_default_socket_with( + "control.sock", + VarRunPolicy { + consult_existing: true, + create_private_dir: false, + }, + Some(Path::new("/valid/xdg")), + |candidate| candidate == Path::new("/valid/xdg"), + ); + + assert_eq!(path, "/valid/xdg/fips/control.sock"); + } + + #[cfg(unix)] + #[test] + fn test_clients_follow_existing_private_var_run_path() { + let path = resolve_default_socket_with( + "control.sock", + VarRunPolicy { + consult_existing: true, + create_private_dir: false, + }, + Some(Path::new("/valid/xdg")), + |candidate| matches!(candidate.to_str(), Some("/var/run/fips" | "/valid/xdg")), + ); + + assert_eq!(path, "/var/run/fips/control.sock"); + } + + #[cfg(unix)] + #[test] + fn test_linux_policy_ignores_var_run_fips() { + let path = resolve_default_socket_with( + "control.sock", + VarRunPolicy { + consult_existing: false, + create_private_dir: false, + }, + Some(Path::new("/valid/xdg")), + |candidate| matches!(candidate.to_str(), Some("/var/run/fips" | "/valid/xdg")), + ); + + assert_eq!(path, "/valid/xdg/fips/control.sock"); + } + + #[cfg(unix)] + #[test] + fn test_managed_socket_parent_matches_only_resolver_candidates() { + let policy = VarRunPolicy { + consult_existing: true, + create_private_dir: true, + }; + + assert!(is_managed_socket_parent_with( + Path::new("/run/fips"), + policy, + Some(Path::new("/valid/xdg")), + )); + assert!(is_managed_socket_parent_with( + Path::new("/var/run/fips"), + policy, + Some(Path::new("/valid/xdg")), + )); + assert!(is_managed_socket_parent_with( + Path::new("/valid/xdg/fips"), + policy, + Some(Path::new("/valid/xdg")), + )); + assert!(!is_managed_socket_parent_with( + Path::new("/tmp"), + policy, + Some(Path::new("/valid/xdg")), + )); + assert!(!is_managed_socket_parent_with( + Path::new("/srv/application/fips"), + policy, + Some(Path::new("/valid/xdg")), + )); + } + + #[cfg(unix)] + #[test] + fn test_linux_managed_socket_parent_excludes_var_run() { + let linux_policy = VarRunPolicy { + consult_existing: false, + create_private_dir: false, + }; + + assert!(!is_managed_socket_parent_with( + Path::new("/var/run/fips"), + linux_policy, + None, + )); + } + /// Mutex serializing tests that mutate `XDG_RUNTIME_DIR`. `cargo test` /// runs tests on multiple threads in the same process, and env mutation /// is process-global, so concurrent env-touching tests would race. diff --git a/src/config/node.rs b/src/config/node.rs index 3709bc00..c3a283bc 100644 --- a/src/config/node.rs +++ b/src/config/node.rs @@ -868,11 +868,11 @@ impl ControlConfig { /// Default control socket path. /// - /// On Unix, delegates to [`super::resolve_default_socket`] for the - /// canonical `/run/fips` → `XDG_RUNTIME_DIR` → `/tmp` order shared with - /// the client-side `default_control_path`. On Windows, returns a TCP - /// port number as a string since Windows does not support Unix domain - /// sockets; the control socket listens on localhost at this port. + /// On Unix, delegates to [`super::resolve_default_socket`] for the shared + /// platform runtime-directory → `XDG_RUNTIME_DIR` → `/tmp` order. On + /// Windows, returns a TCP port number as a string since Windows does not + /// support Unix domain sockets; the control socket listens on localhost at + /// this port. fn default_socket_path() -> String { #[cfg(unix)] { diff --git a/src/control/mod.rs b/src/control/mod.rs index 5e770df8..ddbc384f 100644 --- a/src/control/mod.rs +++ b/src/control/mod.rs @@ -131,6 +131,61 @@ mod unix_impl { use std::path::{Path, PathBuf}; use tokio::net::UnixListener; + /// Ensure the socket's parent exists and report whether this call created + /// the leaf directory. + /// + /// `create_dir` gives us an atomic ownership decision: an `AlreadyExists` + /// result means another actor owns the existing directory, while success + /// means it is safe for this bind to apply FIPS ownership and mode. Missing + /// ancestors are created recursively, but only the requested leaf is later + /// treated as the socket's private directory. + fn ensure_socket_parent(parent: &Path) -> Result { + if parent.as_os_str().is_empty() { + return Ok(false); + } + + match std::fs::create_dir(parent) { + Ok(()) => Ok(true), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + if parent.is_dir() { + Ok(false) + } else { + Err(error) + } + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + let ancestor = parent.parent().ok_or(error)?; + ensure_socket_parent(ancestor)?; + ensure_socket_parent(parent) + } + Err(error) => Err(error), + } + } + + /// Apply access policy to a newly bound control socket. + /// + /// The socket is always group-owned. `managed_parent` is either a private + /// directory this bind created or a canonical FIPS runtime directory. A + /// shared or operator-owned existing parent is omitted so it retains its + /// ownership and mode. + fn set_control_socket_access( + socket_path: &Path, + managed_parent: Option<&Path>, + mut chown_to_fips_group: impl FnMut(&Path), + ) -> Result<(), std::io::Error> { + use std::os::unix::fs::PermissionsExt; + + std::fs::set_permissions(socket_path, std::fs::Permissions::from_mode(0o770))?; + chown_to_fips_group(socket_path); + + if let Some(parent) = managed_parent { + std::fs::set_permissions(parent, std::fs::Permissions::from_mode(0o750))?; + chown_to_fips_group(parent); + } + + Ok(()) + } + /// Control socket listener (Unix domain socket). /// /// Manages the Unix domain socket lifecycle: bind, accept, cleanup. @@ -147,13 +202,20 @@ mod unix_impl { pub fn bind(config: &ControlConfig) -> Result { let socket_path = PathBuf::from(&config.socket_path); - // Create parent directory if it doesn't exist - if let Some(parent) = socket_path.parent() - && !parent.exists() - { - std::fs::create_dir_all(parent)?; - debug!(path = %parent.display(), "Created control socket directory"); - } + // Creation is useful for diagnostics, but ownership is keyed to + // directory identity as well: systemd pre-creates /run/fips on + // every Linux service start and initially owns it as root:root. + let managed_parent = match socket_path.parent() { + Some(parent) => { + let created = ensure_socket_parent(parent)?; + if created { + debug!(path = %parent.display(), "Created private control socket directory"); + } + (created || crate::config::is_managed_socket_parent(parent)) + .then(|| parent.to_owned()) + } + None => None, + }; // Remove stale socket if it exists if socket_path.exists() { @@ -162,14 +224,13 @@ mod unix_impl { let listener = UnixListener::bind(&socket_path)?; - // Make the socket and its parent directory group-accessible so - // 'fips' group members can use fipsctl/fipstop without root. - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&socket_path, std::fs::Permissions::from_mode(0o770))?; - Self::chown_to_fips_group(&socket_path); - if let Some(parent) = socket_path.parent() { - Self::chown_to_fips_group(parent); - } + // Make the socket and its managed private directory group-accessible + // so fips group members can use fipsctl/fipstop. + set_control_socket_access( + &socket_path, + managed_parent.as_deref(), + Self::chown_to_fips_group, + )?; info!(path = %socket_path.display(), "Control socket listening"); @@ -291,6 +352,92 @@ mod unix_impl { self.cleanup(); } } + + #[cfg(test)] + mod tests { + use super::{ensure_socket_parent, set_control_socket_access}; + use std::os::unix::fs::PermissionsExt; + + #[test] + fn parent_setup_distinguishes_existing_and_created_directories() { + let temp = tempfile::tempdir().unwrap(); + let existing = temp.path().join("existing"); + std::fs::create_dir(&existing).unwrap(); + assert!(!ensure_socket_parent(&existing).unwrap()); + + let nested = temp.path().join("missing").join("fips"); + assert!(ensure_socket_parent(&nested).unwrap()); + assert!(nested.is_dir()); + assert!(!ensure_socket_parent(&nested).unwrap()); + } + + #[test] + fn access_setup_leaves_an_existing_shared_parent_unchanged() { + let temp = tempfile::tempdir().unwrap(); + let parent = temp.path().join("shared"); + std::fs::create_dir(&parent).unwrap(); + std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o711)).unwrap(); + let socket = parent.join("control.sock"); + std::fs::File::create(&socket).unwrap(); + + let mut chowned = Vec::new(); + set_control_socket_access(&socket, None, |path| chowned.push(path.to_path_buf())) + .unwrap(); + + assert_eq!(chowned, vec![socket.clone()]); + assert_eq!( + std::fs::metadata(&parent).unwrap().permissions().mode() & 0o777, + 0o711 + ); + assert_eq!( + std::fs::metadata(&socket).unwrap().permissions().mode() & 0o777, + 0o770 + ); + } + + #[test] + fn access_setup_secures_a_new_private_parent() { + let temp = tempfile::tempdir().unwrap(); + let parent = temp.path().join("fips"); + std::fs::create_dir(&parent).unwrap(); + let socket = parent.join("control.sock"); + std::fs::File::create(&socket).unwrap(); + + let mut chowned = Vec::new(); + set_control_socket_access(&socket, Some(&parent), |path| { + chowned.push(path.to_path_buf()) + }) + .unwrap(); + + assert_eq!(chowned, vec![socket, parent.clone()]); + assert_eq!( + std::fs::metadata(&parent).unwrap().permissions().mode() & 0o777, + 0o750 + ); + } + + #[test] + fn access_setup_secures_an_existing_managed_parent() { + let temp = tempfile::tempdir().unwrap(); + let parent = temp.path().join("managed"); + std::fs::create_dir(&parent).unwrap(); + std::fs::set_permissions(&parent, std::fs::Permissions::from_mode(0o700)).unwrap(); + let socket = parent.join("control.sock"); + std::fs::File::create(&socket).unwrap(); + + let mut chowned = Vec::new(); + set_control_socket_access(&socket, Some(&parent), |path| { + chowned.push(path.to_path_buf()) + }) + .unwrap(); + + assert_eq!(chowned, vec![socket, parent.clone()]); + assert_eq!( + std::fs::metadata(&parent).unwrap().permissions().mode() & 0o777, + 0o750 + ); + } + } } // ============================================================================ diff --git a/testing/deb-install/test.sh b/testing/deb-install/test.sh index 432ef1c0..0b542744 100755 --- a/testing/deb-install/test.sh +++ b/testing/deb-install/test.sh @@ -462,6 +462,26 @@ EOF fail "fips.service did not stay up after gateway-enable restart" fi + # systemd removes and recreates RuntimeDirectory=fips across this restart + # as root:root 0750. The daemon must restore the fips group on every start, + # not only when it created the directory itself. + local runtime_access + runtime_access=$(docker exec "$name" stat -c '%a %U:%G' /run/fips 2>/dev/null || true) + if [ "$runtime_access" = "750 root:fips" ]; then + pass "/run/fips ownership restored after service restart" + else + fail "/run/fips wrong after service restart: '$runtime_access' (expected '750 root:fips')" + fi + + if docker exec "$name" bash -c ' + useradd --system --no-create-home --user-group --groups fips fips-test-client + runuser -u fips-test-client -- fipsctl show status >/dev/null + '; then + pass "non-root fips group member reaches control socket after restart" + else + fail "non-root fips group member cannot reach control socket after restart" + fi + docker exec "$name" systemctl start fips-gateway.service >/dev/null 2>&1 || true sleep 3 if docker exec "$name" journalctl -u fips-gateway.service --no-pager 2>/dev/null \