From 6118142b21084b0a0b163829951168eed17bfe28 Mon Sep 17 00:00:00 2001 From: Vova Kryachko Date: Sun, 6 Apr 2025 15:21:18 +0000 Subject: [PATCH] feat(snix-build/oci): Use user's subordinate ids in oci builds. subuid/subgids used to be hardcoded, which resulted in build failures if those did not match the ones of the effective user. fixes #86 Change-Id: I3b0c3e9ef710aa9e3de998891abe10fd1a893189 Reviewed-on: https://cl.snix.dev/c/snix/+/30301 Tested-by: besadii Reviewed-by: Florian Klink --- snix/Cargo.lock | 15 ++- snix/Cargo.nix | 54 +++++++++- snix/build/Cargo.toml | 1 + snix/build/src/buildservice/oci.rs | 41 +------ snix/build/src/oci/mod.rs | 1 + snix/build/src/oci/spec.rs | 113 ++++++++++++++------ snix/build/src/oci/subuid.rs | 165 +++++++++++++++++++++++++++++ 7 files changed, 316 insertions(+), 74 deletions(-) create mode 100644 snix/build/src/oci/subuid.rs diff --git a/snix/Cargo.lock b/snix/Cargo.lock index 6b911776f..03b001703 100644 --- a/snix/Cargo.lock +++ b/snix/Cargo.lock @@ -2574,6 +2574,18 @@ dependencies = [ "libc", ] +[[package]] +name = "nix" +version = "0.29.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71e2746dc3a24dd78b3cfcb7be93368c6de9963d30f43a6a73998a9cf4b17b46" +dependencies = [ + "bitflags 2.6.0", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nix-compat" version = "0.1.0" @@ -4147,6 +4159,7 @@ dependencies = [ "futures", "itertools 0.12.1", "mimalloc", + "nix 0.29.0", "oci-spec", "prost", "prost-build", @@ -5275,7 +5288,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69fff37da548239c3bf9e64a12193d261e8b22b660991c6fd2df057c168f435f" dependencies = [ "cc", - "windows-targets 0.48.5", + "windows-targets 0.52.6", ] [[package]] diff --git a/snix/Cargo.nix b/snix/Cargo.nix index 5daabfeaf..f4ab5e41a 100644 --- a/snix/Cargo.nix +++ b/snix/Cargo.nix @@ -8153,6 +8153,53 @@ rec { }; resolvedDefaultFeatures = [ "default" "fs" ]; }; + "nix 0.29.0" = rec { + crateName = "nix"; + version = "0.29.0"; + edition = "2021"; + sha256 = "0ikvn7s9r2lrfdm3mx1h7nbfjvcc6s9vxdzw7j5xfkd2qdnp9qki"; + authors = [ + "The nix-rust Project Developers" + ]; + dependencies = [ + { + name = "bitflags"; + packageId = "bitflags 2.6.0"; + } + { + name = "cfg-if"; + packageId = "cfg-if"; + } + { + name = "libc"; + packageId = "libc"; + features = [ "extra_traits" ]; + } + ]; + buildDependencies = [ + { + name = "cfg_aliases"; + packageId = "cfg_aliases"; + } + ]; + features = { + "aio" = [ "pin-utils" ]; + "dir" = [ "fs" ]; + "memoffset" = [ "dep:memoffset" ]; + "mount" = [ "uio" ]; + "mqueue" = [ "fs" ]; + "net" = [ "socket" ]; + "pin-utils" = [ "dep:pin-utils" ]; + "ptrace" = [ "process" ]; + "sched" = [ "process" ]; + "signal" = [ "process" ]; + "socket" = [ "memoffset" ]; + "ucontext" = [ "signal" ]; + "user" = [ "feature" ]; + "zerocopy" = [ "fs" "uio" ]; + }; + resolvedDefaultFeatures = [ "default" "feature" "user" ]; + }; "nix-compat" = rec { crateName = "nix-compat"; version = "0.1.0"; @@ -13483,6 +13530,11 @@ rec { name = "mimalloc"; packageId = "mimalloc"; } + { + name = "nix"; + packageId = "nix 0.29.0"; + features = [ "user" ]; + } { name = "oci-spec"; packageId = "oci-spec"; @@ -17658,7 +17710,7 @@ rec { dependencies = [ { name = "windows-targets"; - packageId = "windows-targets 0.48.5"; + packageId = "windows-targets 0.52.6"; target = { target, features }: (target."windows" or false); } ]; diff --git a/snix/build/Cargo.toml b/snix/build/Cargo.toml index 3e208f298..6591bbf08 100644 --- a/snix/build/Cargo.toml +++ b/snix/build/Cargo.toml @@ -25,6 +25,7 @@ bstr = "1.6.0" data-encoding = "2.5.0" futures = "0.3.30" oci-spec = "0.7.0" +nix = { version = "0.29.0", features = ["user"] } serde_json = "1.0.111" snix-tracing = { path = "../tracing" } uuid = { version = "1.7.0", features = ["v4"] } diff --git a/snix/build/src/buildservice/oci.rs b/snix/build/src/buildservice/oci.rs index 1651f2b27..05603b139 100644 --- a/snix/build/src/buildservice/oci.rs +++ b/snix/build/src/buildservice/oci.rs @@ -1,6 +1,5 @@ use anyhow::Context; use bstr::BStr; -use oci_spec::runtime::{LinuxIdMapping, LinuxIdMappingBuilder}; use snix_castore::{ blobservice::BlobService, directoryservice::DirectoryService, @@ -29,11 +28,6 @@ pub struct OCIBuildService { /// Root path in which all bundles are created in bundle_root: PathBuf, - /// uid mappings to set up for the workloads - uid_mappings: Vec, - /// uid mappings to set up for the workloads - gid_mappings: Vec, - /// Handle to a [BlobService], used by filesystems spawned during builds. blob_service: BS, /// Handle to a [DirectoryService], used by filesystems spawned during builds. @@ -49,40 +43,11 @@ impl OCIBuildService { // We map root inside the container to the uid/gid this is running at, // and allocate one for uid 1000 into the container from the range we // got in /etc/sub{u,g}id. - // TODO: actually read uid, and /etc/subuid. Maybe only when we try to build? // FUTUREWORK: use different uids? Self { bundle_root, blob_service, directory_service, - uid_mappings: vec![ - LinuxIdMappingBuilder::default() - .host_id(1000_u32) - .container_id(0_u32) - .size(1_u32) - .build() - .unwrap(), - LinuxIdMappingBuilder::default() - .host_id(100000_u32) - .container_id(1000_u32) - .size(1_u32) - .build() - .unwrap(), - ], - gid_mappings: vec![ - LinuxIdMappingBuilder::default() - .host_id(100_u32) - .container_id(0_u32) - .size(1_u32) - .build() - .unwrap(), - LinuxIdMappingBuilder::default() - .host_id(100000_u32) - .container_id(100_u32) - .size(1_u32) - .build() - .unwrap(), - ], concurrent_builds: tokio::sync::Semaphore::new(MAX_CONCURRENT_BUILDS), } } @@ -108,11 +73,7 @@ where .context("failed to create spec") .map_err(std::io::Error::other)?; - let mut linux = runtime_spec.linux().clone().unwrap(); - - // edit the spec, we need to setup uid/gid mappings. - linux.set_uid_mappings(Some(self.uid_mappings.clone())); - linux.set_gid_mappings(Some(self.gid_mappings.clone())); + let linux = runtime_spec.linux().clone().unwrap(); runtime_spec.set_linux(Some(linux)); diff --git a/snix/build/src/oci/mod.rs b/snix/build/src/oci/mod.rs index a2400c4a6..354e10051 100644 --- a/snix/build/src/oci/mod.rs +++ b/snix/build/src/oci/mod.rs @@ -1,5 +1,6 @@ mod bundle; mod spec; +pub(crate) mod subuid; pub(crate) use bundle::get_host_output_paths; pub(crate) use bundle::make_bundle; diff --git a/snix/build/src/oci/spec.rs b/snix/build/src/oci/spec.rs index 11ebb4f1a..557cf38cc 100644 --- a/snix/build/src/oci/spec.rs +++ b/snix/build/src/oci/spec.rs @@ -1,12 +1,23 @@ //! Module to create a OCI runtime spec for a given [BuildRequest]. use crate::buildservice::{BuildConstraints, BuildRequest}; -use oci_spec::{ - runtime::{Capability, LinuxNamespace, LinuxNamespaceBuilder, LinuxNamespaceType}, - OciSpecError, +use oci_spec::runtime::{ + Capability, LinuxIdMappingBuilder, LinuxNamespace, LinuxNamespaceBuilder, LinuxNamespaceType, }; use std::{collections::HashSet, path::Path}; -use super::scratch_name; +use super::{ + scratch_name, + subuid::{SubordinateError, SubordinateInfo}, +}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum SpecError { + #[error("oci error: {0}")] + OciError(oci_spec::OciSpecError), + #[error("subordinate error: {0}")] + SubordinateError(SubordinateError), +} /// For a given [BuildRequest], return an OCI runtime spec. /// @@ -33,7 +44,7 @@ pub(crate) fn make_spec( request: &BuildRequest, rootless: bool, sandbox_shell: &str, -) -> Result { +) -> Result { let allow_network = request .constraints .contains(&BuildConstraints::NetworkAccess); @@ -57,39 +68,47 @@ pub(crate) fn make_spec( } oci_spec::runtime::SpecBuilder::default() - .process(configure_process( - &request.command_args, - &request.working_dir, - request - .environment_vars - .iter() - .map(|e| { - ( - e.key.as_str(), - // TODO: decide what to do with non-bytes env values - String::from_utf8(e.value.to_vec()).expect("invalid string in env"), - ) - }) - .collect::>(), - rootless, - )?) + .process( + configure_process( + &request.command_args, + &request.working_dir, + request + .environment_vars + .iter() + .map(|e| { + ( + e.key.as_str(), + // TODO: decide what to do with non-bytes env values + String::from_utf8(e.value.to_vec()).expect("invalid string in env"), + ) + }) + .collect::>(), + rootless, + ) + .map_err(SpecError::OciError)?, + ) .linux(configure_linux(allow_network, rootless)?) .root( oci_spec::runtime::RootBuilder::default() .path("root") .readonly(true) - .build()?, + .build() + .map_err(SpecError::OciError)?, ) .hostname("localhost") - .mounts(configure_mounts( - rootless, - allow_network, - request.scratch_paths.iter().map(|e| e.as_path()), - request.inputs.iter(), - &request.inputs_dir, - ro_host_mounts, - )?) + .mounts( + configure_mounts( + rootless, + allow_network, + request.scratch_paths.iter().map(|e| e.as_path()), + request.inputs.iter(), + &request.inputs_dir, + ro_host_mounts, + ) + .map_err(SpecError::OciError)?, + ) .build() + .map_err(SpecError::OciError) } /// Return the Process part of the OCI Runtime spec. @@ -162,7 +181,7 @@ fn configure_process<'a>( fn configure_linux( allow_network: bool, rootless: bool, -) -> Result { +) -> Result { let mut linux = oci_spec::runtime::Linux::default(); // explicitly set namespaces, depending on allow_network. @@ -187,7 +206,8 @@ fn configure_linux( namespace_types .into_iter() .map(|e| LinuxNamespaceBuilder::default().typ(e).build()) - .collect::, _>>()? + .collect::, _>>() + .map_err(SpecError::OciError)? })); linux.set_masked_paths(Some( @@ -217,6 +237,35 @@ fn configure_linux( .map(|e| e.to_string()) .collect::>(), )); + let info = SubordinateInfo::for_effective_user().map_err(SpecError::SubordinateError)?; + linux.set_uid_mappings(Some(vec![ + LinuxIdMappingBuilder::default() + .host_id(info.uid) + .container_id(0_u32) + .size(1_u32) + .build() + .unwrap(), + LinuxIdMappingBuilder::default() + .host_id(info.subuid) + .container_id(1000_u32) + .size(1_u32) + .build() + .unwrap(), + ])); + linux.set_gid_mappings(Some(vec![ + LinuxIdMappingBuilder::default() + .host_id(info.gid) + .container_id(0_u32) + .size(1_u32) + .build() + .unwrap(), + LinuxIdMappingBuilder::default() + .host_id(info.subgid) + .container_id(100_u32) + .size(1_u32) + .build() + .unwrap(), + ])); Ok(linux) } diff --git a/snix/build/src/oci/subuid.rs b/snix/build/src/oci/subuid.rs new file mode 100644 index 000000000..eb3b0245f --- /dev/null +++ b/snix/build/src/oci/subuid.rs @@ -0,0 +1,165 @@ +use std::{ + fs::File, + io::{BufRead, BufReader}, + num::ParseIntError, + path::PathBuf, +}; + +use nix::{ + errno::Errno, + unistd::{Gid, Group, Uid, User}, +}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub(crate) enum SubordinateError { + #[error("can't determine user {0}")] + UidError(Errno), + + #[error("user entry for {0} does not exist")] + NoPasswdEntry(Uid), + + #[error("can't determine group {0}")] + GidError(Errno), + + #[error("group entry for {0} does not exist")] + NoGroupEntry(Gid), + + #[error("io error {0:?}, file {1}")] + IoError(std::io::Error, PathBuf), + + #[error("failed to parse {0} line '{1}', error {2}")] + ParseError(PathBuf, String, ParseIntError), + + #[error("Missing entry in {0}, for {1}({2})")] + MissingEntry(PathBuf, String, u32), +} + +/// Represents a single (subuid,subgid) pair for a user and their group. +/// +/// In practice there are usually many more subordinate ids than just one, but +/// for oci builds we only need one. If we ever need more, we can improve this +/// implementation. +#[derive(Debug, PartialEq, Eq)] +pub(crate) struct SubordinateInfo { + pub uid: u32, + pub gid: u32, + pub subuid: u32, + pub subgid: u32, +} + +impl SubordinateInfo { + /// Parses /etc/subuid and /etc/subgid and returns a single [SubordinateInfo] for the effective user. + pub(crate) fn for_effective_user() -> Result { + let (user, group) = user_info()?; + + let subuid = + first_subordinate_id(&PathBuf::from("/etc/subuid"), user.uid.as_raw(), &user.name)?; + let subgid = first_subordinate_id( + &PathBuf::from("/etc/subgid"), + group.gid.as_raw(), + &group.name, + )?; + Ok(SubordinateInfo { + uid: user.uid.as_raw(), + gid: group.gid.as_raw(), + subuid, + subgid, + }) + } +} + +/// Returns user and group entries for current effective user. +fn user_info() -> Result<(User, Group), SubordinateError> { + let u = Uid::effective(); + let user = User::from_uid(u) + .map_err(SubordinateError::UidError)? + .ok_or(SubordinateError::NoPasswdEntry(u))?; + let g = Gid::effective(); + let group = Group::from_gid(g) + .map_err(SubordinateError::GidError)? + .ok_or(SubordinateError::NoGroupEntry(g))?; + Ok((user, group)) +} + +fn first_subordinate_id(file: &PathBuf, id: u32, name: &str) -> Result { + let f = File::open(file).map_err(|e| SubordinateError::IoError(e, file.clone()))?; + let reader = BufReader::new(f).lines(); + + for line in reader { + let line = line.map_err(|e| SubordinateError::IoError(e, file.clone()))?; + let line = line.trim(); + let parts: Vec<&str> = line.split(':').collect(); + if parts.len() == 3 && (parts[0] == name || id.to_string() == parts[0]) { + let subuid = parts[1] + .parse::() + .map_err(|e| SubordinateError::ParseError(file.clone(), line.into(), e))?; + let range = parts[2] + .parse::() + .map_err(|e| SubordinateError::ParseError(file.clone(), line.into(), e))?; + if range > 0 { + return Ok(subuid); + } + } + } + + Err(SubordinateError::MissingEntry( + file.clone(), + name.into(), + id, + )) +} + +#[cfg(test)] +mod tests { + use crate::oci::subuid::SubordinateError; + + fn create_fixture<'a>(content: impl IntoIterator) -> tempfile::NamedTempFile { + use std::io::Write; + let mut file = tempfile::NamedTempFile::new().expect("Could not create tempfile"); + for line in content.into_iter() { + writeln!(file, "{}", line).expect(""); + } + file + } + + #[test] + fn test_parse_uid_file_with_name_should_return_first_match() { + let file = create_fixture(["nobody:10000:65", "root:1000:2", "0:2:2"]); + let id = super::first_subordinate_id(&file.path().into(), 0, "root") + .expect("Faild to look up subordinate id."); + assert_eq!(id, 1000); + } + + #[test] + fn test_parse_uid_file_with_uid_should_return_first_match() { + let file = create_fixture(["nobody:10000:65", "0:2:2"]); + let id = super::first_subordinate_id(&file.path().into(), 0, "root") + .expect("Failed to look up subordinate id."); + assert_eq!(id, 2); + } + + #[test] + fn test_missing() { + let file = create_fixture(["roots:1000:2", "1000:2:2"]); + let id = super::first_subordinate_id(&file.path().into(), 0, "root") + .expect_err("Expected not to find a matching subordinate entry."); + assert!(matches!(id, SubordinateError::MissingEntry(_, _, _))); + } + + #[test] + fn test_parse_error() { + let file = create_fixture(["root:hello:2", "1000:2:2"]); + let id = super::first_subordinate_id(&file.path().into(), 0, "root") + .expect_err("Expected parsing to fail."); + assert!(matches!(id, SubordinateError::ParseError(_, _, _))); + } + + #[test] + fn test_parse_errors_in_other_users_files_are_ignored() { + let file = create_fixture(["root:hello:2", "1000:2:2"]); + let id = super::first_subordinate_id(&file.path().into(), 1000, "user") + .expect("Failed to look up subordinate id."); + assert_eq!(id, 2); + } +}