Protect muvm-server with a cookie

While our threat model targets single user systems, let's at least
protect muvm-server with an authentication cookie, so only the user
creating the microVM (and root) can access it.

The cookie is stored in the lockfile, instead of the server port (this
was useless in practice and generated confusion as could override the
command line). The lockfile is located in XDG_RUNTIME_DIR, which in
properly configured systems is only accessible to its owner.

Signed-off-by: Sergio Lopez <[email protected]>

Author: slp

Cargo.lock

@@ -22,7 +22,7 @@ serde_json = { version = "1.0.117", default-features = false, features = ["std"]
 tempfile = { version = "3.10.1", default-features = false, features = [] }
 tokio = { version = "1.38.0", default-features = false, features = ["io-util", "macros", "net", "process", "rt-multi-thread", "sync"] }
 tokio-stream = { version = "0.1.15", default-features = false, features = ["net", "sync"] }
-uuid = { version = "1.10.0", default-features = false, features = ["std", "v7"] }
+uuid = { version = "1.10.0", default-features = false, features = ["serde", "std", "v7"] }
 
 [features]
 default = []

crates/muvm/Cargo.toml

@@ -67,7 +67,7 @@ fn main() -> Result<()> {
 
     let options = options().fallback_to_usage().run();
 
-    let (_lock_file, command, command_args, env) = match launch_or_lock(
+    let (cookie, _lock_file, command, command_args, env) = match launch_or_lock(
         options.server_port,
         options.command,
         options.command_args,
@@ -79,11 +79,12 @@ fn main() -> Result<()> {
             return Ok(());
         },
         LaunchResult::LockAcquired {
+            cookie,
             lock_file,
             command,
             command_args,
             env,
-        } => (lock_file, command, command_args, env),
+        } => (cookie, lock_file, command, command_args, env),
     };
 
     {
@@ -374,6 +375,7 @@ fn main() -> Result<()> {
         "MUVM_SERVER_PORT".to_owned(),
         options.server_port.to_string(),
     );
+    env.insert("MUVM_SERVER_COOKIE".to_owned(), cookie.to_string());
 
     if options.direct_x11 {
         let display =

crates/muvm/src/bin/muvm.rs

@@ -2,21 +2,22 @@ use std::collections::HashMap;
 use std::env;
 use std::error::Error;
 use std::fmt::{Display, Formatter};
-use std::fs::File;
+use std::fs::{self, File};
 use std::io::{BufRead, BufReader, Read, Write};
 use std::net::TcpStream;
 use std::path::{Path, PathBuf};
 
 use anyhow::{anyhow, Context, Result};
 use rustix::fs::{flock, FlockOperation};
-use rustix::path::Arg;
+use uuid::Uuid;
 
 use crate::env::prepare_env_vars;
 use crate::utils::launch::Launch;
 
 pub enum LaunchResult {
     LaunchRequested,
     LockAcquired {
+        cookie: Uuid,
         lock_file: File,
         command: PathBuf,
         command_args: Vec<String>,
@@ -59,53 +60,63 @@ pub fn launch_or_lock(
     if let Some(port) = running_server_port {
         let port: u32 = port.parse()?;
         let env = prepare_env_vars(env)?;
-        if let Err(err) = request_launch(port, command, command_args, env) {
+        let cookie = read_cookie()?;
+        if let Err(err) = request_launch(port, cookie, command, command_args, env) {
             return Err(anyhow!("could not request launch to server: {err}"));
         }
         return Ok(LaunchResult::LaunchRequested);
     }
 
-    let (lock_file, running_server_port) = lock_file(server_port)?;
+    let (lock_file, cookie) = lock_file()?;
     match lock_file {
         Some(lock_file) => Ok(LaunchResult::LockAcquired {
+            cookie,
             lock_file,
             command,
             command_args,
             env,
         }),
         None => {
-            if let Some(port) = running_server_port {
-                let env = prepare_env_vars(env)?;
-                let mut tries = 0;
-                loop {
-                    match request_launch(port, command.clone(), command_args.clone(), env.clone()) {
-                        Err(err) => match err.downcast_ref::<LaunchError>() {
-                            Some(&LaunchError::Connection(_)) => {
-                                if tries == 3 {
-                                    return Err(anyhow!(
-                                        "could not request launch to server: {err}"
-                                    ));
-                                } else {
-                                    tries += 1;
-                                }
-                            },
-                            _ => {
+            let env = prepare_env_vars(env)?;
+            let mut tries = 0;
+            loop {
+                match request_launch(
+                    server_port,
+                    cookie,
+                    command.clone(),
+                    command_args.clone(),
+                    env.clone(),
+                ) {
+                    Err(err) => match err.downcast_ref::<LaunchError>() {
+                        Some(&LaunchError::Connection(_)) => {
+                            if tries == 3 {
                                 return Err(anyhow!("could not request launch to server: {err}"));
-                            },
+                            } else {
+                                tries += 1;
+                            }
                         },
-                        Ok(_) => return Ok(LaunchResult::LaunchRequested),
-                    }
+                        _ => {
+                            return Err(anyhow!("could not request launch to server: {err}"));
+                        },
+                    },
+                    Ok(_) => return Ok(LaunchResult::LaunchRequested),
                 }
-            } else {
-                Err(anyhow!(
-                    "muvm is already running but couldn't find its server port, bailing out"
-                ))
             }
         },
     }
 }
 
-fn lock_file(server_port: u32) -> Result<(Option<File>, Option<u32>)> {
+fn read_cookie() -> Result<Uuid> {
+    let run_path = env::var("XDG_RUNTIME_DIR")
+        .context("Failed to read XDG_RUNTIME_DIR environment variable")?;
+    let lock_path = Path::new(&run_path).join("muvm.lock");
+    let data: Vec<u8> = fs::read(lock_path).context("Failed to read lock file")?;
+    assert!(data.len() == 16);
+
+    Uuid::from_slice(&data).context("Failed to read cookie from lock file")
+}
+
+fn lock_file() -> Result<(Option<File>, Uuid)> {
     let run_path = env::var("XDG_RUNTIME_DIR")
         .context("Failed to read XDG_RUNTIME_DIR environment variable")?;
     let lock_path = Path::new(&run_path).join("muvm.lock");
@@ -123,30 +134,23 @@ fn lock_file(server_port: u32) -> Result<(Option<File>, Option<u32>)> {
             .context("Failed to create lock file")?;
         let ret = flock(&lock_file, FlockOperation::NonBlockingLockExclusive);
         if ret.is_err() {
-            let mut data: Vec<u8> = Vec::with_capacity(5);
+            let mut data: Vec<u8> = Vec::with_capacity(16);
             lock_file.read_to_end(&mut data)?;
-            let port = match data.to_string_lossy().parse::<u32>() {
-                Ok(port) => {
-                    if port > 1024 {
-                        Some(port)
-                    } else {
-                        None
-                    }
-                },
-                Err(_) => None,
-            };
-            return Ok((None, port));
+            let cookie = Uuid::from_slice(&data).context("Failed to read cookie from lock file")?;
+            return Ok((None, cookie));
         }
         lock_file
     };
 
+    let cookie = Uuid::now_v7();
     lock_file.set_len(0)?;
-    lock_file.write_all(format!("{server_port}").as_bytes())?;
-    Ok((Some(lock_file), None))
+    lock_file.write_all(cookie.as_bytes())?;
+    Ok((Some(lock_file), cookie))
 }
 
 fn request_launch(
     server_port: u32,
+    cookie: Uuid,
     command: PathBuf,
     command_args: Vec<String>,
     env: HashMap<String, String>,
@@ -155,6 +159,7 @@ fn request_launch(
         TcpStream::connect(format!("127.0.0.1:{server_port}")).map_err(LaunchError::Connection)?;
 
     let launch = Launch {
+        cookie,
         command,
         command_args,
         env,

crates/muvm/src/launch.rs

@@ -1,6 +1,7 @@
+use std::env;
 use std::os::unix::process::ExitStatusExt as _;
 
-use anyhow::Result;
+use anyhow::{Context, Result};
 use log::error;
 use muvm::server::cli_options::options;
 use muvm::server::worker::{State, Worker};
@@ -9,18 +10,27 @@ use tokio::process::Command;
 use tokio::sync::watch;
 use tokio_stream::wrappers::WatchStream;
 use tokio_stream::StreamExt as _;
+use uuid::Uuid;
 
-#[tokio::main]
-async fn main() -> Result<()> {
+fn main() -> Result<()> {
+    let cookie = env::var("MUVM_SERVER_COOKIE")
+        .with_context(|| "Could find server cookie as an environment variable")?;
+
+    let rt = tokio::runtime::Runtime::new().unwrap();
+    rt.block_on(async { tokio_main(cookie).await })
+}
+
+async fn tokio_main(cookie: String) -> Result<()> {
     env_logger::init();
 
     let options = options().run();
+    let cookie = Uuid::try_parse(&cookie).context("Couldn't parse cookie as UUID v7")?;
 
     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(listener, state_tx);
+        let mut worker = Worker::new(cookie, listener, state_tx);
         worker.run().await;
     });
     let command_status = Command::new(&options.command)

crates/muvm/src/server/bin/muvm-server.rs

@@ -13,12 +13,14 @@ use tokio::sync::watch;
 use tokio::task::{JoinError, JoinSet};
 use tokio_stream::wrappers::TcpListenerStream;
 use tokio_stream::StreamExt as _;
+use uuid::Uuid;
 
 use crate::utils::launch::Launch;
 use crate::utils::stdio::make_stdout_stderr;
 
 #[derive(Debug)]
 pub struct Worker {
+    cookie: Uuid,
     listener_stream: TcpListenerStream,
     state_tx: watch::Sender<State>,
     child_set: JoinSet<(PathBuf, ChildResult)>,
@@ -33,8 +35,9 @@ pub struct State {
 type ChildResult = Result<ExitStatus, io::Error>;
 
 impl Worker {
-    pub fn new(listener: TcpListener, state_tx: watch::Sender<State>) -> Self {
+    pub fn new(cookie: Uuid, listener: TcpListener, state_tx: watch::Sender<State>) -> Self {
         Worker {
+            cookie,
             listener_stream: TcpListenerStream::new(listener),
             state_tx,
             child_set: JoinSet::new(),
@@ -56,7 +59,7 @@ impl Worker {
                     };
                     let stream = BufStream::new(stream);
 
-                    match handle_connection(stream).await {
+                    match handle_connection(self.cookie, stream).await {
                         Ok((command, mut child)) => {
                             self.child_set.spawn(async move { (command, child.wait().await) });
                             self.set_child_processes(self.child_set.len());
@@ -158,15 +161,27 @@ async fn read_request(stream: &mut BufStream<TcpStream>) -> Result<Launch> {
     }
 }
 
-async fn handle_connection(mut stream: BufStream<TcpStream>) -> Result<(PathBuf, Child)> {
+async fn handle_connection(
+    server_cookie: Uuid,
+    mut stream: BufStream<TcpStream>,
+) -> Result<(PathBuf, Child)> {
     let mut envs: HashMap<String, String> = env::vars().collect();
 
     let Launch {
+        cookie,
         command,
         command_args,
         env,
     } = read_request(&mut stream).await?;
     debug!(command:?, command_args:?, env:?; "received launch request");
+    if cookie != server_cookie {
+        debug!("invalid cookie in launch request");
+        let msg = "Invalid cookie";
+        stream.write_all(msg.as_bytes()).await.ok();
+        stream.flush().await.ok();
+        return Err(anyhow!(msg));
+    }
+
     envs.extend(env);
 
     let (stdout, stderr) = make_stdout_stderr(&command, &envs)?;

crates/muvm/src/server/worker.rs

@@ -2,9 +2,11 @@ use std::collections::HashMap;
 use std::path::PathBuf;
 
 use serde::{Deserialize, Serialize};
+use uuid::Uuid;
 
 #[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
 pub struct Launch {
+    pub cookie: Uuid,
     pub command: PathBuf,
     pub command_args: Vec<String>,
     pub env: HashMap<String, String>,