src/fsverity: up our FsVerityHashValue trait game

Right now we define

  pub type Sha256HashValue = [u8; 32];
  pub type Sha512HashValue = [u8; 64];

which is simple enough but also painful: we can implement our own traits
for these types (and indeed, we do it for FsVerityHashValue) but we
can't implement things like fmt::Debug because there's a default impl
(which rather unhelpfully prints the hashes out as an array of bytes).

Turn these into proper types using a trivial struct wrapper:

  pub struct Sha256HashValue([u8; 32]);
  pub struct Sha512HashValue([u8; 64]);

which gives us a lot more control.  We can start by implementing
meaningful fmt::Debug traits.

Doing this means that we implicitly lose our implementation of
AsRef<[u8]> and AsMut<[u8]> which we've been mostly using in order to do
various ad-hoc encoding to/from various forms of hex values all over the
codebase.  Move all of those ad-hoc forms of encoding into proper
methods on the trait itself by adding functions like ::to_hex() and
::from_hex() which we can use instead.  We also drop our last instances
of using parse_sha256() for fs-verity digests.

There are a couple of other places where we need the actual bytes,
though: when reading/writing hash values to splitstreams and when
reading/writing the metacopy xattr in erofs images.  Deal with those by
deriving the usual zerocopy FromBytes/IntoBytes traits for our new hash
types and moving the code in the splitstream and erofs modules to
directly include the data inside of the relevant structure types, mark
those types as FromBytes/IntoBytes, and use the zerocopy APIs directly.
In the case of the metacopy attr in the erofs code this provides an
opportunity for a substantial cleanup in an otherwise scary part of the
code.

Signed-off-by: Allison Karlitskaya <[email protected]>

Author: allisonkarlitskaya

Cargo.toml

@@ -36,7 +36,7 @@ thiserror = "2.0.0"
 tokio = "1.24.2"
 toml = "0.8.0"
 xxhash-rust = { version = "0.8.2", features = ["xxh32"] }
-zerocopy = { version = "0.8.0", features = ["derive"] }
+zerocopy = { version = "0.8.0", features = ["derive", "std"] }
 zstd = "0.13.0"
 
 [dev-dependencies]

src/bin/cfsctl.rs

@@ -5,7 +5,12 @@ use clap::{Parser, Subcommand};
 
 use rustix::fs::CWD;
 
-use composefs::{oci, repository::Repository, util::parse_sha256};
+use composefs::{
+    fsverity::{FsVerityHashValue, Sha256HashValue},
+    oci,
+    repository::Repository,
+    util::parse_sha256,
+};
 
 /// cfsctl
 #[derive(Debug, Parser)]
@@ -130,7 +135,7 @@ fn main() -> Result<()> {
         }
         Command::ImportImage { reference } => {
             let image_id = repo.import_image(&reference, &mut std::io::stdin())?;
-            println!("{}", hex::encode(image_id));
+            println!("{}", image_id.to_hex());
         }
         Command::Oci { cmd: oci_cmd } => match oci_cmd {
             OciCommand::ImportLayer { name, sha256 } => {
@@ -140,7 +145,7 @@ fn main() -> Result<()> {
                     name.as_deref(),
                     &mut std::io::stdin(),
                 )?;
-                println!("{}", hex::encode(object_id));
+                println!("{}", object_id.to_hex());
             }
             OciCommand::LsLayer { name } => {
                 oci::ls_layer(&repo, &name)?;
@@ -150,7 +155,7 @@ fn main() -> Result<()> {
             }
             OciCommand::CreateImage { config, name } => {
                 let image_id = oci::image::create_image(&repo, &config, name.as_deref(), None)?;
-                println!("{}", hex::encode(image_id));
+                println!("{}", image_id.to_hex());
             }
             OciCommand::Pull { ref image, name } => {
                 let runtime = tokio::runtime::Builder::new_current_thread()
@@ -161,10 +166,13 @@ fn main() -> Result<()> {
                 runtime.block_on(async move { oci::pull(&repo, image, name.as_deref()).await })?;
             }
             OciCommand::Seal { verity, ref name } => {
-                let (sha256, verity) =
-                    oci::seal(&repo, name, verity.map(parse_sha256).transpose()?.as_ref())?;
+                let (sha256, verity) = oci::seal(
+                    &repo,
+                    name,
+                    verity.map(Sha256HashValue::from_hex).transpose()?.as_ref(),
+                )?;
                 println!("sha256 {}", hex::encode(sha256));
-                println!("verity {}", hex::encode(verity));
+                println!("verity {}", verity.to_hex());
             }
             OciCommand::Mount {
                 ref name,
@@ -182,7 +190,7 @@ fn main() -> Result<()> {
         },
         Command::CreateImage { ref path } => {
             let image_id = composefs::fs::create_image(path, Some(&repo))?;
-            println!("{}", hex::encode(image_id));
+            println!("{}", image_id.to_hex());
         }
         Command::CreateDumpfile { ref path } => {
             composefs::fs::create_dumpfile(path)?;
@@ -193,7 +201,7 @@ fn main() -> Result<()> {
         Command::ImageObjects { name } => {
             let objects = repo.objects_for_image(&name)?;
             for object in objects {
-                println!("{}", hex::encode(object));
+                println!("{}", object.to_hex());
             }
         }
         Command::GC => {

src/bin/composefs-setup-root.rs

@@ -21,7 +21,7 @@ use rustix::{
 use serde::Deserialize;
 
 use composefs::{
-    fsverity::Sha256HashValue,
+    fsverity::{FsVerityHashValue, Sha256HashValue},
     mount::{composefs_fsmount, mount_at, FsHandle},
     mountcompat::{overlayfs_set_fd, overlayfs_set_lower_and_data_fds, prepare_mount},
     repository::Repository,
@@ -204,9 +204,7 @@ fn parse_composefs_cmdline(cmdline: &[u8]) -> Result<Sha256HashValue> {
     // TODO?: officially we need to understand quoting with double-quotes...
     for part in cmdline.split(|c| c.is_ascii_whitespace()) {
         if let Some(digest) = part.strip_prefix(b"composefs=") {
-            let mut value = [0; 32];
-            hex::decode_to_slice(digest, &mut value).context("Parsing composefs=")?;
-            return Ok(value);
+            return Sha256HashValue::from_hex(digest).context("Parsing composefs=");
         }
     }
     bail!("Unable to find composefs= cmdline parameter");
@@ -238,7 +236,7 @@ fn setup_root(args: Args) -> Result<()> {
         Some(cmdline) => cmdline.as_bytes(),
         None => &std::fs::read("/proc/cmdline")?,
     };
-    let image = hex::encode(parse_composefs_cmdline(cmdline)?);
+    let image = parse_composefs_cmdline(cmdline)?.to_hex();
 
     let new_root = match args.root_fs {
         Some(path) => open_root_fs(&path).context("Failed to clone specified root fs")?,
@@ -295,12 +293,9 @@ mod test {
             assert!(parse_composefs_cmdline(case.as_bytes()).is_err());
         }
         let digest = "8b7df143d91c716ecfa5fc1730022f6b421b05cedee8fd52b1fc65a96030ad52";
-        let digest_bytes = hex::decode(digest).unwrap();
         similar_asserts::assert_eq!(
-            parse_composefs_cmdline(format!("composefs={digest}").as_bytes())
-                .unwrap()
-                .as_slice(),
-            &digest_bytes
+            parse_composefs_cmdline(format!("composefs={digest}").as_bytes()).unwrap(),
+            Sha256HashValue::from_hex(digest).unwrap()
         );
     }
 }

src/dumpfile.rs

@@ -12,7 +12,7 @@ use anyhow::Result;
 use rustix::fs::FileType;
 
 use crate::{
-    fsverity::Sha256HashValue,
+    fsverity::{FsVerityHashValue, Sha256HashValue},
     image::{Directory, FileSystem, Inode, Leaf, LeafContent, RegularFile, Stat},
 };
 
@@ -66,7 +66,7 @@ fn write_entry(
     write_escaped(writer, content)?;
     write!(writer, " ")?;
     if let Some(id) = digest {
-        write!(writer, "{}", hex::encode(id))?;
+        write!(writer, "{}", id.to_hex())?;
     } else {
         write_empty(writer)?;
     }
@@ -129,7 +129,7 @@ pub fn write_leaf(
             *size,
             nlink,
             0,
-            format!("{:02x}/{}", id[0], hex::encode(&id[1..])),
+            id.to_object_pathname(),
             &[],
             Some(id),
         ),

src/erofs/composefs.rs

@@ -0,0 +1,33 @@
+use zerocopy::{FromBytes, Immutable, IntoBytes, KnownLayout};
+
+use crate::fsverity::FsVerityHashValue;
+
+/* From linux/fs/overlayfs/overlayfs.h struct ovl_metacopy */
+#[derive(Debug, FromBytes, Immutable, KnownLayout, IntoBytes)]
+#[repr(C)]
+pub(super) struct OverlayMetacopy<H: FsVerityHashValue> {
+    version: u8,
+    len: u8,
+    flags: u8,
+    digest_algo: u8,
+    pub(super) digest: H,
+}
+
+impl<H: FsVerityHashValue> OverlayMetacopy<H> {
+    pub(super) fn new(digest: &H) -> Self {
+        Self {
+            version: 0,
+            len: size_of::<Self>() as u8,
+            flags: 0,
+            digest_algo: H::ALGORITHM,
+            digest: digest.clone(),
+        }
+    }
+
+    pub(super) fn valid(&self) -> bool {
+        self.version == 0
+            && self.len == size_of::<Self>() as u8
+            && self.flags == 0
+            && self.digest_algo == H::ALGORITHM
+    }
+}

src/erofs/mod.rs

@@ -1,3 +1,4 @@
+pub mod composefs;
 pub mod debug;
 pub mod format;
 pub mod reader;

src/erofs/reader.rs

@@ -5,9 +5,12 @@ use std::ops::Range;
 use thiserror::Error;
 use zerocopy::{little_endian::U32, FromBytes, Immutable, KnownLayout};
 
-use super::format::{
-    CompactInodeHeader, ComposefsHeader, DataLayout, DirectoryEntryHeader, ExtendedInodeHeader,
-    InodeXAttrHeader, ModeField, Superblock, XAttrHeader,
+use super::{
+    composefs::OverlayMetacopy,
+    format::{
+        CompactInodeHeader, ComposefsHeader, DataLayout, DirectoryEntryHeader, ExtendedInodeHeader,
+        InodeXAttrHeader, ModeField, Superblock, XAttrHeader,
+    },
 };
 use crate::fsverity::Sha256HashValue;
 
@@ -491,18 +494,17 @@ pub struct ObjectCollector {
 
 impl ObjectCollector {
     fn visit_xattr(&mut self, attr: &XAttr) {
-        // TODO: "4" is a bit magic, isn't it?
+        // This is the index of "trusted".  See XATTR_PREFIXES in format.rs.
         if attr.header.name_index != 4 {
             return;
         }
         if attr.suffix() != b"overlay.metacopy" {
             return;
         }
-        let value = attr.value();
-        // TODO: oh look, more magic values...
-        if value.len() == 36 && value[..4] == [0, 36, 0, 1] {
-            // SAFETY: We already checked that the length is 4 + 32
-            self.objects.insert(value[4..].try_into().unwrap());
+        if let Ok(value) = OverlayMetacopy::read_from_bytes(attr.value()) {
+            if value.valid() {
+                self.objects.insert(value.digest);
+            }
         }
     }
 

src/erofs/writer.rs

@@ -11,7 +11,8 @@ use xxhash_rust::xxh32::xxh32;
 use zerocopy::{Immutable, IntoBytes};
 
 use crate::{
-    erofs::{format, reader::round_up},
+    erofs::{composefs::OverlayMetacopy, format, reader::round_up},
+    fsverity::FsVerityHashValue,
     image,
 };
 
@@ -409,10 +410,12 @@ impl<'a> InodeCollector<'a> {
             ..
         }) = content
         {
-            let metacopy = [&[0, 36, 0, 1], &id[..]].concat();
-            xattrs.add(b"trusted.overlay.metacopy", &metacopy);
+            xattrs.add(
+                b"trusted.overlay.metacopy",
+                OverlayMetacopy::new(id).as_bytes(),
+            );
 
-            let redirect = format!("/{:02x}/{}", id[0], hex::encode(&id[1..]));
+            let redirect = format!("/{}", id.to_object_pathname());
             xattrs.add(b"trusted.overlay.redirect", redirect.as_bytes());
         }
 

src/fsverity/digest.rs

@@ -64,7 +64,7 @@ impl<H: FsVerityHashValue, const LG_BLKSZ: u8> FsVerityHasher<H, LG_BLKSZ> {
             // We had a complete value, but now we're adding new data.
             // This means that we need to add a new hash layer...
             let mut new_layer = FsVerityLayer::new();
-            new_layer.add_data(value.as_ref());
+            new_layer.add_data(value.as_bytes());
             self.layers.push(new_layer);
         }
 
@@ -76,7 +76,7 @@ impl<H: FsVerityHashValue, const LG_BLKSZ: u8> FsVerityHasher<H, LG_BLKSZ> {
 
         for layer in self.layers.iter_mut() {
             // We have a layer we need to hash this value into
-            layer.add_data(value.as_ref());
+            layer.add_data(value.as_bytes());
             if layer.remaining != 0 {
                 return;
             }
@@ -97,7 +97,7 @@ impl<H: FsVerityHashValue, const LG_BLKSZ: u8> FsVerityHasher<H, LG_BLKSZ> {
             for layer in self.layers.iter_mut() {
                 // We have a layer we need to hash this value into
                 if value != H::EMPTY {
-                    layer.add_data(value.as_ref());
+                    layer.add_data(value.as_bytes());
                 }
                 if layer.remaining != (1 << LG_BLKSZ) {
                     // ...but now this layer itself is complete, so get the value of *it*.
@@ -143,7 +143,7 @@ impl<H: FsVerityHashValue, const LG_BLKSZ: u8> FsVerityHasher<H, LG_BLKSZ> {
         context.update(0u8.to_le_bytes()); /* salt_size */
         context.update([0; 4]); /* reserved */
         context.update(self.n_bytes.to_le_bytes());
-        context.update(self.root_hash());
+        context.update(self.root_hash().as_bytes());
         context.update([0].repeat(64 - size_of::<H>()));
         context.update([0; 32]); /* salt */
         context.update([0; 144]); /* reserved */
@@ -162,12 +162,12 @@ mod tests {
     #[test]
     fn test_digest() {
         assert_eq!(
-            hex::encode(FsVerityHasher::<Sha256HashValue, 12>::hash(b"hello world")),
+            FsVerityHasher::<Sha256HashValue, 12>::hash(b"hello world").to_hex(),
             "1e2eaa4202d750a41174ee454970b92c1bc2f925b1e35076d8c7d5f56362ba64"
         );
 
         assert_eq!(
-            hex::encode(FsVerityHasher::<Sha512HashValue, 12>::hash(b"hello world")),
+            FsVerityHasher::<Sha512HashValue, 12>::hash(b"hello world").to_hex(),
             "18430270729d162d4e469daca123ae61893db4b0583d8f7081e3bf4f92b88ba514e7982f10733fb6aa895195c5ae8fd2eb2c47a8be05513ce5a0c51a6f570409"
         );
     }