From f905606375db426fc44bbb59bed52d2450b331b1 Mon Sep 17 00:00:00 2001 From: Joseph Marrero Corchado Date: Thu, 17 Oct 2024 15:54:51 -0400 Subject: [PATCH] Backport https://github.com/coreos/rpm-ostree/pull/5114 Resolves: #RHEL-63021 --- 0001-treefile-Add-ignore-devices.patch | 332 +++++++++++++++++++++++++ rpm-ostree.spec | 2 + 2 files changed, 334 insertions(+) create mode 100644 0001-treefile-Add-ignore-devices.patch diff --git a/0001-treefile-Add-ignore-devices.patch b/0001-treefile-Add-ignore-devices.patch new file mode 100644 index 0000000..f596eb3 --- /dev/null +++ b/0001-treefile-Add-ignore-devices.patch @@ -0,0 +1,332 @@ +From f17b7e57883b4d211d3b0ed427fc4ce66cbe49ee Mon Sep 17 00:00:00 2001 +From: Colin Walters +Date: Tue, 1 Oct 2024 17:56:38 -0400 +Subject: [PATCH 1/1] 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 9839589e..49e193d1 100644 +--- a/docs/treefile.md ++++ b/docs/treefile.md +@@ -36,6 +36,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 649d1b7b..2a4cb12c 100644 +--- a/rpmostree-cxxrs.cxx ++++ b/rpmostree-cxxrs.cxx +@@ -192,6 +192,8 @@ public: + 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 d38fd1db..9d62380e 100644 +--- a/rpmostree-cxxrs.h ++++ b/rpmostree-cxxrs.h +@@ -191,6 +191,8 @@ public: + 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 1a5ae869..8049d502 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(()) + } + +@@ -1533,11 +1562,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 56d8b57f..5356dc8c 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 da6c0ca7..51dfff6d 100644 +--- a/rust/src/treefile.rs ++++ b/rust/src/treefile.rs +@@ -394,6 +394,7 @@ fn treefile_merge(dest: &mut TreeComposeConfig, src: &mut TreeComposeConfig) { + rojig, + selinux, + selinux_label_version, ++ ignore_devices, + ima, + gpg_key, + include, +@@ -1245,6 +1246,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() + } +@@ -2414,6 +2419,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 311de70f..c086bcdd 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 3e40f679..90a11ee3 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} +-- +2.47.0 + diff --git a/rpm-ostree.spec b/rpm-ostree.spec index f4ba596..4268d13 100644 --- a/rpm-ostree.spec +++ b/rpm-ostree.spec @@ -11,6 +11,8 @@ URL: https://github.com/coreos/rpm-ostree # in the upstream git. It also contains vendored Rust sources. Source0: https://github.com/coreos/rpm-ostree/releases/download/v%{version}/rpm-ostree-%{version}.tar.xz +Patch0: 0001-treefile-Add-ignore-devices.patch + ExclusiveArch: %{rust_arches} # ostree not on i686 for RHEL 10