From 78069b4c22695cf9b0056531e510b3b14c742a10 Mon Sep 17 00:00:00 2001 From: John-Erik <79609697+johne8@users.noreply.github.com> Date: Wed, 15 Nov 2023 00:01:14 +0100 Subject: [PATCH 1/3] Update mod.rs, quickfix allow podman to run with --userns=keep-id --- nss/src/cache/mod.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/nss/src/cache/mod.rs b/nss/src/cache/mod.rs index 9d82cdc8..933ff6f5 100644 --- a/nss/src/cache/mod.rs +++ b/nss/src/cache/mod.rs @@ -366,16 +366,19 @@ impl CacheDBBuilder { ))); } - // Checks ownership - if stat.uid() != file.expected_uid || stat.gid() != file.expected_gid { - return Err(CacheError::DatabaseError(format!( - "invalid ownership for {}, expected {}:{} but got {}:{}", - file.path.to_str().unwrap(), - file.expected_uid, - file.expected_gid, - stat.uid(), - stat.gid() - ))); + // skip ownership check if detected owned by nobody. + if stat.uid() != 65534 { + // Checks ownership + if stat.uid() != file.expected_uid || stat.gid() != file.expected_gid { + return Err(CacheError::DatabaseError(format!( + "invalid ownership for {}, expected {}:{} but got {}:{}", + file.path.to_str().unwrap(), + file.expected_uid, + file.expected_gid, + stat.uid(), + stat.gid() + ))); + } } } From 1866d1f2c9394fb65b07c8d3a5c0c2b990a08977 Mon Sep 17 00:00:00 2001 From: John-Erik Haugen Date: Sun, 19 Nov 2023 11:29:57 +0100 Subject: [PATCH 2/3] check cache owner agains overflow(u,g)id when unexpected match --- nss/src/cache/mod.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/nss/src/cache/mod.rs b/nss/src/cache/mod.rs index 933ff6f5..46364b77 100644 --- a/nss/src/cache/mod.rs +++ b/nss/src/cache/mod.rs @@ -345,6 +345,23 @@ impl CacheDBBuilder { Ok(c) } + fn read_file_as_u32(file_path: &str) -> u32 { + match fs::read_to_string(file_path) { + Ok(contents) => { + match contents.trim().parse::() { + Ok(num) => num, + Err(err) => { + eprintln!("Parsing to u32 fail: {}", err); + 0 // fallback to 0 + }, + } + }, + Err(err) => { + eprintln!("error reading file: {}", err); + 0 // fallback to 0 + }, + } + } /// check_file_permissions checks the database files and compares the current ownership and /// permissions with the expected ones. fn check_file_permissions(files: &Vec) -> Result<(), CacheError> { @@ -366,10 +383,12 @@ impl CacheDBBuilder { ))); } - // skip ownership check if detected owned by nobody. - if stat.uid() != 65534 { - // Checks ownership - if stat.uid() != file.expected_uid || stat.gid() != file.expected_gid { + // Checks ownership + if stat.uid() != file.expected_uid || stat.gid() != file.expected_gid { + let overflowuid = Self::read_file_as_u32("/proc/sys/kernel/overflowuid"); + let overflowgid = Self::read_file_as_u32("/proc/sys/kernel/overflowgid"); + // check and don't fail if the file ownership matches kernel overflow uid/gid values + if stat.uid() != overflowuid && stat.gid() != overflowgid { return Err(CacheError::DatabaseError(format!( "invalid ownership for {}, expected {}:{} but got {}:{}", file.path.to_str().unwrap(), @@ -378,6 +397,14 @@ impl CacheDBBuilder { stat.uid(), stat.gid() ))); + }else{ + debug!("unexpected ownership for {}, expected {}:{} but got {}:{}", + file.path.to_str().unwrap(), + file.expected_uid, + file.expected_gid, + stat.uid(), + stat.gid() + ); } } } From 941b339f4f982768706816a09eaec6f1cf35af1c Mon Sep 17 00:00:00 2001 From: John-Erik Haugen Date: Sun, 19 Nov 2023 12:11:48 +0100 Subject: [PATCH 3/3] added check_overflow_uid_gid function --- nss/src/cache/mod.rs | 55 ++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/nss/src/cache/mod.rs b/nss/src/cache/mod.rs index 46364b77..bd6bbefc 100644 --- a/nss/src/cache/mod.rs +++ b/nss/src/cache/mod.rs @@ -345,23 +345,34 @@ impl CacheDBBuilder { Ok(c) } - fn read_file_as_u32(file_path: &str) -> u32 { - match fs::read_to_string(file_path) { - Ok(contents) => { - match contents.trim().parse::() { - Ok(num) => num, - Err(err) => { - eprintln!("Parsing to u32 fail: {}", err); - 0 // fallback to 0 - }, - } - }, - Err(err) => { - eprintln!("error reading file: {}", err); - 0 // fallback to 0 - }, - } + /// check_overflow_uid_gid checks if numbers provided matches with kernel overflow values + /// this is when we are checking owner of cache db, but are running in a namespace, and false values + /// are handed to us. + fn check_overflow_uid_gid(filestat_uid: u32, filestat_gid: u32) -> bool { + + let overflowuid_content = match fs::read_to_string("/proc/sys/kernel/overflowuid") { + Ok(c) => c, + Err(_) => return false, + }; + + let overflowuid = match overflowuid_content.trim().parse::() { + Ok(n) => n, + Err(_) => return false, + }; + + let overflowgid_content = match fs::read_to_string("/proc/sys/kernel/overflowgid") { + Ok(c) => c, + Err(_) => return false, + }; + + let overflowgid = match overflowgid_content.trim().parse::() { + Ok(n) => n, + Err(_) => return false, + }; + + filestat_uid == overflowuid && filestat_gid == overflowgid } + /// check_file_permissions checks the database files and compares the current ownership and /// permissions with the expected ones. fn check_file_permissions(files: &Vec) -> Result<(), CacheError> { @@ -385,10 +396,8 @@ impl CacheDBBuilder { // Checks ownership if stat.uid() != file.expected_uid || stat.gid() != file.expected_gid { - let overflowuid = Self::read_file_as_u32("/proc/sys/kernel/overflowuid"); - let overflowgid = Self::read_file_as_u32("/proc/sys/kernel/overflowgid"); // check and don't fail if the file ownership matches kernel overflow uid/gid values - if stat.uid() != overflowuid && stat.gid() != overflowgid { + if ! Self::check_overflow_uid_gid(stat.uid(), stat.gid()) { return Err(CacheError::DatabaseError(format!( "invalid ownership for {}, expected {}:{} but got {}:{}", file.path.to_str().unwrap(), @@ -397,14 +406,6 @@ impl CacheDBBuilder { stat.uid(), stat.gid() ))); - }else{ - debug!("unexpected ownership for {}, expected {}:{} but got {}:{}", - file.path.to_str().unwrap(), - file.expected_uid, - file.expected_gid, - stat.uid(), - stat.gid() - ); } } }