Create the control socket and its directory with a restrictive mode

bind(2) creates the socket inode with 0777 & ~umask, so under a permissive
umask the control socket was world-accessible for the window between the
bind and the chmod to 0770 that followed it. The parent directory was
worse than a window: create_dir_all takes the same 0777 & ~umask and
nothing ever set a mode on it, so the directory holding the socket stayed
world-writable for the life of the host, and a world-writable parent lets
an unprivileged account plant an entry at the socket path.

Add a small shared helper that binds under a umask masking the "other"
bits and creates directories with an explicit 0750. The socket ends at the
mode it always did, with the existing chmod and chown left as the
authority on it, and 0750 is what the systemd unit and the FreeBSD rc
script already apply to the runtime directory, so no packaged deployment
sees a different mode. The umask is held across the bind alone, and it
only clears bits, so anything else created in that window comes out more
restrictive rather than less. Both the daemon and gateway control sockets
go through the helper; the duplicated bind sequences stay as they are.

The window between the stale-socket probe and the bind is documented at
both sites rather than closed: reaching it needs write access to the
socket's parent directory, which the packaged layouts give to root alone,
and an account holding it can deny the daemon its socket more simply by
squatting the path first.

CHANGELOG.md

@@ -866,6 +866,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   NXDOMAIN. Connecting the socket also means a dead upstream surfaces
   ECONNREFUSED immediately instead of stalling for five seconds.
 
+#### Control socket
+
+- The control socket and the directory holding it are now created with a
+  restrictive mode rather than created wide and narrowed afterwards. `bind(2)`
+  makes the socket inode `0777 & ~umask`, so under a permissive umask the
+  socket was world-accessible for the window between the bind and the `chmod`
+  to 0770 that followed it; the bind now runs under a umask that masks the
+  "other" bits, so the inode is 0770 from creation and the chmod and chown stay
+  the authority on its final mode. The parent directory was worse than a
+  window: it was created with `create_dir_all`, which is also `0777 & ~umask`,
+  and nothing ever set a mode on it, so under a permissive umask the directory
+  holding the socket stayed world-writable for the life of the host, and a
+  world-writable parent lets an unprivileged account plant an entry at the
+  socket path. Directories this code creates now come out 0750, which is what
+  the systemd unit (`RuntimeDirectoryMode=0750`) and the FreeBSD rc script
+  (`install -d -m 0750`) already apply, so no packaged deployment sees a
+  different mode and no `fipsctl` user loses access. Both the daemon and the
+  gateway control sockets are covered. **What this does not close**: the window
+  between the stale-socket probe and the bind is documented at the site rather
+  than removed. Reaching it needs write access to the socket's parent
+  directory, which the packaged layouts give to root alone, and an account
+  holding it can deny the daemon its socket more simply by squatting the path
+  first.
+
 #### Key material and identity files
 
 - Private key writes no longer follow a symlink, and the key file's mode is

src/control/mod.rs

@@ -151,7 +151,7 @@ mod unix_impl {
             if let Some(parent) = socket_path.parent()
                 && !parent.exists()
             {
-                std::fs::create_dir_all(parent)?;
+                crate::utils::sockperm::make_parent(parent)?;
                 debug!(path = %parent.display(), "Created control socket directory");
             }
 
@@ -160,7 +160,9 @@ mod unix_impl {
                 Self::remove_stale_socket(&socket_path)?;
             }
 
-            let listener = UnixListener::bind(&socket_path)?;
+            // Bound under a tightened umask, so the inode is never
+            // world-accessible in the window before the chmod below.
+            let listener = crate::utils::sockperm::bind(&socket_path)?;
 
             // Make the socket and its parent directory group-accessible so
             // 'fips' group members can use fipsctl/fipstop without root.
@@ -183,6 +185,15 @@ mod unix_impl {
         ///
         /// If the file exists but no one is listening, remove it so we can
         /// bind. This handles unclean daemon exits.
+        ///
+        /// The gap between the connect probe and the bind that follows is
+        /// accepted rather than closed. Reaching it needs write access to the
+        /// socket's parent directory, which the packaged layouts give to root
+        /// alone (0750 and root-owned under both systemd and the FreeBSD rc
+        /// script), and an account holding it can deny the daemon its socket
+        /// more simply by squatting the path before the daemon starts. The
+        /// removal itself unlinks a symlink rather than its target, so it is
+        /// not an arbitrary delete.
         fn remove_stale_socket(path: &Path) -> Result<(), std::io::Error> {
             // Try connecting to see if someone is listening
             match std::os::unix::net::UnixStream::connect(path) {

src/gateway/control.rs

@@ -65,7 +65,7 @@ impl GatewayControlSocket {
         if let Some(parent) = socket_path.parent()
             && !parent.exists()
         {
-            std::fs::create_dir_all(parent)?;
+            crate::utils::sockperm::make_parent(parent)?;
             debug!(path = %parent.display(), "Created gateway control socket directory");
         }
 
@@ -74,7 +74,9 @@ impl GatewayControlSocket {
             Self::remove_stale_socket(&socket_path)?;
         }
 
-        let listener = UnixListener::bind(&socket_path)?;
+        // Bound under a tightened umask, so the inode is never
+        // world-accessible in the window before the chmod below.
+        let listener = crate::utils::sockperm::bind(&socket_path)?;
 
         // Set permissions to 0770 and chown to fips group
         use std::os::unix::fs::PermissionsExt;
@@ -93,6 +95,15 @@ impl GatewayControlSocket {
     }
 
     /// Remove a stale socket file from a previous unclean exit.
+    ///
+    /// The gap between the connect probe and the bind that follows is
+    /// accepted rather than closed. Reaching it needs write access to the
+    /// socket's parent directory, which the packaged layouts give to root
+    /// alone (0750 and root-owned under both systemd and the FreeBSD rc
+    /// script), and an account holding it can deny the daemon its socket
+    /// more simply by squatting the path before the daemon starts. The
+    /// removal itself unlinks a symlink rather than its target, so it is
+    /// not an arbitrary delete.
     fn remove_stale_socket(path: &Path) -> Result<(), std::io::Error> {
         match std::os::unix::net::UnixStream::connect(path) {
             Ok(_) => Err(std::io::Error::new(

src/utils/mod.rs

@@ -1,6 +1,9 @@
 //! Utility modules.
 //!
 //! Shared infrastructure that doesn't belong to a specific protocol layer:
-//! session index allocation and other cross-cutting concerns.
+//! session index allocation, socket permission handling, and other
+//! cross-cutting concerns.
 
 pub mod index;
+#[cfg(unix)]
+pub mod sockperm;

src/utils/sockperm.rs

@@ -0,0 +1,157 @@
+//! Permission-safe creation of Unix domain sockets and the directories
+//! holding them.
+//!
+//! The socket inode and its parent directory are created with a mode the
+//! process umask can only tighten, rather than created wide and narrowed
+//! afterwards. The caller's own chmod and chown stay where they are and
+//! remain the authority on the socket's final mode; this closes the window
+//! between creation and that fix-up, and the case of an intermediate
+//! directory that nothing fixes up at all.
+
+use std::path::Path;
+use tokio::net::UnixListener;
+
+/// Mode for a directory this module creates to hold a control socket.
+///
+/// Matches what the packaging already applies (systemd's
+/// `RuntimeDirectoryMode=0750`, `install -d -m 0750` in the FreeBSD rc
+/// script), so no packaged deployment sees a different directory mode than
+/// it does today. Widening it would expose the socket path to accounts that
+/// cannot reach it now; the umask can still tighten it further.
+const SOCKET_DIR_MODE: u32 = 0o750;
+
+/// umask held across the socket bind.
+///
+/// `bind(2)` creates the socket inode with `0777 & !umask`, so under a
+/// permissive umask the socket is world-accessible until the chmod that
+/// follows it. Masking the "other" bits makes the inode 0770 at creation,
+/// which is the mode the caller applies a moment later anyway. Changing
+/// this changes the mode the socket is created with, not the mode it ends
+/// up with.
+const BIND_UMASK: libc::mode_t = 0o007;
+
+/// Restores the process umask when dropped.
+struct UmaskGuard(libc::mode_t);
+
+impl UmaskGuard {
+    /// Install `mask` as the process umask, remembering the previous one.
+    fn tighten(mask: libc::mode_t) -> Self {
+        // SAFETY: umask(2) cannot fail and touches only process state.
+        Self(unsafe { libc::umask(mask) })
+    }
+}
+
+impl Drop for UmaskGuard {
+    fn drop(&mut self) {
+        // SAFETY: as above; restoring the mask this guard replaced.
+        unsafe {
+            libc::umask(self.0);
+        }
+    }
+}
+
+/// Create the directory that will hold a socket, and any missing ancestors.
+///
+/// Directories come out 0750 rather than `0777 & !umask`. Nothing chmods an
+/// intermediate directory afterwards, so one created under a permissive
+/// umask would stay world-writable for the life of the host, and a
+/// world-writable parent lets an unprivileged account plant an entry at the
+/// socket path.
+pub fn make_parent(parent: &Path) -> Result<(), std::io::Error> {
+    use std::os::unix::fs::DirBuilderExt;
+
+    std::fs::DirBuilder::new()
+        .recursive(true)
+        .mode(SOCKET_DIR_MODE)
+        .create(parent)
+}
+
+/// Bind a Unix listener whose inode is never world-accessible.
+///
+/// The umask is process-global, so it is held across the bind alone. It
+/// only clears bits, so anything else created inside that window comes out
+/// more restrictive, never less.
+pub fn bind(path: &Path) -> Result<UnixListener, std::io::Error> {
+    let _umask = UmaskGuard::tighten(BIND_UMASK);
+    UnixListener::bind(path)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::os::unix::fs::PermissionsExt;
+    use std::sync::Mutex;
+
+    /// The umask is process-global, so the tests that set it run one at a
+    /// time. This does not serialize against the rest of the test binary;
+    /// the mask used is 0o022, the ordinary default, so a file another test
+    /// creates in the window is unaffected.
+    static UMASK_LOCK: Mutex<()> = Mutex::new(());
+
+    /// Take the umask lock, ignoring poisoning: a test that fails while
+    /// holding it must not turn its siblings red for an unrelated reason.
+    fn umask_lock() -> std::sync::MutexGuard<'static, ()> {
+        UMASK_LOCK.lock().unwrap_or_else(|e| e.into_inner())
+    }
+
+    /// Read the current umask, which is only observable by replacing it.
+    fn current_umask() -> libc::mode_t {
+        // SAFETY: umask(2) cannot fail; the value read is put straight back.
+        unsafe {
+            let old = libc::umask(0o022);
+            libc::umask(old);
+            old
+        }
+    }
+
+    fn mode_of(path: &Path) -> u32 {
+        std::fs::symlink_metadata(path)
+            .unwrap()
+            .permissions()
+            .mode()
+    }
+
+    #[tokio::test]
+    async fn socket_is_created_without_other_access_under_a_permissive_umask() {
+        let _lock = umask_lock();
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("control.sock");
+
+        let restore = UmaskGuard::tighten(0o022);
+        let listener = bind(&path).unwrap();
+        drop(restore);
+
+        assert_eq!(mode_of(&path) & 0o007, 0);
+        drop(listener);
+    }
+
+    #[tokio::test]
+    async fn socket_bind_leaves_the_process_umask_as_it_found_it() {
+        let _lock = umask_lock();
+        let dir = tempfile::tempdir().unwrap();
+        let path = dir.path().join("control.sock");
+
+        let restore = UmaskGuard::tighten(0o022);
+        let listener = bind(&path).unwrap();
+        let after = current_umask();
+        drop(restore);
+
+        assert_eq!(after, 0o022);
+        drop(listener);
+    }
+
+    #[test]
+    fn socket_parent_and_its_ancestors_are_created_without_other_access() {
+        let _lock = umask_lock();
+        let dir = tempfile::tempdir().unwrap();
+        let intermediate = dir.path().join("run");
+        let parent = intermediate.join("fips");
+
+        let restore = UmaskGuard::tighten(0o022);
+        make_parent(&parent).unwrap();
+        drop(restore);
+
+        assert_eq!(mode_of(&intermediate) & 0o007, 0);
+        assert_eq!(mode_of(&parent) & 0o007, 0);
+    }
+}