Spawn test processes as an unprivileged user by default

Fixes an issue where the connection-checker is allowed to leak traffic
on macOS

6 files changed, 106 insertions, 7 deletions

diff --git a/test/scripts/ssh-setup.sh b/test/scripts/ssh-setup.sh
index 714756f45e..5ac5dea15e 100644
--- a/test/scripts/ssh-setup.sh
+++ b/test/scripts/ssh-setup.sh
@@ -9,6 +9,7 @@ RUNNER_DIR="$1"
 APP_PACKAGE="$2"
 PREVIOUS_APP="$3"
 UI_RUNNER="$4"
+UNPRIVILEGED_USER="$5"
 
 # Copy over test runner to correct place
 
@@ -21,6 +22,9 @@ for file in test-runner connection-checker $APP_PACKAGE $PREVIOUS_APP $UI_RUNNER
     cp -f "$SCRIPT_DIR/$file" "$RUNNER_DIR"
 done
 
+# Unprivileged users need execute rights for connection checker
+chmod 551 "${RUNNER_DIR}/connection-checker"
+
 chown -R root "$RUNNER_DIR/"
 
 # Create service
@@ -69,11 +73,18 @@ function setup_macos {
 </plist>
 EOF
 
+    create_test_user_macos
+
     echo "Starting test runner service"
 
     launchctl load -w $RUNNER_PLIST_PATH
 }
 
+function create_test_user_macos {
+    echo "Adding test user account"
+    sysadminctl -addUser "$UNPRIVILEGED_USER" -fullName "$UNPRIVILEGED_USER" -password "$UNPRIVILEGED_USER"
+}
+
 function setup_systemd {
     RUNNER_SERVICE_PATH="/etc/systemd/system/testrunner.service"
 
@@ -94,10 +105,18 @@ EOF
 
     semanage fcontext -a -t bin_t "$RUNNER_DIR/.*" &> /dev/null || true
 
+    create_test_user_linux
+
     systemctl enable testrunner.service
     systemctl start testrunner.service
 }
 
+function create_test_user_linux {
+    echo "Adding test user account"
+    useradd -m "$UNPRIVILEGED_USER"
+    echo "$UNPRIVILEGED_USER:$UNPRIVILEGED_USER" | chpasswd
+}
+
 if [[ "$(uname -s)" == "Darwin" ]]; then
     setup_macos
     exit 0
diff --git a/test/test-manager/src/vm/provision.rs b/test/test-manager/src/vm/provision.rs
index 143f023fa4..359cf16c33 100644
--- a/test/test-manager/src/vm/provision.rs
+++ b/test/test-manager/src/vm/provision.rs
@@ -10,6 +10,7 @@ use std::{
     net::{IpAddr, SocketAddr, TcpStream},
     path::{Path, PathBuf},
 };
+use test_rpc::UNPRIVILEGED_USER;
 
 /// Returns the directory in the test runner where the test-runner binary is installed.
 pub async fn provision(
@@ -156,7 +157,7 @@ fn blocking_ssh(
 
     // Run the setup script in the test runner
     let cmd = format!(
-        r#"sudo {} {remote_dir} "{app_package_path}" "{app_package_to_upgrade_from_path}" "{gui_package_path}""#,
+        r#"sudo {} {remote_dir} "{app_package_path}" "{app_package_to_upgrade_from_path}" "{gui_package_path}" "{UNPRIVILEGED_USER}""#,
         bootstrap_script_dest.display(),
     );
     log::debug!("Running setup script on remote, cmd: {cmd}");
diff --git a/test/test-rpc/src/lib.rs b/test/test-rpc/src/lib.rs
index cc263b845e..1ec8fb7321 100644
--- a/test/test-rpc/src/lib.rs
+++ b/test/test-rpc/src/lib.rs
@@ -13,6 +13,10 @@ pub mod net;
 pub mod package;
 pub mod transport;
 
+/// Unprivileged user. This is used for things like spawning processes.
+/// This is also used as the password for the same user, as is common practice.
+pub const UNPRIVILEGED_USER: &str = "mole";
+
 #[derive(thiserror::Error, Debug, Serialize, Deserialize, PartialEq, Eq)]
 pub enum Error {
     #[error("Test runner RPC failed")]
diff --git a/test/test-runner/Cargo.toml b/test/test-runner/Cargo.toml
index b16a8c23b2..7206cb394a 100644
--- a/test/test-runner/Cargo.toml
+++ b/test/test-runner/Cargo.toml
@@ -58,7 +58,7 @@ features = ["codec"]
 default-features = false
 
 [target.'cfg(unix)'.dependencies]
-nix = { workspace = true }
+nix = { workspace = true, features = ["user"] }
 
 [target.'cfg(target_os = "linux")'.dependencies]
 rs-release = "0.1.7"
diff --git a/test/test-runner/src/main.rs b/test/test-runner/src/main.rs
index 59fc83672d..79bc17b4ef 100644
--- a/test/test-runner/src/main.rs
+++ b/test/test-runner/src/main.rs
@@ -17,7 +17,7 @@ use test_rpc::{
     net::SockHandleId,
     package::Package,
     transport::GrpcForwarder,
-    AppTrace, Service, SpawnOpts,
+    AppTrace, Service, SpawnOpts, UNPRIVILEGED_USER,
 };
 use tokio::{
     io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader},
@@ -386,11 +386,17 @@ impl Service for TestServer {
         }
 
         cmd.stderr(Stdio::piped());
+        cmd.kill_on_drop(true);
 
-        let mut child = cmd.kill_on_drop(true).spawn().map_err(|error| {
-            log::error!("Failed to spawn {}: {error}", opts.path);
-            test_rpc::Error::Syscall
-        })?;
+        let mut child = util::as_unprivileged_user(UNPRIVILEGED_USER, || cmd.spawn())
+            .map_err(|error| {
+                log::error!("Failed to drop privileges: {error}");
+                test_rpc::Error::Syscall
+            })?
+            .map_err(|error| {
+                log::error!("Failed to spawn {}: {error}", opts.path);
+                test_rpc::Error::Syscall
+            })?;
 
         let pid = child
             .id()
diff --git a/test/test-runner/src/util.rs b/test/test-runner/src/util.rs
index 03a3343214..84cc048b49 100644
--- a/test/test-runner/src/util.rs
+++ b/test/test-runner/src/util.rs
@@ -21,3 +21,72 @@ impl<F: FnOnce() + Send> OnDrop<F> {
         }
     }
 }
+
+#[derive(thiserror::Error, Debug)]
+#[error(transparent)]
+pub struct Error {
+    inner: InnerError,
+}
+
+#[cfg(unix)]
+#[derive(thiserror::Error, Debug)]
+enum InnerError {
+    #[error("Failed to get the specified user")]
+    GetUser(#[source] nix::Error),
+    #[error("The specified user was not found")]
+    MissingUser,
+    #[error("Failed to set uid")]
+    SetUid(#[source] nix::Error),
+    #[error("Failed to set gid")]
+    SetGid(#[source] nix::Error),
+}
+
+#[cfg(target_os = "windows")]
+#[derive(thiserror::Error, Debug)]
+enum InnerError {}
+
+impl From<InnerError> for Error {
+    fn from(inner: InnerError) -> Self {
+        Self { inner }
+    }
+}
+
+#[cfg(target_os = "windows")]
+pub fn as_unprivileged_user<T>(unpriv_user: &str, func: impl FnOnce() -> T) -> Result<T, Error> {
+    // NOTE: no-op
+    let _ = unpriv_user;
+    Ok(func())
+}
+
+#[cfg(unix)]
+pub fn as_unprivileged_user<T>(unpriv_user: &str, func: impl FnOnce() -> T) -> Result<T, Error> {
+    let original_uid = nix::unistd::getuid();
+    let original_gid = nix::unistd::getgid();
+
+    let user = nix::unistd::User::from_name(unpriv_user)
+        .map_err(InnerError::GetUser)?
+        .ok_or(InnerError::MissingUser)?;
+    let uid = user.uid;
+    let gid = user.gid;
+
+    nix::unistd::setegid(gid).map_err(InnerError::SetGid)?;
+    let restore_gid = OnDrop::new(|| {
+        if let Err(error) = nix::unistd::setegid(original_gid) {
+            log::error!("Failed to restore gid: {error}");
+        }
+    });
+
+    nix::unistd::seteuid(uid).map_err(InnerError::SetUid)?;
+    let restore_uid = OnDrop::new(|| {
+        if let Err(error) = nix::unistd::seteuid(original_uid) {
+            log::error!("Failed to restore uid: {error}");
+        }
+    });
+
+    let result = Ok(func());
+
+    drop(restore_uid);
+    drop(restore_gid);
+
+    result
+}