Skip to content

Commit

Permalink
ostree-ext: Serialize xattrs into tar stream as well
Browse files Browse the repository at this point in the history 
We really want this for coreos/rpm-ostree#5222
to be able to rebuild images from their container-synthesized rootfs.

Really, the only xattr we don't want to emit in to the tar stream
is security.selinux for now.

Eventually we should try to switch to putting that into the tar
stream too, but it needs more validation.

Signed-off-by: Colin Walters <[email protected]>
  • Loading branch information
@cgwalters
cgwalters committed Jan 15, 2025
1 parent a5b4af9 commit c0df7af
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 15 deletions.
55 changes: 44 additions & 11 deletions ostree-ext/src/fixture.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ 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 camino::{Utf8Component, Utf8Path, Utf8PathBuf};
Expand All @@ -25,6 +26,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;
Expand All @@ -46,12 +48,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,
}

Expand All @@ -66,9 +75,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<Vec<_>> = parts
.map(|xattr| -> Result<Xattr> {
let (k, v) = xattr
.split_once('=')
.ok_or_else(|| anyhow::anyhow!("Invalid xattr: {xattr}"))?;
let mut k: Vec<u8> = 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())),
Expand All @@ -80,6 +101,7 @@ impl TryFrom<&'static str> for FileDef {
gid: 0,
mode: 0o644,
path: Cow::Borrowed(name.into()),
xattrs,
ty,
})
}
Expand Down Expand Up @@ -165,6 +187,7 @@ static OWNERS: Lazy<Vec<(Regex, &str)>> = 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"),
Expand All @@ -184,6 +207,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
Expand All @@ -206,7 +230,7 @@ m 0 0 1755
d tmp
"## };
pub const CONTENTS_CHECKSUM_V0: &str =
"acc42fb5c796033f034941dc688643bf8beddfd9068d87165344d2b99906220a";
"4449a2b27dd907ffb5e3556018584cfa048edaf3eee4a11ecf21dcd61b6c7a1c";
// 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;
Expand Down Expand Up @@ -267,11 +291,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(),
)]
}
}

Expand All @@ -286,7 +309,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())
}

Expand Down Expand Up @@ -632,7 +655,17 @@ 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 {
Some(xattrs.to_variant())
};
let xattrs = xattrs.as_ref();
let checksum = match &def.ty {
FileDefType::Regular(contents) => self
Expand Down
38 changes: 38 additions & 0 deletions ostree-ext/src/tar/export.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -379,6 +391,31 @@ 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((k, v));
}
self.out.append_pax_extensions(pax_extensions)?;
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)> {
Expand All @@ -402,6 +439,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);
Expand Down
74 changes: 70 additions & 4 deletions ostree-ext/tests/it/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -221,6 +223,7 @@ struct TarExpected {
path: &'static str,
etype: tar::EntryType,
mode: u32,
should_have_security_capability: bool,
}

#[allow(clippy::from_over_into)]
Expand All @@ -230,6 +233,19 @@ impl Into<TarExpected> 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<TarExpected> 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,
}
}
}
Expand All @@ -244,7 +260,7 @@ fn validate_tar_expected<T: std::io::Read>(
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(());
}
Expand All @@ -265,6 +281,21 @@ fn validate_tar_expected<T: std::io::Read>(
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() != c"security.capability".to_bytes_with_nul() {
continue;
}
found = true;
break;
}
assert!(found, "Expected security.capability in {entry_path}");
}
}
}

Expand Down Expand Up @@ -312,6 +343,9 @@ fn common_tar_contents_all() -> impl Iterator<Item = TarExpected> {
]
.into_iter()
.map(Into::into)
.chain(std::iter::once(
("usr/bin/arping", Link, 0o755, true).into(),
))
}

/// Validate metadata (prelude) in a v1 tar.
Expand Down Expand Up @@ -932,7 +966,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();
Expand Down Expand Up @@ -991,9 +1025,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 a2b98a97aae864c69360010c16c871b087ce2f86e915091caff1c61b824e7f54"
);
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;
Expand Down Expand Up @@ -1785,6 +1819,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::<ostree::RepoFile>().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?;
Expand Down Expand Up @@ -1832,6 +1896,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(())
}
Expand Down

0 comments on commit c0df7af

Please sign in to comment.