Radish alpha
h
rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5
Radicle Heartwood Protocol & Stack
Radicle
Git
node/systemd: Check that received sockets are `AF_UNIX`
Merged lorenz opened 8 months ago

Our implementation for the control socket is based on AF_UNIX, and as the note (which is removed suggests, we should actually check that the received socket really is of that domain.

This check was not implemented because it is not exposed via std, and a bit cumbersome to do via libc.

I did not realize that socket2 which neatly sits between std and libc in terms of its abstractions and is cross-platform allows us to do this, and we even already depend on it!

So, add the suggested check.

While at it, refactor the function to return early in cases. Now the progression from a file descriptor to a socket to a listener is nicely captured in the types and not obstructed too much by indentation.

Also log at the error level.

1 file changed +21 -18 8a66e4d0 8a66e4d0
modified crates/radicle-node/src/runtime.rs
@@ -321,28 +321,31 @@ impl Runtime {

    #[cfg(all(feature = "systemd", target_os = "linux"))]
    fn receive_listener() -> Option<Listener> {
-
        use std::os::fd::FromRawFd;
-
        match radicle_systemd::listen::fd("control") {
-
            Ok(Some(fd)) => {
-
                // NOTE: Here, we should make a call to [`fstat(2)`](man:fstat(2))
-
                // and make sure that the file descriptor we received actually
-
                // is `AF_UNIX`. However, this requires fiddling with
-
                // `libc` types or another dependency like `nix`, see
-
                // <https://github.com/lucab/libsystemd-rs/blob/b43fa5e3b5eca3e6aa16a6c2fad87220dc0ad7a0/src/activation.rs#L192-L196>
-
                // systemd also implements such a check, see
-
                // <https://github.com/systemd/systemd/blob/v254/src/libsystemd/sd-daemon/sd-daemon.c#L357-L398>
-
                Some(unsafe {
-
                    // SAFETY: We take ownership of this FD from systemd,
-
                    // which guarantees that it is open.
-
                    Listener::from_raw_fd(fd)
-
                })
+
        let fd = match radicle_systemd::listen::fd("control") {
+
            Ok(Some(fd)) => fd,
+
            Ok(None) => return None,
+
            Err(err) => {
+
                log::error!(target: "node", "Error receiving listener from systemd: {err}");
+
                return None;
            }
-
            Ok(None) => None,
+
        };
+

+
        let socket: socket2::Socket = unsafe { std::os::fd::FromRawFd::from_raw_fd(fd) };
+

+
        let domain = match socket.domain() {
+
            Ok(domain) => domain,
            Err(err) => {
-
                log::trace!(target: "node", "Error receiving file descriptors from systemd: {err}");
-
                None
+
                log::error!(target: "node", "Error receiving listener from systemd when inspecting domain of socket: {err}");
+
                return None;
            }
+
        };
+

+
        if domain != socket2::Domain::UNIX {
+
            log::error!(target: "node", "Dropping listener received from systemd: Domain is not AF_UNIX.");
+
            return None;
        }
+

+
        Some(Listener::from(socket))
    }

    fn bind(path: PathBuf) -> Result<ControlSocket, Error> {