From 3a55e2737c71a9854e6868f4fe3da4fb057c7f45 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 19:28:56 +1300 Subject: [PATCH 01/10] Avoid printing confusing panic message when the test succeeds --- tests/file-read-all.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/file-read-all.rs b/tests/file-read-all.rs index 269441f..dfb558f 100644 --- a/tests/file-read-all.rs +++ b/tests/file-read-all.rs @@ -7,7 +7,7 @@ extern crate rand; use gaol::profile::{Operation, PathPattern, Profile}; use gaol::sandbox::{ChildSandbox, ChildSandboxMethods, Command, Sandbox, SandboxMethods}; -use libc::c_char; +use libc::{c_char, exit}; use rand::Rng; use std::env; use std::ffi::{CString, OsStr}; @@ -15,6 +15,7 @@ use std::fs::File; use std::io::Write; use std::os::unix::prelude::OsStrExt; use std::path::PathBuf; +use std::process; // A conservative overapproximation of `PATH_MAX` on all platforms. const PATH_MAX: usize = 4096; @@ -40,7 +41,9 @@ fn allowance_test() { fn prohibition_test() { let path = PathBuf::from(env::var("GAOL_TEMP_FILE").unwrap()); ChildSandbox::new(prohibition_profile()).activate().unwrap(); - drop(File::open(&path).unwrap()) + if File::open(&path).is_err() { + process::exit(1); + } } pub fn main() { From f3cede2010b9e8cba302c02bfc81bcba0de8b746 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 20:22:29 +1300 Subject: [PATCH 02/10] Fix bug when launching multiple children Parallelizing the forbidden_syscalls test exposes the bug and also reduces test time on my 4-core Skylake laptop: Before: [roc@glory gaol]$ time target/debug/deps/forbidden_syscalls-6fdc4dd40a646c6f real 0m41.322s user 0m0.590s sys 0m1.452s After: [roc@glory gaol]$ time target/debug/deps/forbidden_syscalls-6fdc4dd40a646c6f real 0m14.915s user 0m0.248s sys 0m0.980s --- Cargo.toml | 3 +++ platform/unix/process.rs | 2 +- tests/forbidden-syscalls.rs | 33 +++++++++++++++++++++++---------- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index c86ce29..35936c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,9 @@ libc = "0.2" log = "0.3" rand = "0.3" +[dev-dependencies] +num_cpus = "1" + [[test]] name = "file-read-all" path = "tests/file-read-all.rs" diff --git a/platform/unix/process.rs b/platform/unix/process.rs index 21e69d3..c06752a 100644 --- a/platform/unix/process.rs +++ b/platform/unix/process.rs @@ -68,7 +68,7 @@ impl Process { let mut stat = 0; loop { let pid = unsafe { - waitpid(-1, &mut stat, 0) + waitpid(self.pid, &mut stat, 0) }; if pid < 0 { return Err(io::Error::last_os_error()) diff --git a/tests/forbidden-syscalls.rs b/tests/forbidden-syscalls.rs index 84bba3a..dabef5e 100644 --- a/tests/forbidden-syscalls.rs +++ b/tests/forbidden-syscalls.rs @@ -3,11 +3,13 @@ extern crate gaol; extern crate libc; +extern crate num_cpus; use gaol::profile::Profile; use gaol::sandbox::{ChildSandbox, ChildSandboxMethods, Command, Sandbox, SandboxMethods}; use libc::c_int; use std::env; +use std::thread; #[cfg(target_os="linux")] use gaol::platform::linux::seccomp::ALLOWED_SYSCALLS; @@ -32,16 +34,27 @@ pub fn main() { return test_syscall(arg.parse().unwrap()) } - for syscall in 0..MAX_SYSCALL { - if ALLOWED_SYSCALLS.iter().any(|number| *number == syscall) { - continue - } - let arg = format!("{}", syscall); - let status = Sandbox::new(profile()).start(&mut Command::me().unwrap().arg(&arg[..])) - .unwrap() - .wait() - .unwrap(); - assert!(!status.success()); + let num_cpus = num_cpus::get() as u32; + let handles = (0..num_cpus).into_iter().map(|index| { + thread::spawn(move || { + for syscall in 0..MAX_SYSCALL { + if (syscall % num_cpus) != index { + continue + } + if ALLOWED_SYSCALLS.iter().any(|number| *number == syscall) { + continue + } + let arg = format!("{}", syscall); + let status = Sandbox::new(profile()).start(&mut Command::me().unwrap().arg(&arg[..])) + .unwrap() + .wait() + .unwrap(); + assert!(!status.success()); + } + }) + }).collect::>(); + for h in handles { + h.join().unwrap(); } } From adcde280fce40f6d8c9b4857780426764c7def27 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 17:19:29 +1300 Subject: [PATCH 03/10] Add support for allowing new process creation --- platform/linux/mod.rs | 3 ++- platform/linux/namespace.rs | 1 + platform/linux/seccomp.rs | 37 ++++++++++++++++++++++++++++++++++--- platform/macos/mod.rs | 6 ++++++ profile.rs | 4 ++-- 5 files changed, 45 insertions(+), 6 deletions(-) diff --git a/platform/linux/mod.rs b/platform/linux/mod.rs index d6f5016..64e369a 100644 --- a/platform/linux/mod.rs +++ b/platform/linux/mod.rs @@ -27,7 +27,8 @@ impl OperationSupport for profile::Operation { fn support(&self) -> OperationSupportLevel { match *self { profile::Operation::FileReadAll(_) | - profile::Operation::NetworkOutbound(AddressPattern::All) => { + profile::Operation::NetworkOutbound(AddressPattern::All) | + profile::Operation::CreateNewProcesses => { OperationSupportLevel::CanBeAllowed } profile::Operation::FileReadMetadata(_) | diff --git a/platform/linux/namespace.rs b/platform/linux/namespace.rs index c5ebe89..30ae298 100644 --- a/platform/linux/namespace.rs +++ b/platform/linux/namespace.rs @@ -282,6 +282,7 @@ pub const CLONE_VM: c_int = 0x0000_0100; pub const CLONE_FS: c_int = 0x0000_0200; pub const CLONE_FILES: c_int = 0x0000_0400; pub const CLONE_SIGHAND: c_int = 0x0000_0800; +pub const CLONE_VFORK: c_int = 0x0000_4000; pub const CLONE_THREAD: c_int = 0x0001_0000; pub const CLONE_NEWNS: c_int = 0x0002_0000; pub const CLONE_SYSVSEM: c_int = 0x0004_0000; diff --git a/platform/linux/seccomp.rs b/platform/linux/seccomp.rs index 048d018..854fab8 100644 --- a/platform/linux/seccomp.rs +++ b/platform/linux/seccomp.rs @@ -18,13 +18,14 @@ use platform::linux::namespace::{CLONE_CHILD_CLEARTID, CLONE_FILES, CLONE_FS}; use platform::linux::namespace::{CLONE_PARENT_SETTID, CLONE_SETTLS, CLONE_SIGHAND, CLONE_SYSVSEM}; -use platform::linux::namespace::{CLONE_THREAD, CLONE_VM}; +use platform::linux::namespace::{CLONE_THREAD, CLONE_VM, CLONE_VFORK}; use profile::{Operation, Profile}; use libc::{self, AF_INET, AF_INET6, AF_UNIX, AF_NETLINK}; use libc::{c_char, c_int, c_ulong, c_ushort, c_void}; use libc::{O_NONBLOCK, O_RDONLY, O_NOCTTY, O_CLOEXEC, FIONREAD, FIOCLEX}; use libc::{MADV_NORMAL, MADV_RANDOM, MADV_SEQUENTIAL, MADV_WILLNEED, MADV_DONTNEED}; +use libc::SIGCHLD; use std::ffi::CString; use std::mem; @@ -94,6 +95,9 @@ const NR_recvmsg: u32 = 47; const NR_bind: u32 = 49; const NR_getsockname: u32 = 51; const NR_clone: u32 = 56; +const NR_fork: u32 = 57; +const NR_vfork: u32 = 58; +const NR_execve: u32 = 59; const NR_exit: u32 = 60; const NR_readlink: u32 = 89; const NR_getuid: u32 = 102; @@ -104,6 +108,7 @@ const NR_exit_group: u32 = 231; const NR_set_robust_list: u32 = 273; const NR_sendmmsg: u32 = 307; const NR_getrandom: u32 = 318; +const NR_execveat: u32 = 322; const EM_386: u32 = 3; const EM_PPC: u32 = 20; @@ -186,6 +191,13 @@ static ALLOWED_SYSCALLS_FOR_NETWORK_OUTBOUND: [u32; 3] = [ NR_getsockname, ]; +static ALLOWED_SYSCALLS_FOR_PROCESS_CREATION: [u32; 4] = [ + NR_fork, + NR_vfork, + NR_execve, + NR_execveat, +]; + const ALLOW_SYSCALL: sock_filter = sock_filter { code: RET + K, k: SECCOMP_RET_ALLOW, @@ -295,7 +307,18 @@ impl Filter { }) } - // Only allow normal threads to be created. + let allow_process_creation = profile.allowed_operations().iter().any(|operation| { + match *operation { + Operation::CreateNewProcesses => true, + _ => false, + } + }); + if allow_process_creation { + filter.allow_syscalls(&ALLOWED_SYSCALLS_FOR_PROCESS_CREATION); + } + + // Only allow normal threads to be created, or vfork/fork if they + // are enabled. filter.if_syscall_is(NR_clone, |filter| { filter.if_arg0_is((CLONE_VM | CLONE_FS | @@ -306,7 +329,15 @@ impl Filter { CLONE_SETTLS | CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID) as u32, - |filter| filter.allow_this_syscall()) + |filter| filter.allow_this_syscall()); + if allow_process_creation { + filter.if_arg0_is(SIGCHLD as u32, + |filter| filter.allow_this_syscall()); + filter.if_arg0_is((CLONE_VM | + CLONE_VFORK | + SIGCHLD) as u32, + |filter| filter.allow_this_syscall()); + } }); // Only allow the POSIX values for `madvise`. diff --git a/platform/macos/mod.rs b/platform/macos/mod.rs index 83b4883..e1f4b63 100644 --- a/platform/macos/mod.rs +++ b/platform/macos/mod.rs @@ -38,6 +38,9 @@ impl OperationSupport for profile::Operation { profile::Operation::PlatformSpecific(Operation::MachLookup(_)) => { OperationSupportLevel::CanBeAllowed } + profile::Operation::CreateNewProcesses => { + OperationSupportLevel::NeverAllowed + } } } } @@ -118,6 +121,9 @@ impl ChildSandboxMethods for ChildSandbox { profile::Operation::SystemInfoRead => { sandbox_profile.write_all(b"(allow sysctl-read)\n").unwrap() } + profile::Operation::CreateNewProcesses => { + unimplemented!() + } profile::Operation::PlatformSpecific(Operation::MachLookup(ref service_name)) => { sandbox_profile.write_all(b"(allow mach-lookup (global-name ").unwrap(); write_quoted_string(&mut sandbox_profile, service_name.as_slice()); diff --git a/profile.rs b/profile.rs index 6c3daee..7e3d6e2 100644 --- a/profile.rs +++ b/profile.rs @@ -27,8 +27,6 @@ use std::path::PathBuf; /// /// * Opening any file for writing. /// -/// * Creating new processes. -/// /// * Opening named pipes or System V IPC resources. /// /// * Accessing System V semaphores. @@ -95,6 +93,8 @@ pub enum Operation { NetworkOutbound(AddressPattern), /// System information may be read (via `sysctl` on Unix). SystemInfoRead, + /// Creating new processes (`fork`/`vfork` and `exec` on Linux). + CreateNewProcesses, /// Platform-specific operations. PlatformSpecific(platform::Operation), } From 3607477674e8be353d48634fd462c6b474ec000d Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 17:51:52 +1300 Subject: [PATCH 04/10] Allow arch_prctl GET/SET_FS/GS glibc uses this during process startup. --- platform/linux/seccomp.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/platform/linux/seccomp.rs b/platform/linux/seccomp.rs index 854fab8..4384380 100644 --- a/platform/linux/seccomp.rs +++ b/platform/linux/seccomp.rs @@ -102,6 +102,7 @@ const NR_exit: u32 = 60; const NR_readlink: u32 = 89; const NR_getuid: u32 = 102; const NR_sigaltstack: u32 = 131; +const NR_arch_prctl: u32 = 158; const NR_futex: u32 = 202; const NR_sched_getaffinity: u32 = 204; const NR_exit_group: u32 = 231; @@ -110,6 +111,11 @@ const NR_sendmmsg: u32 = 307; const NR_getrandom: u32 = 318; const NR_execveat: u32 = 322; +const ARCH_SET_GS: u32 = 0x1001; +const ARCH_SET_FS: u32 = 0x1002; +const ARCH_GET_FS: u32 = 0x1003; +const ARCH_GET_GS: u32 = 0x1004; + const EM_386: u32 = 3; const EM_PPC: u32 = 20; const EM_PPC64: u32 = 21; @@ -267,6 +273,14 @@ impl Filter { }; filter.allow_syscalls(&ALLOWED_SYSCALLS); + // glibc uses these during startup + filter.if_syscall_is(NR_arch_prctl, |filter| { + filter.if_arg0_is(ARCH_SET_GS as u32, |filter| filter.allow_this_syscall()); + filter.if_arg0_is(ARCH_SET_FS as u32, |filter| filter.allow_this_syscall()); + filter.if_arg0_is(ARCH_GET_FS as u32, |filter| filter.allow_this_syscall()); + filter.if_arg0_is(ARCH_GET_GS as u32, |filter| filter.allow_this_syscall()); + }); + if profile.allowed_operations().iter().any(|operation| { match *operation { Operation::FileReadAll(_) | Operation::FileReadMetadata(_) => true, From 6065617698afcefa40a657fbe6f90e9c397fc8ec Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 17:55:40 +1300 Subject: [PATCH 05/10] Allow set_tid_address glibc uses this during startup --- platform/linux/seccomp.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/platform/linux/seccomp.rs b/platform/linux/seccomp.rs index 4384380..f4fe9b1 100644 --- a/platform/linux/seccomp.rs +++ b/platform/linux/seccomp.rs @@ -105,6 +105,7 @@ const NR_sigaltstack: u32 = 131; const NR_arch_prctl: u32 = 158; const NR_futex: u32 = 202; const NR_sched_getaffinity: u32 = 204; +const NR_set_tid_address: u32 = 218; const NR_exit_group: u32 = 231; const NR_set_robust_list: u32 = 273; const NR_sendmmsg: u32 = 307; @@ -159,7 +160,7 @@ static FILTER_EPILOGUE: [sock_filter; 1] = [ ]; /// Syscalls that are always allowed. -pub static ALLOWED_SYSCALLS: [u32; 21] = [ +pub static ALLOWED_SYSCALLS: [u32; 22] = [ NR_brk, NR_close, NR_exit, @@ -179,6 +180,7 @@ pub static ALLOWED_SYSCALLS: [u32; 21] = [ NR_sendmmsg, NR_sendto, NR_set_robust_list, + NR_set_tid_address, NR_sigaltstack, NR_write, ]; From c6da6b356111ef16a10ea3f78701a67716c179bd Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 17:57:59 +1300 Subject: [PATCH 06/10] Allow rt_sigaction/rt_sigprocmask since signal-handling is allowed --- platform/linux/seccomp.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/platform/linux/seccomp.rs b/platform/linux/seccomp.rs index f4fe9b1..ff506ed 100644 --- a/platform/linux/seccomp.rs +++ b/platform/linux/seccomp.rs @@ -83,6 +83,8 @@ const NR_mmap: u32 = 9; const NR_mprotect: u32 = 10; const NR_munmap: u32 = 11; const NR_brk: u32 = 12; +const NR_rt_sigaction: u32 = 13; +const NR_rt_sigprocmask: u32 = 14; const NR_rt_sigreturn: u32 = 15; const NR_ioctl: u32 = 16; const NR_access: u32 = 21; @@ -160,7 +162,7 @@ static FILTER_EPILOGUE: [sock_filter; 1] = [ ]; /// Syscalls that are always allowed. -pub static ALLOWED_SYSCALLS: [u32; 22] = [ +pub static ALLOWED_SYSCALLS: [u32; 24] = [ NR_brk, NR_close, NR_exit, @@ -175,6 +177,8 @@ pub static ALLOWED_SYSCALLS: [u32; 22] = [ NR_read, NR_recvfrom, NR_recvmsg, + NR_rt_sigaction, + NR_rt_sigprocmask, NR_rt_sigreturn, NR_sched_getaffinity, NR_sendmmsg, From 43930e6218d0d145de302a8da5e297a3bbaec87f Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 18:07:48 +1300 Subject: [PATCH 07/10] Allow get/setrlimit syscalls Sandboxed processes should be able to use these to reduce their limits. In a future commit we will turn all soft limits into hard limits so it's impossible for a sandboxed child to increase any of its limits. glibc uses prlimit64 during startup. --- platform/linux/seccomp.rs | 8 +++++++- profile.rs | 2 ++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/platform/linux/seccomp.rs b/platform/linux/seccomp.rs index ff506ed..8268699 100644 --- a/platform/linux/seccomp.rs +++ b/platform/linux/seccomp.rs @@ -102,14 +102,17 @@ const NR_vfork: u32 = 58; const NR_execve: u32 = 59; const NR_exit: u32 = 60; const NR_readlink: u32 = 89; +const NR_getrlimit: u32 = 97; const NR_getuid: u32 = 102; const NR_sigaltstack: u32 = 131; const NR_arch_prctl: u32 = 158; +const NR_setrlimit: u32 = 160; const NR_futex: u32 = 202; const NR_sched_getaffinity: u32 = 204; const NR_set_tid_address: u32 = 218; const NR_exit_group: u32 = 231; const NR_set_robust_list: u32 = 273; +const NR_prlimit64: u32 = 302; const NR_sendmmsg: u32 = 307; const NR_getrandom: u32 = 318; const NR_execveat: u32 = 322; @@ -162,18 +165,20 @@ static FILTER_EPILOGUE: [sock_filter; 1] = [ ]; /// Syscalls that are always allowed. -pub static ALLOWED_SYSCALLS: [u32; 24] = [ +pub static ALLOWED_SYSCALLS: [u32; 27] = [ NR_brk, NR_close, NR_exit, NR_exit_group, NR_futex, NR_getrandom, + NR_getrlimit, NR_getuid, NR_mmap, NR_mprotect, NR_munmap, NR_poll, + NR_prlimit64, NR_read, NR_recvfrom, NR_recvmsg, @@ -185,6 +190,7 @@ pub static ALLOWED_SYSCALLS: [u32; 24] = [ NR_sendto, NR_set_robust_list, NR_set_tid_address, + NR_setrlimit, NR_sigaltstack, NR_write, ]; diff --git a/profile.rs b/profile.rs index 7e3d6e2..e36a2b8 100644 --- a/profile.rs +++ b/profile.rs @@ -52,6 +52,8 @@ use std::path::PathBuf; /// informing the kernel that memory pages may be discarded. (It may be possible to restrict /// this in future versions.) /// +/// * Adjusting resource limits downward. +/// /// * Spawning new threads. /// /// * Responding to signals (e.g. `signal`, `sigaltstack`). From 4ab80ac3fd8f134913cc73eba33f99ada22f5b75 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 18:35:06 +1300 Subject: [PATCH 08/10] Allow getting the current time time/gettimeofday are generally called through the vDSO without entering the kernel so blocking them with seccomp doesn't really work anyway. Having sandboxed children fail only when the vDSO is disabled (e.g. when running under rr) is a problem. --- platform/linux/seccomp.rs | 6 +++++- profile.rs | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/platform/linux/seccomp.rs b/platform/linux/seccomp.rs index 8268699..ee0ecec 100644 --- a/platform/linux/seccomp.rs +++ b/platform/linux/seccomp.rs @@ -102,11 +102,13 @@ const NR_vfork: u32 = 58; const NR_execve: u32 = 59; const NR_exit: u32 = 60; const NR_readlink: u32 = 89; +const NR_gettimeofday: u32 = 96; const NR_getrlimit: u32 = 97; const NR_getuid: u32 = 102; const NR_sigaltstack: u32 = 131; const NR_arch_prctl: u32 = 158; const NR_setrlimit: u32 = 160; +const NR_time: u32 = 201; const NR_futex: u32 = 202; const NR_sched_getaffinity: u32 = 204; const NR_set_tid_address: u32 = 218; @@ -165,7 +167,7 @@ static FILTER_EPILOGUE: [sock_filter; 1] = [ ]; /// Syscalls that are always allowed. -pub static ALLOWED_SYSCALLS: [u32; 27] = [ +pub static ALLOWED_SYSCALLS: [u32; 29] = [ NR_brk, NR_close, NR_exit, @@ -173,6 +175,7 @@ pub static ALLOWED_SYSCALLS: [u32; 27] = [ NR_futex, NR_getrandom, NR_getrlimit, + NR_gettimeofday, NR_getuid, NR_mmap, NR_mprotect, @@ -192,6 +195,7 @@ pub static ALLOWED_SYSCALLS: [u32; 27] = [ NR_set_tid_address, NR_setrlimit, NR_sigaltstack, + NR_time, NR_write, ]; diff --git a/profile.rs b/profile.rs index e36a2b8..a8f8fb8 100644 --- a/profile.rs +++ b/profile.rs @@ -54,6 +54,8 @@ use std::path::PathBuf; /// /// * Adjusting resource limits downward. /// +/// * Getting the current real time and timezone. +/// /// * Spawning new threads. /// /// * Responding to signals (e.g. `signal`, `sigaltstack`). From 7502de72cce138aafd779a98feb35384bcc2717f Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 16:46:52 +1300 Subject: [PATCH 09/10] Support before_sandbox/before_exec callbacks to set up subprocess environment This commit is a bit oversized... adding support for these callbacks required creating a way to pass errors back to the parent process, which inspired fixing the error handling in start(), which uncovered some bugs in start(): * Immediate child process leaked as a zombie * pipe_fds[0] leaked in parent * pipe_fds[1] leaked into grandchild --- Cargo.toml | 5 ++ lib.rs | 3 +- platform/linux/namespace.rs | 113 ++++++++++++++++++++++++++++-------- platform/unix/mod.rs | 107 ++++++++++++++++++++++++++++++++++ platform/unix/process.rs | 4 +- sandbox.rs | 8 ++- tests/before-callbacks.rs | 104 +++++++++++++++++++++++++++++++++ 7 files changed, 316 insertions(+), 28 deletions(-) create mode 100644 tests/before-callbacks.rs diff --git a/Cargo.toml b/Cargo.toml index 35936c3..c483cf2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,11 @@ rand = "0.3" [dev-dependencies] num_cpus = "1" +[[test]] +name = "before-callbacks" +path = "tests/before-callbacks.rs" +harness = false + [[test]] name = "file-read-all" path = "tests/file-read-all.rs" diff --git a/lib.rs b/lib.rs index b393feb..c4f436f 100644 --- a/lib.rs +++ b/lib.rs @@ -23,6 +23,8 @@ pub mod platform { pub use platform::macos::{ChildSandbox, Operation, Sandbox}; #[cfg(any(target_os="android", target_os="linux", target_os="macos"))] pub use platform::unix::process::{self, Process}; + #[cfg(any(target_os="android", target_os="linux", target_os="macos"))] + pub use platform::unix::CommandInner; #[cfg(any(target_os="android", target_os="linux"))] pub mod linux; @@ -31,4 +33,3 @@ pub mod platform { #[cfg(any(target_os="android", target_os="linux", target_os="macos"))] pub mod unix; } - diff --git a/platform/linux/namespace.rs b/platform/linux/namespace.rs index 30ae298..0f45b55 100644 --- a/platform/linux/namespace.rs +++ b/platform/linux/namespace.rs @@ -16,13 +16,14 @@ use platform::unix; use profile::{Operation, PathPattern, Profile}; use sandbox::Command; -use libc::{self, c_char, c_int, c_ulong, c_void, gid_t, pid_t, size_t, ssize_t, uid_t}; +use libc::{self, EINVAL, O_CLOEXEC, c_char, c_int, c_ulong, c_void, gid_t, pid_t, size_t, ssize_t, uid_t}; use std::env; use std::ffi::{CString, OsStr, OsString}; use std::fs::{self, File}; use std::io::{self, Write}; use std::iter; use std::mem; +use std::os::unix::io::RawFd; use std::os::unix::prelude::OsStrExt; use std::path::{Path, PathBuf}; use std::ptr; @@ -202,6 +203,50 @@ unsafe fn prepare_user_and_pid_namespaces(parent_uid: uid_t, parent_gid: gid_t) Ok(()) } +unsafe fn fork_wrapper() -> io::Result { + let child = fork(); + if child >= 0 { + Ok(child) + } else { + Err(io::Error::last_os_error()) + } +} + +unsafe fn pipe_write(pipe: RawFd, value: i32) { + assert!(libc::write(pipe, + &value as *const i32 as *const c_void, + mem::size_of::() as size_t) == mem::size_of::() as ssize_t); +} + +unsafe fn pipe_read(pipe: RawFd) -> io::Result> { + let mut ret = Vec::new(); + loop { + let mut v: i32 = 0; + let bytes = libc::read(pipe, + &mut v as *mut i32 as *mut c_void, + mem::size_of::() as size_t); + if bytes == mem::size_of::() as ssize_t { + ret.push(v); + } else if bytes == 0 { + return Ok(ret); + } else if bytes > 0 { + panic!("No idea how we got a partial read in this pipe"); + } else { + return Err(io::Error::last_os_error()) + } + } +} + +unsafe fn handle_error(result: io::Result, pipe: RawFd) -> T { + match result { + Ok(v) => v, + Err(e) => { + pipe_write(pipe, -e.raw_os_error().unwrap_or(EINVAL)); + libc::exit(0); + } + } +} + /// Spawns a child process in a new namespace. /// /// This function is quite tricky. Hic sunt dracones! @@ -227,53 +272,75 @@ pub fn start(profile: &Profile, command: &mut Command) -> io::Result { unsafe { // Create a pipe so we can communicate the PID of our grandchild back. let mut pipe_fds = [0, 0]; - assert!(libc::pipe(&mut pipe_fds[0]) == 0); + if libc::pipe2(&mut pipe_fds[0], O_CLOEXEC) != 0 { + return Err(io::Error::last_os_error()); + } // Set this `prctl` flag so that we can wait on our grandchild. (Otherwise it'll be // reparented to init.) assert!(seccomp::prctl(PR_SET_CHILD_SUBREAPER, 1, 0, 0, 0) == 0); // Fork so that we can unshare without removing our ability to create threads. - if fork() == 0 { - // Close the reading end of the pipe. - libc::close(pipe_fds[0]); - + let forked = match fork_wrapper() { + Ok(pid) => pid, + Err(e) => { + libc::close(pipe_fds[0]); + libc::close(pipe_fds[1]); + return Err(e); + } + }; + if forked == 0 { + handle_error(command.inner.before_sandbox(&[pipe_fds[1]]), pipe_fds[1]); // Set up our user and PID namespaces. The PID namespace won't actually come into // effect until the next fork(), because PIDs are immutable. - prepare_user_and_pid_namespaces(parent_uid, parent_gid).unwrap(); + handle_error(prepare_user_and_pid_namespaces(parent_uid, parent_gid), pipe_fds[1]); // Fork again, to enter the PID namespace. - match fork() { + match handle_error(fork_wrapper(), pipe_fds[1]) { 0 => { // Enter the auxiliary namespaces. - assert!(unshare(unshare_flags) == 0); + if unshare(unshare_flags) != 0 { + handle_error::<()>(Err(io::Error::last_os_error()), pipe_fds[1]); + } + handle_error(command.inner.before_exec(&[pipe_fds[1]]), pipe_fds[1]); // Go ahead and start the command. - drop(unix::process::exec(command)); - abort() + handle_error::<()>(Err(unix::process::exec(command)), pipe_fds[1]); } grandchild_pid => { // Send the PID of our child up to our parent and exit. - assert!(libc::write(pipe_fds[1], - &grandchild_pid as *const pid_t as *const c_void, - mem::size_of::() as size_t) == - mem::size_of::() as ssize_t); + pipe_write(pipe_fds[1], grandchild_pid); libc::exit(0); } } } - // Grandparent execution continues here. First, close the writing end of the pipe. + // Grandparent execution continues here. + + // Reap child zombie. + waitpid(forked, ptr::null_mut(), 0); + + // Close pipe writer end now so that when the child/grandchild close + // theirs, we'll get EOF on reading. libc::close(pipe_fds[1]); // Retrieve our grandchild's PID. - let mut grandchild_pid: pid_t = 0; - assert!(libc::read(pipe_fds[0], - &mut grandchild_pid as *mut i32 as *mut c_void, - mem::size_of::() as size_t) == - mem::size_of::() as ssize_t); + let pipe_vals = pipe_read(pipe_fds[0]); + libc::close(pipe_fds[0]); + let pipe_vals = pipe_vals?; + + // We could get a PID followed by an error from the grandchild. + let grandchild_pid = pipe_vals.iter().find(|v| **v >= 0); + if let Some(err) = pipe_vals.iter().find(|v| **v < 0) { + if let Some(pid) = grandchild_pid { + // Reap failed grandchild zombie. + waitpid(*pid, ptr::null_mut(), 0); + } + return Err(io::Error::from_raw_os_error(-*err)); + } + Ok(Process { - pid: grandchild_pid, + pid: *grandchild_pid.expect("We should have something in the pipe"), }) } } @@ -331,7 +398,6 @@ const _LINUX_CAPABILITY_U32S_3: u32 = 2; const PR_SET_CHILD_SUBREAPER: c_int = 36; extern { - fn abort() -> !; fn capset(hdrp: cap_user_header_t, datap: const_cap_user_data_t) -> c_int; fn chroot(path: *const c_char) -> c_int; fn fork() -> pid_t; @@ -342,6 +408,7 @@ extern { mountflags: c_ulong, data: *const c_void) -> c_int; + fn waitpid(pid: pid_t, stat_loc: *mut c_int, options: c_int) -> pid_t; fn unshare(flags: c_int) -> c_int; } diff --git a/platform/unix/mod.rs b/platform/unix/mod.rs index 77e135e..eceddd0 100644 --- a/platform/unix/mod.rs +++ b/platform/unix/mod.rs @@ -8,5 +8,112 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::io; +use std::os::unix::io::RawFd; + pub mod process; +use sandbox::Command; + +pub trait CommandExt { + /// Schedules a closure to be run after forking but before any sandbox + /// controls are applied. This may not be the final process that will exec. + /// This lets you set up subprocess state that must be initialized before + /// dropping privileges, without disturbing the parent process. + /// + /// The closure is allowed to return an I/O error whose OS error code will + /// be communicated back to the parent and returned as an error from when + /// the start was requested. + /// + /// Multiple closures can be registered and they will be called in order of + /// their registration. If a closure returns `Err` then no further closures + /// will be called and the start operation will immediately return with a + /// failure. + /// TODO on Mac, errors are not yet propagated to start(). + /// + /// # Notes + /// + /// This closure will be run in the context of the child process after a + /// `fork`. This primarily means that any modificatons made to memory on + /// behalf of this closure will **not** be visible to the parent process. + /// This is often a very constrained environment where normal operations + /// like `malloc` or acquiring a mutex are not guaranteed to work (due to + /// other threads perhaps still running when the `fork` was run). + /// + /// Avoid closing any file descriptors in the passed-in list. These are + /// O_CLOEXEC so will automatically close when the command runs. + fn before_sandbox(&mut self, f: F) -> &mut Command + where F: FnMut(&[RawFd]) -> io::Result<()> + Send + Sync + 'static; + /// Schedules a closure to be run after any pre-exec sandbox controls are + /// but before exec, in the process that will exec. On Linux, this closure + /// can call ChildSandbox::activate(), letting you sandbox a foreign + /// executable and then perform process setup steps that must be performed + /// after the sandbox is activated. + /// + /// The closure is allowed to return an I/O error whose OS error code will + /// be communicated back to the parent and returned as an error from when + /// the start was requested. + /// + /// Multiple closures can be registered and they will be called in order of + /// their registration. If a closure returns `Err` then no further closures + /// will be called and the start operation will immediately return with a + /// failure. + /// TODO on Mac, errors are not yet propagated to start(). + /// + /// # Notes + /// + /// This closure will be run in the context of the child process after a + /// `fork`. This primarily means that any modificatons made to memory on + /// behalf of this closure will **not** be visible to the parent process. + /// This is often a very constrained environment where normal operations + /// like `malloc` or acquiring a mutex are not guaranteed to work (due to + /// other threads perhaps still running when the `fork` was run). + /// + /// Avoid closing any file descriptors in the passed-in list. These are + /// O_CLOEXEC so will automatically close when the command runs. + fn before_exec(&mut self, f: F) -> &mut Command + where F: FnMut(&[RawFd]) -> io::Result<()> + Send + Sync + 'static; +} + +pub struct CommandInner { + before_sandbox_closures: Vec io::Result<()> + Send + Sync + 'static>>, + before_exec_closures: Vec io::Result<()> + Send + Sync + 'static>>, +} + +impl CommandInner { + pub fn new() -> CommandInner { + CommandInner { + before_sandbox_closures: Vec::new(), + before_exec_closures: Vec::new(), + } + } + + pub fn before_sandbox(&mut self, preserve_fds: &[RawFd]) -> io::Result<()> { + for c in self.before_sandbox_closures.iter_mut() { + c(preserve_fds)?; + } + self.before_sandbox_closures.clear(); + Ok(()) + } + + pub fn before_exec(&mut self, preserve_fds: &[RawFd]) -> io::Result<()> { + for c in self.before_exec_closures.iter_mut() { + c(preserve_fds)?; + } + self.before_exec_closures.clear(); + Ok(()) + } +} + +impl CommandExt for Command { + fn before_sandbox(&mut self, f: F) -> &mut Command + where F: FnMut(&[RawFd]) -> io::Result<()> + Send + Sync + 'static { + self.inner.before_sandbox_closures.push(Box::new(f)); + self + } + fn before_exec(&mut self, f: F) -> &mut Command + where F: FnMut(&[RawFd]) -> io::Result<()> + Send + Sync + 'static { + self.inner.before_exec_closures.push(Box::new(f)); + self + } +} diff --git a/platform/unix/process.rs b/platform/unix/process.rs index c06752a..ef66865 100644 --- a/platform/unix/process.rs +++ b/platform/unix/process.rs @@ -42,10 +42,12 @@ pub fn exec(command: &Command) -> io::Error { io::Error::last_os_error() } -pub fn spawn(command: &Command) -> io::Result { +pub fn spawn(command: &mut Command) -> io::Result { unsafe { match fork() { 0 => { + drop(command.inner.before_sandbox(&[])); + drop(command.inner.before_exec(&[])); drop(exec(command)); panic!() } diff --git a/sandbox.rs b/sandbox.rs index e3ce6ab..1d38d71 100644 --- a/sandbox.rs +++ b/sandbox.rs @@ -19,7 +19,7 @@ use std::env; use std::ffi::{CString, OsStr}; use std::io; -pub use platform::{ChildSandbox, Sandbox}; +pub use platform::{ChildSandbox, CommandInner, Sandbox}; /// All platform-specific sandboxes implement this trait. /// @@ -60,6 +60,8 @@ pub struct Command { pub args: Vec, /// The environment of the process. pub env: HashMap, + /// Platform-specific inner data + pub(crate) inner: CommandInner, } impl Command { @@ -71,6 +73,7 @@ impl Command { module_path: cstring(module_path), args: Vec::new(), env: HashMap::new(), + inner: CommandInner::new(), } } @@ -99,8 +102,7 @@ impl Command { } /// Executes the command as a child process, which is returned. - pub fn spawn(&self) -> io::Result { + pub fn spawn(&mut self) -> io::Result { process::spawn(self) } } - diff --git a/tests/before-callbacks.rs b/tests/before-callbacks.rs new file mode 100644 index 0000000..12ea3e3 --- /dev/null +++ b/tests/before-callbacks.rs @@ -0,0 +1,104 @@ +// Any copyright is dedicated to the Public Domain. +// http://creativecommons.org/publicdomain/zero/1.0/ + +extern crate gaol; +extern crate libc; + +use gaol::profile::{Operation, PathPattern, Profile}; +use gaol::sandbox::{ChildSandbox, ChildSandboxMethods, Command, Sandbox, SandboxMethods}; +#[cfg(any(target_os="android", target_os="linux", target_os="macos"))] +use gaol::platform::unix::CommandExt; + +use libc::ENOTTY; +use std::env; +use std::fs::metadata; +use std::io; +use std::path::PathBuf; + +#[cfg(any(target_os="android", target_os="linux"))] +fn test_error_propagation() { + fn return_err(_: &[i32]) -> io::Result<()> { + // not a typewriter + Err(io::Error::from_raw_os_error(ENOTTY)) + } + + fn profile() -> Profile { + Profile::new(vec![]).unwrap() + } + let err = + Sandbox::new(profile()).start(&mut Command::me().unwrap() + .arg("child") + .before_sandbox(return_err)); + match err { + Err(e) => assert_eq!(e.raw_os_error(), Some(ENOTTY)), + Ok(_) => panic!(), + }; + + let err = + Sandbox::new(profile()).start(&mut Command::me().unwrap() + .arg("child") + .before_exec(return_err)); + match err { + Err(e) => assert_eq!(e.raw_os_error(), Some(ENOTTY)), + Ok(_) => panic!(), + }; +} + +#[cfg(target_os="macos")] +fn test_error_propagation() { + // TODO this doesn't work yet +} + +#[cfg(any(target_os="android", target_os="linux", target_os="macos"))] +pub fn main() { + fn profile() -> Profile { + let exe = env::current_exe().unwrap(); + // Whitelist a bunch of directories that should let us launch this + // binary OK. But not /tmp. + Profile::new(vec![Operation::FileReadAll(PathPattern::Literal(exe)), + Operation::FileReadAll(PathPattern::Subpath(PathBuf::from("/usr"))), + Operation::FileReadAll(PathPattern::Subpath(PathBuf::from("/bin"))), + Operation::FileReadAll(PathPattern::Subpath(PathBuf::from("/lib64"))), + Operation::FileReadAll(PathPattern::Subpath(PathBuf::from("/lib"))), + Operation::CreateNewProcesses]).unwrap() + } + + match env::args().skip(1).next() { + Some(ref arg) if arg == "child" => return, + _ => {} + } + + fn do_before_sandbox(_: &[i32]) -> io::Result<()> { + metadata("/tmp")?; + Ok(()) + } + + #[cfg(any(target_os="android", target_os="linux"))] + fn do_before_exec(_: &[i32]) -> io::Result<()> { + ChildSandbox::new(profile()).activate().map_err( + |_| io::Error::from_raw_os_error(ENOTTY))?; + if metadata("/tmp").is_err() { + Ok(()) + } else { + Err(io::Error::from_raw_os_error(ENOTTY)) + } + } + #[cfg(not(any(target_os="android", target_os="linux")))] + fn do_before_exec(_: &[i32]) -> io::Result<()> { Ok(()) } + + let status = + Sandbox::new(profile()).start(&mut Command::me().unwrap() + .arg("child") + .before_sandbox(do_before_sandbox) + .before_exec(do_before_exec)) + .unwrap() + .wait() + .unwrap(); + assert!(status.success()); + + test_error_propagation(); +} + +#[cfg(not(any(target_os="android", target_os="linux", target_os="macos")))] +pub fn main() {} + From 07761bce3775a486359617e79ccf5a1505687167 Mon Sep 17 00:00:00 2001 From: Robert O'Callahan Date: Fri, 29 Sep 2017 18:28:17 +1300 Subject: [PATCH 10/10] Make all soft resource limits into hard limits --- platform/linux/namespace.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/platform/linux/namespace.rs b/platform/linux/namespace.rs index 0f45b55..b6526e4 100644 --- a/platform/linux/namespace.rs +++ b/platform/linux/namespace.rs @@ -247,6 +247,23 @@ unsafe fn handle_error(result: io::Result, pipe: RawFd) -> T { } } +/// Make all soft limits hard limits so the sandboxed child cannot increase them. +fn harden_limits() -> io::Result<()> { + for resource in 0..libc::RLIMIT_NLIMITS { + let mut limit = libc::rlimit { rlim_cur: 0, rlim_max: 0 }; + if unsafe { libc::getrlimit(resource, &mut limit as *mut libc::rlimit) } != 0 { + return Err(io::Error::last_os_error()); + } + if limit.rlim_cur != libc::RLIM_INFINITY && limit.rlim_max != limit.rlim_cur { + limit.rlim_max = limit.rlim_cur; + if unsafe { libc::setrlimit(resource, &limit as *const libc::rlimit) } != 0 { + return Err(io::Error::last_os_error()); + } + } + } + Ok(()) +} + /// Spawns a child process in a new namespace. /// /// This function is quite tricky. Hic sunt dracones! @@ -290,6 +307,8 @@ pub fn start(profile: &Profile, command: &mut Command) -> io::Result { } }; if forked == 0 { + handle_error(harden_limits(), pipe_fds[1]); + handle_error(command.inner.before_sandbox(&[pipe_fds[1]]), pipe_fds[1]); // Set up our user and PID namespaces. The PID namespace won't actually come into // effect until the next fork(), because PIDs are immutable.