diff --git a/crates/composefs-boot/src/android_boot.rs b/crates/composefs-boot/src/android_boot.rs index 7b73a7b8..ef7bf93a 100644 --- a/crates/composefs-boot/src/android_boot.rs +++ b/crates/composefs-boot/src/android_boot.rs @@ -3,6 +3,7 @@ //! This module provides functionality to parse Android boot image format version 2 files //! and extract embedded components like kernel, initrd, commandline and dtb. +use std::ffi::CStr; use std::io::{Read, Seek, SeekFrom}; use thiserror::Error; use zerocopy::{ @@ -115,12 +116,12 @@ impl AndroidBootImage { } // mkbootimg splits long command lines (with a null terminator for each) - let primary_len = nul_terminated_len(&header.cmdline); - let extra_len = nul_terminated_len(&header.extra_cmdline); + let primary = nul_terminated_bytes(&header.cmdline); + let extra = nul_terminated_bytes(&header.extra_cmdline); let mut cmdline = [0; TOTAL_CMDLINE_SIZE]; - cmdline[..primary_len].copy_from_slice(&header.cmdline[..primary_len]); - cmdline[primary_len..primary_len + extra_len] - .copy_from_slice(&header.extra_cmdline[..extra_len]); + for (dst, src) in cmdline.iter_mut().zip(primary.iter().chain(extra)) { + *dst = *src; + } Ok(Self { page_size, @@ -179,16 +180,12 @@ impl AndroidBootImage { /// Return the kernel command line stored in the image header. pub fn cmdline(&self) -> Result<&str, AndroidBootError> { - let end = nul_terminated_len(&self.cmdline); - Ok(std::str::from_utf8(&self.cmdline[..end])?) + Ok(std::str::from_utf8(nul_terminated_bytes(&self.cmdline))?) } } -fn nul_terminated_len(bytes: &[u8]) -> usize { - bytes - .iter() - .position(|byte| *byte == 0) - .unwrap_or(bytes.len()) +fn nul_terminated_bytes(bytes: &[u8]) -> &[u8] { + CStr::from_bytes_until_nul(bytes).map_or(bytes, CStr::to_bytes) } fn add_aligned( @@ -305,6 +302,25 @@ pub(crate) mod tests { Ok(()) } + #[test] + fn parses_cmdline_without_nul_terminators() -> Result<(), AndroidBootError> { + let mut bytes = image(b"kernel", b"ramdisk", b""); + let header = BootImageHeaderV2::mut_from_bytes(&mut bytes[..HEADER_SIZE]) + .map_err(|_| AndroidBootError::InvalidHeader)?; + header.cmdline.fill(b'x'); + header.extra_cmdline.fill(b'y'); + + let image = AndroidBootImage::parse(&mut Cursor::new(bytes))?; + let expected = format!( + "{}{}", + "x".repeat(CMDLINE_SIZE), + "y".repeat(EXTRA_CMDLINE_SIZE) + ); + assert_eq!(image.cmdline()?, expected); + assert!(CStr::from_bytes_until_nul(&image.cmdline).is_err()); + Ok(()) + } + #[test] fn rejects_non_utf8_cmdline() -> Result<(), AndroidBootError> { let bytes = image_with_cmdline(b"kernel", b"ramdisk", b"", &[0xff]); diff --git a/crates/composefs-boot/src/selabel.rs b/crates/composefs-boot/src/selabel.rs index a793802f..eaeeda1d 100644 --- a/crates/composefs-boot/src/selabel.rs +++ b/crates/composefs-boot/src/selabel.rs @@ -336,9 +336,9 @@ pub fn open_file( match dir.get_file_opt(filename.as_ref())? { Some(file) => match file { RegularFile::Inline(data) => Ok(Some(Box::new(Cursor::new(data.clone())))), - RegularFile::External(id, ..) | RegularFile::ExternalNoVerity(id, ..) => { - Ok(Some(Box::new(File::from(repo.open_object(id)?)))) - } + RegularFile::External(..) | RegularFile::ExternalPath { .. } => Ok(Some(Box::new( + File::from(repo.open_object(&file.repo_object_id()?)?), + ))), RegularFile::Sparse(..) => Ok(None), }, None => Ok(None), diff --git a/crates/composefs-capi/Cargo.toml b/crates/composefs-capi/Cargo.toml index 722ab703..816e00b3 100644 --- a/crates/composefs-capi/Cargo.toml +++ b/crates/composefs-capi/Cargo.toml @@ -20,6 +20,7 @@ rhel9 = ['composefs/rhel9'] composefs = { workspace = true } hex = { version = "0.4.0", default-features = false, features = ["std"] } libc = "0.2" +log = { version = "0.4", default-features = false } anyhow = "1" rustix = { version = "1", features = ["fs", "mm", "process", "mount"] } zerocopy = "0.8" diff --git a/crates/composefs-capi/build.rs b/crates/composefs-capi/build.rs index a399b0e2..6947e6c9 100644 --- a/crates/composefs-capi/build.rs +++ b/crates/composefs-capi/build.rs @@ -3,6 +3,7 @@ fn main() { cc::Build::new() .file("tests/test_lcfs.c") + .file("tests/ostree-image.c") .include("include/libcomposefs") .warnings(false) .compile("test_lcfs_c"); diff --git a/crates/composefs-capi/src/convert.rs b/crates/composefs-capi/src/convert.rs index ecb1a655..fb4b2e2e 100644 --- a/crates/composefs-capi/src/convert.rs +++ b/crates/composefs-capi/src/convert.rs @@ -3,6 +3,7 @@ use std::ffi::{CStr, CString, OsStr, OsString}; use std::os::unix::ffi::OsStrExt; use std::ptr; +use anyhow::Context; use zerocopy::{FromBytes, IntoBytes}; use composefs::fsverity::{FsVerityHashValue, Sha256HashValue}; @@ -134,30 +135,28 @@ fn ffi_node_to_leaf_content(node: &FfiNode) -> anyhow::Result 0 { - Ok(generic_tree::LeafContent::Regular(RegularFile::Sparse( - node.inode.st_size, - ))) - } else { + } else if node.inode.st_size == 0 { + // libcomposefs ignores the payload and digest of empty files Ok(generic_tree::LeafContent::Regular(RegularFile::Inline( Box::new([]), ))) + } else { + // Like libcomposefs, the payload (e.g. ostree's `xx/.file`) + // is the redirect as is, and the digest only goes into the metacopy + // xattr: they needn't be related. + let redirect = (!node.payload.is_null()).then(|| { + Box::from(OsStr::from_bytes( + unsafe { CStr::from_ptr(node.payload) }.to_bytes(), + )) + }); + let verity = node.digest_set.then(|| { + Sha256HashValue::read_from_bytes(&node.digest).expect("digest size mismatch") + }); + Ok(generic_tree::LeafContent::Regular(RegularFile::external( + redirect, + verity, + node.inode.st_size, + ))) } } t if t == libc::S_IFLNK => { @@ -187,7 +186,9 @@ fn ffi_node_to_leaf_content(node: &FfiNode) -> anyhow::Result) -> *mut FfiNode { +pub(crate) fn filesystem_to_ffi_tree( + fs: &tree::FileSystem, +) -> anyhow::Result<*mut FfiNode> { let mut root = Box::new(FfiNode::default()); stat_to_ffi(&fs.root.stat, &mut root); root.inode.st_mode |= libc::S_IFDIR; @@ -198,9 +199,14 @@ pub(crate) fn filesystem_to_ffi_tree(fs: &tree::FileSystem) -> let root_ptr = Box::into_raw(root); - fs_dir_to_ffi(&fs.root, &fs.leaves, &nlinks, root_ptr, &mut leaf_node_map); + if let Err(e) = fs_dir_to_ffi(&fs.root, &fs.leaves, &nlinks, root_ptr, &mut leaf_node_map) { + // SAFETY: root_ptr came from Box::into_raw above and every node + // created so far is attached to it, so this frees them all. + unsafe { crate::node::lcfs_node_unref(root_ptr) }; + return Err(e); + } - root_ptr + Ok(root_ptr) } fn fs_dir_to_ffi( @@ -209,7 +215,7 @@ fn fs_dir_to_ffi( nlinks: &[u32], parent: *mut FfiNode, leaf_node_map: &mut HashMap, -) { +) -> anyhow::Result<()> { for (name, inode) in dir.sorted_entries() { match inode { generic_tree::Inode::Directory(subdir) => { @@ -220,13 +226,15 @@ fn fs_dir_to_ffi( child.name = CString::new(name_bytes).map_or(ptr::null_mut(), CString::into_raw); child.parent = parent; + // Attach it before recursing, so that on an error the caller + // frees it with the rest of the tree. let child_ptr = Box::into_raw(child); - fs_dir_to_ffi(subdir, leaves, nlinks, child_ptr, leaf_node_map); unsafe { let mut children = (*parent).children_as_vec(); children.push(child_ptr); (*parent).children_put_back(children); } + fs_dir_to_ffi(subdir, leaves, nlinks, child_ptr, leaf_node_map)?; } generic_tree::Inode::Leaf(leaf_id, _) => { let leaf = &leaves[leaf_id.0]; @@ -251,7 +259,8 @@ fn fs_dir_to_ffi( let mut child = Box::new(FfiNode::default()); stat_to_ffi(&leaf.stat, &mut child); - leaf_content_to_ffi(&leaf.content, &mut child); + leaf_content_to_ffi(&leaf.content, &mut child) + .with_context(|| format!("Converting {name:?}"))?; let name_bytes = name.as_bytes(); child.name = CString::new(name_bytes).map_or(ptr::null_mut(), CString::into_raw); child.parent = parent; @@ -271,9 +280,13 @@ fn fs_dir_to_ffi( } } } + Ok(()) } -fn leaf_content_to_ffi(content: &tree::LeafContent, node: &mut FfiNode) { +fn leaf_content_to_ffi( + content: &tree::LeafContent, + node: &mut FfiNode, +) -> anyhow::Result<()> { match content { generic_tree::LeafContent::Regular(reg) => { node.inode.st_mode = (node.inode.st_mode & !libc::S_IFMT) | libc::S_IFREG; @@ -291,10 +304,21 @@ fn leaf_content_to_ffi(content: &tree::LeafContent, node: &mut let path = digest.to_object_pathname(); node.payload = CString::new(path).map_or(ptr::null_mut(), CString::into_raw); } - RegularFile::ExternalNoVerity(digest, size) => { + RegularFile::ExternalPath { + redirect, + verity, + size, + } => { node.inode.st_size = *size; - let path = digest.to_object_pathname(); - node.payload = CString::new(path).map_or(ptr::null_mut(), CString::into_raw); + if let Some(digest) = verity { + node.digest.copy_from_slice(digest.as_bytes()); + node.digest_set = true; + } + if let Some(redirect) = redirect { + node.payload = CString::new(redirect.as_bytes()) + .with_context(|| format!("Invalid redirect {redirect:?}"))? + .into_raw(); + } } RegularFile::Sparse(size) => { node.inode.st_size = *size; @@ -322,4 +346,5 @@ fn leaf_content_to_ffi(content: &tree::LeafContent, node: &mut node.inode.st_mode = (node.inode.st_mode & !libc::S_IFMT) | libc::S_IFSOCK; } } + Ok(()) } diff --git a/crates/composefs-capi/src/image.rs b/crates/composefs-capi/src/image.rs index 2e6d69e6..2302f98c 100644 --- a/crates/composefs-capi/src/image.rs +++ b/crates/composefs-capi/src/image.rs @@ -74,7 +74,14 @@ pub unsafe extern "C" fn lcfs_load_node_from_image_ext( } }; - let root = filesystem_to_ffi_tree(&fs); + let root = match filesystem_to_ffi_tree(&fs) { + Ok(root) => root, + Err(e) => { + log::debug!("Converting the loaded image for the C API: {e:#}"); + set_errno(libc::EINVAL); + return ptr::null_mut(); + } + }; // Apply toplevel_entries filter if specified if !options.is_null() { diff --git a/crates/composefs-capi/src/tests.rs b/crates/composefs-capi/src/tests.rs index c2b20f98..bb70e1b0 100644 --- a/crates/composefs-capi/src/tests.rs +++ b/crates/composefs-capi/src/tests.rs @@ -392,3 +392,144 @@ fn test_clone_deep_rewrites_hardlinks() { node::lcfs_node_unref(root); } } + +unsafe extern "C" { + fn write_ostree_image(fd: libc::c_int) -> libc::c_int; +} + +/// The fsverity digest of the image `tests/ostree-image.c` writes, as written +/// by the C libcomposefs. To regenerate it with the C library installed: +/// +/// ```text +/// cc -DOSTREE_IMAGE_MAIN -o ostree-image tests/ostree-image.c \ +/// $(pkg-config --cflags --libs composefs) +/// ./ostree-image > image && fsverity digest image +/// ``` +const OSTREE_IMAGE_C_DIGEST: &str = + "ed8a80347e9a3b0566dfefcd65509c47551746d9ae6c2dcfd0660ba4810c1013"; + +/// ostree gives each regular file a payload of `xx/.file` and sets +/// the fsverity digest separately; the image must be byte-identical to what +/// the C libcomposefs writes, or overlayfs can't find the file contents. +#[test] +fn test_ostree_image_matches_c() { + use composefs::fsverity::{FsVerityHashValue, Sha256HashValue, compute_verity}; + use composefs::tree::RegularFile; + use std::ffi::OsStr; + use std::io::Read; + use std::os::unix::ffi::OsStrExt; + use zerocopy::IntoBytes; + + let name = CString::new("ostree-image").unwrap(); + let fd = unsafe { libc::memfd_create(name.as_ptr(), 0) }; + assert!(fd >= 0, "memfd_create failed"); + let mut file = unsafe { std::fs::File::from_raw_fd(fd) }; + let ret = unsafe { write_ostree_image(file.as_raw_fd()) }; + assert_eq!(ret, 0, "write_ostree_image failed"); + file.rewind().unwrap(); + let mut image = Vec::new(); + file.read_to_end(&mut image).unwrap(); + + let fs = composefs::erofs::reader::erofs_to_filesystem::(&image).unwrap(); + let file_at = |path: &str| { + let (dir, name) = fs.root.split(OsStr::new(path)).unwrap(); + dir.get_file(name, &fs.leaves).unwrap() + }; + let expected_verity: Vec = (0..32u8) + .map(|i| i.wrapping_mul(7).wrapping_add(3)) + .collect(); + match file_at("/usr/bin/bash") { + RegularFile::ExternalPath { + redirect: Some(redirect), + verity: Some(verity), + size: 1432144, + } => { + assert_eq!( + redirect.as_bytes(), + b"8a/5d74a2f8e3c1a0b9d7e6f5c4b3a2918070605040302010f0e0d0c0b0a09087.file" + ); + assert_eq!(verity.as_bytes(), expected_verity); + } + other => panic!("unexpected /usr/bin/bash: {other:?}"), + } + match file_at("/usr/lib/libfoo.so.1") { + RegularFile::ExternalPath { + redirect: Some(redirect), + verity: None, + size: 8193, + } => assert_eq!( + redirect.as_bytes(), + b"e3/b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.file" + ), + other => panic!("unexpected /usr/lib/libfoo.so.1: {other:?}"), + } + match file_at("/usr/lib/verity-only") { + RegularFile::ExternalPath { + redirect: None, + verity: Some(verity), + size: 4097, + } => assert_eq!(verity.as_bytes(), expected_verity), + other => panic!("unexpected /usr/lib/verity-only: {other:?}"), + } + match file_at("/usr/lib/object") { + RegularFile::External(verity, 5000) => assert_eq!(verity.as_bytes(), expected_verity), + other => panic!("unexpected /usr/lib/object: {other:?}"), + } + + let digest: Sha256HashValue = compute_verity(&image); + assert_eq!(digest.to_hex(), OSTREE_IMAGE_C_DIGEST); +} + +/// A redirect with an interior NUL can't become a C payload, and must fail +/// the conversion instead of silently dropping the redirect. The entries +/// sorting before it (a subdirectory and a hardlinked pair, with xattrs) are +/// already converted by then, so this also exercises freeing a partial tree +/// (meant to be run under ASan or Miri too). +#[test] +fn test_redirect_with_nul_fails() { + use composefs::fsverity::Sha256HashValue; + use composefs::generic_tree::{Directory, Inode, LeafContent, Stat}; + use composefs::tree::{FileSystem, RegularFile}; + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + + let stat = |mode| Stat { + st_mode: mode, + st_uid: 0, + st_gid: 0, + st_mtim_sec: 0, + st_mtim_nsec: 0, + xattrs: [( + Box::from(OsStr::new("user.test")), + Box::from(b"value".as_slice()), + )] + .into(), + }; + let inline = |data: &[u8]| LeafContent::Regular(RegularFile::Inline(Box::from(data))); + let mut fs = FileSystem::::new(stat(0o755)); + + let mut subdir = Directory::new(stat(0o755)); + let id = fs.push_leaf(stat(0o644), inline(b"in subdir")); + subdir.insert(OsStr::new("file"), Inode::leaf(id)); + fs.root + .insert(OsStr::new("a-dir"), Inode::Directory(Box::new(subdir))); + + let id = fs.push_leaf(stat(0o644), inline(b"hardlinked")); + fs.root.insert(OsStr::new("b-link1"), Inode::leaf(id)); + fs.root.insert(OsStr::new("b-link2"), Inode::leaf(id)); + + let id = fs.push_leaf( + stat(0o644), + LeafContent::Regular(RegularFile::external( + Some(Box::from(OsStr::from_bytes(b"8a/5d\0.file"))), + None, + 4096, + )), + ); + fs.root.insert(OsStr::new("z-bad"), Inode::leaf(id)); + + let err = crate::convert::filesystem_to_ffi_tree(&fs).unwrap_err(); + let err = format!("{err:#}"); + assert!(err.contains("Invalid redirect"), "{err}"); + assert!(err.contains("z-bad"), "{err}"); +} diff --git a/crates/composefs-capi/tests/Containerfile.test-capi b/crates/composefs-capi/tests/Containerfile.test-capi index e0c602e0..970bcc0b 100644 --- a/crates/composefs-capi/tests/Containerfile.test-capi +++ b/crates/composefs-capi/tests/Containerfile.test-capi @@ -5,6 +5,11 @@ COPY composefs-c /src/composefs-c COPY libcomposefs_capi.so /usr/lib64/ WORKDIR /src/composefs-c RUN meson setup build && meson compile -C build +# Write an ostree-style image with the C libcomposefs, to compare against ours +COPY ostree-image.c /src/ +RUN gcc -DOSTREE_IMAGE_MAIN -Ilibcomposefs -o /src/ostree-image /src/ostree-image.c \ + -Lbuild/libcomposefs -lcomposefs && \ + LD_LIBRARY_PATH=build/libcomposefs /src/ostree-image > /src/ostree-image-c.erofs RUN cp /usr/lib64/libcomposefs_capi.so build/libcomposefs/libcomposefs.so.1.4.0 && \ ln -sf libcomposefs.so.1.4.0 build/libcomposefs/libcomposefs.so.1 && \ ln -sf libcomposefs.so.1 build/libcomposefs/libcomposefs.so @@ -12,6 +17,10 @@ RUN export LD_LIBRARY_PATH=/src/composefs-c/build/libcomposefs && \ ldd build/tools/mkcomposefs | grep composefs && \ tests/test-checksums.sh build/tools tests/assets \ "config.dump.gz config-with-hard-link.dump.gz special.dump bigfile.dump bigfile-xattr.dump special_v1.dump honggfuzz-long-symlink.dump honggfuzz-longlink-unterminated.dump honggfuzz-bigfile-with-acl.dump no-newline.dump honggfuzz-chardev-nonzero-size.dump longlink.dump honggfuzz-write-inode-data.dump" +RUN export LD_LIBRARY_PATH=/src/composefs-c/build/libcomposefs && \ + /src/ostree-image > /src/ostree-image-rust.erofs && \ + cmp /src/ostree-image-c.erofs /src/ostree-image-rust.erofs && \ + echo "ostree-style image matches the C libcomposefs" RUN export LD_LIBRARY_PATH=/src/composefs-c/build/libcomposefs && \ tests/test-units.sh build/tools && \ echo "=== All C tests passed with Rust libcomposefs ===" diff --git a/crates/composefs-capi/tests/ostree-image.c b/crates/composefs-capi/tests/ostree-image.c new file mode 100644 index 00000000..0e3472e9 --- /dev/null +++ b/crates/composefs-capi/tests/ostree-image.c @@ -0,0 +1,129 @@ +/* SPDX-License-Identifier: GPL-2.0-only OR Apache-2.0 */ +/* + * Build an EROFS image the way ostree does (see ostree_repo_checkout_composefs() + * in ostree's src/libostree/ostree-repo-composefs.c): regular files have a + * payload of "xx/.file" pointing into the bare repo, with an fsverity + * digest only when verity is enabled, and empty files have no payload. + * + * This is linked into the composefs-capi tests, which pin the image's digest, + * and built standalone (with -DOSTREE_IMAGE_MAIN, writing the image to stdout) + * against both the C libcomposefs and ours to compare the output. + */ +#define _GNU_SOURCE + +#include "lcfs-writer.h" +#include +#include +#include +#include + +#define OSTREE_OBJ_A "8a/5d74a2f8e3c1a0b9d7e6f5c4b3a2918070605040302010f0e0d0c0b0a09087.file" +#define OSTREE_OBJ_B "e3/b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855.file" +/* The composefs object path of the digest below */ +#define DIGEST_OBJ "03/0a11181f262d343b424950575e656c737a81888f969da4abb2b9c0c7ced5dc" + +static ssize_t write_fd_cb(void *file, void *buf, size_t count) +{ + return write(*(int *)file, buf, count); +} + +static struct lcfs_node_s *add_node(struct lcfs_node_s *parent, + const char *name, uint32_t mode, uint64_t size) +{ + struct lcfs_node_s *node = lcfs_node_new(); + if (node == NULL) + return NULL; + if (lcfs_node_add_child(parent, node, name) != 0) { + lcfs_node_unref(node); + return NULL; + } + lcfs_node_set_mode(node, mode); + lcfs_node_set_uid(node, 0); + lcfs_node_set_gid(node, 0); + lcfs_node_set_size(node, size); + return node; +} + +/* Returns 0 on success, or -1 with errno set. */ +int write_ostree_image(int fd) +{ + static const char selinux_bin[] = "system_u:object_r:bin_t:s0"; + static const char selinux_lib[] = "system_u:object_r:lib_t:s0"; + uint8_t digest[LCFS_DIGEST_SIZE]; + struct lcfs_node_s *root, *usr, *bin, *lib, *node; + int r = -1; + + for (size_t i = 0; i < sizeof(digest); i++) + digest[i] = (uint8_t)(i * 7 + 3); + + root = lcfs_node_new(); + if (root == NULL) + return -1; + lcfs_node_set_mode(root, S_IFDIR | 0755); + + if ((usr = add_node(root, "usr", S_IFDIR | 0755, 0)) == NULL) + goto out; + if ((bin = add_node(usr, "bin", S_IFDIR | 0755, 0)) == NULL) + goto out; + if ((lib = add_node(usr, "lib", S_IFDIR | 0755, 0)) == NULL) + goto out; + + /* A file with verity enabled: payload and digest are independent */ + if ((node = add_node(bin, "bash", S_IFREG | 0755, 1432144)) == NULL) + goto out; + if (lcfs_node_set_payload(node, OSTREE_OBJ_A) != 0) + goto out; + lcfs_node_set_fsverity_digest(node, digest); + if (lcfs_node_set_xattr(node, "security.selinux", selinux_bin, + sizeof(selinux_bin)) != 0) + goto out; + + /* A file without verity: only the payload */ + if ((node = add_node(lib, "libfoo.so.1", S_IFREG | 0644, 8193)) == NULL) + goto out; + if (lcfs_node_set_payload(node, OSTREE_OBJ_B) != 0) + goto out; + if (lcfs_node_set_xattr(node, "security.selinux", selinux_lib, + sizeof(selinux_lib)) != 0) + goto out; + + /* Not from ostree: a digest without a payload, which gets no redirect */ + if ((node = add_node(lib, "verity-only", S_IFREG | 0644, 4097)) == NULL) + goto out; + lcfs_node_set_fsverity_digest(node, digest); + + /* Not from ostree: the usual composefs layout, payload = digest's path */ + if ((node = add_node(lib, "object", S_IFREG | 0644, 5000)) == NULL) + goto out; + if (lcfs_node_set_payload(node, DIGEST_OBJ) != 0) + goto out; + lcfs_node_set_fsverity_digest(node, digest); + + /* An empty file has no payload */ + if (add_node(lib, "empty", S_IFREG | 0644, 0) == NULL) + goto out; + + if ((node = add_node(bin, "sh", S_IFLNK | 0777, strlen("bash"))) == NULL) + goto out; + if (lcfs_node_set_payload(node, "bash") != 0) + goto out; + + struct lcfs_write_options_s options = { 0 }; + options.format = LCFS_FORMAT_EROFS; + options.version = 0; + options.max_version = 1; + options.file = &fd; + options.file_write_cb = write_fd_cb; + r = lcfs_write_to(root, &options); + +out: + lcfs_node_unref(root); + return r; +} + +#ifdef OSTREE_IMAGE_MAIN +int main(void) +{ + return write_ostree_image(STDOUT_FILENO) == 0 ? 0 : 1; +} +#endif diff --git a/crates/composefs-capi/tests/test-capi-container.sh b/crates/composefs-capi/tests/test-capi-container.sh index 7cdc3e84..472e71b7 100755 --- a/crates/composefs-capi/tests/test-capi-container.sh +++ b/crates/composefs-capi/tests/test-capi-container.sh @@ -14,7 +14,7 @@ cargo build --release -p composefs-capi --manifest-path="$REPO_DIR/Cargo.toml" BUILD_CTX=$(mktemp -d) trap 'rm -rf "$BUILD_CTX"' EXIT -cp "$REPO_DIR/target/release/libcomposefs_capi.so" "$BUILD_CTX/" +cp "$REPO_DIR/target/release/libcomposefs_capi.so" "$SCRIPT_DIR/ostree-image.c" "$BUILD_CTX/" if [ -n "$C_REPO" ]; then cp -a "$(cd "$C_REPO" && pwd)" "$BUILD_CTX/composefs-c" diff --git a/crates/composefs-ctl/src/composefs_info.rs b/crates/composefs-ctl/src/composefs_info.rs index e2872460..85a16045 100644 --- a/crates/composefs-ctl/src/composefs_info.rs +++ b/crates/composefs-ctl/src/composefs_info.rs @@ -18,8 +18,11 @@ //! filesystem doesn't support it, matching the C `lcfs_fd_get_fsverity()` //! behaviour. -use std::collections::HashSet; +use std::borrow::Cow; +use std::collections::{BTreeSet, HashSet}; +use std::ffi::{OsStr, OsString}; use std::io::Write; +use std::os::unix::ffi::OsStrExt; use std::path::Path; use std::{fs::File, io::Read, path::PathBuf}; @@ -31,7 +34,7 @@ use composefs::{ erofs::reader::erofs_to_filesystem, fsverity::{FsVerityHashValue, Sha256HashValue, measure_verity_with_fallback}, generic_tree::{Inode, LeafContent, LeafId}, - tree::{FileSystem, RegularFile}, + tree::FileSystem, }; /// Query information from composefs images. @@ -177,9 +180,9 @@ fn ls_print( match &leaf.content { LeafContent::Regular(regular) => { let is_hardlink = !seen_leaf_ids.insert(*leaf_id); - if !is_hardlink && let RegularFile::External(id, _) = regular { + if !is_hardlink && let Some(path) = regular.backing_path() { write!(out, "\t@ ")?; - print_escaped(out, id.to_object_pathname().as_bytes())?; + print_escaped(out, path.as_bytes())?; } } LeafContent::Symlink(target) => { @@ -239,17 +242,17 @@ fn cmd_dump(_filter: &[String], images: &[PathBuf]) -> Result<()> { Ok(()) } -/// Collect all external object IDs from a parsed filesystem. +/// Collect the backing paths of all external files in a parsed filesystem. /// -/// Iterates the leaves table directly — each `RegularFile::External` entry -/// is a unique content-addressed object. Because `erofs_to_filesystem` -/// deduplicates hard-linked inodes into a single leaf, each object appears -/// exactly once even if it is referenced by multiple paths. -fn collect_objects_from_fs(fs: &FileSystem) -> HashSet { +/// Like `composefs-info` from libcomposefs, these are the payloads (the +/// paths the overlay redirects point at, relative to the object store), so +/// ostree's `xx/.file` objects are listed as they are. Files +/// with only a verity digest have no backing path and aren't listed. +fn collect_objects_from_fs(fs: &FileSystem) -> BTreeSet { fs.leaves .iter() .filter_map(|leaf| match &leaf.content { - LeafContent::Regular(RegularFile::External(id, _)) => Some(id.clone()), + LeafContent::Regular(file) => file.backing_path().map(Cow::into_owned), _ => None, }) .collect() @@ -262,19 +265,25 @@ fn cmd_objects(images: &[PathBuf]) -> Result<()> { let fs = erofs_to_filesystem::(&image_data) .with_context(|| format!("Failed to parse image: {image_path:?}"))?; - let mut objects: Vec = collect_objects_from_fs(&fs).into_iter().collect(); - objects.sort_by_key(|id| id.to_hex()); - - for obj in objects { - println!("{}", obj.to_object_pathname()); + for obj in collect_objects_from_fs(&fs) { + println!("{}", obj.display()); } } Ok(()) } +/// Returns an object's path relative to the object store, without the +/// leading slashes a payload may have, so that joining it to the base +/// directory can't replace it (like C's `abs_to_rel_path()`). +fn object_relative_path(obj: &OsStr) -> &Path { + let bytes = obj.as_bytes(); + let start = bytes.iter().position(|&b| b != b'/').unwrap_or(bytes.len()); + Path::new(OsStr::from_bytes(&bytes[start..])) +} + /// List objects not present in basedir. fn cmd_missing_objects(basedir: &Path, images: &[PathBuf]) -> Result<()> { - let mut all_objects: HashSet = HashSet::new(); + let mut all_objects = BTreeSet::new(); for image_path in images { let image_data = read_image(image_path)?; @@ -283,15 +292,10 @@ fn cmd_missing_objects(basedir: &Path, images: &[PathBuf]) -> Result<()> { all_objects.extend(collect_objects_from_fs(&fs)); } - let mut missing: Vec = all_objects - .into_iter() - .filter(|obj| !basedir.join(obj.to_object_pathname()).exists()) - .collect(); - - missing.sort_by_key(|a| a.to_hex()); - - for obj in missing { - println!("{}", obj.to_object_pathname()); + for obj in all_objects { + if !basedir.join(object_relative_path(&obj)).exists() { + println!("{}", obj.display()); + } } Ok(()) @@ -316,3 +320,24 @@ fn read_image(path: &PathBuf) -> Result> { .with_context(|| format!("Failed to read image: {path:?}"))?; Ok(data) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_object_relative_path() { + let cases = [ + ("ab/cdef", "ab/cdef"), + ("8a/5d74.file", "8a/5d74.file"), + ("/8a/5d74.file", "8a/5d74.file"), + ("//8a/5d74.file", "8a/5d74.file"), + ("/", ""), + ]; + for (obj, expected) in cases { + let rel = object_relative_path(OsStr::new(obj)); + assert_eq!(rel, Path::new(expected), "{obj}"); + assert!(Path::new("/base").join(rel).starts_with("/base"), "{obj}"); + } + } +} diff --git a/crates/composefs-ctl/src/lib.rs b/crates/composefs-ctl/src/lib.rs index 62dba2a0..2043beb0 100644 --- a/crates/composefs-ctl/src/lib.rs +++ b/crates/composefs-ctl/src/lib.rs @@ -1652,28 +1652,17 @@ pub fn dump_files( } Inode::Leaf(leaf_id, _) => { - use composefs::generic_tree::LeafContent::*; - use composefs::tree::RegularFile::*; - if backing_path_only { let leaf = fs.leaf(*leaf_id); - match &leaf.content { - Regular(f) => match f { - Inline(..) | Sparse(..) => { - writeln!(&mut out, "{} inline", file_path.display())?; - } - External(id, _) | ExternalNoVerity(id, _) => { - writeln!( - &mut out, - "{} {}", - file_path.display(), - id.to_object_pathname() - )?; - } - }, - _ => { - writeln!(&mut out, "{} inline", file_path.display())?; + let backing_path = match &leaf.content { + composefs::generic_tree::LeafContent::Regular(f) => f.backing_path(), + _ => None, + }; + match backing_path { + Some(path) => { + writeln!(&mut out, "{} {}", file_path.display(), path.display())? } + None => writeln!(&mut out, "{} inline", file_path.display())?, } continue; diff --git a/crates/composefs-oci/src/delta.rs b/crates/composefs-oci/src/delta.rs index 9f894902..9dcfee36 100644 --- a/crates/composefs-oci/src/delta.rs +++ b/crates/composefs-oci/src/delta.rs @@ -86,11 +86,11 @@ impl DeltaDataSource for ComposeFsDataSource CurrentFile::Inline(Cursor::new(data.to_vec())), - RegularFile::External(id, _size) | RegularFile::ExternalNoVerity(id, _size) => { + RegularFile::External(..) | RegularFile::ExternalPath { .. } => { let fd = self .source .repo - .open_object(id) + .open_object(&file.repo_object_id()?) .with_context(|| format!("Opening source object for {}", path.display()))?; CurrentFile::External(File::from(fd)) } diff --git a/crates/composefs-ostree/src/commit.rs b/crates/composefs-ostree/src/commit.rs index dc09f1c5..c321a220 100644 --- a/crates/composefs-ostree/src/commit.rs +++ b/crates/composefs-ostree/src/commit.rs @@ -305,8 +305,9 @@ impl CommitWriter { match &leaf.content { LeafContent::Regular( - RegularFile::External(obj_id, size) | RegularFile::ExternalNoVerity(obj_id, size), + file @ (RegularFile::External(_, size) | RegularFile::ExternalPath { size, .. }), ) if !should_inline_file::(*size as usize) => { + let obj_id = &file.repo_object_id()?; // Stream through the hasher without buffering the entire file. let mut hasher = Sha256::new(); hasher.update(&*regular_header); @@ -329,8 +330,8 @@ impl CommitWriter { let content: Option> = match &leaf.content { LeafContent::Regular(RegularFile::Inline(data)) => Some(data.to_vec()), LeafContent::Regular( - RegularFile::External(obj_id, _) | RegularFile::ExternalNoVerity(obj_id, _), - ) => Some(repo.read_object(obj_id)?), + file @ (RegularFile::External(..) | RegularFile::ExternalPath { .. }), + ) => Some(repo.read_object(&file.repo_object_id()?)?), LeafContent::Regular(RegularFile::Sparse(_)) => { bail!("Sparse files not supported in ostree commit") } diff --git a/crates/composefs/src/dumpfile.rs b/crates/composefs/src/dumpfile.rs index 8d18e43d..89d46e25 100644 --- a/crates/composefs/src/dumpfile.rs +++ b/crates/composefs/src/dumpfile.rs @@ -191,9 +191,7 @@ pub fn write_leaf( data, None, ), - LeafContent::Regular( - RegularFile::External(id, size) | RegularFile::ExternalNoVerity(id, size), - ) => write_entry( + LeafContent::Regular(RegularFile::External(id, size)) => write_entry( writer, path, stat, @@ -205,6 +203,22 @@ pub fn write_leaf( &[], Some(&id.to_hex()), ), + LeafContent::Regular(RegularFile::ExternalPath { + redirect, + verity, + size, + }) => write_entry( + writer, + path, + stat, + FileType::RegularFile, + *size, + nlink, + 0, + redirect.as_deref().unwrap_or_default(), + &[], + verity.as_ref().map(|id| id.to_hex()).as_deref(), + ), LeafContent::Regular(RegularFile::Sparse(size)) => write_entry( writer, path, @@ -487,14 +501,12 @@ pub fn add_entry_to_filesystem( .. } => { let stat = entry_to_stat(&entry)?; - let leaf_content = if let Some(digest) = fsverity_digest.as_ref() { - let object_id = ObjectID::from_hex(digest)?; - LeafContent::Regular(RegularFile::External(object_id, size)) - } else { - let object_id = ObjectID::from_object_pathname(path.as_os_str().as_bytes()) - .map_err(|e| anyhow::anyhow!("invalid object pathname: {e}"))?; - LeafContent::Regular(RegularFile::ExternalNoVerity(object_id, size)) - }; + let verity = fsverity_digest + .as_deref() + .map(ObjectID::from_hex) + .transpose()?; + let redirect = path.as_deref().map(|p| Box::from(p.as_os_str())); + let leaf_content = LeafContent::Regular(RegularFile::external(redirect, verity, size)); let id = push_leaf(fs, stat, leaf_content); Inode::leaf(id) } @@ -762,6 +774,47 @@ mod tests { Ok(()) } + /// External files keep their payload: only a payload that is the + /// digest's object path becomes `External`; anything else, like ostree's + /// `xx/.file` or no payload at all, is kept as `ExternalPath`. + #[test] + fn test_external_payload_round_trip() -> Result<()> { + const DIGEST: &str = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + const OBJECT: &str = "01/23456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + const OSTREE: &str = + "8a/5d74a2f8e3c1a0b9d7e6f5c4b3a2918070605040302010f0e0d0c0b0a09087.file"; + let cases = [ + (OBJECT, DIGEST, "External"), + (OSTREE, DIGEST, "ExternalPath"), + (OSTREE, "-", "ExternalPath"), + // A digest without a payload, as libcomposefs dumps it + ("-", DIGEST, "ExternalPath"), + ]; + for (payload, digest, variant) in cases { + let dumpfile = format!( + "/ 0 40755 2 0 0 0 0.0 - - -\n/f 4097 100644 1 0 0 0 0.0 {payload} - {digest}\n" + ); + let fs = dumpfile_to_filesystem::(&dumpfile)?; + let Some(Inode::Leaf(id, _)) = fs.root.lookup(OsStr::new("f")) else { + panic!("expected a leaf for {payload}"); + }; + let LeafContent::Regular(file) = &fs.leaf(*id).content else { + panic!("expected a regular file for {payload}"); + }; + let found = match file { + RegularFile::External(..) => "External", + RegularFile::ExternalPath { .. } => "ExternalPath", + other => panic!("unexpected {other:?} for {payload}"), + }; + assert_eq!(found, variant, "{payload} {digest}"); + + let mut out = Vec::new(); + write_dumpfile(&mut out, &fs)?; + assert_eq!(std::str::from_utf8(&out).unwrap(), dumpfile); + } + Ok(()) + } + /// Verify that xattrs with empty values and with a value of "-" /// both survive a round-trip. Previously `write_escaped` used /// the "-" sentinel for empty bytes, which the xattr parser does diff --git a/crates/composefs/src/dumpfile_parse.rs b/crates/composefs/src/dumpfile_parse.rs index 075efc8c..2b14bc2b 100644 --- a/crates/composefs/src/dumpfile_parse.rs +++ b/crates/composefs/src/dumpfile_parse.rs @@ -94,8 +94,9 @@ pub enum Item<'p> { size: u64, /// Number of links nlink: u32, - /// The backing store path - path: Cow<'p, Path>, + /// The backing store path; `None` for a file that only has an + /// fsverity digest, which overlayfs looks up by its own path + path: Option>, /// The fsverity digest fsverity_digest: Option, }, @@ -436,21 +437,22 @@ impl<'p> Entry<'p> { match ty { FileType::RegularFile => { Self::check_rdev(rdev)?; - if let Some(path) = payload.as_ref() { - let path = unescape_to_path(path)?; + if payload.is_some() || fsverity_digest.is_some() { + // libcomposefs accepts a digest without a payload, + // and writes it as a metacopy without a redirect. + if payload.is_none() && content.is_some() { + anyhow::bail!("Inline file cannot have fsverity digest"); + } Item::Regular { size, nlink, - path, + path: payload.map(unescape_to_path).transpose()?, fsverity_digest: fsverity_digest.map(ToOwned::to_owned), } } else { // A dumpfile entry with no backing path or payload is treated as an empty file let content = content.unwrap_or_default(); let content = unescape_limited(content, MAX_INLINE_CONTENT)?; - if fsverity_digest.is_some() { - anyhow::bail!("Inline file cannot have fsverity digest"); - } Item::RegularInline { nlink, size, @@ -560,7 +562,7 @@ impl Item<'_> { pub(crate) fn payload(&self) -> Option<&Path> { match self { - Item::Regular { path, .. } => Some(path), + Item::Regular { path, .. } => path.as_deref(), Item::Symlink { target, .. } => Some(target), Item::Hardlink { target } => Some(target), _ => None, diff --git a/crates/composefs/src/erofs/reader.rs b/crates/composefs/src/erofs/reader.rs index 874e93d0..6ba79633 100644 --- a/crates/composefs/src/erofs/reader.rs +++ b/crates/composefs/src/erofs/reader.rs @@ -1590,36 +1590,32 @@ fn extract_metacopy_digest( Ok(None) } -/// Try to extract the object ID from a redirect xattr (`trusted.overlay.redirect`). +/// Extract the path from a redirect xattr (`trusted.overlay.redirect`). /// -/// The redirect value is a path like `/55/90e94b...` from which we parse -/// the object ID. Returns `None` if no redirect xattr is present. -fn extract_redirect_object_id( - img: &Image, - inode: &InodeType, -) -> anyhow::Result> { +/// The redirect value is a path like `/55/90e94b...`; this returns it without +/// the leading `/`, as the inverse of what the writer does. Returns `None` if +/// no redirect xattr is present. +fn extract_redirect(img: &Image, inode: &InodeType) -> anyhow::Result>> { let Some(xattrs_section) = inode.xattrs()? else { return Ok(None); }; for id in xattrs_section.shared()? { let xattr = img.shared_xattr(id.get())?; - if let Some(obj) = check_redirect_xattr(xattr)? { - return Ok(Some(obj)); + if let Some(path) = check_redirect_xattr(xattr)? { + return Ok(Some(path)); } } for xattr in xattrs_section.local()? { let xattr = xattr?; - if let Some(obj) = check_redirect_xattr(xattr)? { - return Ok(Some(obj)); + if let Some(path) = check_redirect_xattr(xattr)? { + return Ok(Some(path)); } } Ok(None) } -fn check_redirect_xattr( - xattr: &XAttr, -) -> anyhow::Result> { +fn check_redirect_xattr(xattr: &XAttr) -> anyhow::Result>> { if xattr.header.name_index != 4 { return Ok(None); } @@ -1628,10 +1624,7 @@ fn check_redirect_xattr( } let value = xattr.value()?; let path = value.strip_prefix(b"/").unwrap_or(value); - match ObjectID::from_object_pathname(path) { - Ok(id) => Ok(Some(id)), - Err(_) => Ok(None), - } + Ok(Some(Box::from(OsStr::from_bytes(path)))) } /// Check if a single xattr is a valid overlay.metacopy and return the digest. @@ -1882,19 +1875,12 @@ fn populate_directory( } else { match file_type { S_IFREG => { - if let Some(digest) = - extract_metacopy_digest::(img, &child_inode)? - { - tree::LeafContent::Regular(tree::RegularFile::External( - digest, - child_inode.size(), - )) - } else if let Some(id) = - extract_redirect_object_id::(img, &child_inode)? - { - tree::LeafContent::Regular(tree::RegularFile::ExternalNoVerity( - id, - child_inode.size(), + let verity = extract_metacopy_digest::(img, &child_inode)?; + let redirect = extract_redirect(img, &child_inode)?; + let size = child_inode.size(); + if verity.is_some() || redirect.is_some() { + tree::LeafContent::Regular(tree::RegularFile::external( + redirect, verity, size, )) } else if child_inode.data_layout()? == DataLayout::ChunkBased { tree::LeafContent::Regular(tree::RegularFile::Sparse( diff --git a/crates/composefs/src/erofs/writer.rs b/crates/composefs/src/erofs/writer.rs index 0b0de92c..175c8a08 100644 --- a/crates/composefs/src/erofs/writer.rs +++ b/crates/composefs/src/erofs/writer.rs @@ -706,7 +706,7 @@ impl Leaf<'_, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(.., size) - | tree::RegularFile::ExternalNoVerity(.., size) + | tree::RegularFile::ExternalPath { size, .. } | tree::RegularFile::Sparse(size), ) => { let chunk_format = match version.epoch() { @@ -757,7 +757,7 @@ impl Leaf<'_, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(..) - | tree::RegularFile::ExternalNoVerity(..) + | tree::RegularFile::ExternalPath { .. } | tree::RegularFile::Sparse(..), ) => { let n_chunks = self.inline_tail_size / 4; @@ -794,7 +794,7 @@ impl Leaf<'_, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(..) - | tree::RegularFile::ExternalNoVerity(..) + | tree::RegularFile::ExternalPath { .. } | tree::RegularFile::Sparse(..), ) => { const LCFS_MAX_NONINLINE_CHUNKS: usize = 1024; @@ -1178,17 +1178,29 @@ impl<'a, ObjectID: FsVerityHashValue> InodeCollector<'a, ObjectID> { self.version, ); } else if let InodeContent::Leaf(Leaf { - content: tree::LeafContent::Regular(tree::RegularFile::ExternalNoVerity(id, ..)), + content: + tree::LeafContent::Regular(tree::RegularFile::ExternalPath { + redirect, verity, .. + }), .. }) = content { - xattrs.add(format::XATTR_OVERLAY_METACOPY, b"", self.version); - let redirect = format!("/{}", id.to_object_pathname()); - xattrs.add( - format::XATTR_OVERLAY_REDIRECT, - redirect.as_bytes(), - self.version, - ); + // This matches libcomposefs, which writes the redirect as "/" + payload. + match verity { + Some(id) => { + let metacopy = OverlayMetacopy::new(id); + xattrs.add( + format::XATTR_OVERLAY_METACOPY, + metacopy.as_bytes(), + self.version, + ); + } + None => xattrs.add(format::XATTR_OVERLAY_METACOPY, b"", self.version), + } + if let Some(redirect) = redirect { + let redirect = [b"/", redirect.as_bytes()].concat(); + xattrs.add(format::XATTR_OVERLAY_REDIRECT, &redirect, self.version); + } } else if let InodeContent::Leaf(Leaf { content: tree::LeafContent::Regular(tree::RegularFile::Sparse(..)), .. @@ -1263,7 +1275,7 @@ impl<'a, ObjectID: FsVerityHashValue> InodeCollector<'a, ObjectID> { } tree::LeafContent::Regular( tree::RegularFile::External(.., size) - | tree::RegularFile::ExternalNoVerity(.., size) + | tree::RegularFile::ExternalPath { size, .. } | tree::RegularFile::Sparse(size), ) if *size > 0 => { let chunk_count = compute_chunk_count(*size); @@ -1274,7 +1286,9 @@ impl<'a, ObjectID: FsVerityHashValue> InodeCollector<'a, ObjectID> { } else { match &leaf.content { tree::LeafContent::Regular(tree::RegularFile::Inline(data)) => (0, data.len()), - tree::LeafContent::Regular(tree::RegularFile::External(..)) => { + tree::LeafContent::Regular( + tree::RegularFile::External(..) | tree::RegularFile::ExternalPath { .. }, + ) => { (0, 4) // single null chunk index } _ => (0, 0), @@ -2056,7 +2070,7 @@ fn fixup_epoch1_data_blocks( leaf.content, tree::LeafContent::Regular( tree::RegularFile::External(..) - | tree::RegularFile::ExternalNoVerity(..) + | tree::RegularFile::ExternalPath { .. } | tree::RegularFile::Sparse(..) ) ), diff --git a/crates/composefs/src/fs.rs b/crates/composefs/src/fs.rs index 8f882eeb..dccad199 100644 --- a/crates/composefs/src/fs.rs +++ b/crates/composefs/src/fs.rs @@ -308,9 +308,10 @@ fn write_leaf( set_file_contents(dirfd, name, &leaf.stat, data)? } LeafContent::Regular( - RegularFile::External(id, size) | RegularFile::ExternalNoVerity(id, size), + file @ (RegularFile::External(_, size) | RegularFile::ExternalPath { size, .. }), ) => { - let object = repo.open_object(id)?; + let id = file.repo_object_id()?; + let object = repo.open_object(&id)?; // TODO: make this better. At least needs to be EINTR-safe. Could even do reflink in some cases. // Regardless we shouldn't read the whole file into memory. let size = (*size).try_into().context("size overflow")?; @@ -694,10 +695,11 @@ pub fn read_file( ) -> Result> { match file { RegularFile::Inline(data) => Ok(data.clone()), - RegularFile::External(id, size) | RegularFile::ExternalNoVerity(id, size) => { + RegularFile::External(_, size) | RegularFile::ExternalPath { size, .. } => { + let id = file.repo_object_id()?; let capacity: usize = (*size).try_into().context("file too large for memory")?; let mut data = Vec::with_capacity(capacity); - std::fs::File::from(repo.open_object(id)?).read_to_end(&mut data)?; + std::fs::File::from(repo.open_object(&id)?).read_to_end(&mut data)?; ensure!( *size == data.len() as u64, "File content doesn't have the expected length" diff --git a/crates/composefs/src/tree.rs b/crates/composefs/src/tree.rs index 190c3f85..1d10d2ae 100644 --- a/crates/composefs/src/tree.rs +++ b/crates/composefs/src/tree.rs @@ -2,6 +2,10 @@ //! of inlining small files, and having an external fsverity reference for //! larger ones. +use std::borrow::Cow; +use std::ffi::OsStr; +use std::os::unix::ffi::OsStrExt; + use crate::fsverity::FsVerityHashValue; pub use crate::generic_tree::{self, ImageError, Stat}; @@ -19,10 +23,23 @@ pub enum RegularFile { /// The tuple contains (fsverity hash, file size in bytes). /// The fsverity digest is embedded in the overlay metacopy xattr. External(ObjectID, u64), - /// Like `External`, but without embedding the fsverity digest in the - /// overlay metacopy xattr. Used by the C API when the caller set a - /// content-address payload but did not explicitly set a verified digest. - ExternalNoVerity(ObjectID, u64), + /// File stored externally at an explicit path, as libcomposefs models it. + /// + /// Unlike `External`, the overlay redirect isn't derived from a digest. + /// This is what the C API produces for a node with a payload, e.g. ostree's + /// `xx/.file` objects. Build it with [`RegularFile::external`], + /// which keeps the cases `External` and `Sparse` cover out of it. + ExternalPath { + /// The backing file's path relative to the object store root, written + /// as `/` in the overlay redirect xattr. Without one, no + /// redirect is written and overlayfs looks the file up by its own path + /// in the data layers. + redirect: Option>, + /// The fsverity digest to embed in the overlay metacopy xattr, if any. + verity: Option, + /// The file size in bytes. + size: u64, + }, /// File with declared size but no content or external reference. /// Produces ChunkBased layout with null chunk indices. Sparse(u64), @@ -33,7 +50,72 @@ impl RegularFile { pub fn file_size(&self) -> u64 { match self { Self::Inline(data) => data.len() as u64, - Self::External(_, size) | Self::ExternalNoVerity(_, size) | Self::Sparse(size) => *size, + Self::External(_, size) | Self::ExternalPath { size, .. } | Self::Sparse(size) => *size, + } + } + + /// Builds an external file from an overlay redirect and verity digest, + /// the way libcomposefs stores them: independently of each other. + /// + /// An empty redirect counts as none, as in libcomposefs. The result is + /// `External` when the redirect is the digest's object path (the usual + /// composefs layout), `Sparse` when there is neither, and `ExternalPath` + /// otherwise. + pub fn external(redirect: Option>, verity: Option, size: u64) -> Self { + let redirect = redirect.filter(|r| !r.is_empty()); + match (redirect, verity) { + (None, None) => Self::Sparse(size), + (Some(redirect), Some(id)) + if redirect.as_bytes() == id.to_object_pathname().as_bytes() => + { + Self::External(id, size) + } + (redirect, verity) => Self::ExternalPath { + redirect, + verity, + size, + }, + } + } + + /// Returns the path of an external file's backing file relative to the + /// object store root, which the overlay redirect xattr points at: the + /// object path for `External`, the redirect for `ExternalPath`. + /// + /// This is the libcomposefs payload, which `composefs-info` lists. + pub fn backing_path(&self) -> Option> { + match self { + Self::External(id, _) => Some(Cow::Owned(id.to_object_pathname().into())), + Self::ExternalPath { redirect, .. } => redirect.as_deref().map(Cow::Borrowed), + Self::Inline(_) | Self::Sparse(_) => None, + } + } + + /// Returns the object backing an external file in a composefs repository. + /// + /// For `ExternalPath`, this is the verity digest if set, otherwise the + /// redirect parsed as an object pathname. Fails for inline and sparse + /// files, and for an `ExternalPath` that names no object this way (like + /// ostree's `xx/.file` without a verity digest). + pub fn repo_object_id(&self) -> anyhow::Result { + match self { + Self::Inline(_) | Self::Sparse(_) => anyhow::bail!("Not an external file"), + Self::External(id, _) => Ok(id.clone()), + Self::ExternalPath { + verity: Some(id), .. + } => Ok(id.clone()), + Self::ExternalPath { + redirect: Some(redirect), + verity: None, + .. + } => ObjectID::from_object_pathname(redirect.as_bytes()).map_err(|e| { + anyhow::anyhow!("External file path {redirect:?} is not an object: {e}") + }), + Self::ExternalPath { + redirect: None, + verity: None, + .. + } => anyhow::bail!("External file has neither a path nor a verity digest"), } } } @@ -139,4 +221,80 @@ mod tests { .unwrap(); assert_eq!(retrieved_subdir_opt.stat.st_mtim_sec, 20); } + + #[test] + fn test_external() { + let id = Sha256HashValue::from_hex( + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + ) + .unwrap(); + let other = Sha256HashValue::from_hex( + "fedcba9876543210fedcba9876543210fedcba9876543210fedcba9876543210", + ) + .unwrap(); + let object_path = id.to_object_pathname(); + const OSTREE: &str = "8a/5d74.file"; + // (redirect, verity, variant, repo_object_id, backing_path) + let cases = [ + ( + Some(object_path.as_str()), + Some(&id), + "External", + Some(&id), + Some(object_path.as_str()), + ), + // The verity digest names the repository object, whatever the redirect + ( + Some(OSTREE), + Some(&other), + "ExternalPath", + Some(&other), + Some(OSTREE), + ), + ( + Some(object_path.as_str()), + Some(&other), + "ExternalPath", + Some(&other), + Some(object_path.as_str()), + ), + (None, Some(&id), "ExternalPath", Some(&id), None), + (Some(""), Some(&id), "ExternalPath", Some(&id), None), + ( + Some(object_path.as_str()), + None, + "ExternalPath", + Some(&id), + Some(object_path.as_str()), + ), + // ostree's payload without verity names no repository object + (Some(OSTREE), None, "ExternalPath", None, Some(OSTREE)), + (None, None, "Sparse", None, None), + (Some(""), None, "Sparse", None, None), + ]; + for (redirect, verity, variant, object, backing_path) in cases { + let file = RegularFile::external( + redirect.map(|r| Box::from(OsStr::new(r))), + verity.cloned(), + 4096, + ); + let found = match &file { + RegularFile::External(..) => "External", + RegularFile::ExternalPath { .. } => "ExternalPath", + RegularFile::Sparse(..) => "Sparse", + RegularFile::Inline(..) => "Inline", + }; + assert_eq!(found, variant, "{redirect:?} {verity:?}"); + assert_eq!(file.file_size(), 4096); + assert_eq!(file.repo_object_id().ok().as_ref(), object, "{file:?}"); + assert_eq!( + file.backing_path().as_deref(), + backing_path.map(OsStr::new), + "{file:?}" + ); + } + let inline = RegularFile::::Inline(Box::new([1])); + assert!(inline.repo_object_id().is_err()); + assert_eq!(inline.backing_path(), None); + } }