diff --git a/ostree-ext/src/fixture.rs b/ostree-ext/src/fixture.rs index ca27e232..31cd26de 100644 --- a/ostree-ext/src/fixture.rs +++ b/ostree-ext/src/fixture.rs @@ -8,8 +8,10 @@ use crate::container::{Config, ExportOpts, ImageReference, Transport}; use crate::objectsource::{ObjectMeta, ObjectSourceMeta}; use crate::objgv::gv_dirtree; use crate::prelude::*; +use crate::tar::SECURITY_SELINUX_XATTR_C; use crate::{gio, glib}; use anyhow::{anyhow, Context, Result}; +use bootc_utils::CommandRunExt; use camino::{Utf8Component, Utf8Path, Utf8PathBuf}; use cap_std::fs::Dir; use cap_std_ext::cap_std; @@ -25,6 +27,7 @@ use ocidir::oci_spec::image::ImageConfigurationBuilder; use once_cell::sync::Lazy; use regex::Regex; use std::borrow::Cow; +use std::ffi::CString; use std::fmt::Write as _; use std::io::{self, Write}; use std::ops::Add; @@ -46,12 +49,19 @@ enum FileDefType { Directory, } +#[derive(Debug)] +struct Xattr { + key: CString, + value: Box<[u8]>, +} + #[derive(Debug)] pub struct FileDef { uid: u32, gid: u32, mode: u32, path: Cow<'static, Utf8Path>, + xattrs: Box<[Xattr]>, ty: FileDefType, } @@ -66,9 +76,21 @@ impl TryFrom<&'static str> for FileDef { let name = parts.next().ok_or_else(|| anyhow!("Missing file name"))?; let contents = parts.next(); let contents = move || contents.ok_or_else(|| anyhow!("Missing file contents: {}", value)); - if parts.next().is_some() { - anyhow::bail!("Invalid filedef: {}", value); - } + let xattrs: Result> = parts + .map(|xattr| -> Result { + let (k, v) = xattr + .split_once('=') + .ok_or_else(|| anyhow::anyhow!("Invalid xattr: {xattr}"))?; + let mut k: Vec = k.to_owned().into(); + k.push(0); + let r = Xattr { + key: CString::from_vec_with_nul(k).unwrap(), + value: Vec::from(v.to_owned()).into(), + }; + Ok(r) + }) + .collect(); + let xattrs = xattrs?.into(); let ty = match tydef { "r" => FileDefType::Regular(contents()?.into()), "l" => FileDefType::Symlink(Cow::Borrowed(contents()?.into())), @@ -80,6 +102,7 @@ impl TryFrom<&'static str> for FileDef { gid: 0, mode: 0o644, path: Cow::Borrowed(name.into()), + xattrs, ty, }) } @@ -165,6 +188,7 @@ static OWNERS: Lazy> = Lazy::new(|| { ("usr/lib/modules/.*/initramfs", "initramfs"), ("usr/lib/modules", "kernel"), ("usr/bin/(ba)?sh", "bash"), + ("usr/bin/arping", "arping"), ("usr/lib.*/emptyfile.*", "bash"), ("usr/bin/hardlink.*", "testlink"), ("usr/etc/someconfig.conf", "someconfig"), @@ -184,6 +208,7 @@ r usr/lib/modules/5.10.18-200.x86_64/initramfs this-is-an-initramfs m 0 0 755 r usr/bin/bash the-bash-shell l usr/bin/sh bash +r usr/bin/arping arping-binary security.capability=0sAAAAAgAgAAAAAAAAAAAAAAAAAAA= m 0 0 644 # Some empty files r usr/lib/emptyfile @@ -206,7 +231,7 @@ m 0 0 1755 d tmp "## }; pub const CONTENTS_CHECKSUM_V0: &str = - "acc42fb5c796033f034941dc688643bf8beddfd9068d87165344d2b99906220a"; + "bd3d13c3059e63e6f8a3d6d046923ded730d90bd7a055c9ad93312111ea7d395"; // 1 for ostree commit, 2 for max frequency packages, 3 as empty layer pub const LAYERS_V0_LEN: usize = 3usize; pub const PKGS_V0_LEN: usize = 7usize; @@ -267,11 +292,10 @@ impl SeLabel { } pub fn xattrs(&self) -> Vec<(&[u8], &[u8])> { - vec![(b"security.selinux\0", self.to_str().as_bytes())] - } - - pub fn new_xattrs(&self) -> glib::Variant { - self.xattrs().to_variant() + vec![( + SECURITY_SELINUX_XATTR_C.to_bytes_with_nul(), + self.to_str().as_bytes(), + )] } } @@ -286,7 +310,7 @@ pub fn create_dirmeta(path: &Utf8Path, selinux: bool) -> glib::Variant { } else { None }; - let xattrs = label.map(|v| v.new_xattrs()); + let xattrs = label.map(|v| v.xattrs().to_variant()); ostree::create_directory_metadata(&finfo, xattrs.as_ref()) } @@ -632,7 +656,18 @@ impl Fixture { } else { None }; - let xattrs = label.map(|v| v.new_xattrs()); + let mut xattrs = label.as_ref().map(|v| v.xattrs()).unwrap_or_default(); + xattrs.extend( + def.xattrs + .iter() + .map(|xattr| (xattr.key.as_bytes_with_nul(), &xattr.value[..])), + ); + let xattrs = if xattrs.is_empty() { + None + } else { + xattrs.sort_by(|a, b| a.0.cmp(b.0)); + Some(xattrs.to_variant()) + }; let xattrs = xattrs.as_ref(); let checksum = match &def.ty { FileDefType::Regular(contents) => self @@ -724,6 +759,21 @@ impl Fixture { gio::Cancellable::NONE, )?; + // Verify that this is what is expected. + let commit_object = self.srcrepo.load_commit(&commit)?.0; + let content_checksum = ostree::commit_get_content_checksum(&commit_object).unwrap(); + if content_checksum != CONTENTS_CHECKSUM_V0 { + // Only spew this once + static DUMP_OSTREE: std::sync::Once = std::sync::Once::new(); + DUMP_OSTREE.call_once(|| { + let _ = Command::new("ostree") + .arg(format!("--repo={}", self.path.join("src/repo"))) + .args(["ls", "-X", "-C", "-R", commit.as_str()]) + .run(); + }); + } + assert_eq!(CONTENTS_CHECKSUM_V0, content_checksum.as_str()); + Ok(()) } diff --git a/ostree-ext/src/tar/export.rs b/ostree-ext/src/tar/export.rs index 5543ac42..eac79ab6 100644 --- a/ostree-ext/src/tar/export.rs +++ b/ostree-ext/src/tar/export.rs @@ -13,11 +13,23 @@ use ostree::gio; use std::borrow::Borrow; use std::borrow::Cow; use std::collections::HashSet; +use std::ffi::CStr; use std::io::BufReader; /// The repository mode generated by a tar export stream. pub const BARE_SPLIT_XATTRS_MODE: &str = "bare-split-xattrs"; +/// The SELinux xattr. Because the ostree xattrs require an embedded NUL, we +/// store that version as a constant. +pub(crate) const SECURITY_SELINUX_XATTR_C: &CStr = c"security.selinux"; +/// Then derive a string version (without the NUL) from the above. +pub(crate) const SECURITY_SELINUX_XATTR: &str = const { + match SECURITY_SELINUX_XATTR_C.to_str() { + Ok(r) => r, + Err(_) => unreachable!(), + } +}; + // This is both special in the tar stream *and* it's in the ostree commit. const SYSROOT: &str = "sysroot"; // This way the default ostree -> sysroot/ostree symlink works. @@ -379,6 +391,32 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { Ok(true) } + /// Append all xattrs to the tar stream *except* security.selinux, because + /// that one doesn't become visible in `podman run` anyways, so we couldn't + /// rely on it in some cases. + /// https://github.com/containers/storage/blob/0d4a8d2aaf293c9f0464b888d932ab5147a284b9/pkg/archive/archive.go#L85 + #[context("Writing tar xattrs")] + fn append_tarstream_xattrs(&mut self, xattrs: &glib::Variant) -> Result<()> { + let v = xattrs.data_as_bytes(); + let v = v.try_as_aligned().unwrap(); + let v = gvariant::gv!("a(ayay)").cast(v); + let mut pax_extensions = Vec::new(); + for entry in v { + let (k, v) = entry.to_tuple(); + let k = CStr::from_bytes_with_nul(k).unwrap(); + let k = k + .to_str() + .with_context(|| format!("Found non-UTF8 xattr: {k:?}"))?; + if k == SECURITY_SELINUX_XATTR { + continue; + } + pax_extensions.push((format!("SCHILY.xattr.{k}"), v)); + } + self.out + .append_pax_extensions(pax_extensions.iter().map(|(k, v)| (k.as_str(), *v)))?; + Ok(()) + } + /// Write a content object, returning the path/header that should be used /// as a hard link to it in the target path. This matches how ostree checkouts work. fn append_content(&mut self, checksum: &str) -> Result<(Utf8PathBuf, tar::Header)> { @@ -402,6 +440,7 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { // refer to. Otherwise the importing logic won't have the xattrs available // when importing file content. self.append_ostree_xattrs(checksum, &xattrs)?; + self.append_tarstream_xattrs(&xattrs)?; if let Some(instream) = instream { ensure!(meta.file_type() == gio::FileType::Regular); diff --git a/ostree-ext/tests/it/main.rs b/ostree-ext/tests/it/main.rs index f460a423..2d221810 100644 --- a/ostree-ext/tests/it/main.rs +++ b/ostree-ext/tests/it/main.rs @@ -5,6 +5,8 @@ use camino::Utf8Path; use cap_std::fs::{Dir, DirBuilder, DirBuilderExt}; use cap_std_ext::cap_std; use containers_image_proxy::oci_spec; +use gvariant::aligned_bytes::TryAsAligned; +use gvariant::{Marker, Structure}; use oci_image::ImageManifest; use oci_spec::image as oci_image; use ocidir::oci_spec::image::{Arch, DigestAlgorithm}; @@ -221,6 +223,7 @@ struct TarExpected { path: &'static str, etype: tar::EntryType, mode: u32, + should_have_security_capability: bool, } #[allow(clippy::from_over_into)] @@ -230,6 +233,19 @@ impl Into for (&'static str, tar::EntryType, u32) { path: self.0, etype: self.1, mode: self.2, + should_have_security_capability: false, + } + } +} + +#[allow(clippy::from_over_into)] +impl Into for (&'static str, tar::EntryType, u32, bool) { + fn into(self) -> TarExpected { + TarExpected { + path: self.0, + etype: self.1, + mode: self.2, + should_have_security_capability: self.3, } } } @@ -244,7 +260,7 @@ fn validate_tar_expected( let mut seen_paths = HashSet::new(); // Verify we're injecting directories, fixes the absence of `/tmp` in our // images for example. - for entry in entries { + for mut entry in entries { if expected.is_empty() { return Ok(()); } @@ -265,6 +281,21 @@ fn validate_tar_expected( header.entry_type(), entry_path ); + if exp.should_have_security_capability { + let pax = entry + .pax_extensions()? + .ok_or_else(|| anyhow::anyhow!("Missing pax extensions for {entry_path}"))?; + let mut found = false; + for ent in pax { + let ent = ent?; + if ent.key_bytes() != b"SCHILY.xattr.security.capability" { + continue; + } + found = true; + break; + } + assert!(found, "Expected security.capability in {entry_path}"); + } } } @@ -312,6 +343,9 @@ fn common_tar_contents_all() -> impl Iterator { ] .into_iter() .map(Into::into) + .chain(std::iter::once( + ("sysroot/ostree/repo/objects/b0/48a4f451e9fdfaaec911c1fe07d5d1d39be02f932b827c25458d3b15ae589e.file", Regular, 0o755, true).into(), + )) } /// Validate metadata (prelude) in a v1 tar. @@ -335,6 +369,7 @@ fn test_tar_export_structure() -> Result<()> { let fixture = Fixture::new_v1()?; let src_tar = fixture.export_tar()?; + std::fs::copy(fixture.path.join(src_tar), "/tmp/test.tar").unwrap(); let mut src_tar = fixture .dir .open(src_tar) @@ -932,7 +967,7 @@ async fn test_container_chunked() -> Result<()> { .created_by() .as_ref() .unwrap(), - "8 components" + "9 components" ); } let import = imp.import(prep).await.context("Init pull derived").unwrap(); @@ -991,9 +1026,9 @@ r usr/bin/bash bash-v0 assert!(second.0.commit.is_none()); assert_eq!( first.1, - "ostree export of commit fe4ba8bbd8f61a69ae53cde0dd53c637f26dfbc87717b2e71e061415d931361e" + "ostree export of commit 6e6afc49d902daa2456c858818e0ad8bf9afe79cdcca738c5676c0e175c1def1" ); - assert_eq!(second.1, "8 components"); + assert_eq!(second.1, "9 components"); assert_eq!(store::list_images(fixture.destrepo()).unwrap().len(), 1); let n = store::count_layer_references(fixture.destrepo())? as i64; @@ -1785,6 +1820,36 @@ async fn test_container_xattr() -> Result<()> { _ => unreachable!(), }; + // Verify security.capability is in the ostree commit + let arping = "/usr/bin/arping"; + { + let ostree_root = fixture + .srcrepo() + .read_commit(fixture.testref(), gio::Cancellable::NONE)? + .0; + let arping_ostree = ostree_root.resolve_relative_path(arping); + assert_eq!( + arping_ostree.query_file_type( + gio::FileQueryInfoFlags::NOFOLLOW_SYMLINKS, + gio::Cancellable::NONE + ), + gio::FileType::Regular + ); + let arping_ostree = arping_ostree.downcast_ref::().unwrap(); + let arping_ostree_xattrs = arping_ostree.xattrs(gio::Cancellable::NONE)?; + let v = arping_ostree_xattrs.data_as_bytes(); + let v = v.try_as_aligned().unwrap(); + let v = gvariant::gv!("a(ayay)").cast(v); + assert!(v + .iter() + .find(|entry| { + let k = entry.to_tuple().0; + let k = std::ffi::CStr::from_bytes_with_nul(k).unwrap(); + k.to_str().ok() == Some("security.capability") + }) + .is_some()); + } + // Build a derived image let derived_path = &fixture.path.join("derived.oci"); oci_clone(basepath, derived_path).await?; @@ -1832,6 +1897,8 @@ async fn test_container_xattr() -> Result<()> { ) .read()?; assert!(out.contains("'user.foo', [byte 0x62, 0x61, 0x72]")); + let out = cmd!(sh, "ostree --repo=dest/repo ls -X {merge_commit} {arping}").read()?; + assert!(out.contains("'security.capability'")); Ok(()) }