diff --git a/src/base_plugin/mod.rs b/src/base_plugin/mod.rs index f9f9c41..325e6c3 100644 --- a/src/base_plugin/mod.rs +++ b/src/base_plugin/mod.rs @@ -28,10 +28,6 @@ use crate::{parsers, threadstate}; // a handful of libsinsp/libscap consts // duplicated here -const SE_EINPROGRESS: i64 = 115; -const SE_ETIMEOUT: i64 = 110; -const SE_EAGAIN: i64 = 11; - const STDIN_FD: u64 = 0; const STDOUT_FD: u64 = 1; const STDERR_FD: u64 = 2; @@ -219,44 +215,34 @@ impl EderaPlugin { }) } - /// distinct from `error` in that not all syscall errors indicate true failure & etc + /// Mirrors libsinsp `evt.failed` (which routes through the same + /// `extract_error_count` as `evt.count.error`): a syscall failed iff it has + /// a return value and that value is negative. A non-negative value + /// (including an fd or a byte count) is a success. `` when there's no + /// return value. pub fn extract_failed(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - if parsers::has_retval(zone_evt) { - // The return value is always the first parameter of the syscall event - // It could have different names depending on the event type `res`,`fd`, etc. - let retval = - i64::from_ne_bytes(zone_evt.event_params[0].param_data.as_slice().try_into()?); - if (retval != 0) - && (retval != SE_EINPROGRESS) - && (retval != SE_EAGAIN) - && (retval != SE_ETIMEOUT) - { - Ok(true) - } else { - Ok(false) - } - } else { - Err(anyhow!("event should have return code, but none found!")) - } + parsers::syscall_failed(zone_evt) + .ok_or_else(|| anyhow!("event should have return code, but none found!")) }) } pub fn extract_retval_str(&mut self, mut req: ExtractRequest) -> Result { self.with_zone_syscall_evt_ctx(&mut req, |zone_evt| { - if parsers::has_retval(zone_evt) { - // The return value is always the first parameter of the syscall event - // It could have different names depending on the event type `res`,`fd`, etc. - let retval = - i64::from_ne_bytes(zone_evt.event_params[0].param_data.as_slice().try_into()?); - if retval >= 0 { - Ok(CString::new("SUCCESS").expect("should cstring")) - } else { - Ok(CString::new(zone_evt.event_params[0].param_pretty.clone()) - .expect("should cstring")) - } + let Some(retval) = parsers::get_retval(zone_evt) else { + return Err(anyhow!("event should have return code, but none found!")); + }; + if retval >= 0 { + Ok(CString::new("SUCCESS").expect("should cstring")) } else { - Err(anyhow!("event should have return code, but none found!")) + // A negative return is an errno; render its name (e.g. ENOENT), + // matching libsinsp `evt.res`. We can't use the param's pretty + // string here: the return param is typed PT_FD and decoded + // unsigned upstream, so a negative errno would render as a huge + // positive integer. `unsigned_abs` avoids an overflow panic on a + // wire value of i64::MIN. + let errno = nix::errno::Errno::from_raw(retval.unsigned_abs() as i32); + Ok(CString::new(format!("{errno:?}")).expect("should cstring")) } }) } @@ -447,12 +433,7 @@ impl EderaPlugin { } fn get_count_error(evt: &ZoneKernelSyscallEvent) -> Result { - if parsers::has_retval(evt) { - let retval = i64::from_ne_bytes(evt.event_params[0].param_data.as_slice().try_into()?); - if retval < 0 { Ok(1) } else { Ok(0) } - } else { - Ok(0) - } + Ok(parsers::syscall_failed(evt).unwrap_or(false) as u64) } fn with_zone_threadinfo_ctx(&mut self, req: &mut ExtractRequest, f: F) -> Option @@ -570,11 +551,7 @@ impl EderaPlugin { let mut current_tid = *tid; let mut res: Option = None; - loop { - let Some(thread) = self.get_main_thread(zid, ¤t_tid) else { - break; - }; - + while let Some(thread) = self.get_main_thread(zid, ¤t_tid) { if predicate(&thread) { res = Some(thread.clone()); } @@ -586,7 +563,6 @@ impl EderaPlugin { break; } } - res } @@ -3093,3 +3069,42 @@ impl EderaPlugin { f(&context.decoded_evt) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::proto::generated::protect::control::v1::ZoneKernelEventParam; + + fn param(name: &str, pretty: &str) -> ZoneKernelEventParam { + ZoneKernelEventParam { + name: name.to_string(), + param_pretty: pretty.to_string(), + ..Default::default() + } + } + + // A failed openat still carries the attempted path in its `name` param, so + // `fs.path.name` can report which file a denied access targeted even though + // no fd was created (issue #2859, item 2). + #[test] + fn failed_openat_still_yields_path() { + let evt = ZoneKernelSyscallEvent { + event_type: event_codes::PPME_SYSCALL_OPENAT_2_X as u32, + event_params: vec![ + param("fd", "-2"), // ENOENT: no fd created + param("dirfd", "-100"), + param("name", "/opt/host-canary"), + ], + ..Default::default() + }; + + let paths: Vec = EderaPlugin::get_paths_from_evt_params(&evt) + .into_iter() + .filter_map(|p| match p { + EventPathType::Singular(s) => Some(s), + _ => None, + }) + .collect(); + assert_eq!(paths, vec!["/opt/host-canary".to_string()]); + } +} diff --git a/src/parsers.rs b/src/parsers.rs index 5534021..bd35dcf 100644 --- a/src/parsers.rs +++ b/src/parsers.rs @@ -178,6 +178,15 @@ pub fn get_retval(evt: &ZoneKernelSyscallEvent) -> Option { } } +/// libsinsp `evt.failed` / error-count semantics (see libsinsp +/// `sinsp_filter_check_event::extract_error_count`): a syscall failed iff it has +/// a return value and that value is negative. A non-negative value (including an +/// fd or a byte count) is a success. `None` means the event carries no return +/// value, which extracts as ``. +pub fn syscall_failed(evt: &ZoneKernelSyscallEvent) -> Option { + get_retval(evt).map(|retval| retval < 0) +} + /// Extract FD number from an event. /// /// Modern BPF driver only captures exit events, so this extracts FD from exit event parameters. @@ -370,3 +379,61 @@ pub fn lookup_service(port: u32, proto: &str) -> String { format!("{}/{}", port, proto) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::proto::generated::protect::control::v1::ZoneKernelEventParam; + + // Builds an exit event carrying `retval` as its first param, the way the + // driver reports a syscall return value: a signed 64-bit two's-complement + // value (negative on error). `event_type` must be an exit code. + fn exit_event_with_retval(event_type: event_codes, retval: i64) -> ZoneKernelSyscallEvent { + ZoneKernelSyscallEvent { + event_type: event_type as u32, + event_category: "EC_FILE | EC_SYSCALL".to_string(), + event_params: vec![ZoneKernelEventParam { + name: "fd".to_string(), + param_data: retval.to_ne_bytes().to_vec(), + ..Default::default() + }], + ..Default::default() + } + } + + #[test] + fn successful_open_is_not_failed() { + // A successful open returns a positive fd, which must read as success - + // this is the regression the old `retval != 0` check got wrong. + let evt = exit_event_with_retval(event_codes::PPME_SYSCALL_OPENAT_2_X, 3); + assert!(has_retval(&evt)); + assert_eq!(get_retval(&evt), Some(3)); + assert_eq!(syscall_failed(&evt), Some(false)); + } + + #[test] + fn failed_open_enoent_is_failed() { + let evt = exit_event_with_retval(event_codes::PPME_SYSCALL_OPENAT_2_X, -2); + assert_eq!(get_retval(&evt), Some(-2)); + assert_eq!(syscall_failed(&evt), Some(true)); + } + + #[test] + fn eagain_return_is_failed() { + // EAGAIN arrives as a negative return (-11), so it reads as failed, in + // line with libsinsp `evt.failed` (no errno is special-cased there). + let evt = exit_event_with_retval(event_codes::PPME_SOCKET_CONNECT_X, -11); + assert_eq!(get_retval(&evt), Some(-11)); + assert_eq!(syscall_failed(&evt), Some(true)); + } + + #[test] + fn enter_event_has_no_retval() { + // Enter events carry no return value; failure extracts as (None). + let evt = exit_event_with_retval(event_codes::PPME_SYSCALL_OPENAT_2_E, 0); + assert!(is_enter(&evt)); + assert!(!has_retval(&evt)); + assert_eq!(get_retval(&evt), None); + assert_eq!(syscall_failed(&evt), None); + } +}