diff --git a/docs/design-per-rule-actions.md b/docs/design-per-rule-actions.md new file mode 100644 index 000000000..d320ec3dd --- /dev/null +++ b/docs/design-per-rule-actions.md @@ -0,0 +1,25 @@ +# Per-rule actions + NEW_LISTENER: design notes + +## `UserNotif` is unit + +`SECCOMP_RET_USER_NOTIF` return-data bits are ignored by the kernel; the +supervisor's `seccomp_notif` carries a kernel-assigned `id` and the syscall +args, never the filter's return data. A `UserNotif(u32)` would be a tag the +kernel discards, so the unit variant is the honest representation. + +## Open questions + +- **`IdenticalActions` over-strict.** `match == mismatch` is rejected even when + every rule overrides; relax or drop the check. +- **`apply_filter_with_listener` duplicates `apply_filter_with_flags`.** The rc + contract differs (positive rc = listener fd, not a TSYNC offender); could + share a helper returning the raw `c_long`. +- **`rc > 0 -> ThreadSync` footgun in `apply_filter_with_flags`.** Unreachable + today (no public caller passes `NEW_LISTENER`); guard if that changes. +- **No notification consumer API.** Callers do the ioctls themselves. The + low-level bindings are landing upstream in `libc` + ([#5224](https://github.com/rust-lang/libc/pull/5224)); a high-level safe + supervisor API (TOCTOU-safe memory access, fd injection, dispatch) would be a + separate crate. +- **JSON frontend has no per-rule `action`.** Add an optional key plumbed to + `new_with_action`/`always`. diff --git a/src/backend/filter.rs b/src/backend/filter.rs index e7757a0ae..bc4c3d4dd 100644 --- a/src/backend/filter.rs +++ b/src/backend/filter.rs @@ -129,9 +129,11 @@ impl SeccompFilter { let chain: Vec<_> = chain .into_iter() .map(|rule| { + // A rule may override the filter's match_action with its own action. + let action = rule.action().unwrap_or_else(|| match_action.clone()); let mut bpf: BpfProgram = rule.into(); - // Last statement is the on-match action of the filter. - bpf.push(bpf_stmt(BPF_RET | BPF_K, u32::from(match_action.clone()))); + // Last statement is the on-match action for this rule. + bpf.push(bpf_stmt(BPF_RET | BPF_K, u32::from(action))); bpf }) .collect(); @@ -457,4 +459,129 @@ mod tests { let bpfprog: BpfProgram = filter.try_into().unwrap(); assert_eq!(bpfprog, instructions); } + + #[test] + fn test_per_rule_action_overrides_match_action() { + // syscall 42 takes a per-rule action; syscall 7 falls back to match_action. + let rules: BTreeMap> = [ + (42, vec![SeccompRule::always(SeccompAction::UserNotif)]), + (7, Vec::new()), + ] + .into_iter() + .collect(); + + let filter = SeccompFilter::new( + rules, + SeccompAction::Allow, + SeccompAction::Trace(0), + ARCH.try_into().unwrap(), + ) + .unwrap(); + + let prog: BpfProgram = filter.try_into().unwrap(); + + let user_notif = u32::from(SeccompAction::UserNotif); + let trace = u32::from(SeccompAction::Trace(0)); + let allow = u32::from(SeccompAction::Allow); + + let rets: Vec = prog + .iter() + .filter(|f| f.code == (BPF_RET | BPF_K)) + .map(|f| f.k) + .collect(); + + assert!( + rets.contains(&user_notif), + "expected UserNotif, got {rets:?}" + ); + assert!(rets.contains(&trace), "expected Trace, got {rets:?}"); + assert!(rets.contains(&allow), "expected Allow, got {rets:?}"); + } + + #[test] + fn test_first_matching_rule_wins_its_action() { + let rules: BTreeMap> = [( + 42, + vec![ + SeccompRule::always(SeccompAction::UserNotif), + SeccompRule::always(SeccompAction::Trap), + ], + )] + .into_iter() + .collect(); + + let filter = SeccompFilter::new( + rules, + SeccompAction::Allow, + SeccompAction::Log, + ARCH.try_into().unwrap(), + ) + .unwrap(); + + let prog: BpfProgram = filter.try_into().unwrap(); + let rets: Vec = prog + .iter() + .filter(|f| f.code == (BPF_RET | BPF_K)) + .map(|f| f.k) + .collect(); + + assert!(rets.contains(&u32::from(SeccompAction::UserNotif))); + assert!(rets.contains(&u32::from(SeccompAction::Trap))); + // BPF text can't show runtime selection, so assert RET ordering instead. + let user_notif_pos = rets + .iter() + .position(|&k| k == u32::from(SeccompAction::UserNotif)) + .unwrap(); + let trap_pos = rets + .iter() + .position(|&k| k == u32::from(SeccompAction::Trap)) + .unwrap(); + assert!( + user_notif_pos < trap_pos, + "first rule's action must precede the second's" + ); + } + + #[test] + fn test_conditional_rule_action_emitted() { + // new_with_action: conditions + per-rule action. The rule's UserNotif is + // emitted; the overridden match_action (Log) is absent. + let rules: BTreeMap> = [( + 42, + vec![SeccompRule::new_with_action( + vec![Cond::new(0, ArgLen::Dword, Eq, 7).unwrap()], + SeccompAction::UserNotif, + ) + .unwrap()], + )] + .into_iter() + .collect(); + + let filter = SeccompFilter::new( + rules, + SeccompAction::Allow, + SeccompAction::Log, + ARCH.try_into().unwrap(), + ) + .unwrap(); + + let prog: BpfProgram = filter.try_into().unwrap(); + + assert!( + prog.iter().any(|f| f.code == (BPF_LD | BPF_W | BPF_ABS)), + "expected an argument load" + ); + assert!( + prog.iter().any(|f| { + f.code == (BPF_RET | BPF_K) && f.k == u32::from(SeccompAction::UserNotif) + }), + "expected a UserNotif return" + ); + assert!( + !prog + .iter() + .any(|f| { f.code == (BPF_RET | BPF_K) && f.k == u32::from(SeccompAction::Log) }), + "match_action must not be emitted when overridden" + ); + } } diff --git a/src/backend/rule.rs b/src/backend/rule.rs index b87e62a0d..4876088fb 100644 --- a/src/backend/rule.rs +++ b/src/backend/rule.rs @@ -1,19 +1,23 @@ // Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause -use crate::backend::{bpf::*, condition::SeccompCondition, Error, Result}; +use crate::backend::{bpf::*, condition::SeccompCondition, Error, Result, SeccompAction}; /// Rule that a filter attempts to match for a syscall. /// -/// If all conditions match then rule gets matched. -/// A syscall can have many rules associated. If either of them matches, the `match_action` of the -/// [`SeccompFilter`] is triggered. +/// If all conditions match then the rule is matched. A syscall can have many +/// rules associated; the first match wins. A matched rule with its own +/// [`action`](Self::new_with_action) takes that action, otherwise the +/// `match_action` of the [`SeccompFilter`] is triggered. /// /// [`SeccompFilter`]: struct.SeccompFilter.html #[derive(Clone, Debug, PartialEq, Eq)] pub struct SeccompRule { /// Conditions of rule that need to match in order for the rule to get matched. conditions: Vec, + /// Action to take when this rule matches. `None` falls back to the + /// [`SeccompFilter`]'s `match_action`. + action: Option, } impl SeccompRule { @@ -21,6 +25,9 @@ impl SeccompRule { /// of argument values, map the syscall number to an empty vector of rules when constructing /// the [`SeccompFilter`](super::SeccompFilter) instead. /// + /// On match this rule triggers the filter's `match_action`; for a per-rule + /// action see [`new_with_action`](Self::new_with_action). + /// /// # Arguments /// /// * `conditions` - Vector of [`SeccompCondition`]s that the syscall must match. @@ -39,17 +46,51 @@ impl SeccompRule { /// /// [`SeccompCondition`]: struct.SeccompCondition.html pub fn new(conditions: Vec) -> Result { - let instance = Self { conditions }; + let instance = Self { + conditions, + action: None, + }; instance.validate()?; Ok(instance) } + /// Creates a rule that takes `action` on match, overriding the filter's + /// `match_action`. Requires at least one condition; for an unconditional + /// per-syscall action use [`always`](Self::always). + pub fn new_with_action( + conditions: Vec, + action: SeccompAction, + ) -> Result { + if conditions.is_empty() { + return Err(Error::EmptyRule); + } + let instance = Self { + conditions, + action: Some(action), + }; + instance.validate()?; + + Ok(instance) + } + + /// An unconditional rule: matches the syscall regardless of arguments and + /// takes `action`. + pub fn always(action: SeccompAction) -> Self { + SeccompRule { + conditions: Vec::new(), + action: Some(action), + } + } + + pub(crate) fn action(&self) -> Option { + self.action.clone() + } + /// Performs semantic checks on the SeccompRule. fn validate(&self) -> Result<()> { - // Rules with no conditions are not allowed. Syscalls mappings to empty rule vectors are to - // be used instead, for matching only on the syscall number. - if self.conditions.is_empty() { + // A condition-less rule is only valid via `always` (which sets an action). + if self.conditions.is_empty() && self.action.is_none() { return Err(Error::EmptyRule); } @@ -148,7 +189,7 @@ mod tests { use super::SeccompRule; use crate::backend::bpf::*; use crate::backend::{ - Error, SeccompCmpArgLen as ArgLen, SeccompCmpOp::*, SeccompCondition as Cond, + Error, SeccompAction, SeccompCmpArgLen as ArgLen, SeccompCmpOp::*, SeccompCondition as Cond, }; #[test] @@ -156,6 +197,24 @@ mod tests { assert_eq!(SeccompRule::new(vec![]).unwrap_err(), Error::EmptyRule); } + #[test] + fn test_new_with_action_requires_conditions() { + assert_eq!( + SeccompRule::new_with_action(vec![], SeccompAction::UserNotif).unwrap_err(), + Error::EmptyRule + ); + } + + #[test] + fn test_new_with_action_and_always_carry_action() { + let cond = Cond::new(0, ArgLen::Dword, Eq, 1).unwrap(); + let rule = SeccompRule::new_with_action(vec![cond], SeccompAction::UserNotif).unwrap(); + assert_eq!(rule.action(), Some(SeccompAction::UserNotif)); + + let always = SeccompRule::always(SeccompAction::Trap); + assert_eq!(always.action(), Some(SeccompAction::Trap)); + } + // Checks that rule gets translated correctly into BPF statements. #[test] fn test_rule_bpf_output() { diff --git a/src/lib.rs b/src/lib.rs index a0d122377..69663e527 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -199,6 +199,7 @@ use std::io::Read; use std::collections::HashMap; use std::fmt::{Display, Formatter}; use std::io; +use std::os::unix::io::{FromRawFd, OwnedFd, RawFd}; #[cfg(feature = "json")] use frontend::json::{Error as JsonFrontendError, JsonCompiler}; @@ -325,6 +326,52 @@ pub fn apply_filter_all_threads(bpf_filter: BpfProgramRef) -> Result<()> { apply_filter_with_flags(bpf_filter, libc::SECCOMP_FILTER_FLAG_TSYNC) } +/// Apply a BPF filter with `SECCOMP_FILTER_FLAG_NEW_LISTENER` and return the +/// listener fd (Linux 5.0+). [`SeccompAction::UserNotif`] notifications are +/// consumed via the `SECCOMP_IOCTL_NOTIF_*` ioctls (see `seccomp_unotify(2)`); +/// at least one rule should use UserNotif, or the listener never fires. +/// +/// Unlike [`apply_filter`], a successful call returns the positive listener fd. +/// +/// [`SeccompAction::UserNotif`]: enum.SeccompAction.html#variant.UserNotif +pub fn apply_filter_with_listener(bpf_filter: BpfProgramRef) -> Result { + if bpf_filter.is_empty() { + return Err(Error::EmptyFilter); + } + + // SAFETY: Safe because syscall arguments are valid. + let rc = unsafe { libc::prctl(libc::PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) }; + if rc != 0 { + return Err(Error::Prctl(io::Error::last_os_error())); + } + + let bpf_prog = sock_fprog { + len: bpf_filter.len() as u16, + filter: bpf_filter.as_ptr(), + }; + let bpf_prog_ptr = &bpf_prog as *const sock_fprog; + + // SAFETY: + // Safe because the kernel performs a `copy_from_user` on the filter and leaves the memory + // untouched. We can therefore use a reference to the BpfProgram, without needing ownership. + let rc = unsafe { + libc::syscall( + libc::SYS_seccomp, + libc::SECCOMP_SET_MODE_FILTER, + libc::SECCOMP_FILTER_FLAG_NEW_LISTENER, + bpf_prog_ptr, + ) + }; + + if rc < 0 { + return Err(Error::Seccomp(io::Error::last_os_error())); + } + + // SAFETY: on success seccomp(2) with NEW_LISTENER returns a newly-allocated, + // open file descriptor; we take ownership and close it on drop. + Ok(unsafe { OwnedFd::from_raw_fd(rc as RawFd) }) +} + /// Apply a BPF filter to the calling thread. /// /// # Arguments diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index cb6c1f350..6600c0dca 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -8,8 +8,8 @@ use std::collections::BTreeMap; use seccompiler::SeccompCmpArgLen::*; use seccompiler::SeccompCmpOp::*; use seccompiler::{ - apply_filter, sock_filter, BpfProgram, Error, SeccompAction, SeccompCondition as Cond, - SeccompFilter, SeccompRule, + apply_filter, apply_filter_with_listener, sock_filter, BpfProgram, Error, SeccompAction, + SeccompCondition as Cond, SeccompFilter, SeccompRule, }; use std::convert::TryInto; use std::env::consts::ARCH; @@ -789,3 +789,48 @@ fn test_filter_apply() { .join() .unwrap(); } + +#[test] +fn test_apply_filter_with_listener() { + use std::os::unix::io::{AsRawFd, OwnedFd}; + + // Empty program: rejected without enabling seccomp. + assert_eq!(unsafe { libc::prctl(libc::PR_GET_SECCOMP) }, 0); + assert!(matches!( + apply_filter_with_listener(&Vec::new()).unwrap_err(), + Error::EmptyFilter + )); + assert_eq!(unsafe { libc::prctl(libc::PR_GET_SECCOMP) }, 0); + + // Install in a thread (filters are process-wide). getpid routes to UserNotif + // but is never called, so the thread exits cleanly. Requires Linux 5.0+. + let fd: OwnedFd = thread::spawn(|| { + let rules: BTreeMap> = [( + libc::SYS_getpid, + vec![SeccompRule::always(SeccompAction::UserNotif)], + )] + .into_iter() + .collect(); + + let filter = SeccompFilter::new( + rules, + SeccompAction::Allow, // mismatch: everything else allowed + SeccompAction::Log, // match_action — distinct (passes validate), unused + ARCH.try_into().unwrap(), + ) + .unwrap(); + let prog: BpfProgram = filter.try_into().unwrap(); + + apply_filter_with_listener(&prog).unwrap() + }) + .join() + .unwrap(); + + let raw = fd.as_raw_fd(); + assert!(raw >= 0, "listener fd must be non-negative, got {raw}"); + assert!( + unsafe { libc::fcntl(raw, libc::F_GETFD) } >= 0, + "listener fd {raw} should be a valid open fd" + ); + // OwnedFd closes on drop. +}