diff --git a/Cargo.lock b/Cargo.lock index cd2f2e8..97c4d00 100644 --- a/Cargo.lock +++ b/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", diff --git a/bin/cluelesshd/Cargo.toml b/bin/cluelesshd/Cargo.toml index 8868d36..aa12397 100644 --- a/bin/cluelesshd/Cargo.toml +++ b/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" diff --git a/bin/cluelesshd/src/connection.rs b/bin/cluelesshd/src/connection.rs index 7ac97de..76349f2 100644 --- a/bin/cluelesshd/src/connection.rs +++ b/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::::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, rpc_client: Arc, @@ -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_term: Option, channel: Channel, process_exit_send: mpsc::Sender>>, process_exit_recv: mpsc::Receiver>>, @@ -196,7 +265,7 @@ async fn handle_session_channel(channel: Channel, rpc_client: Arc) 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 { - ws_row: height_rows as u16, - ws_col: width_chars as u16, - ws_xpixel: width_px as u16, - ws_ypixel: height_px as u16, - }, + height_rows, + width_chars, + width_px, + height_px, term_modes, ) .await @@ -398,11 +465,30 @@ impl SessionState { Ok(()) } - async fn pty_req(&mut self, term: String, winsize: Winsize, term_modes: Vec) -> Result<()> { - let pty = pty::Pty::new(term, winsize, term_modes).await?; - let controller = pty.controller().try_clone_to_owned()?; + async fn pty_req( + &mut self, + term: String, - self.pty = Some(pty); + width_chars: u32, + height_rows: u32, + width_px: u32, + height_px: u32, + term_modes: Vec, + ) -> 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.as_ref().map(|pty| pty.term()).unwrap_or_default(), + self.pty_term.clone(), 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" diff --git a/bin/cluelesshd/src/main.rs b/bin/cluelesshd/src/main.rs index 1053047..ef3f4cb 100644 --- a/bin/cluelesshd/src/main.rs +++ b/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()); - - let result = libc::close_range( + // libc close_range is not async-signal-safe, so syscall directly. + let result = libc::syscall( + libc::SYS_close_range, (PRIVSEP_CONNECTION_RPC_CLIENT_FD as u32) + 1, std::ffi::c_uint::MAX, 0, ); - if result == -1 { - return Err(std::io::Error::last_os_error()); + if result.is_negative() { + 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. diff --git a/bin/cluelesshd/src/pty.rs b/bin/cluelesshd/src/pty.rs index 674e057..2e2b446 100644 --- a/bin/cluelesshd/src/pty.rs +++ b/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, - - controller: OwnedFd, - - user_pty: OwnedFd, + pub controller: OwnedFd, + pub user_pty: OwnedFd, } impl Pty { - pub async fn new(term: String, winsize: Winsize, modes: Vec) -> Result { - tokio::task::spawn_blocking(move || Self::new_blocking(term, winsize, modes)).await? + pub async fn new(winsize: Winsize, modes: Vec) -> Result { + tokio::task::spawn_blocking(move || Self::new_blocking(winsize, modes)).await? } - pub fn new_blocking(term: String, winsize: Winsize, modes: Vec) -> Result { + pub fn new_blocking(winsize: Winsize, modes: Vec) -> Result { // 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 { - 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<()> { diff --git a/bin/cluelesshd/src/rpc.rs b/bin/cluelesshd/src/rpc.rs index 7a514bf..21d09c2 100644 --- a/bin/cluelesshd/src/rpc.rs +++ b/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, }, + 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, +} + #[derive(Debug, Serialize, Deserialize)] struct ShellRequest { - /// Whether a PTY is used. - /// If true, the PTY fd is passed as ancillary data. - /// If false, the response will contain the 3 stdio fds - /// as ancillary data. - pty: Option, + /// Whether a PTY is used and if yes, the TERM env var. + pty_term: Option, command: Option, 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, String>, @@ -114,9 +128,9 @@ pub struct Server { server_recv_recv: mpsc::Receiver<(Request, Vec)>, host_keys: Vec, authenticated_user: Option, - /// 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. - shell_process: Option<(Child, Vec)>, + + pty_user: Option, + shell_process: Option, } 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) -> Result<()> { + async fn receive_message(&mut self, req: Request, fds: Vec) -> 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 - .shell(&mut fds, &user, req) - .await - .map_err(|err| err.to_string()); + let result = self.shell(&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( - &mut self, - fds: &mut Vec, - user: &User, - req: ShellRequest, - ) -> Result> { + async fn shell(&mut self, user: &User, req: ShellRequest) -> Result> { 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 { - if fds.len() != 1 { - bail!("invalid request: shell with PTY must send one FD"); - } - let user_pty = fds.remove(0); - crate::pty::start_session_for_command(user_pty, pty.term, &mut cmd)?; + ensure!( + has_pty == self.pty_user.is_some(), + "Mismatch between client and server PTY requests" + ); + + 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()?); - fds2.push(stdin); - fds1.push(stdout.try_clone()?); - fds2.push(stdout); - fds1.push(stderr.try_clone()?); - fds2.push(stderr); + fds1.push(stdin); + fds1.push(stdout); + fds1.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, + ) -> Result> { + 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::().await?; + resp.result.map_err(|err| eyre!(err))?; + + Ok(fds) + } + + pub async fn shell( &self, command: Option, - pty: Option, - term: String, + pty_term: Option, env: Vec<(String, String)>, ) -> Result> { - 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) -> Result<()> { + // TODO: remove support for ancillary? let data = postcard::to_allocvec(&req)?; let socket = self.socket.as_fd().try_clone_to_owned()?; diff --git a/lib/cluelessh-connection/src/lib.rs b/lib/cluelessh-connection/src/lib.rs index 2a82241..34c6438 100644 --- a/lib/cluelessh-connection/src/lib.rs +++ b/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" => { diff --git a/lib/cluelessh-protocol/src/lib.rs b/lib/cluelessh-protocol/src/lib.rs index c8f94e4..2b29565 100644 --- a/lib/cluelessh-protocol/src/lib.rs +++ b/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, diff --git a/lib/cluelessh-transport/src/server.rs b/lib/cluelessh-transport/src/server.rs index ff5f383..20b56de 100644 --- a/lib/cluelessh-transport/src/server.rs +++ b/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); }