diff --git a/CHANGELOG.md b/CHANGELOG.md index 0da8ca7..77b0fff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ unreleased ---------- +- Switch seccomp backend to seccompiler + - This adds several new structs that act as wrappers around the underlying seccompiler structs + - Macros are defined in extrasafe now to replace the ones provided by + libseccomp for comparing and filtering syscall arguments +- Add `#[must_use]` attributes to several structs + 0.2.0 ----- - reexport syscalls dependency diff --git a/Cargo.toml b/Cargo.toml index 9562aab..9879518 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,7 +10,7 @@ keywords = ["security", "seccomp", "syscall"] categories = ["os::linux-apis"] [dependencies] -libseccomp = "^0.3" +seccompiler = { version = "^0.4", default-features = false } libc = "^0.2" syscalls = { version = "^0.6", default-features = false } @@ -23,5 +23,5 @@ tempfile = "^3" tokio = "^1.15" hyper = { version = "^0.14", features = ["http1", "server", "runtime", "tcp"] } warp = "^0.3" -reqwest = "^0.11" +reqwest = { version = "^0.11", default-features = false, features = ["rustls-tls"] } rusqlite = "^0.26" diff --git a/README.md b/README.md index 598767a..4695748 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ fn main() { You've used safe and unsafe Rust: now your code can be extrasafe. -extrasafe is a wrapper around [libseccomp](https://libseccomp.readthedocs.io/en/latest/), which uses [the Linux kernel's seccomp](https://www.kernel.org/doc/html/latest/userspace-api/seccomp_filter.html) syscall-filtering functionality to prevent your program from calling syscalls you don't need. Seccomp is used by systemd, Chrome, application sandboxes like bubblewrap and firejail, and container runtimes. Seccomp by itself is not a complete sandboxing system. +extrasafe is a wrapper around [the Linux kernel's seccomp](https://www.kernel.org/doc/html/latest/userspace-api/seccomp_filter.html) syscall-filtering functionality to prevent your program from calling syscalls you don't need. Seccomp is used by systemd, Chrome, application sandboxes like bubblewrap and firejail, and container runtimes. Seccomp by itself is not a complete sandboxing system. The goal of extrasafe is to make it easy to add extra security to your own programs without having to rely on external configuration by the person running the software. diff --git a/design.md b/design.md index 66d7650..5d7d50c 100644 --- a/design.md +++ b/design.md @@ -10,14 +10,13 @@ This is the entry point. You create a SafetyContext, in which you gather rules v - RuleSet A trait that provides a collection of simple and conditional rules. You can think of this as a facet of a security policy, like allowing IO, network, or clock access. There are implementors provided by extrasafe and you can also define your own. - SeccompRule -A syscall and an optional number of conditions on the syscall's arguments. You can make comparisons on the arguments of the syscall, but the conditions can't dereference pointers so you can't do e.g. string comparisons on file paths. +A syscall and an optional number of conditions (`SeccompArgumentFilter`s) on the syscall's arguments. You can make comparisons on the arguments of the syscall, but the conditions can't dereference pointers so you can't do e.g. string comparisons on file paths. The comparisons are such that when the comparison returns **true**, the syscall is allowed. -A single SeccompRule may contain multiple conditions, which are anded together by libseccomp, that is, they all must be true for the syscall to be allowed. If multiple rules are loaded, the syscall is allowed if any of the rules allow it. +A single SeccompRule may contain multiple conditions, which are anded together by seccompiler, that is, they all must be true for the syscall to be allowed. If multiple rules are loaded for a single syscall, the syscall is allowed if any of the rules allow it. - -One thing to note is that libseccomp will silently override conditional rules on a syscall if an unconditional one is added (in the context of extrasafe, where all filters are allow-based). extrasafe catches this and provides an error with the conflicting syscall and the names of the RuleSets that provided the rules. +That is, the argument filters within a SeccompRule are and-ed together, but the SeccompRules themselves are or-ed together. # Typical usage @@ -30,7 +29,7 @@ DNS requires accessing /etc/resolv.conf and ssl typically requires opening a bun ### Network calls -TODO: tcp sockets and accept, tcp clients opening connections +TODO: see tests/examples, describe tcp sockets and accept, tcp clients opening connections ### Threads vs processes and IPC TODO, threads don't give as much isolation as processes due to sharing namespace, fds, environment variables, etc. diff --git a/examples/user-guide.rs b/examples/user-guide.rs index 307c36d..c6cb49f 100644 --- a/examples/user-guide.rs +++ b/examples/user-guide.rs @@ -22,9 +22,8 @@ fn simple_example() { } fn custom_ruleset() { - use extrasafe::{SeccompRule, RuleSet}; + use extrasafe::*; use extrasafe::syscalls::Sysno; - use libseccomp::scmp_cmp; use std::collections::HashMap; @@ -42,7 +41,7 @@ fn custom_ruleset() { let rule = SeccompRule::new(Sysno::socket) .and_condition( - scmp_cmp!($arg0 & SOCK_STREAM == SOCK_STREAM)); + seccomp_arg_filter!(arg0 & SOCK_STREAM == SOCK_STREAM)); HashMap::from([ (Sysno::socket, vec![rule,]) ]) diff --git a/src/builtins/danger_zone.rs b/src/builtins/danger_zone.rs index 582e19f..07ae946 100644 --- a/src/builtins/danger_zone.rs +++ b/src/builtins/danger_zone.rs @@ -2,7 +2,6 @@ use std::collections::{HashMap, HashSet}; -//use libseccomp::scmp_cmp; use syscalls::Sysno; use crate::{SeccompRule, RuleSet}; @@ -20,13 +19,13 @@ use super::YesReally; /// do not provide isolation from each other. You can still access other threads' memory and /// potentially get them to do operations that are not allowed in the current thread's seccomp /// context. +#[must_use] pub struct Threads { allowed: HashSet, } impl Threads { /// Create a new [`Threads`] ruleset with nothing allowed by default. - #[must_use] pub fn nothing() -> Threads { Threads { allowed: HashSet::new(), @@ -34,7 +33,6 @@ impl Threads { } /// Allow creating new threads and processes. - #[must_use] pub fn allow_create(mut self) -> Threads { self.allowed.extend([Sysno::clone, Sysno::clone3]); @@ -46,7 +44,6 @@ impl Threads { /// # Security considerations /// An attacker with arbitrary code execution and access to a high resolution timer can mount /// timing attacks (e.g. spectre). - #[must_use] pub fn allow_sleep(mut self) -> YesReally { self.allowed .extend([Sysno::clock_nanosleep, Sysno::nanosleep]); @@ -64,13 +61,13 @@ impl RuleSet for Threads { // let mut rules = HashMap::new(); // let clone = SeccompRule::new(Sysno::clone) - // .and_condition(scmp_cmp!($arg2 & CLONE_THREAD == CLONE_THREAD)); + // .and_condition(seccomp_arg_filter!(arg2 & CLONE_THREAD == CLONE_THREAD)); // rules.entry(Sysno::clone) // .or_insert_with(Vec::new) // .push(clone); // let clone3 = SeccompRule::new(Sysno::clone3) - // .and_condition(scmp_cmp!($arg2 & CLONE_THREAD == CLONE_THREAD)); + // .and_condition(seccomp_arg_filter!(arg2 & CLONE_THREAD == CLONE_THREAD)); // rules.entry(Sysno::clone3) // .or_insert_with(Vec::new) // .push(clone3); @@ -89,6 +86,7 @@ impl RuleSet for Threads { /// `tests/inherit_filters.rs`) but depending on your filter it could still do bad things. /// /// Note that this also allows the `clone` syscall. +#[must_use] pub struct ForkAndExec; impl RuleSet for ForkAndExec { fn simple_rules(&self) -> Vec { @@ -106,13 +104,13 @@ impl RuleSet for ForkAndExec { // let mut custom = HashMap::new(); // // let clone = SeccompRule::new(Sysno::clone) - // .and_condition(scmp_cmp!($arg2 & CLONE_PARENT == CLONE_PARENT)); + // .and_condition(seccomp_arg_filter!(arg2 & CLONE_PARENT == CLONE_PARENT)); // custom.entry(Sysno::clone) // .or_insert_with(Vec::new) // .push(clone); // let clone3 = SeccompRule::new(Sysno::clone3) - // .and_condition(scmp_cmp!($arg2 & CLONE_PARENT == CLONE_PARENT)); + // .and_condition(seccomp_arg_filter!(arg2 & CLONE_PARENT == CLONE_PARENT)); // custom.entry(Sysno::clone3) // .or_insert_with(Vec::new) // .push(clone3); diff --git a/src/builtins/mod.rs b/src/builtins/mod.rs index b2be6cb..43b3c50 100644 --- a/src/builtins/mod.rs +++ b/src/builtins/mod.rs @@ -3,19 +3,18 @@ /// A struct whose purpose is to make you read the documentation for the function you're calling. /// If you're reading this, go read the documentation for the function that is returning this /// object. +#[must_use] pub struct YesReally { inner: T, } impl YesReally { /// Confirm you really wanted to call the function and return its result. - #[must_use] pub fn yes_really(self) -> T { self.inner } /// Make a [`YesReally`]. - #[must_use] pub fn new(inner: T) -> YesReally { YesReally { inner, diff --git a/src/builtins/network.rs b/src/builtins/network.rs index b14202f..c959c1d 100644 --- a/src/builtins/network.rs +++ b/src/builtins/network.rs @@ -2,7 +2,6 @@ use std::collections::{HashMap, HashSet}; -use libseccomp::scmp_cmp; use syscalls::Sysno; use super::YesReally; @@ -63,6 +62,7 @@ const NET_WRITE_SYSCALLS: &[Sysno] = &[Sysno::sendto, Sysno::sendmsg, Sysno::sen /// care to consider whether you can split up your program (e.g. across separate threads) into a /// part that opens and writes to files and a part that speaks to the network. This is a good /// security practice in general. +#[must_use] pub struct Networking { /// Syscalls that are allowed allowed: HashSet, @@ -72,7 +72,6 @@ pub struct Networking { impl Networking { /// By default, allow no networking syscalls. - #[must_use] pub fn nothing() -> Networking { Networking { allowed: HashSet::new(), @@ -82,7 +81,6 @@ impl Networking { /// Allow a running TCP server to continue running. Does not allow `socket` or `bind`, /// preventing new sockets from being created. - #[must_use] pub fn allow_running_tcp_servers(mut self) -> Networking { self.allowed.extend(NET_IO_SYSCALLS); self.allowed.extend(NET_READ_SYSCALLS); @@ -98,7 +96,6 @@ impl Networking { /// You probably don't need to use this. In most cases you can just run your server and then /// use [`allow_running_tcp_servers`](Self::allow_running_tcp_servers). See /// `examples/network_server.rs` for an example with warp. - #[must_use] pub fn allow_start_tcp_servers(mut self) -> YesReally { const AF_INET: u64 = libc::AF_INET as u64; const AF_INET6: u64 = libc::AF_INET6 as u64; @@ -106,15 +103,15 @@ impl Networking { // IPv4 let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_INET == AF_INET)) - .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_INET == AF_INET)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_STREAM == SOCK_STREAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); // IPv6 let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_INET6 == AF_INET6)) - .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_INET6 == AF_INET6)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_STREAM == SOCK_STREAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); @@ -131,7 +128,6 @@ impl Networking { /// Allow a running UDP socket to continue running. Does not allow `socket` or `bind`, /// preventing new sockets from being created. - #[must_use] pub fn allow_running_udp_sockets(mut self) -> Networking { self.allowed.extend(NET_IO_SYSCALLS); self.allowed.extend(NET_READ_SYSCALLS); @@ -146,7 +142,6 @@ impl Networking { /// /// You probably don't need to use this. In most cases you can just run your server and then /// use [`allow_running_udp_sockets`](Self::allow_running_udp_sockets). - #[must_use] pub fn allow_start_udp_servers(mut self) -> YesReally { const AF_INET: u64 = libc::AF_INET as u64; const AF_INET6: u64 = libc::AF_INET6 as u64; @@ -154,15 +149,15 @@ impl Networking { // IPv4 let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_INET == AF_INET)) - .and_condition(scmp_cmp!($arg1 & SOCK_DGRAM == SOCK_DGRAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_INET == AF_INET)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_DGRAM == SOCK_DGRAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); // IPv6 let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_INET6 == AF_INET6)) - .and_condition(scmp_cmp!($arg1 & SOCK_DGRAM == SOCK_DGRAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_INET6 == AF_INET6)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_DGRAM == SOCK_DGRAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); @@ -191,7 +186,6 @@ impl Networking { /// /// In some cases you can create the socket ahead of time, but that isn't possible with e.g. /// reqwest, so we allow socket but not bind here. - #[must_use] pub fn allow_start_tcp_clients(mut self) -> Networking { const AF_INET: u64 = libc::AF_INET as u64; const AF_INET6: u64 = libc::AF_INET6 as u64; @@ -199,15 +193,15 @@ impl Networking { // IPv4 let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_INET == AF_INET)) - .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_INET == AF_INET)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_STREAM == SOCK_STREAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); // IPv6 let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_INET6 == AF_INET6)) - .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_INET6 == AF_INET6)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_STREAM == SOCK_STREAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); @@ -225,7 +219,6 @@ impl Networking { /// /// This is technically the same as /// [`allow_running_tcp_servers`](Self::allow_running_tcp_servers). - #[must_use] pub fn allow_running_tcp_clients(mut self) -> Networking { self.allowed.extend(NET_IO_SYSCALLS); self.allowed.extend(NET_READ_SYSCALLS); @@ -240,7 +233,6 @@ impl Networking { /// /// You probably don't need to use this. In most cases you can just run your server and then /// use [`allow_running_unix_servers`](Self::allow_running_unix_servers). - #[must_use] pub fn allow_start_unix_servers(mut self) -> YesReally { const AF_UNIX: u64 = libc::AF_UNIX as u64; const SOCK_STREAM: u64 = libc::SOCK_STREAM as u64; @@ -248,15 +240,15 @@ impl Networking { // We allow both stream and dgram unix sockets let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_UNIX == AF_UNIX)) - .and_condition(scmp_cmp!($arg1 & SOCK_STREAM == SOCK_STREAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_UNIX == AF_UNIX)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_STREAM == SOCK_STREAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); // DGRAM let rule = SeccompRule::new(Sysno::socket) - .and_condition(scmp_cmp!($arg0 & AF_UNIX == AF_UNIX)) - .and_condition(scmp_cmp!($arg1 & SOCK_DGRAM == SOCK_DGRAM)); + .and_condition(seccomp_arg_filter!(arg0 & AF_UNIX == AF_UNIX)) + .and_condition(seccomp_arg_filter!(arg1 & SOCK_DGRAM == SOCK_DGRAM)); self.custom.entry(Sysno::socket) .or_insert_with(Vec::new) .push(rule); @@ -271,7 +263,6 @@ impl Networking { /// Allow a running Unix server to continue running. Does not allow `socket` or `bind`, /// preventing new sockets from being created. - #[must_use] pub fn allow_running_unix_servers(mut self) -> Networking { self.allowed.extend(NET_IO_SYSCALLS); self.allowed.extend(NET_READ_SYSCALLS); @@ -285,7 +276,6 @@ impl Networking { /// /// This is technically the same as /// [`allow_running_unix_servers`](Self::allow_running_unix_servers). - #[must_use] pub fn allow_running_unix_clients(mut self) -> Networking { self.allowed.extend(NET_IO_SYSCALLS); self.allowed.extend(NET_READ_SYSCALLS); diff --git a/src/builtins/systemio.rs b/src/builtins/systemio.rs index 26f21ac..d332cde 100644 --- a/src/builtins/systemio.rs +++ b/src/builtins/systemio.rs @@ -4,7 +4,6 @@ use std::collections::{HashSet, HashMap}; use std::fs::File; use std::os::unix::io::AsRawFd; -use libseccomp::*; use syscalls::Sysno; use crate::{RuleSet, SeccompRule}; @@ -26,6 +25,7 @@ const IO_UNLINK_SYSCALLS: &[Sysno] = &[Sysno::unlink, Sysno::unlinkat]; /// A [`RuleSet`] representing syscalls that perform IO - open/close/read/write/seek/stat. /// /// Configurable to allow subsets of IO syscalls and specific fds. +#[must_use] pub struct SystemIO { /// Syscalls that are allowed allowed: HashSet, @@ -35,7 +35,6 @@ pub struct SystemIO { impl SystemIO { /// By default, allow no IO syscalls. - #[must_use] pub fn nothing() -> SystemIO { SystemIO { allowed: HashSet::new(), @@ -44,7 +43,6 @@ impl SystemIO { } /// Allow all IO syscalls. - #[must_use] pub fn everything() -> SystemIO { SystemIO::nothing() .allow_read() @@ -58,7 +56,6 @@ impl SystemIO { /// Allow `read` syscalls. - #[must_use] pub fn allow_read(mut self) -> SystemIO { self.allowed.extend(IO_READ_SYSCALLS); @@ -66,7 +63,6 @@ impl SystemIO { } /// Allow `write` syscalls. - #[must_use] pub fn allow_write(mut self) -> SystemIO { self.allowed.extend(IO_WRITE_SYSCALLS); @@ -74,7 +70,6 @@ impl SystemIO { } /// Allow `unlink` syscalls. - #[must_use] pub fn allow_unlink(mut self) -> SystemIO { self.allowed.extend(IO_UNLINK_SYSCALLS); @@ -88,7 +83,6 @@ impl SystemIO { /// The reason this function returns a [`YesReally`] is because it's easy to accidentally combine /// it with another ruleset that allows `write` - for example the Network ruleset - even if you /// only want to read files. Consider using `allow_open_directories()` or `allow_open_files()`. - #[must_use] pub fn allow_open(mut self) -> YesReally { self.allowed.extend(IO_OPEN_SYSCALLS); @@ -100,7 +94,6 @@ impl SystemIO { /// Note that the `openat2` syscall (which is not exposed by glibc anyway according to the /// syscall manpage, and so probably isn't very common) is not supported here because it has a /// separate configuration struct instead of a flag bitset. - #[must_use] pub fn allow_open_readonly(mut self) -> SystemIO { const O_WRONLY: u64 = libc::O_WRONLY as u64; const O_RDWR: u64 = libc::O_RDWR as u64; @@ -117,13 +110,13 @@ impl SystemIO { // flags are the second argument for open but the third for openat let rule = SeccompRule::new(Sysno::open) - .and_condition(scmp_cmp!($arg1 & WRITECREATE == 0)); + .and_condition(seccomp_arg_filter!(arg1 & WRITECREATE == 0)); self.custom.entry(Sysno::open) .or_insert_with(Vec::new) .push(rule); let rule = SeccompRule::new(Sysno::openat) - .and_condition(scmp_cmp!($arg2 & WRITECREATE == 0)); + .and_condition(seccomp_arg_filter!(arg2 & WRITECREATE == 0)); self.custom.entry(Sysno::openat) .or_insert_with(Vec::new) .push(rule); @@ -132,7 +125,6 @@ impl SystemIO { } /// Allow `stat` syscalls. - #[must_use] pub fn allow_metadata(mut self) -> SystemIO { self.allowed.extend(IO_METADATA_SYSCALLS); @@ -140,7 +132,6 @@ impl SystemIO { } /// Allow `ioctl` and `fcntl` syscalls. - #[must_use] pub fn allow_ioctl(mut self) -> SystemIO { self.allowed.extend(IO_IOCTL_SYSCALLS); @@ -148,7 +139,6 @@ impl SystemIO { } /// Allow `close` syscalls. - #[must_use] pub fn allow_close(mut self) -> SystemIO { self.allowed.extend(IO_CLOSE_SYSCALLS); @@ -156,10 +146,9 @@ impl SystemIO { } /// Allow reading from stdin - #[must_use] pub fn allow_stdin(mut self) -> SystemIO { let rule = SeccompRule::new(Sysno::read) - .and_condition(scmp_cmp!($arg0 == 0)); + .and_condition(seccomp_arg_filter!(arg0 == 0)); self.custom.entry(Sysno::read) .or_insert_with(Vec::new) .push(rule); @@ -168,10 +157,9 @@ impl SystemIO { } /// Allow writing to stdout - #[must_use] pub fn allow_stdout(mut self) -> SystemIO { let rule = SeccompRule::new(Sysno::write) - .and_condition(scmp_cmp!($arg0 == 1)); + .and_condition(seccomp_arg_filter!(arg0 == 1)); self.custom.entry(Sysno::write) .or_insert_with(Vec::new) .push(rule); @@ -180,10 +168,9 @@ impl SystemIO { } /// Allow writing to stderr - #[must_use] pub fn allow_stderr(mut self) -> SystemIO { let rule = SeccompRule::new(Sysno::write) - .and_condition(scmp_cmp!($arg0 == 2)); + .and_condition(seccomp_arg_filter!(arg0 == 2)); self.custom.entry(Sysno::write) .or_insert_with(Vec::new) .push(rule); @@ -198,19 +185,19 @@ impl SystemIO { /// /// If another file or socket is opened after the file provided to this function is closed, /// it's possible that the fd will be reused and therefore may be read from. - #[must_use] + #[allow(clippy::missing_panics_doc)] pub fn allow_file_read(mut self, file: &File) -> SystemIO { - let fd = file.as_raw_fd(); + let fd = file.as_raw_fd().try_into().expect("provided fd was negative"); for &syscall in IO_READ_SYSCALLS { let rule = SeccompRule::new(syscall) - .and_condition(scmp_cmp!($arg0 == fd.try_into().expect("fd provided was negative"))); + .and_condition(seccomp_arg_filter!(arg0 == fd)); self.custom.entry(syscall) .or_insert_with(Vec::new) .push(rule); } for &syscall in IO_METADATA_SYSCALLS { let rule = SeccompRule::new(syscall) - .and_condition(scmp_cmp!($arg0 == fd.try_into().expect("fd provided was negative"))); + .and_condition(seccomp_arg_filter!(arg0 == fd)); self.custom.entry(syscall) .or_insert_with(Vec::new) .push(rule); @@ -226,11 +213,11 @@ impl SystemIO { /// /// If another file or socket is opened after the file provided to this function is closed, /// it's possible that the fd will be reused and therefore may be written to. - #[must_use] + #[allow(clippy::missing_panics_doc)] pub fn allow_file_write(mut self, file: &File) -> SystemIO { - let fd = file.as_raw_fd(); + let fd = file.as_raw_fd().try_into().expect("provided fd was negative"); let rule = SeccompRule::new(Sysno::write) - .and_condition(scmp_cmp!($arg0 == fd.try_into().expect("fd provided was negative"))); + .and_condition(seccomp_arg_filter!(arg0 == fd)); self.custom.entry(Sysno::write) .or_insert_with(Vec::new) .push(rule); diff --git a/src/builtins/time.rs b/src/builtins/time.rs index e94f0be..2127013 100644 --- a/src/builtins/time.rs +++ b/src/builtins/time.rs @@ -7,6 +7,7 @@ use syscalls::Sysno; use crate::{SeccompRule, RuleSet}; +#[must_use] /// Enable syscalls related to time. pub struct Time { /// Syscalls that are allowed @@ -15,7 +16,6 @@ pub struct Time { impl Time { /// Create a new Time [`RuleSet`] with nothing allowed by default. - #[must_use] pub fn nothing() -> Time { Time { allowed: HashSet::new(), @@ -26,7 +26,6 @@ impl Time { /// [`vDSO`](https://man7.org/linux/man-pages/man7/vdso.7.html) to compute the time directly with /// rdtsc rather than calling the `clock_gettime` syscall, so in most cases you don't need to /// actually enable this. - #[must_use] pub fn allow_gettime(mut self) -> Time { self.allowed .extend([Sysno::clock_gettime, Sysno::clock_getres]); diff --git a/src/lib.rs b/src/lib.rs index fb0750c..4e5b6a6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,9 +1,10 @@ #![deny(non_ascii_idents)] #![deny(unsafe_code)] #![deny(unused_results)] +#![allow(clippy::unwrap_or_default)] // explicit is better than implicit #![allow(clippy::new_without_default)] // Denied in CI -//#![warn(missing_docs)] +#![warn(missing_docs)] #![warn(trivial_casts, trivial_numeric_casts)] //! extrasafe is a library that makes it easy to improve your program's security by selectively @@ -12,44 +13,153 @@ //! See the [`SafetyContext`] struct's documentation and the tests/ and examples/ directories for //! more information on how to use it. -use libseccomp::*; +// Filter is the entire, top-level seccomp filter chain. All SeccompilerRules are or-ed together. +// Vec<(i64, Vec)>, Vec is empty if Rule has no filters. +// Rule is a syscall + multiple argument filters. All argument filters are and-ed together in a +// single Rule. +// ArgumentFilter is a single condition on a single argument +// Comparator is used in an ArgumentFilter to choose the comparison operation +pub use seccompiler::SeccompFilter as SeccompilerFilter; +pub use seccompiler::SeccompRule as SeccompilerRule; +pub use seccompiler::SeccompCondition as SeccompilerArgumentFilter; +pub use seccompiler::Error as SeccompilerError; +pub use seccompiler::SeccompCmpOp as SeccompilerComparator; + +use seccompiler::SeccompAction; pub use syscalls; +#[macro_use] +pub mod macros; +pub use macros::*; + pub mod builtins; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use std::fmt; +#[derive(Debug, Clone, PartialEq)] +/// A restriction on the arguments of a syscall. May be combined with other +/// [`SeccompArgumentFilter`] as part of a single [`SeccompRule`], in which case they are and-ed +/// together and must all return true for the syscall to be allowed. +/// +/// Because some syscalls take 32 bit arguments which may or may not be sign-extended to 64 bits +/// when passed to the linux kernel, there is an option to indicate whether the argument is 32 or +/// 64 bits. It shouldn't need to be used frequently. +/// See for more details +/// # Examples +/// +/// ``` +/// # use extrasafe::*; +/// # const SOCK_STREAM: u64 = libc::SOCK_STREAM as u64; +/// # const AF_INET: u64 = libc::AF_INET as u64; +/// // if syscall (specified elsewhere) is `read`, allow reading from stdin +/// seccomp_arg_filter!(arg0 == 1); +/// // if syscall is `socket`, allow IPV4 sockets only +/// seccomp_arg_filter!(arg0 & AF_INET == AF_INET); +/// // if syscall is `socket`, allow TCP sockets only +/// seccomp_arg_filter!(arg0 & SOCK_STREAM == SOCK_STREAM); +/// ``` +/// +/// You should use the [`seccomp_arg_filter!`] macros to create these. +pub struct SeccompArgumentFilter { + /// Which syscall argument to filter. Starts at 0 for the first argument. + pub arg_idx: u8, + /// What operation should be used to compare to the user-provided value. + comparator: SeccompilerComparator, + /// The user-provided value to compare the argument against. + pub value: u64, + /// Whether the argument is 64 bits or 32 bits. See the docstring for why this is needed. + pub is_64bit: bool, +} + +impl SeccompArgumentFilter { + #[must_use] + /// Create a new [`SeccompArgumentFilter`]. You should probably use the [`seccomp_arg_filter!`] + /// instead. + pub fn new(arg_idx: u8, comparator: SeccompilerComparator, value: u64) -> SeccompArgumentFilter { + // TODO: add quirks mode file and check whether syscall's parameter at index `arg_idx` is + // 32 or 64 bit (and also I guess if it even has that many arguments) + SeccompArgumentFilter::new64(arg_idx, comparator, value) + } + + #[must_use] + /// Create a new [`SeccompArgumentFilter`] that checks all 64 bits of the provided argument. + /// You should probably use the [`seccomp_arg_filter!`] instead. + pub fn new64(arg_idx: u8, comparator: SeccompilerComparator, value: u64) -> SeccompArgumentFilter { + SeccompArgumentFilter { + arg_idx, + comparator, + value, + is_64bit: true, + } + } + + #[must_use] + /// Create a new [`SeccompArgumentFilter`] that checks 32 bits of the provided argument. + /// You should probably use the [`seccomp_arg_filter!`] instead. See the struct's documentation + /// for why this is needed. + pub fn new32(arg_idx: u8, comparator: SeccompilerComparator, value: u32) -> SeccompArgumentFilter { + // Note that it doesn't matter if we convert with or without sign extension here since the + // point is that we'll only compare the least significant 32 bits anyway. + let value = u64::from(value); + SeccompArgumentFilter { + arg_idx, + comparator, + value, + is_64bit: false, + } + } + + pub(crate) fn into_seccompiler(self) -> Result { + use seccompiler::SeccompCmpArgLen; + let arg_len = if self.is_64bit { SeccompCmpArgLen::Qword } else { SeccompCmpArgLen::Dword }; + Ok(SeccompilerArgumentFilter::new(self.arg_idx, arg_len, + self.comparator, self.value)?) + } +} + #[derive(Debug, Clone)] +#[must_use] /// A seccomp rule. pub struct SeccompRule { /// The syscall being filtered pub syscall: syscalls::Sysno, - // Yes, technically this is not the correct usage of "comparators" but it's fine. - /// Comparisons applied to the syscall's args. The SeccompRule allows the syscall if all comparators - /// evaluate to true. - pub comparators: Vec, + /// Filters on the syscall's arguments. The SeccompRule allows the syscall if all argument + /// filters evaluate to true. + pub argument_filters: Vec, } impl SeccompRule { /// Constructs a new [`SeccompRule`] that unconditionally allows the given syscall. - #[must_use] pub fn new(syscall: syscalls::Sysno) -> SeccompRule { SeccompRule { syscall, - comparators: Vec::new(), + argument_filters: Vec::new(), } } /// Adds a condition to the [`SeccompRule`] which must evaluate to true in order for the syscall to be /// allowed. - #[must_use] - pub fn and_condition(mut self, comparator: ScmpArgCompare) -> SeccompRule { - self.comparators.push(comparator); + pub fn and_condition(mut self, argument_filter: SeccompArgumentFilter) -> SeccompRule { + self.argument_filters.push(argument_filter); self } + + /// Convert an extrasafe `SeccompRule` to a seccompiler `SeccompilerRule`. Seccompiler's rules + /// require that at least one `ArgumentFilter`, so if we have a "simple rule" in extrasafe + /// terminology, we return `Option::None`. + pub(crate) fn into_seccompiler(self) -> Result, ExtraSafeError> { + if self.argument_filters.is_empty() { + return Ok(None); + } + + let argument_filters: Vec = self.argument_filters.into_iter() + .map(SeccompArgumentFilter::into_seccompiler).collect::>()?; + + Ok(Some(SeccompilerRule::new(argument_filters)?)) + } } #[derive(Debug, Clone)] @@ -98,6 +208,8 @@ impl RuleSet for &T { pub struct SafetyContext { /// May either be a single simple rule or multiple conditional rules, but not both. rules: HashMap>, + /// The errno returned when a syscall does not match one of the seccomp rules. Defaults to 1. + errno: u32, } impl SafetyContext { @@ -105,16 +217,19 @@ impl SafetyContext { /// [`apply_to_current_thread`](Self::apply_to_current_thread) or /// [`apply_to_all_threads`](Self::apply_to_all_threads) is called. pub fn new() -> SafetyContext { - #[cfg(not(target_arch = "x86_64"))] - { - compile_error!("Extrasafe currently only supports the x86_64 architecture. You will likely see other errors about Sysno enum variants not existing; this is why."); - } - SafetyContext { rules: HashMap::new(), + errno: 1, } } + /// Set the errno to the provided value when a syscall does not match one of the seccomp rules + /// in this `SafetyContext`. + pub fn with_errno(mut self, errno: u32) -> SafetyContext { + self.errno = errno; + self + } + /// Gather unconditional and conditional rules to be provided to the seccomp context. #[allow(clippy::needless_pass_by_value)] fn gather_rules(rules: impl RuleSet) -> Vec { @@ -153,8 +268,8 @@ impl SafetyContext { for labeled_existing_rule in existing_rules { let existing_rule = &labeled_existing_rule.1; - let new_is_simple = new_rule.comparators.is_empty(); - let existing_is_simple = existing_rule.comparators.is_empty(); + let new_is_simple = new_rule.argument_filters.is_empty(); + let existing_is_simple = existing_rule.argument_filters.is_empty(); // if one rule is conditional and the other is simple, let the user know there // would be a conflict and raise an error. @@ -209,40 +324,53 @@ impl SafetyContext { self.apply(true) } + /// Actually do the application of the rules. If `all_threads` is True, applies the rules to + /// all threads via seccomp tsync. + #[allow(unsafe_code)] fn apply(mut self, all_threads: bool) -> Result<(), ExtraSafeError> { - // This guard will not currently ever be hit because libseccomp-rs will fail to build - // before we get here. If we ever move off of it or if libseccomp-rs decides to do a - // no-op build on non-linux platform, having this guard here means end users will still - // have to explicitly acknowledge that extrasafe isn't running on that platform. - if cfg!(not(target_os = "linux")) { - return Err(ExtraSafeError::UnsupportedOSError); - } + self = self.enable(builtins::BasicCapabilities)?; - let mut ctx = ScmpFilterContext::new_filter(ScmpAction::Errno(libc::EPERM))?; + // Turn our internal HashMap into a BTreeMap for seccompiler, being careful to avoid + // https://github.com/rust-vmm/seccompiler/issues/42 i.e. don't use BTreeMap's collect impl + // because it will ignore duplicates. - if all_threads { - ctx.set_filter_attr(ScmpFilterAttr::CtlTsync, 1)?; - } - else { - // this is the default but we set it just to be sure. - ctx.set_filter_attr(ScmpFilterAttr::CtlTsync, 0)?; - } + let mut rules_map: BTreeMap> = BTreeMap::new(); - // We don't have to care if the architecture was already added. - // And since this is a new `ScmpFilterContext()`, it should not even be possible. - let _: bool = ctx.add_arch(ScmpArch::Native)?; + for (syscall, labeled_rules) in self.rules { + let syscall = syscall.id().into(); - self = self.enable(builtins::BasicCapabilities)?; - for LabeledSeccompRule(_origin, rule) in self.rules.into_values().flatten() { - if rule.comparators.is_empty() { - ctx.add_rule(ScmpAction::Allow, rule.syscall.id())?; - } - else { - ctx.add_rule_conditional(ScmpAction::Allow, rule.syscall.id(), &rule.comparators)?; + let mut seccompiler_rules = Vec::new(); + for LabeledSeccompRule(_origin, rule) in labeled_rules { + // If there are conditional rules, insert them to the vec + if let Some(seccompiler_rule) = rule.into_seccompiler()? { + seccompiler_rules.push(seccompiler_rule); + } + // otherwise, keep the vec empty, which indicates to seccompiler that the syscall + // should be allowed without restriction } + let result = rules_map.insert(syscall, seccompiler_rules); + assert!(result.is_none(), "extrasafe logic error: somehow inserted the same syscall's rules twice"); } - ctx.load()?; + + #[cfg(not(all(target_arch = "x86_64", target_os = "linux")))] + compile_error!("extrasafe is currently only supported on linux x86_64"); + + let seccompiler_filter = SeccompilerFilter::new( + rules_map, + SeccompAction::Errno(self.errno), + SeccompAction::Allow, + std::env::consts::ARCH.try_into().expect("invalid arches are prevented above"), + )?; + + let bpf_filter: seccompiler::BpfProgram = seccompiler_filter.try_into()?; + + if all_threads { + seccompiler::apply_filter_all_threads(&bpf_filter)?; + } + else { + seccompiler::apply_filter(&bpf_filter)?; + } Ok(()) } @@ -251,41 +379,43 @@ impl SafetyContext { #[derive(Debug)] /// The error type produced by [`SafetyContext`] pub enum ExtraSafeError { - /// Error created when trying to apply filters on non-Linux operating systems. Should never - /// occur. - UnsupportedOSError, /// Error created when a simple Seccomp rule would override a conditional rule, or when trying to add a /// conditional rule when there's already a simple rule with the same syscall. ConditionalNoEffectError(syscalls::Sysno, &'static str, &'static str), /// An error from the underlying seccomp library. - SeccompError(libseccomp::error::SeccompError), + SeccompError(SeccompilerError), } impl fmt::Display for ExtraSafeError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Self::UnsupportedOSError => f.write_str("extrasafe is only usable on Linux."), &Self::ConditionalNoEffectError(sysno, a, b) => write!( f, "A conditional rule on syscall `{}` from RuleSet `{}` would be overridden \ by a simple rule from RuleSet `{}`.", sysno, a, b, ), - Self::SeccompError(err) => write!(f, "A libseccomp error occured {:?}", err), + Self::SeccompError(err) => write!(f, "Another seccomp error occured {:?}", err), } } } -impl From for ExtraSafeError { - fn from(value: libseccomp::error::SeccompError) -> Self { +impl From for ExtraSafeError { + fn from(value: SeccompilerError) -> Self { Self::SeccompError(value) } } +impl From for ExtraSafeError { + fn from(value: seccompiler::BackendError) -> Self { + Self::SeccompError(SeccompilerError::from(value)) + } +} + impl std::error::Error for ExtraSafeError { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { match self { - Self::UnsupportedOSError | Self::ConditionalNoEffectError(..) => None, + Self::ConditionalNoEffectError(..) => None, Self::SeccompError(err) => Some(err), } } diff --git a/src/macros.rs b/src/macros.rs new file mode 100644 index 0000000..242c645 --- /dev/null +++ b/src/macros.rs @@ -0,0 +1,126 @@ +//! Macros for extrasafe + +// Heavily inspired by the libseccomp-rs macros, but written from scratch. + +/// A macro to easily create [`crate::SeccompArgumentFilter`]s. Note that because internally it uses a +/// helper macro, to use this macro you should just `use extrasafe::*` if possible. +/// Usage: +/// ``` +/// use extrasafe::*; +/// // usage: `seccomp_arg_filter!( );` +/// // or `seccomp_arg_filter!( & == );` +/// // arg0 through arg5 are supported +/// // operations <=, <, >=, >, ==, != are supported +/// let argfilter = seccomp_arg_filter!(arg0 < 5); +/// // Masked equality is also supported to check specific bits are set. +/// // The following checks the second bit of the syscall's 4th argument is set. +/// let argfilter = seccomp_arg_filter!(arg4 & 0b10 == 0b10); +/// ``` +#[macro_export] +macro_rules! seccomp_arg_filter { + ($argno:ident <= $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::Le, $value) + }; + ($argno:ident < $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::Lt, $value) + }; + ($argno:ident >= $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::Ge, $value) + }; + ($argno:ident > $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::Gt, $value) + }; + ($argno:ident == $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::Eq, $value) + }; + ($argno:ident != $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::Ne, $value) + }; + ($argno:ident & $mask:tt == $value:expr) => { + $crate::SeccompArgumentFilter::new(match_argno!($argno), $crate::SeccompilerComparator::MaskedEq($mask), $value) + }; + ($_other:expr) => {compile_error!("usage: `arg[0-5] {<=, <, >=, >, ==, !=} ` or `arg[0-5] & == `")}; +} + +#[doc(hidden)] +#[macro_export] +/// Internal macro for `seccomp_arg_filter!` +macro_rules! match_argno { + (arg0) => {0}; + (arg1) => {1}; + (arg2) => {2}; + (arg3) => {3}; + (arg4) => {4}; + (arg5) => {5}; + ($_other:expr) => {compile_error!("Seccomp argument filters must start with argX where X is 0-5")}; +} + +/// These tests just test that the macro expands correctly, not that the comparators do what they +/// say they do in seccompiler. +#[cfg(test)] +mod tests { + use crate::*; + + #[test] + fn test_comparison_le() { + let cmp = seccomp_arg_filter!(arg0 <= 10); + assert_eq!( + cmp, + SeccompArgumentFilter::new(0, SeccompilerComparator::Le, 10) + ); + } + + #[test] + fn test_comparison_lt() { + let cmp = seccomp_arg_filter!(arg1 < 10); + assert_eq!( + cmp, + SeccompArgumentFilter::new(1, SeccompilerComparator::Lt, 10) + ); + } + + #[test] + fn test_comparison_ge() { + let cmp = seccomp_arg_filter!(arg2 >= 5); + assert_eq!( + cmp, + SeccompArgumentFilter::new(2, SeccompilerComparator::Ge, 5) + ); + } + + #[test] + fn test_comparison_gt() { + let cmp = seccomp_arg_filter!(arg3 > 200); + assert_eq!( + cmp, + SeccompArgumentFilter::new(3, SeccompilerComparator::Gt, 200) + ); + } + + #[test] + fn test_comparison_eq() { + let cmp = seccomp_arg_filter!(arg4 == 0); + assert_eq!( + cmp, + SeccompArgumentFilter::new(4, SeccompilerComparator::Eq, 0) + ); + } + + #[test] + fn test_comparison_ne() { + let cmp = seccomp_arg_filter!(arg5 != 80); + assert_eq!( + cmp, + SeccompArgumentFilter::new(5, SeccompilerComparator::Ne, 80) + ); + } + + #[test] + fn test_comparison_mask() { + let cmp = seccomp_arg_filter!(arg2 & 0x1337 == 0x37); + assert_eq!( + cmp, + SeccompArgumentFilter::new(2, SeccompilerComparator::MaskedEq(0x1337), 0x37) + ); + } +} diff --git a/tests/arg_comparisons.rs b/tests/arg_comparisons.rs new file mode 100644 index 0000000..fa9f328 --- /dev/null +++ b/tests/arg_comparisons.rs @@ -0,0 +1,152 @@ +#![allow(unsafe_code)] +// allow unsafe to call syscalls directly + +use extrasafe::*; +use builtins::SystemIO; +use syscalls::Sysno; + +use std::collections::HashMap; + +/// Ioctl with the 2nd arg restricted on all 64 bits +struct IoctlRestricted64(u64); +impl RuleSet for IoctlRestricted64 { + fn simple_rules(&self) -> Vec { + Vec::new() + } + + fn conditional_rules(&self) -> HashMap> { + let cmp = SeccompArgumentFilter::new64(1, SeccompilerComparator::Eq, self.0); + let rule = SeccompRule::new(Sysno::ioctl) + .and_condition(cmp); + + HashMap::from([(Sysno::ioctl, vec![rule])]) + } + + fn name(&self) -> &'static str { + "ioctl restricted 64" + } +} + +/// Ioctl but the 2nd arg restricted, but only the least-significant 32 bits +struct IoctlRestricted32(u32); +impl RuleSet for IoctlRestricted32 { + fn simple_rules(&self) -> Vec { + Vec::new() + } + + fn conditional_rules(&self) -> HashMap> { + let cmp = SeccompArgumentFilter::new32(1, SeccompilerComparator::Eq, self.0); + let rule = SeccompRule::new(Sysno::ioctl) + .and_condition(cmp); + + HashMap::from([(Sysno::ioctl, vec![rule])]) + } + + fn name(&self) -> &'static str { + "ioctl restricted 32" + } +} + + +struct GetUidRestricted; +impl RuleSet for GetUidRestricted { + fn simple_rules(&self) -> Vec { + Vec::new() + } + + fn conditional_rules(&self) -> HashMap> { + let rule = SeccompRule::new(Sysno::getuid) + // 10 is arbitrary but it's unlikely that the value of the register ever will happen to + // be 10 + .and_condition(seccomp_arg_filter!(arg0 == 10)); + + HashMap::from([(Sysno::getuid, vec![rule])]) + } + + fn name(&self) -> &'static str { + "getuid restricted" + } +} + +#[test] +/// Try to restrict arguments on syscall that does not receive arguments, does not error and should +/// just fail (unless value in register happens to be that value, which is highly unlikely) +fn cmp_arg_syscall_unused_parameter() { + // SAFETY: getuid just gives the current user's uid + let uid1 = unsafe { libc::getuid() }; + assert!(uid1 > 0); + + extrasafe::SafetyContext::new() + .enable(SystemIO::nothing() + .allow_stdout() + .allow_stderr()).unwrap() + .enable(GetUidRestricted).unwrap() + .apply_to_current_thread().unwrap(); + + // SAFETY: getuid just gives the current user's uid + let uid2 = unsafe { libc::getuid() }; + assert!(uid1 != uid2); +} + +macro_rules! assert_errno { + ($e: expr) => { + let errno = std::io::Error::last_os_error().raw_os_error().unwrap(); + assert_eq!(errno, $e); + } +} + +// See https://github.com/rust-vmm/seccompiler/issues/59 for more details on below two tests +#[test] +fn cmp_arg_64bit_ioctl_musl_glibc_diff() { + let value: u64 = 0x8000_0000; + let seccomp_errno = 999; + extrasafe::SafetyContext::new() + .with_errno(seccomp_errno) + .enable(SystemIO::nothing() + .allow_stdout() + .allow_stderr()).unwrap() + .enable(IoctlRestricted64(value)).unwrap() + .apply_to_current_thread().unwrap(); + + // On glibc, the second parameter is a u64, so the value seen by the kernel matches the value + // in our seccomp filter, and the ioctl call is allowed. It then sets errno to -9 since 4321 is + // not a valid fd. + #[cfg(target_env = "gnu")] + { + let ret = unsafe { libc::ioctl(4321, value, 0) }; + assert_eq!(ret, -1); + assert_errno!(libc::EBADF); + } + // On musl, the second parameter is an i32 and the value gets signed extended by musl when + // passed to the kernel. It's therefore different than the one we pass in our seccomp filter, + // so the ioctl call doesn't match and seccomp makes the return value of ioctl itself -999 + #[cfg(target_env = "musl")] + { + let ret = unsafe { libc::ioctl(4321, value as i32, 0) }; + assert_eq!(ret, -1); + assert_errno!(seccomp_errno as i32); // apparently errno is also i32 on musl + } +} + +#[test] +fn cmp_arg_32bit_ioctl_musl_glibc_same() { + let value: u32 = 0x8000_0000; + let seccomp_errno = 999; + extrasafe::SafetyContext::new() + .with_errno(seccomp_errno) + .enable(SystemIO::nothing() + .allow_stdout() + .allow_stderr()).unwrap() + .enable(IoctlRestricted32(value)).unwrap() + .apply_to_current_thread().unwrap(); + + // here both tests are the same except for value being i32 on musl + #[cfg(target_env = "gnu")] + let ret = unsafe { libc::ioctl(4321, u64::from(value), 0) }; + + #[cfg(target_env = "musl")] + let ret = unsafe { libc::ioctl(4321, i32::from(value), 0) }; + + assert_eq!(ret, -1); + assert_errno!(libc::EBADF); +} diff --git a/tests/bad_combination.rs b/tests/bad_combination.rs index d323b5f..b44071b 100644 --- a/tests/bad_combination.rs +++ b/tests/bad_combination.rs @@ -78,7 +78,7 @@ fn invalid_combination_new_conditional_different_name() { } #[test] -/// Test that adding a conditional and simple rule in the same RuleSet produces an error +/// Test that adding a conditional and simple rule in the same `RuleSet` produces an error fn invalid_combination_read_and_stdin() { let res = extrasafe::SafetyContext::new() @@ -93,7 +93,7 @@ fn invalid_combination_read_and_stdin() { } #[test] -/// Test that adding duplicate simple rules in the same RuleSet doesn't produce an error +/// Test that adding duplicate simple rules in the same `RuleSet` doesn't produce an error fn not_invalid_combination_duplicate_simple() { let res = extrasafe::SafetyContext::new() @@ -108,7 +108,7 @@ fn not_invalid_combination_duplicate_simple() { } #[test] -/// Test that adding duplicate simple rules in the same RuleSet doesn't produce an error +/// Test that adding duplicate simple rules in the same `RuleSet` doesn't produce an error fn not_invalid_combination_duplicate_simple2() { let res = extrasafe::SafetyContext::new() @@ -124,7 +124,7 @@ fn not_invalid_combination_duplicate_simple2() { } #[test] -/// Test that adding duplicate conditional rules in the same RuleSet doesn't produce an error +/// Test that adding duplicate conditional rules in the same `RuleSet` doesn't produce an error fn not_invalid_combination_duplicate_conditional() { let res = extrasafe::SafetyContext::new() @@ -139,7 +139,7 @@ fn not_invalid_combination_duplicate_conditional() { } #[test] -/// Test that adding duplicate conditional rules in the same RuleSet doesn't produce an error +/// Test that adding duplicate conditional rules from different `RuleSet`s doesn't produce an error fn not_invalid_combination_duplicate_conditional2() { let res = extrasafe::SafetyContext::new() diff --git a/tests/inherit_filters.rs b/tests/inherit_filters.rs index 360534c..fdf5ca2 100644 --- a/tests/inherit_filters.rs +++ b/tests/inherit_filters.rs @@ -40,6 +40,8 @@ fn new_process_inherits_restrictions() { .apply_to_current_thread() .unwrap(); + // Note that this actually fails not because of the cat but because the new process is not even + // allowed to open ld or glibc let res = std::process::Command::new("cat") .arg("/proc/cpuinfo") .status(); diff --git a/tests/multiple_conditions.rs b/tests/multiple_conditions.rs new file mode 100644 index 0000000..298ff34 --- /dev/null +++ b/tests/multiple_conditions.rs @@ -0,0 +1,22 @@ +use std::io::Write; + +use extrasafe::*; + +#[test] +/// Test that if multiple `RuleSets` have conditional rules, any of them will work i.e. they are +/// or-ed together across all `RuleSets`. +fn multiple_rulsets_conditional() { + SafetyContext::new() + .enable(builtins::SystemIO::nothing() + .allow_stdout() + ).unwrap() + .enable(builtins::SystemIO::nothing() + .allow_stderr() + ).unwrap() + .apply_to_current_thread().unwrap(); + + let res = writeln!(std::io::stdout(), "we can print to stdout"); + assert!(res.is_ok(), "failed to write to stdout: {:?}", res.unwrap_err()); + let res = writeln!(std::io::stderr(), "we can print to stderr"); + assert!(res.is_ok(), "failed to write to stderr: {:?}", res.unwrap_err()); +} diff --git a/tests/ruleset_union.rs b/tests/ruleset_union.rs new file mode 100644 index 0000000..2567bea --- /dev/null +++ b/tests/ruleset_union.rs @@ -0,0 +1,96 @@ +use std::path::Path; +use std::fs::File; +use std::io::{Read, Write}; + +use extrasafe::*; +use builtins::SystemIO; + + +// Tests to make sure we don't run into this issue +// https://github.com/rust-vmm/seccompiler/issues/42 +// which we shouldn't (by design) + +#[test] +/// Test multiple uses of the same syscall across different `RuleSets` don't cancel each other out. +fn different_rulesets_same_syscall() { + SafetyContext::new() + // First RuleSet: stdout, stderr + .enable(SystemIO::nothing() + .allow_read() + .allow_stdout() + .allow_stderr() + .allow_metadata() + ).unwrap() + .enable( + // Second RuleSet: stderr only + SystemIO::nothing() + .allow_stderr() + .allow_metadata() + .allow_close(), + ) + .unwrap() + .apply_to_current_thread() + .unwrap(); + + // Try to write to stdout and stderr + let res = writeln!(std::io::stdout(), "we can print to stdout"); + assert!(res.is_ok(), "failed to write to stdout: {:?}", res.unwrap_err()); + let res = writeln!(std::io::stderr(), "we can print to stderr"); + assert!(res.is_ok(), "failed to write to stderr: {:?}", res.unwrap_err()); +} + +fn create_testfile(path: &Path, filename: &str) -> File { + let path = path.join(filename); + let mut file = File::create(&path).unwrap(); + file.write_all(filename.as_bytes()).unwrap(); + // sync and close + file.sync_all().unwrap(); + drop(file); + + // reopen for reading + File::open(&path).unwrap() +} + + +#[test] +/// Same as above but with mask instead of == and also 3 rulesets +fn different_rulesets_same_syscall2() { + // create tempdir + let dir = tempfile::tempdir().unwrap(); + + // Create 3 files, write to them + + let path = dir.path().to_path_buf(); + + let mut file1 = create_testfile(&path, "testfile1.txt"); + let mut file2 = create_testfile(&path, "testfile2.txt"); + let mut file3 = create_testfile(&path, "testfile3.txt"); + + // Add three different rulesets each allowing reads to a different file + SafetyContext::new() + .enable(SystemIO::nothing() + .allow_stdout() + .allow_stderr() + ).unwrap() + .enable(SystemIO::nothing() + .allow_file_read(&file1) + ).unwrap() + .enable(SystemIO::nothing() + .allow_file_read(&file2) + ).unwrap() + .enable(SystemIO::nothing() + .allow_file_read(&file3) + ).unwrap() + .apply_to_current_thread() + .unwrap(); + + let mut s = String::new(); + let res = file1.read_to_string(&mut s); + assert!(res.is_ok(), "Failed to read file1: {:?}", res.unwrap_err()); + let res = file2.read_to_string(&mut s); + assert!(res.is_ok(), "Failed to read file2: {:?}", res.unwrap_err()); + let res = file3.read_to_string(&mut s); + assert!(res.is_ok(), "Failed to read file3: {:?}", res.unwrap_err()); + + assert_eq!(s, "testfile1.txttestfile2.txttestfile3.txt"); +} diff --git a/tests/tests_can_fail.rs b/tests/tests_can_fail.rs new file mode 100644 index 0000000..b4f5872 --- /dev/null +++ b/tests/tests_can_fail.rs @@ -0,0 +1,13 @@ +#[test] +#[should_panic] +#[allow(clippy::assertions_on_constants)] +/// Test that even if everything (besides default enabled syscalls) is denied with seccomp, tests can fail +/// This is also manually tested by commenting out the assert line and checking that the test +/// failure propagates to the cli +fn seccomp_active_tests_fail() { + let res = extrasafe::SafetyContext::new() + .apply_to_current_thread(); + assert!(res.is_ok(), "Extrasafe failed {:?}", res.unwrap_err()); + + assert!(false, "should fail"); +} diff --git a/todo.txt b/todo.txt index 593ebe1..6684d14 100644 --- a/todo.txt +++ b/todo.txt @@ -1,3 +1,39 @@ +# New functionality + +## Other "sandboxing" features +https://chromium.googlesource.com/chromium/src.git/+/HEAD/docs/linux/sandboxing.md#User-namespaces-sandbox + +User namespaces option? Something like: + +rust +``` +SafetyContext::new() +.isolate_process() +``` + +using unshare maybe? https://docs.kernel.org/userspace-api/unshare.html unshare is a bit tricky because e.g. newpid doesn't actually unshare the current thread, only its children. so maybe it would be better with a wrapper around creating a thread like ctx.run below. + +Maybe even a macro similar to tokio::main? is something like + +``` +fn my_context() -> SafetyContext { +} + +#[extrasafe::main(my_context)] +fn main() { + foo(); + //... +} +``` + +possible? it should ideally also work transparently with tokio::main as well as long as you put it first + +see also https://blog.lizzie.io/linux-containers-in-500-loc.html + +## Simple mode + +Something either like "Network::everything()" (rather than Network::nothing etc) and "SystemIO::everything" or a separate wrapper over the existing RuleSet mechanism. + # Nice to haves - more builtin profiles @@ -32,35 +68,8 @@ see https://github.com/servo/gaol/ and chrome? -# New functionality -https://chromium.googlesource.com/chromium/src.git/+/HEAD/docs/linux/sandboxing.md#User-namespaces-sandbox - -User namespaces option? Something like: - -rust -``` -SafetyContext::new() -.isolate_process() -``` - -see also https://blog.lizzie.io/linux-containers-in-500-loc.html - # Remove all dependencies If you're using extrasafe to provide extra security, it then becomes a target for vulnerabilities, including supply-chain attacks. -### libseccomp -This is the hardest part to remove as we'd have to either rewrite the bpf generator, or record the output of libseccomp for our specific use-cases and commit it to the repository. For cases like allowing lists of fds to be read from/written to, we'd have to do some additional work to template the generated bpf code, which is essentially what libseccomp is already doing. - -There are actually two levels of dependencies here: [The Rust bindings](https://github.com/libseccomp-rs/libseccomp-rs) and [the actual C library](https://github.com/seccomp/libseccomp) - -maybe use the code from gaol? -https://github.com/servo/gaol/ -https://github.com/servo/gaol/blob/4544946c5c922ee619a932ae5c3fd1d26f143384/platform/linux/seccomp.rs#L217 - -also can consider seccompiler, which at least brings the code into pure rust -and possibly gives us more flexibility on filtering syscall arguments, which is -a weakness of libseccomp -https://crates.io/crates/seccompiler - ### syscalls Relatively easy to remove by copying directly into the repository, but comes with a maintenance burden of having to update the lists when new syscalls are created. diff --git a/user-guide.md b/user-guide.md index d8e11b3..c8b8b7e 100644 --- a/user-guide.md +++ b/user-guide.md @@ -90,8 +90,7 @@ If you want to use syscalls that aren't included in any of the builtin rulesets, In the meantime, you can create your own: ```rust -use extrasafe::{SeccompRule, RuleSet}; -use libseccomp::scmp_cmp; +use extrasafe::*; use syscalls::Sysno; use std::collections::HashMap; @@ -110,7 +109,7 @@ impl RuleSet for MyRuleSet { let rule = SeccompRule::new(Sysno::socket) .and_condition( - scmp_cmp!($arg0 & SOCK_STREAM == SOCK_STREAM)); + seccomp_arg_filter!(arg0 & SOCK_STREAM == SOCK_STREAM)); HashMap::from([ (Sysno::socket, vec![rule,]) ]) @@ -127,7 +126,7 @@ extrasafe::SafetyContext::new() .apply_to_current_thread().unwrap(); ``` -See the [libseccomp rust bindings documentation](https://docs.rs/libseccomp/latest/libseccomp/macro.scmp_cmp.html) for more information on how to use the comparator generator macro. +See the [extrasafe documentation](https://docs.rs/extrasafe/latest/macro.seccomp_arg_filter.html) for more information on how to use the comparator generator macro. Currently [the syscalls crate's](https://crates.io/crates/syscalls) [`Sysno` enum](https://docs.rs/syscalls/latest/syscalls/enum.Sysno.html) is used in the `RuleSet` interface. It's convenient because the enum is defined separately for each target architecture such that the syscall gets mapped to the correct syscall number (which may differ on different architectures).