From cc30d58c18353905154173bab850d3610c7d01bc Mon Sep 17 00:00:00 2001 From: Zach Reizner Date: Tue, 23 Jan 2018 21:16:42 -0800 Subject: [PATCH] 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 Tested-by: Zach Reizner Reviewed-by: Dylan Reid --- src/main.rs | 11 +++++- src/plugin/mod.rs | 90 +++++++++++++++++++++++++++++++++++++++++-- src/plugin/process.rs | 66 ++++++++++++++++++++----------- tests/plugin.policy | 47 ++++++++++++++++++++++ tests/plugins.rs | 48 +++++++++++++++-------- 5 files changed, 219 insertions(+), 43 deletions(-) create mode 100644 tests/plugin.policy diff --git a/src/main.rs b/src/main.rs index 27fedfde62..7e2d10380b 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 4d4805d729..31ff8a0cc6 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 { + // 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 fb9ad7dc2d..5fd31b67fe 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, objects: HashMap, shared_vcpu_state: Arc>, @@ -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 { + /// + /// 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) -> Result { 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>> = 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 { - 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 0000000000..960c8e5e7a --- /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 349634dfd6..94b07672a1 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::()); 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]