diff --git a/crates/houndd/examples/gate-smoke.rs b/crates/houndd/examples/gate-smoke.rs index ff89f4b..d46d3bf 100644 --- a/crates/houndd/examples/gate-smoke.rs +++ b/crates/houndd/examples/gate-smoke.rs @@ -113,6 +113,9 @@ fn run(mount: &Path) -> bool { Arc::new(|p: &Path, name: &str, v: fanotify::Verdict| { println!(" detect: {} ({name}) {v:?}", p.display()); }), + // No fast path in the smoke test: every event must reach a worker, + // so phase 3 can genuinely stall one. + Arc::new(|_e: &fanotify::Event| None), ); println!("gate armed on {MOUNT} — 1 reader, 4 workers, watchdog live"); std::thread::sleep(Duration::from_millis(100)); diff --git a/crates/houndd/src/caps.rs b/crates/houndd/src/caps.rs index f7ef004..7393c71 100644 --- a/crates/houndd/src/caps.rs +++ b/crates/houndd/src/caps.rs @@ -118,22 +118,41 @@ pub fn retain_only(caps: &[u32]) -> io::Result { } let (lo, hi) = to_words(caps); + let want = lo as u64; - // 1. Bounding set first, while CAP_SETPCAP is still held. Dropping one - // we never had, or one this kernel does not define, returns EINVAL - // and is not interesting; anything else is worth knowing about. + // 1. Bounding set first, while CAP_SETPCAP is still held. + // + // Only capabilities actually PRESENT are attempted. Under systemd + // the unit's CapabilityBoundingSet= has already narrowed the set, + // and the service does not hold CAP_SETPCAP afterwards — so + // blindly dropping all sixty-odd fails EPERM sixty-odd times and + // reports a scary error about work that was already done + // correctly. Checking first makes the operation idempotent, which + // is what it should have been. + let present = bounding_now().unwrap_or(u64::MAX); let mut refused = 0usize; for c in 0..=CAP_LAST_CAP_CEILING { if caps.contains(&c) { continue; } + if c < 64 && present & (1u64 << c) == 0 { + continue; // already gone + } // SAFETY: prctl with a constant option and a capability number. let rc = unsafe { libc::prctl(PR_CAPBSET_DROP, c as libc::c_ulong, 0, 0, 0) }; if rc != 0 && io::Error::last_os_error().raw_os_error() != Some(libc::EINVAL) { refused += 1; } } - if refused > 0 { + // Judge by the end state, not by the return codes: what matters is + // that nothing beyond `caps` remains, however it got that way. + if let Some(after) = bounding_now() { + if after & !want != 0 { + return Err(io::Error::other(format!( + "{refused} capabilities could not be dropped; CapBnd is {after:#x}, wanted {want:#x}" + ))); + } + } else if refused > 0 { return Err(io::Error::other(format!( "{refused} capabilities could not be dropped from the bounding set" ))); diff --git a/crates/houndd/src/engine.rs b/crates/houndd/src/engine.rs index 498a5f1..7e6ed19 100644 --- a/crates/houndd/src/engine.rs +++ b/crates/houndd/src/engine.rs @@ -50,6 +50,16 @@ pub trait ScanEngine: Send + Sync { fn scan_bytes(&self, _bytes: &[u8]) -> Option { None } + + /// Answer from memory alone, without reading the file. + /// + /// `Some(verdict)` means we have judged this exact file version + /// before; `None` means it must be scanned. The execution gate uses + /// this to answer repeat executions without waking a worker, which is + /// most of the traffic on a machine that is actually doing something. + fn cached_verdict(&self, _md: &std::fs::Metadata) -> Option> { + None + } } /// The ClamAV-backed engine: `clamscan` + `freshclam` over their diff --git a/crates/houndd/src/fanotify.rs b/crates/houndd/src/fanotify.rs index 8ab907a..df58004 100644 --- a/crates/houndd/src/fanotify.rs +++ b/crates/houndd/src/fanotify.rs @@ -142,6 +142,19 @@ impl Event { std::fs::read_link(format!("/proc/self/fd/{}", self.fd)).ok() } + /// Full metadata for the descriptor, for cache lookups that must not + /// read the file. + pub fn metadata(&self) -> Option { + use std::os::unix::io::{AsRawFd, FromRawFd}; + // SAFETY: borrowed for the duration of the call and immediately + // forgotten, so the descriptor is not closed twice. + let f = unsafe { std::fs::File::from_raw_fd(self.fd) }; + let md = f.metadata().ok(); + let _ = f.as_raw_fd(); + std::mem::forget(f); + md + } + /// Size of the file behind the descriptor, via `fstat`. pub fn size(&self) -> Option { // SAFETY: zeroed stat is a valid initial value; fd is ours. @@ -385,14 +398,29 @@ impl Gate { Arc::clone(&self.responder) } - /// Watch a whole mount. Used in tests against a dedicated tmpfs so a - /// bug cannot reach the real filesystem. + /// Watch a mount, by its vfsmount. + /// + /// **Only safe when the watcher shares a mount namespace with the + /// processes it is watching.** A vfsmount is per-namespace, so a + /// daemon in a private namespace marks its own copy and receives + /// nothing from anybody else — while still reporting itself armed. + /// Under systemd that is the default situation, because several + /// ordinary hardening options force a private mount namespace. + /// + /// Kept for the smoke-test example, which runs outside systemd. + /// Production uses [`Gate::mark_filesystem`]. pub fn mark_mount(&self, path: &Path) -> io::Result<()> { self.mark(path, FAN_MARK_ADD | FAN_MARK_MOUNT) } - /// Watch an entire filesystem. This is the production mark, and the - /// reason the watchdog is not optional. + /// Watch an entire filesystem, by its superblock. + /// + /// A superblock is shared across mount namespaces, so this sees every + /// process on the machine regardless of where the watcher lives. It + /// also still scopes correctly: a superblock is exactly one + /// filesystem, so marking a dedicated mount covers that and nothing + /// else. This is the production mark, and the reason the watchdog is + /// not optional. pub fn mark_filesystem(&self, path: &Path) -> io::Result<()> { self.mark(path, FAN_MARK_ADD | FAN_MARK_FILESYSTEM) } @@ -417,6 +445,35 @@ impl Gate { Ok(()) } + /// Wait until the group has events, or `timeout_ms` elapses. + /// + /// The reader used to poll: read, get `EAGAIN`, sleep a millisecond, + /// try again. That millisecond lands on the critical path of every + /// `execve` on a watched filesystem — measured at +2.7 ms per exec on + /// this machine, nearly all of it waiting for a sleep to finish rather + /// than doing any work. Blocking on the descriptor means the reader + /// wakes when the kernel has something to say and not before. + /// + /// The timeout exists only so the loop can notice `stop()`. + pub fn wait_readable(&self, timeout_ms: i32) -> io::Result { + let mut fds = libc::pollfd { + fd: self.responder.fan.as_raw_fd(), + events: libc::POLLIN, + revents: 0, + }; + // SAFETY: one valid descriptor, and the struct outlives the call. + let rc = unsafe { libc::poll(&mut fds, 1, timeout_ms) }; + if rc < 0 { + let err = io::Error::last_os_error(); + // A signal interrupting the wait is not an error. + if err.kind() == io::ErrorKind::Interrupted { + return Ok(false); + } + return Err(err); + } + Ok(rc > 0 && fds.revents & libc::POLLIN != 0) + } + /// Read whatever events are pending. Returns empty on `EAGAIN`. pub fn read_events(&self) -> io::Result> { let mut buf = [0u8; 8192]; @@ -521,6 +578,7 @@ impl Gate { cfg: GateConfig, scan: Arc Option + Send + Sync>, on_detect: Arc, + fast: Arc Option + Send + Sync>, ) -> Vec> { let (tx, rx) = std::sync::mpsc::channel::(); let rx = Arc::new(Mutex::new(rx)); @@ -535,13 +593,41 @@ impl Gate { .name("hound-gate-reader".into()) .spawn(move || { while running.load(Ordering::SeqCst) { - match gate.read_events() { - Ok(events) if events.is_empty() => { - std::thread::sleep(Duration::from_millis(1)); + // Block until the kernel has something, so no + // exec ever waits on a timer of ours. + match gate.wait_readable(100) { + Ok(false) => continue, + Ok(true) => {} + Err(e) => { + eprintln!("gate: poll failed: {e}"); + std::thread::sleep(Duration::from_millis(20)); + continue; } + } + match gate.read_events() { + Ok(events) if events.is_empty() => {} Ok(events) => { for e in events { let (seq, fd) = (e.seq, e.fd); + + // Fast path: a verdict already in + // memory is answered here, without + // the channel handoff or a worker + // wakeup. Re-executing a binary we + // have already judged is the + // overwhelmingly common event on a + // busy machine, and it costs an + // fstat and a map lookup — the + // reader still never scans, which + // is what keeps the watchdog + // meaningful. + if e.needs_response() { + if let Some(allow) = fast(&e) { + gate.answer_and_retire(seq, fd, allow); + continue; + } + } + if tx.send(e).is_err() { // No workers left: fail open. gate.answer_and_retire(seq, fd, true); diff --git a/crates/houndd/src/main.rs b/crates/houndd/src/main.rs index 5a0a8ac..dc30d21 100644 --- a/crates/houndd/src/main.rs +++ b/crates/houndd/src/main.rs @@ -223,15 +223,26 @@ impl DaemonState { } else { s.exec_gate_paths.clone() }; + // ALWAYS mark the filesystem, never the mount — even when the + // operator scoped the gate to one path. + // + // systemd gives this service a private mount namespace (any of + // ProtectProc, ProtectKernelTunables or ProtectControlGroups is + // enough to force one). FAN_MARK_MOUNT marks a vfsmount, and a + // private namespace holds its own vfsmount for the same + // filesystem — so the daemon marks its copy, every other process + // on the machine uses the host's copy, and not one event is ever + // delivered. The gate reports itself armed and silently protects + // nothing, which is the worst way for a security feature to fail. + // + // FAN_MARK_FILESYSTEM marks the SUPERBLOCK, which is shared + // across namespaces. Scoping still works, because a superblock is + // exactly one filesystem: marking a dedicated mount covers that + // mount and nothing else. let mut marked = Vec::new(); for p in &paths { let path = std::path::Path::new(p); - let result = if s.exec_gate_paths.is_empty() { - gate.mark_filesystem(path) - } else { - gate.mark_mount(path) - }; - match result { + match gate.mark_filesystem(path) { Ok(()) => marked.push(p.clone()), Err(e) => { events.push("gate", "warn", format!("could not watch {p}: {e}")); @@ -286,7 +297,7 @@ impl DaemonState { std::sync::Arc::new(|_path: &std::path::Path, bytes: &[u8]| { engine::engine().scan_bytes(bytes) }), - std::sync::Arc::new( + std::sync::Arc::new( move |path: &std::path::Path, name: &str, verdict: fanotify::Verdict| { match verdict { fanotify::Verdict::Blocked => { @@ -329,6 +340,15 @@ impl DaemonState { } }, ), + // Fast path: fstat the descriptor the kernel already gave us + // and ask the engine whether it has judged this exact file + // version before. No read, no scan, no worker. + std::sync::Arc::new(|event: &fanotify::Event| { + let md = event.metadata()?; + engine::engine() + .cached_verdict(&md) + .map(|verdict| verdict.is_none()) + }), ); events.push( diff --git a/crates/houndd/src/native.rs b/crates/houndd/src/native.rs index 85b0c03..eceb6ee 100644 --- a/crates/houndd/src/native.rs +++ b/crates/houndd/src/native.rs @@ -162,6 +162,12 @@ impl ScanEngine for HoundEngine { }) } + fn cached_verdict(&self, md: &std::fs::Metadata) -> Option> { + self.cache + .get(&FileKey::from_metadata(md)) + .map(|v| v.map(|name| name.to_string())) + } + fn scan_bytes(&self, bytes: &[u8]) -> Option { let set = self.rules.current(); let mut scanner = yara_x::Scanner::new(&set.rules); diff --git a/dist/hound_0.1.0_amd64.deb b/dist/hound_0.1.0_amd64.deb index 07d3426..ffc8a82 100644 Binary files a/dist/hound_0.1.0_amd64.deb and b/dist/hound_0.1.0_amd64.deb differ diff --git a/packaging/systemd/houndd.service b/packaging/systemd/houndd.service index 1301e7f..5d1f7e9 100644 --- a/packaging/systemd/houndd.service +++ b/packaging/systemd/houndd.service @@ -25,7 +25,12 @@ RestartSec=2s # someone else needs DAC_OVERRIDE; stripping the execute bit off a file # we do not own needs FOWNER. CapabilityBoundingSet=CAP_SYS_ADMIN CAP_DAC_READ_SEARCH CAP_DAC_OVERRIDE CAP_FOWNER -AmbientCapabilities=CAP_SYS_ADMIN CAP_DAC_READ_SEARCH CAP_DAC_OVERRIDE CAP_FOWNER +# AmbientCapabilities is deliberately NOT set. Ambient capabilities are +# inherited by child processes, and the daemon shells out to freshclam, +# rpm and pacman on some paths — none of which should start life holding +# CAP_SYS_ADMIN. A process running as root already receives everything in +# the bounding set as permitted and effective, so Ambient adds nothing +# here except a way for it to leak. NoNewPrivileges=yes # ── Filesystem ─────────────────────────────────────────────────────── @@ -74,6 +79,16 @@ LockPersonality=yes RestrictAddressFamilies=AF_UNIX AF_INET AF_INET6 SystemCallArchitectures=native SystemCallFilter=@system-service +# fanotify_init and fanotify_mark live in @privileged, which +# @system-service deliberately excludes — so the base filter kills the +# daemon with SIGSYS the moment the execution gate is switched on. This +# was invisible until the gate was armed for the first time on a real +# install: the service starts fine with the gate off. +# +# Granted individually rather than by adding @privileged, which would +# also admit setuid, chroot, bpf, kexec_load and pivot_root. Two +# syscalls is the whole requirement. +SystemCallFilter=fanotify_init fanotify_mark SystemCallFilter=~@clock @cpu-emulation @debug @module @mount @obsolete @raw-io @reboot @swap UMask=0077