From 85234842f941d99de75a134619fabaff4347b24e Mon Sep 17 00:00:00 2001 From: Hound Date: Thu, 20 Aug 2026 23:27:34 -0500 Subject: [PATCH] houndd: gate covers writes too; inotify becomes the fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes Phase 1. The gate now asks for FAN_CLOSE_WRITE alongside the permission events, so a threat written to disk is quarantined and a threat being executed is refused — one mechanism, one mark, no watch-descriptor ceiling and no blind spots outside a configured list. Verified live. The nicest evidence is an error message: $ chmod +x /tmp/hound-live/malware.sh chmod: cannot access '/tmp/hound-live/malware.sh': No such file or directory Hound had already quarantined it. `hound quarantine list` shows the entry, the clean binary beside it still runs, and CapPrm/CapEff/CapBnd read 000000000020000e. Three bugs, each of which looked like working code: * A file descriptor number is not an identity. The kernel allocates an fd per event and recycles the number the moment we close it, so one write arrives as FAN_OPEN_PERM on fd 6 and then FAN_CLOSE_WRITE on fd 6 again. Idempotency keyed on the fd treated the second as a duplicate of the first and dropped it — detection ran, matched EICAR, and threw the result away. Events now carry a monotonic seq that is never reused. * rename(2) fails EXDEV across filesystems, and for quarantine that is the common case rather than the exotic one: the vault is under /var/lib while threats land on /home, in a tmpfs, on a USB stick or in a container overlay. Quarantine now falls back to copy-then-unlink, unlinking only once the copy is safely down, and seals the stored file at 0600 with every execute bit cleared. * The capability set was too small to do the job. CAP_DAC_READ_SEARCH lets us read a threat but not unlink it, so quarantine failed EACCES as root. The set is now four capabilities — SYS_ADMIN, DAC_READ_SEARCH, DAC_OVERRIDE, FOWNER. DAC_OVERRIDE is close to "write anywhere" and that is worth being honest about; an antivirus that quarantines cannot avoid it, because the threat is by definition in a directory somebody else owns. What the reduction still buys is what it excludes, and there is a test asserting SYS_MODULE, SYS_BOOT, SYS_PTRACE, NET_ADMIN, NET_RAW, AUDIT_CONTROL and SETUID never creep back in. Narrowing further means a separate privileged helper for quarantine. realtime.rs is now documented as the unprivileged fallback and does not start when the gate is armed — running both would scan everything twice and quarantine the same file from two threads. 99 tests pass. HOUNDD_GATE_DEBUG=1 dumps every event and decision. Co-Authored-By: Claude Opus 5 --- crates/houndd/examples/gate-smoke.rs | 3 + crates/houndd/src/caps.rs | 78 ++++++-- crates/houndd/src/fanotify.rs | 270 +++++++++++++++++++++++---- crates/houndd/src/main.rs | 83 ++++++-- crates/houndd/src/quarantine.rs | 111 ++++++++++- crates/houndd/src/realtime.rs | 19 +- 6 files changed, 491 insertions(+), 73 deletions(-) diff --git a/crates/houndd/examples/gate-smoke.rs b/crates/houndd/examples/gate-smoke.rs index d6e99a1..ff89f4b 100644 --- a/crates/houndd/examples/gate-smoke.rs +++ b/crates/houndd/examples/gate-smoke.rs @@ -110,6 +110,9 @@ fn run(mount: &Path) -> bool { excludes: Vec::new(), }, scan, + Arc::new(|p: &Path, name: &str, v: fanotify::Verdict| { + println!(" detect: {} ({name}) {v:?}", p.display()); + }), ); 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 0145da8..f7ef004 100644 --- a/crates/houndd/src/caps.rs +++ b/crates/houndd/src/caps.rs @@ -1,10 +1,27 @@ //! Capability reduction. //! -//! The execution gate needs `CAP_SYS_ADMIN` to call `fanotify_init`, and -//! `CAP_DAC_READ_SEARCH` to read files whose owner has not invited us. -//! Nothing else. But a daemon started by systemd arrives holding the full -//! root set, which includes loading kernel modules, rebooting the -//! machine, and rewriting the audit log. +//! Four capabilities, out of the forty-one a root daemon starts with: +//! +//! | | | +//! |---|---| +//! | `CAP_SYS_ADMIN` | `fanotify_init` and `fanotify_mark` | +//! | `CAP_DAC_READ_SEARCH` | read any file to scan it | +//! | `CAP_DAC_OVERRIDE` | unlink a threat into the vault | +//! | `CAP_FOWNER` | strip the exec bits off a file we do not own | +//! +//! The last two are worth being honest about, because they are not small. +//! `CAP_DAC_OVERRIDE` is close to "write anywhere", and an antivirus that +//! quarantines cannot do without it: the threat is by definition in a +//! directory somebody else owns. What the reduction still buys is +//! everything it excludes — `CAP_SYS_MODULE`, `CAP_SYS_BOOT`, +//! `CAP_SYS_PTRACE`, `CAP_NET_ADMIN`, `CAP_NET_RAW`, `CAP_AUDIT_CONTROL`, +//! `CAP_MAC_ADMIN`, `CAP_SETUID`. A compromised Hound cannot load a +//! rootkit, reboot the box, attach to other processes, forge packets or +//! rewrite the audit trail. +//! +//! Narrowing this further means moving quarantine into a separate +//! privileged helper so the scanning process holds neither DAC capability. +//! Worth doing; not worth blocking the gate on. //! //! We are asking people to run a root daemon that can block execution. //! The least we can do is make it hold only what it needs, so that a @@ -33,9 +50,19 @@ use std::io; /// Capability numbers we care about, from `linux/capability.h`. +pub const CAP_DAC_OVERRIDE: u32 = 1; pub const CAP_DAC_READ_SEARCH: u32 = 2; +pub const CAP_FOWNER: u32 = 3; pub const CAP_SYS_ADMIN: u32 = 21; +/// Everything the daemon needs and nothing else. +pub const GATE_CAPS: [u32; 4] = [ + CAP_SYS_ADMIN, + CAP_DAC_READ_SEARCH, + CAP_DAC_OVERRIDE, + CAP_FOWNER, +]; + /// Highest capability the running kernel could define. 63 is the ceiling /// of the v3 two-word ABI; dropping past what exists is harmless. const CAP_LAST_CAP_CEILING: u32 = 63; @@ -166,13 +193,13 @@ pub fn bounding_now() -> Option { /// Exactly what the execution gate needs, and nothing more. pub fn drop_to_gate_minimum() -> io::Result { - let dropped = retain_only(&[CAP_SYS_ADMIN, CAP_DAC_READ_SEARCH])?; + let dropped = retain_only(&GATE_CAPS)?; if !dropped { return Ok(false); } // Trust the kernel, verify anyway: the failure mode here is a syscall // that returns success and changes nothing. - let (lo, _) = to_words(&[CAP_SYS_ADMIN, CAP_DAC_READ_SEARCH]); + let (lo, _) = to_words(&GATE_CAPS); let want = lo as u64; match (effective_now(), bounding_now()) { (Some(eff), Some(bnd)) if eff == want && bnd == want => Ok(true), @@ -202,12 +229,39 @@ mod tests { } #[test] - fn the_gate_set_is_exactly_two_bits() { - let (lo, hi) = to_words(&[CAP_SYS_ADMIN, CAP_DAC_READ_SEARCH]); - assert_eq!(lo.count_ones(), 2, "no capability may sneak in"); + fn the_gate_set_is_exactly_four_bits() { + let (lo, hi) = to_words(&GATE_CAPS); + assert_eq!(lo.count_ones(), 4, "no capability may sneak in"); assert_eq!(hi, 0); } + #[test] + fn the_dangerous_capabilities_stay_out() { + // The whole point of the reduction. If any of these ever appear in + // GATE_CAPS, a compromised Hound can load a rootkit, reboot the + // machine, read other processes' memory, forge packets, or rewrite + // the audit log. + const CAP_SYS_MODULE: u32 = 16; + const CAP_SYS_PTRACE: u32 = 19; + const CAP_SYS_BOOT: u32 = 22; + const CAP_NET_ADMIN: u32 = 12; + const CAP_NET_RAW: u32 = 13; + const CAP_AUDIT_CONTROL: u32 = 30; + const CAP_SETUID: u32 = 7; + let (lo, _) = to_words(&GATE_CAPS); + for (name, bit) in [ + ("CAP_SYS_MODULE", CAP_SYS_MODULE), + ("CAP_SYS_PTRACE", CAP_SYS_PTRACE), + ("CAP_SYS_BOOT", CAP_SYS_BOOT), + ("CAP_NET_ADMIN", CAP_NET_ADMIN), + ("CAP_NET_RAW", CAP_NET_RAW), + ("CAP_AUDIT_CONTROL", CAP_AUDIT_CONTROL), + ("CAP_SETUID", CAP_SETUID), + ] { + assert_eq!(lo & (1 << bit), 0, "{name} must never be retained"); + } + } + #[test] fn capabilities_above_31_cross_into_the_high_word() { let (lo, hi) = to_words(&[40]); @@ -246,8 +300,8 @@ mod tests { #[test] fn the_gate_set_matches_what_proc_would_report() { - let (lo, _) = to_words(&[CAP_SYS_ADMIN, CAP_DAC_READ_SEARCH]); - assert_eq!(lo as u64, 0x20_0004, "must match the CapEff mask on the wire"); + let (lo, _) = to_words(&GATE_CAPS); + assert_eq!(lo as u64, 0x20_000e, "must match the CapEff mask in /proc"); } #[test] diff --git a/crates/houndd/src/fanotify.rs b/crates/houndd/src/fanotify.rs index 1ea1a71..8ab907a 100644 --- a/crates/houndd/src/fanotify.rs +++ b/crates/houndd/src/fanotify.rs @@ -43,9 +43,16 @@ const FAN_CLOEXEC: u32 = 0x0000_0001; const FAN_NONBLOCK: u32 = 0x0000_0002; const FAN_CLASS_CONTENT: u32 = 0x0000_0004; +const FAN_CLOSE_WRITE: u64 = 0x0000_0008; const FAN_OPEN_PERM: u64 = 0x0001_0000; const FAN_OPEN_EXEC_PERM: u64 = 0x0004_0000; +/// The permission bits. An event carrying one of these has a process +/// blocked behind it and MUST be answered; an event carrying none of them +/// is a notification and must NOT be — writing a response for one is a +/// protocol error. +const PERM_MASK: u64 = FAN_OPEN_PERM | FAN_OPEN_EXEC_PERM; + const FAN_MARK_ADD: u32 = 0x0000_0001; const FAN_MARK_MOUNT: u32 = 0x0000_0010; const FAN_MARK_FILESYSTEM: u32 = 0x0000_0100; @@ -84,12 +91,22 @@ struct Response { response: u32, } -/// One decoded permission event. +/// One decoded event. #[derive(Debug)] pub struct Event { - /// Descriptor for the file being opened. Ours to close. + /// Our own identity for this event. + /// + /// It exists because **a file descriptor number is not a stable + /// identity**. The kernel allocates an fd per event and recycles the + /// number as soon as we close it, so a single write produces an + /// `FAN_OPEN_PERM` on fd 6 and then an `FAN_CLOSE_WRITE` on fd 6 + /// again. Keying "have I handled this?" on the fd made the second + /// event look like a duplicate of the first and silently dropped it. + /// Sequence numbers are never reused. + pub seq: u64, + /// Descriptor for the file. Ours to close, exactly once. pub fd: RawFd, - /// The process being held. + /// The process being held, for permission events. pub pid: i32, pub mask: u64, } @@ -100,6 +117,24 @@ impl Event { self.mask & FAN_OPEN_EXEC_PERM != 0 } + /// True when a process is blocked waiting for our answer. + /// + /// The distinction drives everything downstream: a permission event is + /// a deadline, a notification is a chore. Answering a notification is + /// a protocol error, and failing to answer a permission event freezes + /// a process. + pub fn needs_response(&self) -> bool { + self.mask & PERM_MASK != 0 + } + + /// True when this is a completed write — a file just changed on disk. + /// Nobody is waiting; this is the post-hoc path that replaces what + /// inotify used to do, with whole-filesystem coverage and no watch + /// descriptor limit. + pub fn is_write(&self) -> bool { + self.mask & FAN_CLOSE_WRITE != 0 + } + /// Resolve what the descriptor points at, via `/proc/self/fd`. /// /// This is a readlink, not an open, so it generates no event. @@ -216,7 +251,9 @@ pub mod policy { /// exactly once, whether by the scan loop or by the watchdog. pub struct Responder { fan: OwnedFd, - answered: Mutex>, + /// Events already finished, by sequence number — never by fd, which + /// the kernel recycles. + answered: Mutex>, allowed: AtomicU64, denied: AtomicU64, timed_out: AtomicU64, @@ -233,22 +270,36 @@ impl Responder { } } - /// Register an event as in flight so the watchdog can adopt it. - fn register(&self, fd: RawFd) { - self.answered.lock().expect("responder poisoned").remove(&fd); - } - /// Answer an event. Idempotent: a second call for the same descriptor /// is a no-op, which is what makes the watchdog safe to race with the /// scan loop. - pub fn answer(&self, fd: RawFd, allow: bool, timed_out: bool) { + pub fn answer(&self, seq: u64, fd: RawFd, allow: bool, timed_out: bool) { + self.finish(seq, fd, allow, timed_out, true) + } + + /// Close a notification event's descriptor without answering it. + /// + /// The kernel hands out a descriptor for notifications too, and leaking + /// them exhausts the fd table just as thoroughly as leaking a + /// permission event's. + pub fn close_only(&self, seq: u64, fd: RawFd) { + self.finish(seq, fd, true, false, false) + } + + fn finish(&self, seq: u64, fd: RawFd, allow: bool, timed_out: bool, respond: bool) { { let mut answered = self.answered.lock().expect("responder poisoned"); - if !answered.insert(fd) { + if !answered.insert(seq) { return; } } + if !respond { + // SAFETY: we own this fd and nothing is waiting on it. + unsafe { libc::close(fd) }; + return; + } + let response = Response { fd, response: if allow { FAN_ALLOW } else { FAN_DENY }, @@ -296,9 +347,11 @@ impl Responder { /// An initialised fanotify group. pub struct Gate { responder: Arc, - /// Events registered but not yet answered, with their deadlines. - inflight: Arc>>, + /// Events registered but not yet answered: `(seq, fd, deadline)`. + inflight: Arc>>, running: Arc, + /// Source of event identities. Monotonic, never reused. + seq: Arc, } impl Gate { @@ -324,6 +377,7 @@ impl Gate { responder: Arc::new(Responder::new(fan)), inflight: Arc::new(Mutex::new(Vec::new())), running: Arc::new(AtomicBool::new(false)), + seq: Arc::new(AtomicU64::new(1)), }) } @@ -352,7 +406,7 @@ impl Gate { libc::fanotify_mark( self.responder.fan.as_raw_fd(), flags, - FAN_OPEN_EXEC_PERM | FAN_OPEN_PERM, + FAN_OPEN_EXEC_PERM | FAN_OPEN_PERM | FAN_CLOSE_WRITE, libc::AT_FDCWD, c_path.as_ptr(), ) @@ -381,15 +435,36 @@ impl Gate { } return Err(err); } - let events = decode(&buf[..n as usize]); + let mut events = decode(&buf[..n as usize]); + for e in &mut events { + e.seq = self.seq.fetch_add(1, Ordering::Relaxed); + } + if std::env::var_os("HOUNDD_GATE_DEBUG").is_some() { + for e in &events { + eprintln!( + "gate/debug: fd={} pid={} mask={:#x} exec={} write={} perm={} path={:?}", + e.fd, + e.pid, + e.mask, + e.is_exec(), + e.is_write(), + e.needs_response(), + e.path() + ); + } + } // Register before returning so the watchdog can adopt an event even // if the caller panics on the way to scanning it. { let deadline = Instant::now() + DEADLINE; let mut inflight = self.inflight.lock().expect("inflight poisoned"); for e in &events { - self.responder.register(e.fd); - inflight.push((e.fd, deadline)); + // Only permission events go to the watchdog. Handing it a + // notification would make it write a response nobody asked + // for. + if e.needs_response() { + inflight.push((e.seq, e.fd, deadline)); + } } } Ok(events) @@ -411,21 +486,21 @@ impl Gate { while running.load(Ordering::SeqCst) { std::thread::sleep(Duration::from_millis(50)); let now = Instant::now(); - let mut expired = Vec::new(); + let mut expired: Vec<(u64, RawFd)> = Vec::new(); { let mut list = inflight.lock().expect("inflight poisoned"); - list.retain(|(fd, deadline)| { + list.retain(|(seq, fd, deadline)| { if now >= *deadline { - expired.push(*fd); + expired.push((*seq, *fd)); false } else { true } }); } - for fd in expired { + for (seq, fd) in expired { // Fail OPEN. Always. - responder.answer(fd, true, true); + responder.answer(seq, fd, true, true); } } }) @@ -445,6 +520,7 @@ impl Gate { self: &Arc, cfg: GateConfig, scan: Arc Option + Send + Sync>, + on_detect: Arc, ) -> Vec> { let (tx, rx) = std::sync::mpsc::channel::(); let rx = Arc::new(Mutex::new(rx)); @@ -465,10 +541,10 @@ impl Gate { } Ok(events) => { for e in events { - let fd = e.fd; + let (seq, fd) = (e.seq, e.fd); if tx.send(e).is_err() { // No workers left: fail open. - gate.answer_and_retire(fd, true); + gate.answer_and_retire(seq, fd, true); } } } @@ -488,6 +564,7 @@ impl Gate { let gate = Arc::clone(self); let rx = Arc::clone(&rx); let scan = Arc::clone(&scan); + let on_detect = Arc::clone(&on_detect); let running = Arc::clone(&self.running); let excludes = cfg.excludes.clone(); let max_size = cfg.max_size; @@ -511,16 +588,38 @@ impl Gate { max_size, &excludes, ); - let allow = match decision { - policy::Decision::AllowNow(_) => true, - policy::Decision::Scan => path - .as_deref() - .and_then(|p| { - event.content(max_size as usize).and_then(|b| scan(p, &b)) - }) - .is_none(), + + let hit = match decision { + policy::Decision::AllowNow(_) => None, + policy::Decision::Scan => path.as_deref().and_then(|p| { + event.content(max_size as usize).and_then(|b| scan(p, &b)) + }), }; - gate.answer_and_retire(event.fd, allow); + + if std::env::var_os("HOUNDD_GATE_DEBUG").is_some() { + eprintln!( + "gate/debug: seq={} decision={:?} hit={:?} path={:?} size={:?}", + event.seq, decision, hit, path, event.size() + ); + } + + if event.needs_response() { + // Someone is blocked. Answer first, report + // second — the report must never sit between + // a process and its verdict. + gate.answer_and_retire(event.seq, event.fd, hit.is_none()); + if let (Some(name), Some(p)) = (&hit, path.as_deref()) { + on_detect(p, name, Verdict::Blocked); + } + } else { + // A completed write: nothing is waiting, so + // close the descriptor and hand it to the + // daemon to quarantine. + gate.responder().close_only(event.seq, event.fd); + if let (Some(name), Some(p)) = (&hit, path.as_deref()) { + on_detect(p, name, Verdict::Written); + } + } } }) .expect("spawning a gate worker must not fail"), @@ -529,10 +628,10 @@ impl Gate { handles } - /// Mark an event as answered so the watchdog stops tracking it. - pub fn retire(&self, fd: RawFd) { + /// Stop the watchdog tracking an event. + pub fn retire(&self, seq: u64) { let mut list = self.inflight.lock().expect("inflight poisoned"); - list.retain(|(f, _)| *f != fd); + list.retain(|(s, _, _)| *s != seq); } pub fn stop(&self) { @@ -587,18 +686,28 @@ impl Gate { } }; - self.answer_and_retire(event.fd, allow); + self.answer_and_retire(event.seq, event.fd, allow); } Ok((events.len(), denied)) } /// Answer an event and stop the watchdog tracking it. - pub fn answer_and_retire(&self, fd: RawFd, allow: bool) { - self.retire(fd); - self.responder.answer(fd, allow, false); + pub fn answer_and_retire(&self, seq: u64, fd: RawFd, allow: bool) { + self.retire(seq); + self.responder.answer(seq, fd, allow, false); } } +/// How a detection reached us, which decides what the daemon does with it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Verdict { + /// Caught at `execve` and refused. The file never ran. + Blocked, + /// Caught after a completed write. It is on disk and should be + /// quarantined — this is the path that replaces inotify. + Written, +} + /// How the gate should behave once running. #[derive(Debug, Clone)] pub struct GateConfig { @@ -642,6 +751,7 @@ fn decode(buf: &[u8]) -> Vec { } if meta.fd >= 0 { out.push(Event { + seq: 0, // stamped by read_events, which owns the counter fd: meta.fd, pid: meta.pid, mask: meta.mask, @@ -815,7 +925,7 @@ mod tests { fn event_for(path: &Path) -> (Event, std::fs::File) { let f = std::fs::File::open(path).unwrap(); let fd = f.as_raw_fd(); - (Event { fd, pid: 1, mask: FAN_OPEN_PERM }, f) + (Event { seq: 1, fd, pid: 1, mask: FAN_OPEN_PERM }, f) } #[test] @@ -851,6 +961,84 @@ mod tests { let _ = std::fs::remove_file(&p); } + // ── permission events versus notifications ── + + #[test] + fn permission_events_must_be_answered() { + for mask in [FAN_OPEN_PERM, FAN_OPEN_EXEC_PERM, FAN_OPEN_PERM | FAN_CLOSE_WRITE] { + let e = Event { seq: 1, fd: 3, pid: 1, mask }; + assert!(e.needs_response(), "mask {mask:#x} has a process blocked on it"); + } + } + + #[test] + fn a_completed_write_must_not_be_answered() { + let e = Event { seq: 1, fd: 3, pid: 1, mask: FAN_CLOSE_WRITE }; + assert!(!e.needs_response(), "answering a notification is a protocol error"); + assert!(e.is_write()); + assert!(!e.is_exec()); + } + + #[test] + fn exec_and_write_are_distinguishable() { + let exec = Event { seq: 1, fd: 3, pid: 1, mask: FAN_OPEN_EXEC_PERM }; + assert!(exec.is_exec() && !exec.is_write()); + let write = Event { seq: 2, fd: 4, pid: 1, mask: FAN_CLOSE_WRITE }; + assert!(write.is_write() && !write.is_exec()); + } + + #[test] + fn the_watchdog_only_adopts_permission_events() { + // A notification handed to the watchdog would have it write a + // response for an event nobody is waiting on. + let notify = Event { seq: 1, fd: 9, pid: 1, mask: FAN_CLOSE_WRITE }; + let perm = Event { seq: 2, fd: 10, pid: 1, mask: FAN_OPEN_EXEC_PERM }; + let adopted: Vec = [¬ify, &perm] + .iter() + .filter(|e| e.needs_response()) + .map(|e| e.fd) + .collect(); + assert_eq!(adopted, vec![10]); + } + + #[test] + fn decoding_preserves_the_write_bit() { + let buf = encode(FAN_METADATA_VERSION, FAN_CLOSE_WRITE, 5, 77); + let events = decode(&buf); + assert_eq!(events.len(), 1); + assert!(events[0].is_write()); + assert!(!events[0].needs_response()); + } + + #[test] + fn the_same_fd_number_twice_is_two_events() { + // The kernel recycles an event fd's NUMBER as soon as we close it, + // so one write arrives as FAN_OPEN_PERM on fd 6 and then + // FAN_CLOSE_WRITE on fd 6 again. Keying identity on the fd made the + // second look like a duplicate and dropped it silently — the write + // path appeared to work and quarantined nothing. + let first = Event { seq: 1, fd: 6, pid: 99, mask: FAN_OPEN_PERM }; + let second = Event { seq: 2, fd: 6, pid: 99, mask: FAN_CLOSE_WRITE }; + assert_eq!(first.fd, second.fd, "the kernel really does reuse the number"); + assert_ne!(first.seq, second.seq, "identity must not come from the fd"); + } + + #[test] + fn read_events_stamps_unique_sequence_numbers() { + let Ok(gate) = Gate::init() else { return }; // needs root; skip otherwise + let a = gate.seq.fetch_add(1, Ordering::Relaxed); + let b = gate.seq.fetch_add(1, Ordering::Relaxed); + assert_ne!(a, b); + assert!(b > a, "sequence numbers must be monotonic"); + gate.stop(); + } + + #[test] + fn decode_leaves_seq_for_read_events_to_stamp() { + let buf = encode(FAN_METADATA_VERSION, FAN_OPEN_PERM, 3, 1); + assert_eq!(decode(&buf)[0].seq, 0, "decode must not invent identities"); + } + #[test] fn gate_config_defaults_exclude_pseudo_filesystems() { let cfg = GateConfig::default(); diff --git a/crates/houndd/src/main.rs b/crates/houndd/src/main.rs index b13112e..26eddd4 100644 --- a/crates/houndd/src/main.rs +++ b/crates/houndd/src/main.rs @@ -131,16 +131,29 @@ impl DaemonState { let realtime = realtime::RealtimeMonitor::new(settings.clone(), quarantine.clone(), events.clone()); - // If realtime is enabled, bring the monitor up. Failures are - // non-fatal (e.g. no inotify) — the daemon still serves scans. let s = settings.get(); - if s.realtime_enabled && !s.paused { + let (gate, gate_detail, gate_paths) = Self::arm_gate(&s, &events, &quarantine); + + // The inotify monitor is now the FALLBACK, not the primary path. + // + // When the gate is armed it already sees every completed write on + // the whole filesystem, with no watch-descriptor ceiling and no + // blind spots outside the configured directories. Running both + // would scan everything twice and quarantine the same file from + // two threads. inotify survives only for the unprivileged case, + // where fanotify is not available at all. + if gate.is_some() { + events.push( + "realtime", + "info", + "inotify monitor idle — the execution gate covers writes".into(), + ); + } else if s.realtime_enabled && !s.paused { match realtime.start() { Ok(()) => events.push("realtime", "info", "real-time monitor started".into()), Err(e) => events.push("realtime", "warn", format!("real-time monitor idle: {e}")), }; } - let (gate, gate_detail, gate_paths) = Self::arm_gate(&s, &events); events.push("info", "info", "houndd daemon started".into()); @@ -165,6 +178,7 @@ impl DaemonState { fn arm_gate( s: &Settings, events: &events::EventLog, + quarantine: &quarantine::Quarantine, ) -> (Option>, String, Vec) { if !s.exec_gate { eprintln!("gate: disabled in settings"); @@ -225,9 +239,9 @@ impl DaemonState { events.push( "gate", "info", - "capabilities reduced to CAP_SYS_ADMIN + CAP_DAC_READ_SEARCH".into(), + "capabilities reduced to the four the gate needs".into(), ); - eprintln!("gate: capabilities reduced to CAP_SYS_ADMIN + CAP_DAC_READ_SEARCH"); + eprintln!("gate: capabilities reduced to 4 of 41 (CapEff 0x20000e)"); } Ok(false) => {} Err(e) => { @@ -243,23 +257,60 @@ impl DaemonState { let excludes = s.exclude_paths.clone(); let max_size = s.max_file_size_mb.saturating_mul(1024 * 1024); let ev = events.clone(); + let quarantine_on_write = s.on_detect == "quarantine"; + let q = quarantine.clone(); gate.serve( fanotify::GateConfig { workers: 4, max_size, excludes, }, - std::sync::Arc::new(move |path: &std::path::Path, bytes: &[u8]| { - let hit = engine::engine().scan_bytes(bytes); - if let Some(name) = &hit { - ev.push( - "gate", - "critical", - format!("blocked execution of {} ({name})", path.display()), - ); - } - hit + std::sync::Arc::new(|_path: &std::path::Path, bytes: &[u8]| { + engine::engine().scan_bytes(bytes) }), + std::sync::Arc::new( + move |path: &std::path::Path, name: &str, verdict: fanotify::Verdict| { + match verdict { + fanotify::Verdict::Blocked => { + ev.push( + "gate", + "critical", + format!("blocked execution of {} ({name})", path.display()), + ); + } + fanotify::Verdict::Written => { + // Nothing was waiting on this one, so the file is + // already on disk. This is the path that replaces + // what inotify used to do, with whole-filesystem + // coverage and no watch-descriptor ceiling. + if !quarantine_on_write { + ev.push( + "gate", + "critical", + format!("threat written to {} ({name})", path.display()), + ); + return; + } + match q.add(&path.to_string_lossy(), name) { + Ok(entry) => ev.push( + "quarantine", + "critical", + format!( + "quarantined {} ({name}) as {}", + path.display(), + entry.id + ), + ), + Err(e) => ev.push( + "quarantine", + "warn", + format!("could not quarantine {}: {e}", path.display()), + ), + }; + } + } + }, + ), ); events.push( diff --git a/crates/houndd/src/quarantine.rs b/crates/houndd/src/quarantine.rs index 297334d..20e4f3f 100644 --- a/crates/houndd/src/quarantine.rs +++ b/crates/houndd/src/quarantine.rs @@ -87,8 +87,9 @@ impl Quarantine { let dest = dir.join(&id); let meta_path = dir.join(format!("{id}.meta.json")); - // Move the bytes in. - std::fs::rename(&src, &dest)?; + // Move the bytes in, across filesystems if need be. + move_file(&src, &dest)?; + seal(&dest); let size = std::fs::metadata(&dest).map(|m| m.len()).unwrap_or(0); let entry = QuarantineEntry { @@ -116,7 +117,7 @@ impl Quarantine { if let Some(parent) = dest.parent() { std::fs::create_dir_all(parent)?; } - std::fs::rename(&file, &dest)?; + move_file(&file, &dest)?; entry.restored = true; std::fs::write(&meta_path, serde_json::to_string_pretty(&entry)?)?; @@ -142,6 +143,53 @@ impl Quarantine { /// Stable-ish id from the original path + a time component so two quarrantines /// of the same file at different times get distinct ids. +/// Move a file, falling back to copy-and-delete across filesystems. +/// +/// `rename(2)` fails with `EXDEV` when source and destination are on +/// different filesystems, and for quarantine that is the common case, not +/// the exotic one: the vault lives under `/var/lib`, while the things worth +/// quarantining show up on `/home` (often its own partition), in a tmpfs, +/// on a USB stick, or inside a container's overlay. A bare rename means +/// quarantine silently fails exactly where it is most needed. +fn move_file(src: &std::path::Path, dest: &std::path::Path) -> anyhow::Result<()> { + match std::fs::rename(src, dest) { + Ok(()) => Ok(()), + Err(e) if is_cross_device(&e) => { + std::fs::copy(src, dest) + .map_err(|e| anyhow::anyhow!("copying {} to the vault: {e}", src.display()))?; + // Only unlink once the copy is safely down. Losing the original + // without having stored it would destroy evidence. + std::fs::remove_file(src).map_err(|e| { + let _ = std::fs::remove_file(dest); + anyhow::anyhow!("removing {} after copying it: {e}", src.display()) + })?; + Ok(()) + } + Err(e) => Err(anyhow::anyhow!( + "moving {} to the vault: {e}", + src.display() + )), + } +} + +/// EXDEV, however the platform spells it. +fn is_cross_device(e: &std::io::Error) -> bool { + e.raw_os_error() == Some(18) +} + +/// Strip every execute bit and make the file root-only. +/// +/// The vault holds live malware. It should not be runnable by anyone who +/// wanders into the directory, and a restore puts the original mode back +/// from the metadata rather than trusting what is on disk. +fn seal(path: &std::path::Path) { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let _ = std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)); + } +} + fn make_id(path: &std::path::Path) -> String { let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) @@ -217,6 +265,63 @@ mod tests { let _ = std::fs::remove_dir_all(&data); } + #[test] + fn quarantine_works_across_filesystems() { + // The bug this covers: rename(2) returns EXDEV between filesystems, + // and the vault is almost never on the same one as the threat. + // /dev/shm is a tmpfs on every mainstream distro, so this exercises + // a real cross-device move rather than a simulated one. + let shm = std::path::Path::new("/dev/shm"); + if !shm.is_dir() { + return; + } + let data = tmp_data("xdev"); + let _env_guard = crate::test_util::locked(); + std::env::set_var("XDG_DATA_HOME", &data); + + let src = shm.join(format!("hound-xdev-{}", std::process::id())); + std::fs::write(&src, b"pretend malware").unwrap(); + + let q = Quarantine::new(); + let entry = q + .add(src.to_str().unwrap(), "Test.CrossDevice") + .expect("cross-device quarantine must work"); + + assert!(!src.exists(), "the original must be gone"); + assert!( + std::path::Path::new(&entry.quarantined_path).exists(), + "the vault copy must exist" + ); + assert_eq!(entry.size, 15); + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&entry.quarantined_path) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600, "the vault must strip exec bits"); + } + + // And back again, across the same boundary. + let restored = q.restore(&entry.id).expect("cross-device restore must work"); + assert!(restored.restored); + assert!(src.exists(), "the file must return to where it came from"); + let _ = std::fs::remove_file(&src); + let _ = q.remove(&entry.id); + std::env::remove_var("XDG_DATA_HOME"); + let _ = std::fs::remove_dir_all(&data); + } + + #[test] + fn exdev_is_recognised() { + let e = std::io::Error::from_raw_os_error(18); + assert!(is_cross_device(&e)); + let enoent = std::io::Error::from_raw_os_error(2); + assert!(!is_cross_device(&enoent)); + } + #[test] fn ids_are_distinct() { let a = make_id(std::path::Path::new("/tmp/x")); diff --git a/crates/houndd/src/realtime.rs b/crates/houndd/src/realtime.rs index bb2f00b..31c66f1 100644 --- a/crates/houndd/src/realtime.rs +++ b/crates/houndd/src/realtime.rs @@ -1,4 +1,21 @@ -//! Real-time interception. +//! Real-time interception — the **unprivileged fallback**. +//! +//! This was the primary path before the execution gate landed. It is not +//! any more, and the reason is structural rather than a matter of taste: +//! inotify reports a file *after* it has been written, so it can quarantine +//! but never refuse; it needs a watch per directory, so it silently misses +//! anything outside the configured list and hits an 8,192-watch ceiling on +//! a busy tree; and new directories race the walk that adds watches to them. +//! +//! `crates/houndd/src/fanotify.rs` has none of those properties: one +//! filesystem-wide mark, no ceiling, no blind spots, and the ability to +//! deny an `execve` outright. The daemon starts the gate when it can and +//! only falls back to this module when it cannot — which today means an +//! unprivileged run, since fanotify needs CAP_SYS_ADMIN. +//! +//! Kept rather than deleted because "runs without root" is a real mode +//! that developers and non-sudo users need, and a degraded monitor beats +//! no monitor. Do not add features here; add them to the gate. //! //! A background thread owns an inotify instance watching the configured //! directories (recursively — we walk each dir and add a watch per