crosvm: run plugin process in a jail by default

The plugin process is similar to a virtual device from the perspective
of crosvm. Therefore, the plugin process should be run in a jail,
similar to the other devices in crosvm.

TEST=cargo build --features plugin; ./build_test
BUG=chromium:800626

Change-Id: I881d7b0f8a11e2626f69a5fa0eee0aa59bb6b6be
Reviewed-on: https://chromium-review.googlesource.com/882131
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Dylan Reid <dgreid@chromium.org>

5 files changed, 219 insertions, 43 deletions

diff --git a/src/main.rs b/src/main.rs
index 27fedfd..7e2d103 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -307,7 +307,14 @@ fn set_argument(cfg: &mut Config, name: &str, value: Option<&str>) -> argument::
             } else if cfg.plugin.is_some() {
                 return Err(argument::Error::TooManyArguments("`plugin` already given".to_owned()));
             }
-            cfg.plugin = Some(PathBuf::from(value.unwrap().to_owned()));
+            let plugin = PathBuf::from(value.unwrap().to_owned());
+            if plugin.is_relative() {
+                return Err(argument::Error::InvalidValue {
+                  value: plugin.to_string_lossy().into_owned(),
+                  expected: "the plugin path must be an absolute path",
+                })
+            }
+            cfg.plugin = Some(plugin);
         }
         "help" => return Err(argument::Error::PrintHelp),
         _ => unreachable!(),
@@ -354,7 +361,7 @@ fn run_vm(args: std::env::Args) -> i32 {
           Argument::value("cid", "CID", "Context ID for virtual sockets"),
           Argument::value("seccomp-policy-dir", "PATH", "Path to seccomp .policy files."),
           #[cfg(feature = "plugin")]
-          Argument::value("plugin", "PATH", "Path to plugin process to run under crosvm."),
+          Argument::value("plugin", "PATH", "Absolute path to plugin process to run under crosvm."),
           Argument::short_flag('h', "help", "Print help message.")];
 
     let mut cfg = Config::default();
diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs
index 4d4805d..31ff8a0 100644
--- a/src/plugin/mod.rs
+++ b/src/plugin/mod.rs
@@ -10,6 +10,7 @@ use std::fs::File;
 use std::io;
 use std::os::unix::io::{IntoRawFd, FromRawFd};
 use std::os::unix::net::UnixDatagram;
+use std::path::Path;
 use std::result;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Barrier};
@@ -17,13 +18,15 @@ use std::thread;
 use std::time::{Duration, Instant};
 
 use libc::{socketpair, ioctl, c_ulong, AF_UNIX, SOCK_SEQPACKET, FIOCLEX, EAGAIN, EINTR, EINVAL,
-           ENOENT, EPERM, EDEADLK, ENOTTY, EEXIST, EBADF, EOVERFLOW, SIGCHLD};
+           ENOENT, EPERM, EDEADLK, ENOTTY, EEXIST, EBADF, EOVERFLOW, SIGCHLD, MS_NOSUID, MS_NODEV};
 
 use protobuf::ProtobufError;
 
+use io_jail::{self, Minijail};
 use kvm::{Kvm, Vm, Vcpu, VcpuExit, IoeventAddress, NoDatamatch};
 use sys_util::{EventFd, MmapError, Killable, SignalFd, SignalFdError, Poller, Pollable,
-               GuestMemory, Result as SysResult, Error as SysError, register_signal_handler};
+               GuestMemory, Result as SysResult, Error as SysError, register_signal_handler,
+               geteuid, getegid};
 
 use Config;
 
@@ -39,6 +42,7 @@ pub enum Error {
     CloneVcpuSocket(io::Error),
     CreateEventFd(SysError),
     CreateIrqChip(SysError),
+    CreateJail(io_jail::Error),
     CreateKvm(SysError),
     CreateMainSocket(SysError),
     CreateSignalFd(SignalFdError),
@@ -48,9 +52,18 @@ pub enum Error {
     CreateVm(SysError),
     DecodeRequest(ProtobufError),
     EncodeResponse(ProtobufError),
+    MountLib(io_jail::Error),
+    MountLib64(io_jail::Error),
+    MountPlugin(io_jail::Error),
+    MountPluginLib(io_jail::Error),
+    MountRoot(io_jail::Error),
+    NoVarEmpty,
+    ParsePivotRoot(io_jail::Error),
+    ParseSeccomp(io_jail::Error),
     PluginFailed(i32),
     PluginKill(SysError),
     PluginKilled(i32),
+    PluginRunJail(io_jail::Error),
     PluginSocketHup,
     PluginSocketPoll(SysError),
     PluginSocketRecv(SysError),
@@ -59,6 +72,8 @@ pub enum Error {
     PluginTimeout,
     PluginWait(SysError),
     Poll(SysError),
+    SetGidMap(io_jail::Error),
+    SetUidMap(io_jail::Error),
     SigChild {
         pid: u32,
         signo: u32,
@@ -76,6 +91,7 @@ impl fmt::Display for Error {
             Error::CloneVcpuSocket(ref e) => write!(f, "failed to clone vcpu socket: {:?}", e),
             Error::CreateEventFd(ref e) => write!(f, "failed to create eventfd: {:?}", e),
             Error::CreateIrqChip(ref e) => write!(f, "failed to create kvm irqchip: {:?}", e),
+            Error::CreateJail(ref e) => write!(f, "failed to create jail: {}", e),
             Error::CreateKvm(ref e) => write!(f, "error creating Kvm: {:?}", e),
             Error::CreateMainSocket(ref e) => {
                 write!(f, "error creating main request socket: {:?}", e)
@@ -89,9 +105,18 @@ impl fmt::Display for Error {
             Error::CreateVm(ref e) => write!(f, "error creating vm: {:?}", e),
             Error::DecodeRequest(ref e) => write!(f, "failed to decode plugin request: {}", e),
             Error::EncodeResponse(ref e) => write!(f, "failed to encode plugin response: {}", e),
+            Error::MountLib(ref e) => write!(f, "failed to mount: {}", e),
+            Error::MountLib64(ref e) => write!(f, "failed to mount: {}", e),
+            Error::MountPlugin(ref e) => write!(f, "failed to mount: {}", e),
+            Error::MountPluginLib(ref e) => write!(f, "failed to mount: {}", e),
+            Error::MountRoot(ref e) => write!(f, "failed to mount: {}", e),
+            Error::NoVarEmpty => write!(f, "no /var/empty for jailed process to pivot root into"),
+            Error::ParsePivotRoot(ref e) => write!(f, "failed to set jail pivot root: {}", e),
+            Error::ParseSeccomp(ref e) => write!(f, "failed to parse jail seccomp filter: {}", e),
             Error::PluginFailed(ref e) => write!(f, "plugin exited with error: {}", e),
             Error::PluginKill(ref e) => write!(f, "error sending kill signal to plugin: {:?}", e),
             Error::PluginKilled(ref e) => write!(f, "plugin exited with signal {}", e),
+            Error::PluginRunJail(ref e) => write!(f, "failed to run jail: {}", e),
             Error::PluginSocketHup => write!(f, "plugin request socket has been hung up"),
             Error::PluginSocketPoll(ref e) => {
                 write!(f, "failed to poll plugin request sockets: {:?}", e)
@@ -106,6 +131,8 @@ impl fmt::Display for Error {
             Error::PluginTimeout => write!(f, "plugin did not exit within timeout"),
             Error::PluginWait(ref e) => write!(f, "error waiting for plugin to exit: {:?}", e),
             Error::Poll(ref e) => write!(f, "failed to poll all FDs: {:?}", e),
+            Error::SetGidMap(ref e) => write!(f, "failed to set gidmap for jail: {}", e),
+            Error::SetUidMap(ref e) => write!(f, "failed to set uidmap for jail: {}", e),
             Error::SigChild {
                 pid,
                 signo,
@@ -163,6 +190,46 @@ fn mmap_to_sys_err(e: MmapError) -> SysError {
     }
 }
 
+fn create_plugin_jail(root: &Path, seccomp_policy: &Path) -> Result<Minijail> {
+    // All child jails run in a new user namespace without any users mapped,
+    // they run as nobody unless otherwise configured.
+    let mut j = Minijail::new().map_err(Error::CreateJail)?;
+    j.namespace_pids();
+    j.namespace_user();
+    j.uidmap(&format!("{0} {0} 1", geteuid()))
+        .map_err(Error::SetUidMap)?;
+    j.gidmap(&format!("{0} {0} 1", getegid()))
+        .map_err(Error::SetGidMap)?;
+    j.namespace_user_disable_setgroups();
+    // Don't need any capabilities.
+    j.use_caps(0);
+    // Create a new mount namespace with an empty root FS.
+    j.namespace_vfs();
+    j.enter_pivot_root(root).map_err(Error::ParsePivotRoot)?;
+    // Run in an empty network namespace.
+    j.namespace_net();
+    j.no_new_privs();
+    // Use TSYNC only for the side effect of it using SECCOMP_RET_TRAP, which will correctly kill
+    // the entire plugin process if a worker thread commits a seccomp violation.
+    j.set_seccomp_filter_tsync();
+    j.parse_seccomp_filters(seccomp_policy)
+        .map_err(Error::ParseSeccomp)?;
+    j.use_seccomp_filter();
+    // Don't do init setup.
+    j.run_as_init();
+
+    // Create a tmpfs in the plugin's root directory so that we can bind mount it's executable
+    // file into it.  The size=67108864 is size=64*1024*1024 or size=64MB.
+    j.mount_with_data(Path::new("none"),
+                         Path::new("/"),
+                         "tmpfs",
+                         (MS_NOSUID | MS_NODEV) as usize,
+                         "size=67108864")
+        .map_err(Error::MountRoot)?;
+
+    Ok(j)
+}
+
 /// Each `PluginObject` represents one object that was instantiated by the guest using the `Create`
 /// request.
 ///
@@ -228,12 +295,27 @@ pub fn run_config(cfg: Config) -> Result<()> {
     // quickly.
     let sigchld_fd = SignalFd::new(SIGCHLD).map_err(Error::CreateSignalFd)?;
 
+    let jail = if cfg.multiprocess {
+        // An empty directory for jailed plugin pivot root.
+        let empty_root_path = Path::new("/var/empty");
+        if !empty_root_path.exists() {
+            return Err(Error::NoVarEmpty);
+        }
+
+        let policy_path = cfg.seccomp_policy_dir.join("plugin.policy");
+        let jail = create_plugin_jail(empty_root_path, &policy_path)?;
+        Some(jail)
+    } else {
+        None
+    };
+
+    let plugin_path = cfg.plugin.as_ref().unwrap().as_path();
     let vcpu_count = cfg.vcpu_count.unwrap_or(1);
     let mem = GuestMemory::new(&[]).unwrap();
     let kvm = Kvm::new().map_err(Error::CreateKvm)?;
     let mut vm = Vm::new(&kvm, mem).map_err(Error::CreateVm)?;
     vm.create_irq_chip().map_err(Error::CreateIrqChip)?;
-    let mut plugin = Process::new(vcpu_count, &mut vm, &cfg.plugin.unwrap())?;
+    let mut plugin = Process::new(vcpu_count, &mut vm, plugin_path, jail)?;
 
     let exit_evt = EventFd::new().map_err(Error::CreateEventFd)?;
     let kill_signaled = Arc::new(AtomicBool::new(false));
@@ -385,7 +467,7 @@ pub fn run_config(cfg: Config) -> Result<()> {
                             Ok(Some(siginfo)) => {
                                 // If the plugin process has ended, there is no need to continue
                                 // processing plugin connections, so we break early.
-                                if siginfo.ssi_pid == plugin.pid() {
+                                if siginfo.ssi_pid == plugin.pid() as u32 {
                                     break 'poll;
                                 }
                                 // Because SIGCHLD is not expected from anything other than the
diff --git a/src/plugin/process.rs b/src/plugin/process.rs
index fb9ad7d..5fd31b6 100644
--- a/src/plugin/process.rs
+++ b/src/plugin/process.rs
@@ -3,21 +3,22 @@
 // found in the LICENSE file.
 
 use std::collections::hash_map::{HashMap, Entry, VacantEntry};
+use std::env::set_var;
 use std::fs::File;
 use std::net::Shutdown;
 use std::os::unix::io::{AsRawFd, RawFd};
 use std::os::unix::net::UnixDatagram;
-use std::os::unix::process::ExitStatusExt;
 use std::path::Path;
-use std::process::{Command, Child};
+use std::process::Command;
 use std::sync::{Arc, Mutex, RwLock};
 use std::thread::JoinHandle;
 
-use libc::EINVAL;
+use libc::{waitpid, pid_t, EINVAL, WNOHANG, WIFEXITED, WEXITSTATUS, WTERMSIG};
 
 use protobuf;
 use protobuf::Message;
 
+use io_jail::Minijail;
 use kvm::{Vm, IoeventAddress, NoDatamatch, IrqSource, IrqRoute, dirty_log_bitmap_size};
 use sys_util::{EventFd, MemoryMapping, Killable, Scm, Poller, Pollable, SharedMemory,
                GuestAddress, Result as SysResult, Error as SysError};
@@ -44,7 +45,7 @@ pub enum ProcessStatus {
 /// an unprivileged manner as a child process spawned via a path to a arbitrary executable.
 pub struct Process {
     started: bool,
-    plugin_proc: Child,
+    plugin_pid: pid_t,
     request_sockets: Vec<UnixDatagram>,
     objects: HashMap<u32, PluginObject>,
     shared_vcpu_state: Arc<RwLock<SharedVcpuState>>,
@@ -66,7 +67,11 @@ impl Process {
     ///
     /// This will immediately spawn the plugin process and wait for the child to signal that it is
     /// ready to start. This call may block indefinitely.
-    pub fn new(cpu_count: u32, vm: &mut Vm, cmd_path: &Path) -> Result<Process> {
+    ///
+    /// Set the `jail` argument to spawn the plugin process within the preconfigured jail.
+    /// Due to an API limitation in libminijail necessitating that this function set an environment
+    /// variable, this function is not thread-safe.
+    pub fn new(cpu_count: u32, vm: &mut Vm, cmd: &Path, jail: Option<Minijail>) -> Result<Process> {
         let (request_socket, child_socket) = new_seqpacket_pair().map_err(Error::CreateMainSocket)?;
 
         let mut vcpu_sockets: Vec<(UnixDatagram, UnixDatagram)> = Vec::with_capacity(cpu_count as
@@ -77,11 +82,20 @@ impl Process {
         let mut per_vcpu_states: Vec<Arc<Mutex<PerVcpuState>>> = Vec::new();
         per_vcpu_states.resize(cpu_count as usize, Default::default());
 
-        // TODO(zachr): use a minijail
-        let plugin_proc = Command::new(cmd_path)
-            .env("CROSVM_SOCKET", child_socket.as_raw_fd().to_string())
-            .spawn()
-            .map_err(Error::PluginSpawn)?;
+        let plugin_pid = match jail {
+            Some(jail) => {
+                set_var("CROSVM_SOCKET", child_socket.as_raw_fd().to_string());
+                jail.run(cmd, &[0, 1, 2, child_socket.as_raw_fd()], &[])
+                    .map_err(Error::PluginRunJail)?
+            }
+            None => {
+                Command::new(cmd)
+                    .env("CROSVM_SOCKET", child_socket.as_raw_fd().to_string())
+                    .spawn()
+                    .map_err(Error::PluginSpawn)?
+                    .id() as pid_t
+            }
+        };
 
         // Very important to drop the child socket so that the pair will properly hang up if the
         // plugin process exits or closes its end.
@@ -91,7 +105,7 @@ impl Process {
 
         let mut plugin = Process {
             started: false,
-            plugin_proc,
+            plugin_pid,
             request_sockets,
             objects: Default::default(),
             shared_vcpu_state: Default::default(),
@@ -160,8 +174,8 @@ impl Process {
     }
 
     /// Returns the process ID of the plugin process.
-    pub fn pid(&self) -> u32 {
-        self.plugin_proc.id()
+    pub fn pid(&self) -> pid_t {
+        self.plugin_pid
     }
 
     /// Returns a slice of each socket that should be polled.
@@ -211,17 +225,25 @@ impl Process {
 
     /// Waits without blocking for the plugin process to exit and returns the status.
     pub fn try_wait(&mut self) -> SysResult<ProcessStatus> {
-        match self.plugin_proc.try_wait() {
-            Ok(Some(status)) => {
-                match status.code() {
-                    Some(0) => Ok(ProcessStatus::Success),
-                    Some(code) => Ok(ProcessStatus::Fail(code)),
-                    // If there was no exit code there must be a signal.
-                    None => Ok(ProcessStatus::Signal(status.signal().unwrap())),
+        let mut status = 0;
+        // Safe because waitpid is given a valid pointer of correct size and mutability, and the
+        // return value is checked.
+        let ret = unsafe { waitpid(self.plugin_pid, &mut status, WNOHANG) };
+        match ret {
+            -1 => Err(SysError::last()),
+            0 => Ok(ProcessStatus::Running),
+            _ => {
+                // Trivially safe
+                if unsafe { WIFEXITED(status) } {
+                    match unsafe { WEXITSTATUS(status) } { // Trivially safe
+                        0 => Ok(ProcessStatus::Success),
+                        code => Ok(ProcessStatus::Fail(code)),
+                    }
+                } else {
+                    // Plugin terminated but has no exit status, so it must have been signaled.
+                    Ok(ProcessStatus::Signal(unsafe { WTERMSIG(status) })) // Trivially safe
                 }
             }
-            Ok(None) => Ok(ProcessStatus::Running),
-            Err(e) => Err(io_to_sys_err(e)),
         }
     }
 
diff --git a/tests/plugin.policy b/tests/plugin.policy
new file mode 100644
index 0000000..960c8e5
--- /dev/null
+++ b/tests/plugin.policy
@@ -0,0 +1,47 @@
+# Copyright 2017 The Chromium OS Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+close: 1
+dup: 1
+dup2: 1
+execve: 1
+exit_group: 1
+futex: 1
+lseek: 1
+mprotect: 1
+munmap: 1
+read: 1
+recvfrom: 1
+sched_getaffinity: 1
+set_robust_list: 1
+sigaltstack: 1
+# Disallow clone's other than new threads.
+clone: arg0 & 0x00010000
+write: 1
+eventfd2: 1
+poll: 1
+getpid: 1
+# Allow PR_SET_NAME only.
+prctl: arg0 == 15
+access: 1
+arch_prctl: 1
+brk: 1
+exit: 1
+fcntl: 1
+fstat: 1
+ftruncate: 1
+getcwd: 1
+getrlimit: 1
+madvise: 1
+memfd_create: 1
+mmap: 1
+open: 1
+recvmsg: 1
+restart_syscall: 1
+rt_sigaction: 1
+rt_sigprocmask: 1
+sendmsg: 1
+set_tid_address: 1
+stat: 1
+writev: 1
diff --git a/tests/plugins.rs b/tests/plugins.rs
index 349634d..94b0767 100644
--- a/tests/plugins.rs
+++ b/tests/plugins.rs
@@ -26,8 +26,8 @@ impl Drop for RemovePath {
     }
 }
 
-fn get_crosvm_path() -> PathBuf {
-    let mut crosvm_path = current_exe()
+fn get_target_path() -> PathBuf {
+    current_exe()
         .ok()
         .map(|mut path| {
                  path.pop();
@@ -36,24 +36,26 @@ fn get_crosvm_path() -> PathBuf {
                  }
                  path
              })
-        .expect("failed to get crosvm binary directory");
-    crosvm_path.push("crosvm");
-    crosvm_path
+        .expect("failed to get crosvm binary directory")
 }
 
 fn build_plugin(src: &str) -> RemovePath {
     let mut out_bin = PathBuf::from("target");
-    let mut libcrosvm_plugin = get_crosvm_path();
-    libcrosvm_plugin.set_file_name("libcrosvm_plugin.so");
+    let libcrosvm_plugin_dir = get_target_path();
     out_bin.push(thread_rng()
                      .gen_ascii_chars()
                      .take(10)
                      .collect::<String>());
     let mut child = Command::new(var_os("CC").unwrap_or(OsString::from("cc")))
-        .args(&["-Icrosvm_plugin", "-pthread", "-o"])
+        .args(&["-Icrosvm_plugin", "-pthread", "-o"]) // crosvm.h location and set output path.
         .arg(&out_bin)
-        .arg(libcrosvm_plugin)
-        .args(&["-xc", "-"])
+        .arg("-L") // Path of shared object to link to.
+        .arg(&libcrosvm_plugin_dir)
+        .arg("-lcrosvm_plugin")
+        .arg("-Wl,-rpath") // Search for shared object in the same path when exec'd.
+        .arg(&libcrosvm_plugin_dir)
+        .args(&["-Wl,-rpath", "."]) // Also check current directory in case of sandboxing.
+        .args(&["-xc", "-"]) // Read source code from piped stdin.
         .stdin(Stdio::piped())
         .spawn()
         .expect("failed to spawn compiler");
@@ -70,10 +72,24 @@ fn build_plugin(src: &str) -> RemovePath {
     RemovePath(PathBuf::from(out_bin))
 }
 
-fn run_plugin(bin_path: &Path) {
-    let mut child = Command::new(get_crosvm_path())
-        .args(&["run", "-c", "1", "--plugin"])
-        .arg(bin_path)
+fn run_plugin(bin_path: &Path, with_sandbox: bool) {
+    let mut crosvm_path = get_target_path();
+    crosvm_path.push("crosvm");
+    let mut cmd = Command::new(crosvm_path);
+    cmd.args(&["run",
+                "-c",
+                "1",
+                "--seccomp-policy-dir",
+                "tests",
+                "--plugin"])
+        .arg(bin_path
+                 .canonicalize()
+                 .expect("failed to canonicalize plugin path"));
+    if !with_sandbox {
+        cmd.arg("--disable-sandbox");
+    }
+
+    let mut child = cmd
         .spawn()
         .expect("failed to spawn crosvm");
     for _ in 0..12 {
@@ -91,7 +107,9 @@ fn run_plugin(bin_path: &Path) {
 
 fn test_plugin(src: &str) {
     let bin_path = build_plugin(src);
-    run_plugin(&bin_path.0);
+    // Run with and without the sandbox enabled.
+    run_plugin(&bin_path.0, false);
+    run_plugin(&bin_path.0, true);
 }
 
 #[test]