From d954b4eb55a0554eb5639e194dd686e39a8a77d2 Mon Sep 17 00:00:00 2001 From: qjerome Date: Thu, 5 Dec 2024 13:50:57 +0100 Subject: [PATCH] fix: bug user/group parsing triggering strategy --- kunai/src/bin/main.rs | 21 ++----- kunai/src/cache.rs | 116 +++++++++++++------------------------- kunai/src/util/account.rs | 12 +++- 3 files changed, 54 insertions(+), 95 deletions(-) diff --git a/kunai/src/bin/main.rs b/kunai/src/bin/main.rs index 89a85f4..05899eb 100644 --- a/kunai/src/bin/main.rs +++ b/kunai/src/bin/main.rs @@ -1559,30 +1559,17 @@ impl<'s> EventConsumer<'s> { ti: &bpf_events::TaskInfo, ) -> TaskAdditionalInfo { // getting user and group information for task - let user = self + let (user, group) = self .cache - .get_user_in_ns(mnt_ns, &ti.uid) + .get_user_group_in_ns(mnt_ns, &ti.uid, &ti.gid) .inspect_err(|e| { if !e.is_unknown_ns() { error!("failed to get task user: {e}") } }) - .unwrap_or_default() - .cloned(); - - // getting group information for task - let group = self - .cache - .get_group_in_ns(mnt_ns, &ti.gid) - .inspect_err(|e| { - if !e.is_unknown_ns() { - error!("failed to get task group: {e}") - } - }) - .unwrap_or_default() - .cloned(); + .unwrap_or_default(); - TaskAdditionalInfo::new(user, group) + TaskAdditionalInfo::new(user.cloned(), group.cloned()) } #[inline(always)] diff --git a/kunai/src/cache.rs b/kunai/src/cache.rs index bac49ba..dda5a0a 100644 --- a/kunai/src/cache.rs +++ b/kunai/src/cache.rs @@ -199,7 +199,7 @@ impl Path { #[derive(Debug, PartialEq, Eq, Hash, Clone)] struct Key { - mnt_namespace: u32, + mnt_namespace: Mnt, path: PathBuf, size: u64, modified: SystemTime, @@ -210,7 +210,7 @@ struct Key { impl Default for Key { fn default() -> Self { Key { - mnt_namespace: 0, + mnt_namespace: Mnt::default(), path: PathBuf::default(), size: 0, modified: SystemTime::UNIX_EPOCH, @@ -222,7 +222,7 @@ impl Default for Key { impl Key { #[inline(always)] - fn from_path_in_ns(ns: &Mnt, path: &Path) -> Result { + fn from_path_in_ns(ns: Mnt, path: &Path) -> Result { let pb = path.to_path_buf(); // checking if the file still exists @@ -233,7 +233,7 @@ impl Key { let meta = pb.metadata()?; let k = Key { - mnt_namespace: ns.inum, + mnt_namespace: ns, path: pb.clone(), size: meta.size(), modified: SystemTime::from(&Time::new(meta.mtime(), meta.mtime_nsec())), @@ -269,16 +269,11 @@ impl Key { unsafe impl Send for Key {} unsafe impl Sync for Key {} -#[derive(Debug)] -struct UsersAndGroups { - users: Users, - groups: Groups, -} - pub struct Cache { mnt_namespaces: LruHashMap>, hashes: LruHashMap, - users_groups: LruHashMap, + users: LruHashMap, + groups: LruHashMap, // since hashes and signatures are not computed // at the same time. It seems a better option // to separate into two HashMaps to prevent @@ -293,7 +288,8 @@ impl Cache { pub fn with_max_entries(cap: usize) -> Self { Cache { mnt_namespaces: LruHashMap::with_max_entries(NS_CACHE_SIZE), - users_groups: LruHashMap::with_max_entries(NS_CACHE_SIZE), + users: LruHashMap::with_max_entries(NS_CACHE_SIZE), + groups: LruHashMap::with_max_entries(NS_CACHE_SIZE), hashes: LruHashMap::with_max_entries(cap), signatures: LruHashMap::with_max_entries(cap), } @@ -311,80 +307,48 @@ impl Cache { /// Get a [User] structure corresponding to user id `uid` #[inline(always)] - pub fn get_user_in_ns(&mut self, ns: Mnt, uid: &u32) -> Result, Error> { + pub fn get_user_group_in_ns( + &mut self, + ns: Mnt, + uid: &u32, + gid: &u32, + ) -> Result<(Option<&User>, Option<&Group>), Error> { let Some(mnt_ns) = self.mnt_namespaces.get(&ns) else { return Err(Error::UnknownMntNs(ns)); }; - // we have already parsed users and groups - if self.users_groups.contains_key(&ns) { - // we have an entry for uid so we return it - if self - .users_groups - .get(&ns) - .map(|ung| ung.users.contains_uid(uid)) - .unwrap_or_default() - { - return Ok(self - .users_groups - .get(&ns) - .and_then(|users| users.users.get_by_uid(uid))); - } - } - // we haven't yet parsed users and groups or we don't find an entry - let users = mnt_ns.do_in_namespace(|| { - Ok(UsersAndGroups { - users: Users::from_sys().map_err(namespace::Error::other)?, - groups: Groups::from_sys().map_err(namespace::Error::other)?, - }) - })?; - - self.users_groups.insert(ns, users); + let user_group = mnt_ns.do_in_namespace(|| { + // getting user + let ukey = Key::from_path_in_ns(ns, &Users::sys_path().into()) + .map_err(namespace::Error::other)?; + + if !self.users.contains_key(&ukey) { + self.users.insert( + ukey.clone(), + Users::from_sys().map_err(namespace::Error::other)?, + ); + } - Ok(self - .users_groups - .get(&ns) - .and_then(|ung| ung.users.get_by_uid(uid))) - } + let user = self.users.get(&ukey).and_then(|u| u.get_by_uid(uid)); - /// Get a [Group] structure corresponding to group id `gid` - #[inline(always)] - pub fn get_group_in_ns(&mut self, ns: Mnt, gid: &u32) -> Result, Error> { - let Some(mnt_ns) = self.mnt_namespaces.get(&ns) else { - return Err(Error::UnknownMntNs(ns)); - }; + // getting group + let gkey = Key::from_path_in_ns(ns, &Groups::sys_path().into()) + .map_err(namespace::Error::other)?; - // we have already parsed users and groups - if self.users_groups.contains_key(&ns) { - // we have an entry for uid so we return it - if self - .users_groups - .get(&ns) - .map(|ung| ung.groups.contains_gid(gid)) - .unwrap_or_default() - { - return Ok(self - .users_groups - .get(&ns) - .and_then(|users| users.groups.get_by_gid(gid))); + if !self.groups.contains_key(&gkey) { + self.groups.insert( + gkey.clone(), + Groups::from_sys().map_err(namespace::Error::other)?, + ); } - } - // we haven't yet parsed users and groups or we don't find an entry - let users = mnt_ns.do_in_namespace(|| { - Ok(UsersAndGroups { - users: Users::from_sys().map_err(namespace::Error::other)?, - groups: Groups::from_sys().map_err(namespace::Error::other)?, - }) - })?; + let group = self.groups.get(&gkey).and_then(|g| g.get_by_gid(gid)); - self.users_groups.insert(ns, users); + Ok((user, group)) + })?; - Ok(self - .users_groups - .get(&ns) - .and_then(|ung| ung.groups.get_by_gid(gid))) + Ok(user_group) } #[inline(always)] @@ -403,7 +367,7 @@ impl Cache { let pb = path.to_path_buf(); // we create key to check if we already have cached // signatures for that file - let key = Key::from_path_in_ns(&ns, path).map_err(namespace::Error::other)?; + let key = Key::from_path_in_ns(ns, path).map_err(namespace::Error::other)?; let sigs = match self.signatures.get(&key) { // we have caches signatures @@ -442,7 +406,7 @@ impl Cache { let res = mnt_ns.do_in_namespace(|| { let pb = path.to_path_buf(); - let key = Key::from_path_in_ns(&ns, path).map_err(namespace::Error::other)?; + let key = Key::from_path_in_ns(ns, path).map_err(namespace::Error::other)?; if !self.hashes.contains_key(&key) { let h = Hashes::from_path_ref(pb); diff --git a/kunai/src/util/account.rs b/kunai/src/util/account.rs index db729bc..245140f 100644 --- a/kunai/src/util/account.rs +++ b/kunai/src/util/account.rs @@ -98,6 +98,10 @@ impl Users { Self::default() } + pub const fn sys_path() -> &'static str { + "/etc/passwd" + } + #[inline] pub fn extend_from_vec>(&mut self, v: Vec) -> Result<&mut Self, Error> { let mut lines = Vec::new(); @@ -140,7 +144,7 @@ impl Users { pub fn from_sys() -> Result { let mut out = Self::new(); - out.extend_from_file("/etc/passwd")?; + out.extend_from_file(Self::sys_path())?; Ok(out) } @@ -184,6 +188,10 @@ impl Groups { Self::default() } + pub const fn sys_path() -> &'static str { + "/etc/group" + } + #[inline] pub fn extend_from_vec>(&mut self, v: Vec) -> Result<&mut Self, Error> { let mut lines = Vec::new(); @@ -215,7 +223,7 @@ impl Groups { pub fn from_sys() -> Result { let mut out = Self::default(); - out.extend_from_file("/etc/group")?; + out.extend_from_file(Self::sys_path())?; Ok(out) }