Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/design-per-rule-actions.md
Original file line number Diff line number Diff line change
@@ -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`.
131 changes: 129 additions & 2 deletions src/backend/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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<i64, Vec<SeccompRule>> = [
(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<u32> = 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<i64, Vec<SeccompRule>> = [(
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<u32> = 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<i64, Vec<SeccompRule>> = [(
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"
);
}
}
77 changes: 68 additions & 9 deletions src/backend/rule.rs
Original file line number Diff line number Diff line change
@@ -1,26 +1,33 @@
// 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<SeccompCondition>,
/// Action to take when this rule matches. `None` falls back to the
/// [`SeccompFilter`]'s `match_action`.
action: Option<SeccompAction>,
}

impl SeccompRule {
/// Creates a new rule. Rules with 0 conditions are not allowed; to match a syscall regardless
/// 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.
Expand All @@ -39,17 +46,51 @@ impl SeccompRule {
///
/// [`SeccompCondition`]: struct.SeccompCondition.html
pub fn new(conditions: Vec<SeccompCondition>) -> Result<Self> {
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<SeccompCondition>,
action: SeccompAction,
) -> Result<Self> {
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<SeccompAction> {
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);
}

Expand Down Expand Up @@ -148,14 +189,32 @@ 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]
fn test_validate_rule() {
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() {
Expand Down
47 changes: 47 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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<OwnedFd> {
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
Expand Down
Loading