libpriv/kernel: Add padding between dracut initramfs and random CPIO

[jlebon]

Any arbitrary amount of padding between concatenated CPIOs is allowed:

https://www.kernel.org/doc/Documentation/early-userspace/buffer-format.txt

However, some initramfs decompressors in the kernel expect a minimal amount of padding to detect EOF and allow the kernel outer loop to try out other decompressors for the next payload. This is the case for lz4. This upstream patch allows the lz4 decompressor to treat padding as EOF:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2c484419efc09e7234c667aa72698cb79ba8d8ed

But it assumes that the padding is at least 4 bytes.

Since padding doesn't hurt regardless of the compression used, let's just unconditionally add it.

Fixes #4683.

[cgwalters]

Ah, thanks. Indeed if this is sufficient that'd be great.

I would say though it'd be cleaner here to deduplicate things and change the Rust API to take a file descriptor as input, then we can have one implementation only in Rust.

[cgwalters]

How about
std::io::copy(&mut std::io::repeat(0).take(misalignment), &mut f)?;

[cgwalters]

How about

diff --git a/rpmostree-cxxrs.cxx b/rpmostree-cxxrs.cxx
index 4dd46f69..4f4dd700 100644
--- a/rpmostree-cxxrs.cxx
+++ b/rpmostree-cxxrs.cxx
@@ -2341,7 +2341,8 @@ extern "C"
       ::rust::Str abs_path, ::rpmostreecxx::GFileInfo const &file_info, ::rust::Str username,
       ::rust::Str groupname, ::rust::String *return$) noexcept;
 
-  ::rust::repr::Fat rpmostreecxx$cxxbridge1$get_dracut_random_cpio () noexcept;
+  ::rust::repr::PtrLen
+  rpmostreecxx$cxxbridge1$append_dracut_random_cpio (::std::int32_t fd) noexcept;
 
   ::rust::repr::PtrLen
   rpmostreecxx$cxxbridge1$initramfs_overlay_generate (::rust::Vec< ::rust::String> const &files,
@@ -4417,11 +4418,14 @@ tmpfiles_translate (::rust::Str abs_path, ::rpmostreecxx::GFileInfo const &file_
   return ::std::move (return$.value);
 }
 
-::rust::Slice< ::std::uint8_t const>
-get_dracut_random_cpio () noexcept
+void
+append_dracut_random_cpio (::std::int32_t fd)
 {
-  return ::rust::impl< ::rust::Slice< ::std::uint8_t const> >::slice (
-      rpmostreecxx$cxxbridge1$get_dracut_random_cpio ());
+  ::rust::repr::PtrLen error$ = rpmostreecxx$cxxbridge1$append_dracut_random_cpio (fd);
+  if (error$.ptr)
+    {
+      throw ::rust::impl< ::rust::Error>::error (error$);
+    }
 }
 
 ::std::int32_t
diff --git a/rpmostree-cxxrs.h b/rpmostree-cxxrs.h
index ce564475..323e1d0f 100644
--- a/rpmostree-cxxrs.h
+++ b/rpmostree-cxxrs.h
@@ -1917,7 +1917,7 @@ rpm_importer_new (::rust::Str pkg_name, ::rust::Str ostree_branch,
 ::rust::String tmpfiles_translate (::rust::Str abs_path, ::rpmostreecxx::GFileInfo const &file_info,
                                    ::rust::Str username, ::rust::Str groupname);
 
-::rust::Slice< ::std::uint8_t const> get_dracut_random_cpio () noexcept;
+void append_dracut_random_cpio (::std::int32_t fd);
 
 ::std::int32_t initramfs_overlay_generate (::rust::Vec< ::rust::String> const &files,
                                            ::rpmostreecxx::GCancellable &cancellable);
diff --git a/rust/src/cliwrap/kernel_install.rs b/rust/src/cliwrap/kernel_install.rs
index 9b352448..cc2a0077 100644
--- a/rust/src/cliwrap/kernel_install.rs
+++ b/rust/src/cliwrap/kernel_install.rs
@@ -8,7 +8,7 @@ use cap_std::fs::FileType;
 use cap_std::fs_utf8::Dir as Utf8Dir;
 use cap_std_ext::cap_std;
 use fn_error_context::context;
-use std::io::Write;
+use std::os::fd::AsRawFd;
 use std::process::Command;
 
 /// Primary entrypoint to running our wrapped `kernel-install` handling.
@@ -95,16 +95,11 @@ fn run_dracut(kernel_dir: &str) -> Result<()> {
             res
         ));
     }
-    let mut f = std::fs::OpenOptions::new()
+    let f = std::fs::OpenOptions::new()
         .write(true)
         .append(true)
         .open(&tmp_initramfs_path)?;
-    // See similar code in rpmostree-kernel.cxx. */
-    let misalignment = (f.metadata()?.len() % 4) as usize;
-    let zeros = &[b'\0', b'\0', b'\0', b'\0', b'\0', b'\0', b'\0', b'\0'];
-    f.write(&zeros[..8 - misalignment])?;
-    // Also append the dev/urandom bits here, see the duplicate bits in rpmostree-kernel.cxx
-    f.write(crate::initramfs::get_dracut_random_cpio())?;
+    crate::initramfs::append_dracut_random_cpio(f.as_raw_fd())?;
     drop(f);
     let utf8_tmp_dir_path = Utf8Path::from_path(tmp_dir.path().strip_prefix("/")?)
         .context("Error turning Path to Utf8Path")?;
diff --git a/rust/src/initramfs.rs b/rust/src/initramfs.rs
index 2095a64c..fb184f7a 100644
--- a/rust/src/initramfs.rs
+++ b/rust/src/initramfs.rs
@@ -4,10 +4,12 @@
 use crate::cxxrsutil::*;
 use anyhow::{Context, Result};
 use camino::Utf8Path;
+use cap_std::io_lifetimes::AsFilelike;
 use cap_std_ext::cap_std;
 use cap_std_ext::prelude::CapStdExtCommandExt;
 use fn_error_context::context;
 use ostree_ext::{gio, glib, prelude::*};
+use rustix::fd::BorrowedFd;
 use std::collections::BTreeSet;
 use std::collections::HashSet;
 use std::io::prelude::*;
@@ -134,9 +136,33 @@ fn generate_initramfs_overlay_etc<P: glib::IsA<gio::Cancellable>>(
     generate_initramfs_overlay(root, files, cancellable)
 }
 
-pub(crate) fn get_dracut_random_cpio() -> &'static [u8] {
+fn impl_append_dracut_random_cpio(fd: BorrowedFd) -> Result<()> {
+    let fd = fd.as_filelike_view::<std::fs::File>();
+    // Add padding; see https://www.kernel.org/doc/Documentation/early-userspace/buffer-format.txt
+    let mut len = (&*fd).seek(std::io::SeekFrom::End(0))?;
+    // We *always* ensure 4 bytes of NUL padding, no matter what because it aids lz4
+    len += std::io::copy(&mut std::io::repeat(0).take(4), &mut &*fd)?;
+    // Now also ensure alignment to 4; also xref https://github.com/dracutdevs/dracut/blob/master/src/dracut-cpio/src/main.rs#L644
+    const ALIGN: u64 = 4;
+    let misalignment = len % ALIGN;
+    if misalignment > 0 {
+        std::io::copy(
+            &mut std::io::repeat(0).take(ALIGN - misalignment),
+            &mut &*fd,
+        )?;
+    }
+
     // Generated with: fakeroot /bin/sh -c 'cd dracut-urandom && find . -print0 | sort -z | (mknod dev/random c 1 8 && mknod dev/urandom c 1 9 && cpio -o --null -H newc -R 0:0 --reproducible --quiet -D . -O /tmp/dracut-urandom.cpio)'
-    include_bytes!("../../src/libpriv/dracut-random.cpio.gz")
+    let buf = include_bytes!("../../src/libpriv/dracut-random.cpio.gz");
+    (&*fd).write_all(buf)?;
+    Ok(())
+}
+
+/// Append a small static initramfs chunk which includes /dev/{u,}random, because
+/// dracut's FIPS module may fail to do so in restricted environments.
+pub(crate) fn append_dracut_random_cpio(fd: i32) -> CxxResult<()> {
+    let fd = unsafe { BorrowedFd::borrow_raw(fd) };
+    impl_append_dracut_random_cpio(fd).map_err(Into::into)
 }
 
 /// cxx-rs entrypoint; we can't use generics and need to return a raw integer for fd
@@ -173,4 +199,19 @@ mod test {
         let _ = tmpd.metadata("initramfs").context("stat")?;
         Ok(())
     }
+
+    #[test]
+    fn test_append_initramfs() -> Result<()> {
+        use std::os::fd::AsFd;
+        // Current size of static file
+        let dracut_random_len = 171u64;
+        let tmpf = tempfile::tempfile()?;
+        impl_append_dracut_random_cpio(tmpf.as_fd()).unwrap();
+        assert_eq!(tmpf.metadata()?.len(), dracut_random_len + 4);
+        let mut tmpf = tempfile::tempfile()?;
+        tmpf.write_all(b"x")?;
+        impl_append_dracut_random_cpio(tmpf.as_fd()).unwrap();
+        assert_eq!(tmpf.metadata()?.len(), dracut_random_len + 8);
+        Ok(())
+    }
 }
diff --git a/rust/src/lib.rs b/rust/src/lib.rs
index b18cd891..e0f6dd80 100644
--- a/rust/src/lib.rs
+++ b/rust/src/lib.rs
@@ -436,7 +436,7 @@ pub mod ffi {
 
     // initramfs.rs
     extern "Rust" {
-        fn get_dracut_random_cpio() -> &'static [u8];
+        fn append_dracut_random_cpio(fd: i32) -> Result<()>;
         fn initramfs_overlay_generate(
             files: &Vec<String>,
             cancellable: Pin<&mut GCancellable>,
diff --git a/src/libpriv/rpmostree-kernel.cxx b/src/libpriv/rpmostree-kernel.cxx
index 676e5ca5..1242e3fc 100644
--- a/src/libpriv/rpmostree-kernel.cxx
+++ b/src/libpriv/rpmostree-kernel.cxx
@@ -580,21 +580,7 @@ rpmostree_run_dracut (int rootfs_dfd, const char *const *argv, const char *kver,
    * https://bugzilla.redhat.com/show_bug.cgi?id=1401444
    * https://bugzilla.redhat.com/show_bug.cgi?id=1380866
    * */
-  auto random_cpio_data = rpmostreecxx::get_dracut_random_cpio ();
-  off_t off = lseek (tmpf.fd, 0, SEEK_END);
-  if (off < 0)
-    return glnx_throw_errno_prefix (error, "lseek");
-  /* add some padding between the two; first, make sure there's at least 4 bytes
-   * of NULs to satisfy certain compressors (e.g. lz4) and second, make sure
-   * it's 4-byte aligned to be nice (it's a hard requirement for uncompressed
-   * CPIOs, which isn't the case here right now but could've easily been; also
-   * e.g. GRUB does this) */
-  guint misalignment = off % 4;
-  const char *zeros = "\0\0\0\0\0\0\0\0";
-  if (glnx_loop_write (tmpf.fd, zeros + misalignment, 8 - misalignment) < 0)
-    return glnx_throw_errno_prefix (error, "write(padding)");
-  if (glnx_loop_write (tmpf.fd, random_cpio_data.data (), random_cpio_data.length ()) < 0)
-    return glnx_throw_errno_prefix (error, "write(random_cpio_data)");
+  CXX_TRY (rpmostreecxx::append_dracut_random_cpio (tmpf.fd), error);
 
   if (rebuild_from_initramfs)
     (void)unlinkat (rootfs_dfd, rebuild_from_initramfs, 0);

[cgwalters]

Calculations are wrong of course...

EDIT: Should be fixed and added a unit test, though this also deserves integration testing

[jmarrero]

@cgwalters should I add this to the release or we are waiting for a integration test?

[ericcurtin]

We were thinking of proposing a change to lz4 initrd compression in Fedora, with this change lz4 initrds would still not be FIPS compliant I guess (checking my assumptions)? If this weakens FIPS, it will still make alternate compression algorithms unusable in many scenarios because of the weakening in FIPS compliance.

We should still merge this when ready, as it's still an improvement regardless.

[jlebon]

Thanks, I'll update it based on your Rust diff soon and add an integration test for this.

@cgwalters should I add this to the release or we are waiting for a integration test?

There's actually another (unrelated, small) patch I'd like to get in before cutting a release. Will get that out today. (Edit: #4710).

We were thinking of proposing a change to lz4 initrd compression in Fedora, with this change lz4 initrds would still not be FIPS compliant I guess (checking my assumptions)? If this weakens FIPS, it will still make alternate compression algorithms unusable in many scenarios because of the weakening in FIPS compliance.

We should still merge this when ready, as it's still an improvement regardless.

I'm not sure I follow the link with FIPS compliance. Hmm OK, I think you might be thinking that with this, the /dev/[u]random CPIO appended is no longer unpacked? If so, note that's not the case.

[cgwalters]

with this change lz4 initrds would still not be FIPS compliant I guess

No; the issue boils down to whether dracut has privileges to create the device nodes at build time, it is orthogonal to the compression algorithm.

[cgwalters]

Looks like the unit test needs a tweak since we're now appending another 4 bytes in the already-aligned case right?