use namespaces

Cargo.lock

@@ -1353,9 +1353,9 @@ dependencies = [
 
 [[package]]
 name = "rustix"
-version = "0.38.34"
+version = "0.38.35"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f"
+checksum = "a85d50532239da68e9addb745ba38ff4612a242c1c7ceea689c4bc7c2f43c36f"
 dependencies = [
  "bitflags",
  "errno",

bin/cluelesshd/Cargo.toml

@@ -12,7 +12,7 @@ tokio = { version = "1.39.2", features = ["full"] }
 tracing.workspace = true
 eyre.workspace = true
 tracing-subscriber = { version = "0.3.18", features = ["env-filter", "json"] }
-rustix = { version = "0.38.34", features = ["pty", "termios", "procfs", "process", "stdio", "net"] }
+rustix = { version = "0.38.35", features = ["pty", "termios", "procfs", "process", "stdio", "net", "fs", "thread"] }
 users = "0.11.0"
 futures = "0.3.30"
 thiserror = "1.0.63"

bin/cluelesshd/src/connection.rs

@@ -1,11 +1,11 @@
 use std::{
-    os::fd::{BorrowedFd, FromRawFd, OwnedFd},
+    os::fd::{FromRawFd, OwnedFd},
+    path::Path,
     pin::Pin,
     sync::Arc,
 };
 
 use crate::{
-    pty::{self, Pty},
     rpc, MemFd, SerializedConnectionState, PRIVSEP_CONNECTION_RPC_CLIENT_FD,
     PRIVSEP_CONNECTION_STATE_FD, PRIVSEP_CONNECTION_STREAM_FD,
 };
@@ -18,7 +18,11 @@ use cluelessh_tokio::{
     Channel,
 };
 use eyre::{bail, ensure, Result, WrapErr};
-use rustix::termios::Winsize;
+use rustix::{
+    fs::UnmountFlags,
+    process::WaitOptions,
+    thread::{Pid, UnshareFlags},
+};
 use tokio::{
     fs::File,
     io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt},
@@ -28,8 +32,6 @@ use tokio::{
 use tracing::{debug, error, info, info_span, warn, Instrument};
 
 pub async fn connection() -> Result<()> {
-    rustix::fs::fcntl_getfd(unsafe { BorrowedFd::borrow_raw(PRIVSEP_CONNECTION_STATE_FD) })
-        .unwrap();
     let mut memfd =
         unsafe { MemFd::<SerializedConnectionState>::from_raw_fd(PRIVSEP_CONNECTION_STATE_FD) }
             .wrap_err("failed to open memfd")?;
@@ -45,6 +47,10 @@ pub async fn connection() -> Result<()> {
 async fn connection_inner(state: SerializedConnectionState) -> Result<()> {
     let config = state.config;
 
+    if rustix::process::getuid().is_root() {
+        unshare_namespaces()?;
+    }
+
     if let Some(uid) = state.setgid {
         debug!(?uid, "Setting GID to drop privileges");
         let result = unsafe { libc::setgid(uid) };
@@ -124,6 +130,69 @@ async fn connection_inner(state: SerializedConnectionState) -> Result<()> {
     Ok(())
 }
 
+fn unshare_namespaces() -> Result<()> {
+    // The complicated incarnation to get a mount namespace working.
+    rustix::thread::unshare(
+        UnshareFlags::NEWNS
+            | UnshareFlags::NEWNET
+            | UnshareFlags::NEWIPC
+            | UnshareFlags::NEWPID
+            | UnshareFlags::NEWTIME,
+    )
+    .wrap_err("unsharing namespaces")?;
+
+    // After creating the PID namespace, we fork immediately so we can get PID 1.
+    // We never exec, we just let the child live on happily.
+    // The parent immediately waits for it, and then doesn't do anything really.
+    // TODO: this is a bit sus....
+    unsafe {
+        let result = libc::fork();
+        if result == -1 {
+            return Err(std::io::Error::last_os_error()).wrap_err("setting propagation flags")?;
+        }
+        if result > 0 {
+            // Parent
+            let code = rustix::process::waitpid(
+                Some(Pid::from_raw_unchecked(result)),
+                WaitOptions::empty(),
+            );
+            match code {
+                Err(_) => libc::exit(2),
+                Ok(None) => libc::exit(1),
+                Ok(Some(code)) => libc::exit(code.as_raw() as i32),
+            }
+        }
+    }
+
+    let result = unsafe {
+        libc::mount(
+            c"none".as_ptr(),
+            c"/".as_ptr(),
+            std::ptr::null(),
+            libc::MS_REC | libc::MS_PRIVATE,
+            std::ptr::null(),
+        )
+    };
+    if result == -1 {
+        return Err(std::io::Error::last_os_error()).wrap_err("setting propagation flags")?;
+    }
+
+    let new_root = Path::new("empty-new-root");
+    let old_root = &new_root.join("old-root");
+
+    std::fs::create_dir_all(new_root)?;
+    std::fs::create_dir_all(&old_root)?;
+
+    rustix::fs::bind_mount(new_root, new_root).wrap_err("bind mount the empty dir")?;
+
+    rustix::process::pivot_root(new_root, old_root).wrap_err("pivoting root")?;
+
+    // TODO: can we get rid of it entirely?
+    rustix::fs::unmount("/old-root", UnmountFlags::DETACH).wrap_err("unmounting old root")?;
+
+    Ok(())
+}
+
 async fn handle_connection(
     mut conn: cluelessh_tokio::server::ServerConnection<TcpStream>,
     rpc_client: Arc<rpc::Client>,
@@ -145,7 +214,7 @@ async fn handle_connection(
                         return Ok(());
                     }
                     SshStatus::Disconnect => {
-                        info!("Received disconnect from client");
+                        debug!("Received disconnect from client");
                         return Ok(());
                     }
                 },
@@ -175,7 +244,7 @@ async fn handle_connection(
 }
 
 struct SessionState {
-    pty: Option<Pty>,
+    pty_term: Option<String>,
     channel: Channel,
     process_exit_send: mpsc::Sender<Result<Option<i32>>>,
     process_exit_recv: mpsc::Receiver<Result<Option<i32>>>,
@@ -196,7 +265,7 @@ async fn handle_session_channel(channel: Channel, rpc_client: Arc<rpc::Client>)
     let (process_exit_send, process_exit_recv) = tokio::sync::mpsc::channel(1);
 
     let mut state = SessionState {
-        pty: None,
+        pty_term: None,
         channel,
         process_exit_send,
         process_exit_recv,
@@ -295,12 +364,10 @@ impl SessionState {
                         match self
                             .pty_req(
                                 term,
-                                Winsize {
+                                height_rows,
-                                    ws_row: height_rows as u16,
+                                width_chars,
-                                    ws_col: width_chars as u16,
+                                width_px,
-                                    ws_xpixel: width_px as u16,
+                                height_px,
-                                    ws_ypixel: height_px as u16,
-                                },
                                 term_modes,
                             )
                             .await
@@ -398,11 +465,30 @@ impl SessionState {
         Ok(())
     }
 
-    async fn pty_req(&mut self, term: String, winsize: Winsize, term_modes: Vec<u8>) -> Result<()> {
+    async fn pty_req(
-        let pty = pty::Pty::new(term, winsize, term_modes).await?;
+        &mut self,
-        let controller = pty.controller().try_clone_to_owned()?;
+        term: String,
 
-        self.pty = Some(pty);
+        width_chars: u32,
+        height_rows: u32,
+        width_px: u32,
+        height_px: u32,
+        term_modes: Vec<u8>,
+    ) -> Result<()> {
+        let mut fd = self
+            .rpc_client
+            .pty_req(width_chars, height_rows, width_px, height_px, term_modes)
+            .await?;
+
+        ensure!(
+            fd.len() == 1,
+            "Incorrect amount of FDs received: {}",
+            fd.len()
+        );
+
+        self.pty_term = Some(term);
+
+        let controller = fd.remove(0);
         self.writer = Some(Box::pin(File::from_std(std::fs::File::from(
             controller.try_clone()?,
         ))));
@@ -411,22 +497,16 @@ impl SessionState {
     }
 
     async fn shell(&mut self, shell_command: Option<&str>) -> Result<()> {
-        let pty = match &self.pty {
-            Some(pty) => Some(pty.user_fd()?),
-            None => None,
-        };
-
         let mut fds = self
             .rpc_client
-            .exec(
+            .shell(
                 shell_command.map(ToOwned::to_owned),
-                pty,
+                self.pty_term.clone(),
-                self.pty.as_ref().map(|pty| pty.term()).unwrap_or_default(),
                 self.envs.clone(),
             )
             .await?;
 
-        if self.pty.is_some() {
+        if self.pty_term.is_some() {
             ensure!(
                 fds.len() == 0,
                 "RPC Server sent back FDs despite being in PTY mode"

bin/cluelesshd/src/main.rs

@@ -24,7 +24,7 @@ use eyre::{bail, eyre, Context, Result};
 use rustix::fs::MemfdFlags;
 use serde::{Deserialize, Serialize};
 use tokio::net::{TcpListener, TcpStream};
-use tracing::{error, info};
+use tracing::{error, info, warn};
 
 use tracing_subscriber::EnvFilter;
 
@@ -39,7 +39,12 @@ struct Args {
 async fn main() -> eyre::Result<()> {
     match std::env::var("CLUELESSH_PRIVSEP_PROCESS") {
         Ok(privsep_process) => match privsep_process.as_str() {
-            "connection" => connection::connection().await,
+            "connection" => {
+                if let Err(err) = connection::connection().await {
+                    error!(?err, "Error in connection child process");
+                }
+                Ok(())
+            }
             _ => bail!("unknown CLUELESSH_PRIVSEP_PROCESS: {privsep_process}"),
         },
         Err(_) => {
@@ -50,6 +55,10 @@ async fn main() -> eyre::Result<()> {
 
             setup_tracing(&config);
 
+            if !rustix::process::getuid().is_root() {
+                warn!("Daemon not started as root. This disables several security mitigations and permits logging in as any other user");
+            }
+
             let addr: SocketAddr = SocketAddr::new(config.net.ip, config.net.port);
             info!(%addr, "Starting server");
 
@@ -220,15 +229,15 @@ async fn spawn_connection_child(
 
             // Ensure that all FDs are closed except stdout (for logging), and the 3 arguments.
             drop(rustix::stdio::take_stdin());
-            drop(rustix::stdio::take_stderr());
+            // libc close_range is not async-signal-safe, so syscall directly.
-
+            let result = libc::syscall(
-            let result = libc::close_range(
+                libc::SYS_close_range,
                 (PRIVSEP_CONNECTION_RPC_CLIENT_FD as u32) + 1,
                 std::ffi::c_uint::MAX,
                 0,
             );
-            if result == -1 {
+            if result.is_negative() {
-                return Err(std::io::Error::last_os_error());
+                return Err(std::io::Error::from_raw_os_error(-(result as i32)));
             }
 
             // Ensure our new FDs stay open, as they will be acquired in the new process.

bin/cluelesshd/src/pty.rs

@@ -1,6 +1,6 @@
 //! PTY-related operations for setting up the session.
 
-use std::os::fd::{AsFd, BorrowedFd, OwnedFd};
+use std::os::fd::OwnedFd;
 
 use eyre::{Context, Result};
 use rustix::{
@@ -11,19 +11,16 @@ use rustix::{
 use tokio::process::Command;
 
 pub struct Pty {
-    term: String,
+    pub controller: OwnedFd,
-
+    pub user_pty: OwnedFd,
-    controller: OwnedFd,
-
-    user_pty: OwnedFd,
 }
 
 impl Pty {
-    pub async fn new(term: String, winsize: Winsize, modes: Vec<u8>) -> Result<Self> {
+    pub async fn new(winsize: Winsize, modes: Vec<u8>) -> Result<Self> {
-        tokio::task::spawn_blocking(move || Self::new_blocking(term, winsize, modes)).await?
+        tokio::task::spawn_blocking(move || Self::new_blocking(winsize, modes)).await?
     }
 
-    pub fn new_blocking(term: String, winsize: Winsize, modes: Vec<u8>) -> Result<Self> {
+    pub fn new_blocking(winsize: Winsize, modes: Vec<u8>) -> Result<Self> {
         // Create new PTY:
         let controller = rustix::pty::openpt(OpenptFlags::RDWR | OpenptFlags::NOCTTY)
             .wrap_err("opening controller pty")?;
@@ -47,23 +44,10 @@ impl Pty {
         rustix::termios::tcsetattr(&user_pty, rustix::termios::OptionalActions::Flush, &termios)?;
 
         Ok(Self {
-            term,
             controller,
             user_pty,
         })
     }
-
-    pub fn term(&self) -> String {
-        self.term.clone()
-    }
-
-    pub fn user_fd(&self) -> Result<OwnedFd> {
-        self.user_pty.try_clone().wrap_err("cloning PTY user")
-    }
-
-    pub fn controller(&self) -> BorrowedFd<'_> {
-        self.controller.as_fd()
-    }
 }
 
 pub fn start_session_for_command(user_pty: OwnedFd, term: String, cmd: &mut Command) -> Result<()> {

bin/cluelesshd/src/rpc.rs

@@ -15,6 +15,7 @@ use cluelessh_keys::signature::Signature;
 use cluelessh_protocol::auth::CheckPubkey;
 use cluelessh_protocol::auth::VerifySignature;
 use eyre::bail;
+use eyre::ensure;
 use eyre::eyre;
 use eyre::Context;
 use eyre::OptionExt;
@@ -25,6 +26,7 @@ use rustix::net::RecvFlags;
 use rustix::net::SendAncillaryBuffer;
 use rustix::net::SendAncillaryMessage;
 use rustix::net::SendFlags;
+use rustix::termios::Winsize;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use tokio::process::Child;
@@ -55,6 +57,7 @@ enum Request {
         pubkey_alg_name: String,
         pubkey: Vec<u8>,
     },
+    PtyReq(PtyRequest),
     /// Executes a command on the host.
     /// IMPORTANT: This is the critical operation, and we must ensure that it is secure.
     /// To ensure that even a compromised auth process cannot escalate privileges via this RPC,
@@ -63,13 +66,19 @@ enum Request {
     Wait,
 }
 
+#[derive(Debug, Serialize, Deserialize)]
+struct PtyRequest {
+    height_rows: u32,
+    width_chars: u32,
+    width_px: u32,
+    height_px: u32,
+    term_modes: Vec<u8>,
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 struct ShellRequest {
-    /// Whether a PTY is used.
+    /// Whether a PTY is used and if yes, the TERM env var.
-    /// If true, the PTY fd is passed as ancillary data.
+    pty_term: Option<String>,
-    /// If false, the response will contain the 3 stdio fds
-    /// as ancillary data.
-    pty: Option<ShellRequestPty>,
     command: Option<String>,
     env: Vec<(String, String)>,
 }
@@ -99,6 +108,11 @@ struct ShellResponse {
     result: Result<(), String>,
 }
 
+#[derive(Debug, Serialize, Deserialize)]
+struct PtyResponse {
+    result: Result<(), String>,
+}
+
 #[derive(Debug, Serialize, Deserialize)]
 struct WaitResponse {
     result: Result<Option<i32>, String>,
@@ -114,9 +128,9 @@ pub struct Server {
     server_recv_recv: mpsc::Receiver<(Request, Vec<OwnedFd>)>,
     host_keys: Vec<PlaintextPrivateKey>,
     authenticated_user: Option<users::User>,
-    /// We keep the owned FDs here around to avoid a race condition where the child would
+
-    /// think stdout is closed before the client process opens it.
+    pty_user: Option<OwnedFd>,
-    shell_process: Option<(Child, Vec<OwnedFd>)>,
+    shell_process: Option<Child>,
 }
 
 fn server_thread(
@@ -152,6 +166,7 @@ impl Server {
             host_keys,
             server_recv_recv,
             authenticated_user: None,
+            pty_user: None,
             shell_process: None,
         })
     }
@@ -171,7 +186,7 @@ impl Server {
         }
     }
 
-    async fn receive_message(&mut self, req: Request, mut fds: Vec<OwnedFd>) -> Result<()> {
+    async fn receive_message(&mut self, req: Request, fds: Vec<OwnedFd>) -> Result<()> {
         trace!(?req, ?fds, "Received RPC message");
 
         match req {
@@ -245,6 +260,42 @@ impl Server {
 
                 self.respond(CheckPubkeyResponse { is_ok }).await?;
             }
+            Request::PtyReq(req) => {
+                if self.pty_user.is_some() {
+                    self.respond(ShellResponse {
+                        result: Err("already requests pty".to_owned()),
+                    })
+                    .await?;
+
+                    return Ok(());
+                }
+
+                let result = crate::pty::Pty::new(
+                    Winsize {
+                        ws_row: req.width_chars as u16,
+                        ws_col: req.height_rows as u16,
+                        ws_xpixel: req.width_px as u16,
+                        ws_ypixel: req.height_px as u16,
+                    },
+                    req.term_modes,
+                )
+                .await;
+
+                let (controller, user) = match result {
+                    Ok(pty) => (vec![pty.controller], Ok(pty.user_pty)),
+                    Err(err) => (vec![], Err(err)),
+                };
+
+                self.respond_ancillary(
+                    ShellResponse {
+                        result: user.as_ref().map(drop).map_err(ToString::to_string),
+                    },
+                    controller,
+                )
+                .await?;
+
+                self.pty_user = user.ok();
+            }
             Request::Shell(req) => {
                 if self.shell_process.is_some() {
                     self.respond(ShellResponse {
@@ -264,10 +315,7 @@ impl Server {
                     return Ok(());
                 };
 
-                let result = self
+                let result = self.shell(&user, req).await.map_err(|err| err.to_string());
-                    .shell(&mut fds, &user, req)
-                    .await
-                    .map_err(|err| err.to_string());
 
                 self.respond_ancillary(
                     ShellResponse {
@@ -285,7 +333,7 @@ impl Server {
                     .await?;
                 }
                 Some(child) => {
-                    let result = child.0.wait().await;
+                    let result = child.wait().await;
 
                     self.respond(WaitResponse {
                         result: result
@@ -302,12 +350,7 @@ impl Server {
         Ok(())
     }
 
-    async fn shell(
+    async fn shell(&mut self, user: &User, req: ShellRequest) -> Result<Vec<OwnedFd>> {
-        &mut self,
-        fds: &mut Vec<OwnedFd>,
-        user: &User,
-        req: ShellRequest,
-    ) -> Result<Vec<OwnedFd>> {
         let shell = user.shell();
 
         let mut cmd = Command::new(shell);
@@ -317,14 +360,20 @@ impl Server {
         }
         cmd.env_clear();
 
-        let has_pty = req.pty.is_some();
+        let has_pty = req.pty_term.is_some();
 
-        if let Some(pty) = req.pty {
+        ensure!(
-            if fds.len() != 1 {
+            has_pty == self.pty_user.is_some(),
-                bail!("invalid request: shell with PTY must send one FD");
+            "Mismatch between client and server PTY requests"
-            }
+        );
-            let user_pty = fds.remove(0);
+
-            crate::pty::start_session_for_command(user_pty, pty.term, &mut cmd)?;
+        if let Some(term) = req.pty_term {
+            let Some(pty_fd) = &self.pty_user else {
+                bail!("no pty requested before");
+            };
+            let pty_fd = pty_fd.try_clone()?;
+
+            crate::pty::start_session_for_command(pty_fd, term, &mut cmd)?;
         } else {
             cmd.stdin(Stdio::piped());
             cmd.stdout(Stdio::piped());
@@ -346,22 +395,18 @@ impl Server {
 
         // See Server::shell_process
         let mut fds1 = Vec::new();
-        let mut fds2 = Vec::new();
 
         if !has_pty {
             let stdin = shell.stdin.take().unwrap().into_owned_fd()?;
             let stdout = shell.stdout.take().unwrap().into_owned_fd()?;
             let stderr = shell.stderr.take().unwrap().into_owned_fd()?;
 
-            fds1.push(stdin.try_clone()?);
+            fds1.push(stdin);
-            fds2.push(stdin);
+            fds1.push(stdout);
-            fds1.push(stdout.try_clone()?);
+            fds1.push(stderr);
-            fds2.push(stdout);
-            fds1.push(stderr.try_clone()?);
-            fds2.push(stderr);
         }
 
-        self.shell_process = Some((shell, vec![]));
+        self.shell_process = Some(shell);
 
         Ok(fds1)
     }
@@ -442,26 +487,45 @@ impl Client {
         resp.is_ok.map_err(|err| eyre!(err))
     }
 
-    pub async fn exec(
+    pub async fn pty_req(
+        &self,
+        width_chars: u32,
+        height_rows: u32,
+        width_px: u32,
+        height_px: u32,
+        term_modes: Vec<u8>,
+    ) -> Result<Vec<OwnedFd>> {
+        self.send_request(
+            &Request::PtyReq(PtyRequest {
+                height_rows,
+                width_chars,
+                width_px,
+                height_px,
+                term_modes,
+            }),
+            vec![],
+        )
+        .await?;
+
+        let (resp, fds) = self.recv_response_ancillary::<PtyResponse>().await?;
+        resp.result.map_err(|err| eyre!(err))?;
+
+        Ok(fds)
+    }
+
+    pub async fn shell(
         &self,
         command: Option<String>,
-        pty: Option<OwnedFd>,
+        pty_term: Option<String>,
-        term: String,
         env: Vec<(String, String)>,
     ) -> Result<Vec<OwnedFd>> {
-        let has_pty = pty.is_some();
-        let fds = match pty {
-            Some(fd) => vec![fd],
-            None => vec![],
-        };
-
         self.send_request(
             &Request::Shell(ShellRequest {
-                pty: has_pty.then_some(ShellRequestPty { term }),
+                pty_term,
                 command,
                 env,
             }),
-            fds,
+            vec![],
         )
         .await?;
 
@@ -487,6 +551,7 @@ impl Client {
     }
 
     async fn send_request(&self, req: &Request, fds: Vec<OwnedFd>) -> Result<()> {
+        // TODO: remove support for ancillary?
         let data = postcard::to_allocvec(&req)?;
 
         let socket = self.socket.as_fd().try_clone_to_owned()?;

lib/cluelessh-connection/src/lib.rs

@@ -456,7 +456,7 @@ impl ChannelsState {
                             return Err(peer_error!("server tried to open shell"));
                         }
 
-                        info!(channel = %our_channel, "Opening shell");
+                        debug!(channel = %our_channel, "Opening shell");
                         ChannelRequest::Shell { want_reply }
                     }
                     "exec" => {

lib/cluelessh-protocol/src/lib.rs

@@ -280,7 +280,7 @@ pub mod auth {
 
     use cluelessh_format::{numbers, NameList};
     use cluelessh_transport::{packet::Packet, peer_error, Result};
-    use tracing::{debug, info};
+    use tracing::debug;
 
     pub struct ServerAuth {
         has_failed: bool,
@@ -363,7 +363,7 @@ pub mod auth {
             let method_name = p.utf8_string()?;
 
             if method_name != "none" {
-                info!(
+                debug!(
                     %username,
                     %service_name,
                     %method_name,

lib/cluelessh-transport/src/server.rs

@@ -121,7 +121,7 @@ impl ServerConnection {
 
                     let reason_string = numbers::disconnect_reason_to_string(reason);
 
-                    info!(%reason, %reason_string, %description, "Client disconnecting");
+                    debug!(%reason, %reason_string, %description, "Client disconnecting");
 
                     return Err(SshStatus::Disconnect);
                 }