Resolves: #RHEL-63021
This commit is contained in:
Joseph Marrero Corchado 2024-10-17 15:54:51 -04:00
parent 3d6de0747e
commit f905606375
2 changed files with 334 additions and 0 deletions

View File

@ -0,0 +1,332 @@
From f17b7e57883b4d211d3b0ed427fc4ce66cbe49ee Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
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 <walters@verbum.org>
---
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 <typename C> explicit Slice (C &c) : Slice (c.data (), c.size ()) {}
+
Slice &operator= (const Slice<T> &) &noexcept = default;
Slice &operator= (Slice<T> &&) &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 <typename C> explicit Slice (C &c) : Slice (c.data (), c.size ()) {}
+
Slice &operator= (const Slice<T> &) &noexcept = default;
Slice &operator= (Slice<T> &&) &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<u32> {
- 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<bool>,
#[serde(skip_serializing_if = "Option::is_none")]
+ pub(crate) ignore_devices: Option<bool>,
+ #[serde(skip_serializing_if = "Option::is_none")]
pub(crate) selinux_label_version: Option<u32>,
#[serde(skip_serializing_if = "Option::is_none")]
pub(crate) ima: Option<bool>,
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

View File

@ -11,6 +11,8 @@ URL: https://github.com/coreos/rpm-ostree
# in the upstream git. It also contains vendored Rust sources. # 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 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} ExclusiveArch: %{rust_arches}
# ostree not on i686 for RHEL 10 # ostree not on i686 for RHEL 10