Use the same server instance for privileged launches too

WhatAmISupposedToPutHere · slp · commit 1acbfa76e334 · 2025-01-27T10:42:45.000+01:00
We can keep the root uid in our saved set user id, and copy it to ruid/euid
after fork, if launching as root is requested. Writing to drop_caches becomes
a bit tricker when running unprivileged, but we can acquire the privileges in
a forked process for it to work and not affect other launches.

Signed-off-by: Sasha Finkelstein &lt;fnkl.kernel@gmail.com&gt;
diff --git a/crates/muvm/src/bin/muvm.rs b/crates/muvm/src/bin/muvm.rs
@@ -77,6 +77,7 @@ fn main() -> Result<ExitCode> {
         options.env,
         options.interactive,
         options.tty,
+        options.privileged,
     )? {
         LaunchResult::LaunchRequested(code) => {
             // There was a muvm instance already running and we've requested it
@@ -264,7 +265,7 @@ fn main() -> Result<ExitCode> {
                 .context("Failed to connect to `passt`")?
                 .into()
         } else {
-            start_passt(options.server_port, options.root_server_port)
+            start_passt(options.server_port)
                 .context("Failed to start `passt`")?
                 .into()
         };
@@ -410,10 +411,6 @@ fn main() -> Result<ExitCode> {
         "MUVM_SERVER_PORT".to_owned(),
         options.server_port.to_string(),
     );
-    env.insert(
-        "MUVM_ROOT_SERVER_PORT".to_owned(),
-        options.root_server_port.to_string(),
-    );
     env.insert("MUVM_SERVER_COOKIE".to_owned(), cookie.to_string());
 
     let display = env::var("DISPLAY").context("X11 forwarding requested but DISPLAY is unset")?;
@@ -467,7 +464,7 @@ fn main() -> Result<ExitCode> {
         }
     }
 
-    spawn_monitor(options.root_server_port, cookie);
+    spawn_monitor(options.server_port, cookie);
 
     {
         // Start and enter the microVM. Unless there is some error while creating the
diff --git a/crates/muvm/src/cli_options.rs b/crates/muvm/src/cli_options.rs
@@ -13,12 +13,12 @@ pub struct Options {
     pub mem: Option<MiB>,
     pub vram: Option<MiB>,
     pub passt_socket: Option<PathBuf>,
-    pub root_server_port: u32,
     pub server_port: u32,
     pub fex_images: Vec<String>,
     pub merged_rootfs: bool,
     pub interactive: bool,
     pub tty: bool,
+    pub privileged: bool,
     pub command: PathBuf,
     pub command_args: Vec<String>,
 }
@@ -98,12 +98,6 @@ pub fn options() -> OptionParser<Options> {
         .help("Instead of starting passt, connect to passt socket at PATH")
         .argument("PATH")
         .optional();
-    let root_server_port = long("root-server-port")
-        .short('r')
-        .help("Set the port to be used in root server mode")
-        .argument("ROOT_SERVER_PORT")
-        .fallback(3335)
-        .display_fallback();
     let server_port = long("server-port")
         .short('p')
         .help("Set the port to be used in server mode")
@@ -118,6 +112,12 @@ pub fn options() -> OptionParser<Options> {
         .short('t')
         .help("Allocate a tty for the command")
         .switch();
+    let privileged = long("privileged")
+        .help(
+            "Run the command as root inside the vm.
+        This notably does not allow root access to the host fs.",
+        )
+        .switch();
     let command = positional("COMMAND").help("the command you want to execute in the vm");
     let command_args = any::<String, _, _>("COMMAND_ARGS", |arg| {
         (!["--help", "-h"].contains(&&*arg)).then_some(arg)
@@ -131,12 +131,12 @@ pub fn options() -> OptionParser<Options> {
         mem,
         vram,
         passt_socket,
-        root_server_port,
         server_port,
         fex_images,
         merged_rootfs,
         interactive,
         tty,
+        privileged,
         // positionals
         command,
         command_args,
diff --git a/crates/muvm/src/guest/bin/muvm-guest.rs b/crates/muvm/src/guest/bin/muvm-guest.rs
@@ -13,6 +13,7 @@ use muvm::guest::net::configure_network;
 use muvm::guest::socket::setup_socket_proxy;
 use muvm::guest::user::setup_user;
 use muvm::guest::x11::setup_x11_forwarding;
+use nix::unistd::{setresgid, setresuid, Gid, Uid};
 use rustix::process::{getrlimit, setrlimit, Resource};
 
 fn main() -> Result<()> {
@@ -61,13 +62,6 @@ fn main() -> Result<()> {
         .spawn()
         .context("Failed to execute `muvm-hidpipe` as child process")?;
 
-    // Before switching to the user, start another instance of muvm-server to serve
-    // launch requests as root.
-    let muvm_server_path = find_muvm_exec("muvm-server")?;
-    Command::new(muvm_server_path)
-        .spawn()
-        .context("Failed to execute `muvm-server` as child process")?;
-
     let run_path = match setup_user(options.username, options.uid, options.gid) {
         Ok(p) => p,
         Err(err) => return Err(err).context("Failed to set up user, bailing out"),
@@ -81,6 +75,8 @@ fn main() -> Result<()> {
 
     setup_x11_forwarding(run_path)?;
 
+    setresuid(options.uid, Uid::from(0), Uid::from(0))?;
+    setresgid(options.gid, Gid::from(0), Gid::from(0))?;
     debug!(command:? = options.command, command_args:? = options.command_args; "exec");
     let err = Command::new(&options.command)
         .args(options.command_args)
diff --git a/crates/muvm/src/guest/user.rs b/crates/muvm/src/guest/user.rs
@@ -4,13 +4,13 @@ use std::os::unix::fs::{chown, PermissionsExt as _};
 use std::path::{Path, PathBuf};
 
 use anyhow::{anyhow, Context, Result};
-use nix::unistd::{setgid, setuid, Gid, Uid, User};
+use nix::unistd::{setresgid, setresuid, Gid, Uid, User};
 
 pub fn setup_user(username: String, uid: Uid, gid: Gid) -> Result<PathBuf> {
     setup_directories(uid, gid)?;
 
-    setgid(gid).context("Failed to setgid")?;
-    setuid(uid).context("Failed to setuid")?;
+    setresgid(gid, gid, Gid::from(0)).context("Failed to setgid")?;
+    setresuid(uid, uid, Uid::from(0)).context("Failed to setuid")?;
 
     let path = tempfile::Builder::new()
         .prefix(&format!("muvm-run-{uid}-"))
diff --git a/crates/muvm/src/launch.rs b/crates/muvm/src/launch.rs
@@ -86,6 +86,7 @@ fn acquire_socket_lock() -> Result<(File, u32)> {
     Err(anyhow!("Ran out of ports."))
 }
 
+#[allow(clippy::too_many_arguments)]
 fn wrapped_launch(
     server_port: u32,
     cookie: Uuid,
@@ -94,9 +95,19 @@ fn wrapped_launch(
     env: HashMap<String, String>,
     interactive: bool,
     tty: bool,
+    privileged: bool,
 ) -> Result<ExitCode> {
     if !interactive {
-        request_launch(server_port, cookie, command, command_args, env, 0, false)?;
+        request_launch(
+            server_port,
+            cookie,
+            command,
+            command_args,
+            env,
+            0,
+            false,
+            privileged,
+        )?;
         return Ok(ExitCode::from(0));
     }
     let run_path = env::var("XDG_RUNTIME_DIR")
@@ -122,6 +133,7 @@ fn wrapped_launch(
         env,
         vsock_port,
         tty,
+        privileged,
     )?;
     let code = run_io_host(listener, tty)?;
     drop(raw_tty);
@@ -135,13 +147,23 @@ pub fn launch_or_lock(
     env: Vec<(String, Option<String>)>,
     interactive: bool,
     tty: bool,
+    privileged: bool,
 ) -> Result<LaunchResult> {
     let running_server_port = env::var("MUVM_SERVER_PORT").ok();
     if let Some(port) = running_server_port {
         let port: u32 = port.parse()?;
         let env = prepare_env_vars(env)?;
         let cookie = read_cookie()?;
-        return match wrapped_launch(port, cookie, command, command_args, env, interactive, tty) {
+        return match wrapped_launch(
+            port,
+            cookie,
+            command,
+            command_args,
+            env,
+            interactive,
+            tty,
+            privileged,
+        ) {
             Err(err) => Err(anyhow!("could not request launch to server: {err}")),
             Ok(code) => Ok(LaunchResult::LaunchRequested(code)),
         };
@@ -168,6 +190,7 @@ pub fn launch_or_lock(
                     env.clone(),
                     interactive,
                     tty,
+                    privileged,
                 ) {
                     Err(err) => match err.downcast_ref::<LaunchError>() {
                         Some(&LaunchError::Connection(_)) => {
@@ -230,6 +253,7 @@ fn lock_file() -> Result<(Option<File>, Uuid)> {
     Ok((Some(lock_file), cookie))
 }
 
+#[allow(clippy::too_many_arguments)]
 pub fn request_launch(
     server_port: u32,
     cookie: Uuid,
@@ -238,6 +262,7 @@ pub fn request_launch(
     env: HashMap<String, String>,
     vsock_port: u32,
     tty: bool,
+    privileged: bool,
 ) -> Result<()> {
     let mut stream =
         TcpStream::connect(format!("127.0.0.1:{server_port}")).map_err(LaunchError::Connection)?;
@@ -249,6 +274,7 @@ pub fn request_launch(
         env,
         vsock_port,
         tty,
+        privileged,
     };
 
     stream
diff --git a/crates/muvm/src/monitor.rs b/crates/muvm/src/monitor.rs
@@ -44,7 +44,16 @@ fn set_guest_pressure(server_port: u32, cookie: Uuid, pressure: GuestPressure) -
         let command = PathBuf::from("/muvmdropcaches");
         let command_args = vec![];
         let env = HashMap::new();
-        request_launch(server_port, cookie, command, command_args, env, 0, false)?;
+        request_launch(
+            server_port,
+            cookie,
+            command,
+            command_args,
+            env,
+            0,
+            false,
+            true,
+        )?;
     }
 
     let wsf: u32 = pressure.into();
@@ -53,7 +62,16 @@ fn set_guest_pressure(server_port: u32, cookie: Uuid, pressure: GuestPressure) -
     let command = PathBuf::from("/sbin/sysctl");
     let command_args = vec![format!("vm.watermark_scale_factor={}", wsf)];
     let env = HashMap::new();
-    request_launch(server_port, cookie, command, command_args, env, 0, false)
+    request_launch(
+        server_port,
+        cookie,
+        command,
+        command_args,
+        env,
+        0,
+        false,
+        true,
+    )
 }
 
 fn run(server_port: u32, cookie: Uuid) {
diff --git a/crates/muvm/src/net.rs b/crates/muvm/src/net.rs
@@ -14,7 +14,7 @@ where
     Ok(UnixStream::connect(passt_socket_path)?)
 }
 
-pub fn start_passt(server_port: u32, root_server_port: u32) -> Result<UnixStream> {
+pub fn start_passt(server_port: u32) -> Result<UnixStream> {
     // SAFETY: The child process should not inherit the file descriptor of
     // `parent_socket`. There is no documented guarantee of this, but the
     // implementation as of writing atomically sets `SOCK_CLOEXEC`.
@@ -40,9 +40,7 @@ pub fn start_passt(server_port: u32, root_server_port: u32) -> Result<UnixStream
     // See https://doc.rust-lang.org/std/io/index.html#io-safety
     let child = Command::new("passt")
         .args(["-q", "-f", "-t"])
-        .arg(format!(
-            "{server_port}:{server_port},{root_server_port}:{root_server_port}"
-        ))
+        .arg(format!("{server_port}:{server_port}"))
         .arg("--fd")
         .arg(format!("{}", child_fd.into_raw_fd()))
         .spawn();
diff --git a/crates/muvm/src/server/bin/muvm-server.rs b/crates/muvm/src/server/bin/muvm-server.rs
@@ -1,12 +1,11 @@
 use std::env;
 use std::os::unix::process::ExitStatusExt as _;
-use std::path::PathBuf;
 
 use anyhow::{Context, Result};
 use log::error;
 use muvm::server::cli_options::options;
 use muvm::server::worker::{State, Worker};
-use nix::unistd::geteuid;
+use nix::unistd::{getgid, getuid, setresgid, setresuid, Gid, Uid};
 use tokio::net::TcpListener;
 use tokio::process::Command;
 use tokio::sync::watch;
@@ -26,32 +25,23 @@ async fn tokio_main(cookie: String) -> Result<()> {
     env_logger::init();
 
     let cookie = Uuid::try_parse(&cookie).context("Couldn't parse cookie as UUID v7")?;
-    let uid: u32 = geteuid().into();
+    let uid = getuid();
+    let gid = getgid();
+    setresuid(uid, uid, Uid::from(0))?;
+    setresgid(gid, gid, Gid::from(0))?;
 
-    let (server_port, command, command_args) = if uid == 0 {
-        let server_port = if let Ok(server_port) = env::var("MUVM_ROOT_SERVER_PORT") {
-            server_port.parse()?
-        } else {
-            3335
-        };
-        (
-            server_port,
-            PathBuf::from("/bin/sleep"),
-            vec!["inf".to_string()],
-        )
-    } else {
-        let options = options().run();
-        (options.server_port, options.command, options.command_args)
-    };
+    let options = options().run();
 
-    let listener = TcpListener::bind(format!("0.0.0.0:{}", server_port)).await?;
+    let listener = TcpListener::bind(format!("0.0.0.0:{}", options.server_port)).await?;
     let (state_tx, state_rx) = watch::channel(State::new());
 
     let mut worker_handle = tokio::spawn(async move {
         let mut worker = Worker::new(cookie, listener, state_tx);
         worker.run().await;
     });
-    let command_status = Command::new(&command).args(command_args).status();
+    let command_status = Command::new(&options.command)
+        .args(options.command_args)
+        .status();
     tokio::pin!(command_status);
     let mut state_rx = WatchStream::new(state_rx);
 
@@ -79,12 +69,12 @@ async fn tokio_main(cookie: String) -> Result<()> {
                             if let Some(code) = status.code() {
                                 eprintln!(
                                     "{:?} process exited with status code: {code}",
-                                    command
+                                    options.command
                                 );
                             } else {
                                 eprintln!(
                                     "{:?} process terminated by signal: {}",
-                                    command,
+                                    options.command,
                                     status
                                         .signal()
                                         .expect("either one of status code or signal should be set")
@@ -95,7 +85,7 @@ async fn tokio_main(cookie: String) -> Result<()> {
                     Err(err) => {
                         eprintln!(
                             "Failed to execute {:?} as child process: {err}",
-                            command
+                            options.command
                         );
                     },
                 }
diff --git a/crates/muvm/src/server/worker.rs b/crates/muvm/src/server/worker.rs
diff --git a/crates/muvm/src/utils/launch.rs b/crates/muvm/src/utils/launch.rs
