From a9ca65c151d2c27838dffb4135833897fa72a956 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 1 Oct 2024 17:56:38 -0400 Subject: [PATCH] treefile: Add ignore-devices We hit another case where people are pulling a container image with devices in `/dev` in the tar stream; they're then trying to commit this to an ostree. There's much better ways to fix this: - Change the image to stop including devices as there's no reason to do so - Switch to logically bound images instead of physically bound - Use the composefs backend for c/storage Eventually I may look at "quoting" generally in ostree, but it's fairly invasive: https://github.com/ostreedev/ostree/issues/2568 In practice today, simply ignoring the files will happen to work for "podman run" of such images; podman will just use overlayfs to stitch together the `diff` directories, and doesn't try to do any validation of their contents today. (Queue the composefs integration, which *would* do that but would also fix this anwyays) Signed-off-by: Colin Walters --- docs/treefile.md | 4 ++ rpmostree-cxxrs.cxx | 11 ++-- rpmostree-cxxrs.h | 5 +- rust/src/composepost.rs | 74 +++++++++++++++++++-------- rust/src/lib.rs | 2 +- rust/src/treefile.rs | 7 +++ src/libpriv/rpmostree-postprocess.cxx | 2 +- tests/compose/test-installroot.sh | 8 +++ 8 files changed, 84 insertions(+), 29 deletions(-) diff --git a/docs/treefile.md b/docs/treefile.md index ad5cb04b86..daac3a3c90 100644 --- a/docs/treefile.md +++ b/docs/treefile.md @@ -40,6 +40,10 @@ It supports the following parameters: * `selinux`: boolean, optional: Defaults to `true`. If `false`, then no SELinux labeling will be performed on the server side. + * `ignore-devices`: boolean, optional: Defaults to `true`. If `true`, then + all character and block device files found in the target root (except overlayfs + whiteouts, which are automatically "quoted") will be ignored. + * `ima`: boolean, optional: Defaults to `false`. Propagate any IMA signatures in input RPMs into the final OSTree commit. diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx index 649d1b7b8c..2a4cb12cee 100644 --- a/rpmostree-cxxrs.cxx +++ b/rpmostree-cxxrs.cxx @@ -192,6 +192,8 @@ class Slice final : private detail::copy_assignable_if::value> Slice () noexcept; Slice (T *, std::size_t count) noexcept; + template explicit Slice (C &c) : Slice (c.data (), c.size ()) {} + Slice &operator= (const Slice &) &noexcept = default; Slice &operator= (Slice &&) &noexcept = default; @@ -2206,8 +2208,8 @@ extern "C" ::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile &treefile, ::rust::Str next_version, bool unified_core) noexcept; - ::rust::repr::PtrLen - rpmostreecxx$cxxbridge1$compose_postprocess_final_pre (::std::int32_t rootfs_dfd) noexcept; + ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$compose_postprocess_final_pre ( + ::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile const &treefile) noexcept; ::rust::repr::PtrLen rpmostreecxx$cxxbridge1$compose_postprocess_final ( ::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile const &treefile) noexcept; @@ -4011,9 +4013,10 @@ compose_postprocess (::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile &treefi } void -compose_postprocess_final_pre (::std::int32_t rootfs_dfd) +compose_postprocess_final_pre (::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile const &treefile) { - ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$compose_postprocess_final_pre (rootfs_dfd); + ::rust::repr::PtrLen error$ + = rpmostreecxx$cxxbridge1$compose_postprocess_final_pre (rootfs_dfd, treefile); if (error$.ptr) { throw ::rust::impl< ::rust::Error>::error (error$); diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h index d38fd1dbbd..9d62380e50 100644 --- a/rpmostree-cxxrs.h +++ b/rpmostree-cxxrs.h @@ -191,6 +191,8 @@ class Slice final : private detail::copy_assignable_if::value> Slice () noexcept; Slice (T *, std::size_t count) noexcept; + template explicit Slice (C &c) : Slice (c.data (), c.size ()) {} + Slice &operator= (const Slice &) &noexcept = default; Slice &operator= (Slice &&) &noexcept = default; @@ -1867,7 +1869,8 @@ void composepost_nsswitch_altfiles (::std::int32_t rootfs_dfd); void compose_postprocess (::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile &treefile, ::rust::Str next_version, bool unified_core); -void compose_postprocess_final_pre (::std::int32_t rootfs_dfd); +void compose_postprocess_final_pre (::std::int32_t rootfs_dfd, + ::rpmostreecxx::Treefile const &treefile); void compose_postprocess_final (::std::int32_t rootfs_dfd, ::rpmostreecxx::Treefile const &treefile); diff --git a/rust/src/composepost.rs b/rust/src/composepost.rs index 72ca67ea61..96f2c8d40c 100644 --- a/rust/src/composepost.rs +++ b/rust/src/composepost.rs @@ -295,40 +295,69 @@ fn is_overlay_whiteout(meta: &cap_std::fs::Metadata) -> bool { (meta.mode() & libc::S_IFMT) == libc::S_IFCHR && meta.rdev() == 0 } -/// Auto-synthesize embedded overlayfs whiteouts; for more information -/// see https://github.com/ostreedev/ostree/pull/2722/commits/0085494e350c72599fc5c0e00422885d80b3c660 -#[context("Postprocessing embedded overlayfs")] -fn postprocess_embedded_ovl_whiteouts(root: &Dir) -> Result<()> { +/// Automatically "quote" embeded overlayfs whiteouts as regular files, and +/// if configured error out on devices or ignore them. +/// For more on overlayfs, see https://github.com/ostreedev/ostree/pull/2722/commits/0085494e350c72599fc5c0e00422885d80b3c660 +#[context("Postprocessing devices")] +fn postprocess_devices(root: &Dir, treefile: &Treefile) -> Result<()> { const OSTREE_WHITEOUT_PREFIX: &str = ".ostree-wh."; - fn recurse(root: &Dir, path: &Utf8Path) -> Result { - let mut n = 0; + let mut n_overlay = 0u64; + let mut n_devices = 0u64; + fn recurse( + root: &Dir, + path: &Utf8Path, + ignore_devices: bool, + n_overlay: &mut u64, + n_devices: &mut u64, + ) -> Result<()> { for entry in root.read_dir(path)? { let entry = entry?; let meta = entry.metadata()?; let name = PathBuf::from(entry.file_name()); let name: Utf8PathBuf = name.try_into()?; if meta.is_dir() { - n += recurse(root, &path.join(name))?; + recurse(root, &path.join(name), ignore_devices, n_overlay, n_devices)?; continue; } - if !is_overlay_whiteout(&meta) { + let is_device = matches!(meta.mode() & libc::S_IFMT, libc::S_IFCHR | libc::S_IFBLK); + if !is_device { continue; - }; + } let srcpath = path.join(&name); - let targetname = format!("{OSTREE_WHITEOUT_PREFIX}{name}"); - let destpath = path.join(&targetname); - root.remove_file(srcpath)?; - root.atomic_write_with_perms(destpath, "", meta.permissions())?; - n += 1; + if is_overlay_whiteout(&meta) { + let targetname = format!("{OSTREE_WHITEOUT_PREFIX}{name}"); + let destpath = path.join(&targetname); + root.remove_file(srcpath)?; + root.atomic_write_with_perms(destpath, "", meta.permissions())?; + *n_overlay += 1; + continue; + } + if ignore_devices { + root.remove_file(srcpath)?; + *n_devices += 1; + } else { + anyhow::bail!("Unsupported device file: {srcpath}") + } } - Ok(n) + Ok(()) } - let n = recurse(root, ".".into())?; - if n > 0 { - println!("Processed {n} embedded whiteouts"); + recurse( + root, + ".".into(), + treefile.get_ignore_devices(), + &mut n_overlay, + &mut n_devices, + )?; + if n_overlay > 0 { + println!("Processed {n_overlay} embedded whiteouts"); } else { println!("No embedded whiteouts found"); } + if n_devices > 0 { + println!("Ignored {n_devices} device files"); + } else { + println!("No device files found"); + } Ok(()) } @@ -420,7 +449,7 @@ pub(crate) fn postprocess_cleanup_rpmdb(rootfs_dfd: i32) -> CxxResult<()> { /// it as the bits of that function that we've chosen to implement in Rust. /// It takes care of all things that are really required to use rpm-ostree /// on the target host. -pub fn compose_postprocess_final_pre(rootfs_dfd: i32) -> CxxResult<()> { +pub fn compose_postprocess_final_pre(rootfs_dfd: i32, treefile: &Treefile) -> CxxResult<()> { let rootfs_dfd = unsafe { &crate::ffiutil::ffi_dirfd(rootfs_dfd)? }; // These tasks can safely run in parallel, so just for fun we do so via rayon. let tasks = [ @@ -430,7 +459,7 @@ pub fn compose_postprocess_final_pre(rootfs_dfd: i32) -> CxxResult<()> { ]; tasks.par_iter().try_for_each(|f| f(rootfs_dfd))?; // This task recursively traverses the filesystem and hence should be serial. - postprocess_embedded_ovl_whiteouts(rootfs_dfd)?; + postprocess_devices(rootfs_dfd, treefile)?; Ok(()) } @@ -1536,11 +1565,12 @@ OSTREE_VERSION='33.4' // We don't actually test creating whiteout devices here as that // may not work. let td = cap_tempfile::tempdir(cap_std::ambient_authority())?; + let tf = crate::treefile::tests::new_test_tf_basic("")?; // Verify no-op case - postprocess_embedded_ovl_whiteouts(&td).unwrap(); + postprocess_devices(&td, &tf).unwrap(); td.create("foo")?; td.symlink("foo", "bar")?; - postprocess_embedded_ovl_whiteouts(&td).unwrap(); + postprocess_devices(&td, &tf).unwrap(); assert!(td.try_exists("foo")?); assert!(td.try_exists("bar")?); diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 7052304e48..3cd535967f 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -299,7 +299,7 @@ pub mod ffi { next_version: &str, unified_core: bool, ) -> Result<()>; - fn compose_postprocess_final_pre(rootfs_dfd: i32) -> Result<()>; + fn compose_postprocess_final_pre(rootfs_dfd: i32, treefile: &Treefile) -> Result<()>; fn compose_postprocess_final(rootfs_dfd: i32, treefile: &Treefile) -> Result<()>; fn convert_var_to_tmpfiles_d(rootfs_dfd: i32, cancellable: &GCancellable) -> Result<()>; fn rootfs_prepare_links( diff --git a/rust/src/treefile.rs b/rust/src/treefile.rs index ced64b4dba..43e96744e3 100644 --- a/rust/src/treefile.rs +++ b/rust/src/treefile.rs @@ -450,6 +450,7 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) { rojig, selinux, selinux_label_version, + ignore_devices, ima, gpg_key, include, @@ -1320,6 +1321,10 @@ impl Treefile { self.parsed.base.selinux.unwrap_or(true) } + pub(crate) fn get_ignore_devices(&self) -> bool { + self.parsed.base.ignore_devices.unwrap_or(true) + } + pub(crate) fn get_selinux_label_version(&self) -> u32 { self.parsed.base.selinux_label_version.unwrap_or_default() } @@ -2505,6 +2510,8 @@ pub(crate) struct BaseComposeConfigFields { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) selinux: Option, #[serde(skip_serializing_if = "Option::is_none")] + pub(crate) ignore_devices: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub(crate) selinux_label_version: Option, #[serde(skip_serializing_if = "Option::is_none")] pub(crate) ima: Option, diff --git a/src/libpriv/rpmostree-postprocess.cxx b/src/libpriv/rpmostree-postprocess.cxx index 311de70f0c..c086bcdda4 100644 --- a/src/libpriv/rpmostree-postprocess.cxx +++ b/src/libpriv/rpmostree-postprocess.cxx @@ -381,7 +381,7 @@ postprocess_final (int rootfs_dfd, rpmostreecxx::Treefile &treefile, gboolean un auto selinux = treefile.get_selinux (); - ROSCXX_TRY (compose_postprocess_final_pre (rootfs_dfd), error); + ROSCXX_TRY (compose_postprocess_final_pre (rootfs_dfd, treefile), error); if (selinux) { diff --git a/tests/compose/test-installroot.sh b/tests/compose/test-installroot.sh index 3e40f6791f..5eab03cc9e 100755 --- a/tests/compose/test-installroot.sh +++ b/tests/compose/test-installroot.sh @@ -7,6 +7,8 @@ dn=$(cd "$(dirname "$0")" && pwd) # This is used to test postprocessing with treefile vs not treefile_set "boot-location" '"new"' +# On by default now: +# treefile_set "ignore-devices" 'True' # This test is a bit of a degenerative case of the supermin abstration. We need # to be able to interact with the compose output directly, feed it back to @@ -56,6 +58,7 @@ testdate=$(date) runasroot sh -xec " # https://github.com/ostreedev/ostree/pull/2717/commits/e234b630f85b97e48ecf45d5aaba9b1aa64e6b54 mknod -m 000 ${instroot}-directcommit/usr/share/foowhiteout c 0 0 +mknod -m 000 ${instroot}-directcommit/usr/share/devzero c 1 5 echo \"${testdate}\" > ${instroot}-directcommit/usr/share/rpm-ostree-composetest-split.txt ! test -f ${instroot}-directcommit/${integrationconf} rpm-ostree compose commit --repo=${repo} ${treefile} ${instroot}-directcommit @@ -69,6 +72,11 @@ ostree --repo=${repo} ls ${treeref} /usr/share ostree --repo=${repo} ls ${treeref} /usr/share/.ostree-wh.foowhiteout >out.txt grep -Ee '^-00000' out.txt +# And the devzero should have been ignored +if ostree --repo=${repo} ls ${treeref} /usr/share/devzero; then + echo "found devzero" 1>&2; exit 1 +fi + ostree --repo=${repo} cat ${treeref} /usr/share/rpm-ostree-composetest-split.txt >out.txt grep \"${testdate}\" out.txt ostree --repo=${repo} cat ${treeref} /${integrationconf}