From f25a3ca63604a476ad38641c817b18b9c0a58e47 Mon Sep 17 00:00:00 2001 From: Silenio Quarti Date: Thu, 29 Aug 2024 10:25:41 -0400 Subject: [PATCH 1/2] image-rs: Handle gzip whiteouts correctly Fixes: #604 Signed-off-by: Silenio Quarti --- image-rs/src/unpack.rs | 106 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/image-rs/src/unpack.rs b/image-rs/src/unpack.rs index 0647fa5c3..1851a601a 100644 --- a/image-rs/src/unpack.rs +++ b/image-rs/src/unpack.rs @@ -2,7 +2,7 @@ // // SPDX-License-Identifier: Apache-2.0 -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use filetime::FileTime; use futures::StreamExt; use log::{debug, warn}; @@ -39,6 +39,55 @@ fn is_attr_available(path: &Path) -> Result { } } +const WHITEOUT_PREFIX: &str = ".wh."; +const WHITEOUT_OPAQUE_DIR: &str = ".wh..wh..opq"; + +/// Returns whether the file name is a whiteout file +fn is_whiteout(name: &str) -> bool { + name.starts_with(WHITEOUT_PREFIX) +} + +/// Converts a whiteout file or opaque directory. See OverlayFS and Aufs documentation for details +/// https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt +/// https://aufs.sourceforge.net/aufs.html +async fn convert_whiteout( + name: &str, + path: &Path, + uid: u32, + gid: u32, + mode: Option, + destination: &Path, +) -> Result<()> { + let parent = path + .parent() + .ok_or(anyhow!("Invalid whiteout parent for path: {:?}", path))?; + + // Handle opaque directories + if name == WHITEOUT_OPAQUE_DIR { + let destination_parent = destination.join(parent); + xattr::set(destination_parent, "trusted.overlay.opaque", b"y")?; + return Ok(()); + } + + // Handle whiteout files + let original_name = name + .strip_prefix(WHITEOUT_PREFIX) + .ok_or(anyhow!("Failed to strip whiteout prefix for: {}", name))?; + let original_path = parent.join(original_name); + let path = CString::new(format!( + "{}/{}", + destination.display(), + original_path.display() + ))?; + + let ret = unsafe { nix::libc::mknod(path.as_ptr(), nix::libc::S_IFCHR, 0) }; + if ret != 0 { + bail!("mknod: {:?} error: {:?}", path, io::Error::last_os_error()); + } + + set_perms_ownerships(&path, ChownType::LChown, uid, gid, mode).await +} + /// Unpack the contents of tarball to the destination path pub async fn unpack(input: R, destination: &Path) -> Result<()> { if destination.exists() { @@ -53,9 +102,10 @@ pub async fn unpack(input: R, destination: &Path) -> Resul fs::create_dir_all(destination).await?; + let attr_available = is_attr_available(destination)?; let mut archive = ArchiveBuilder::new(input) .set_ignore_zeros(true) - .set_unpack_xattrs(is_attr_available(destination)?) + .set_unpack_xattrs(attr_available) .set_preserve_permissions(true) .build(); @@ -67,6 +117,12 @@ pub async fn unpack(input: R, destination: &Path) -> Resul while let Some(file) = entries.next().await { let mut file = file?; + let entry_path = file.path()?; + let entry_name = entry_path + .file_name() + .unwrap_or_default() + .to_str() + .ok_or(anyhow!("Invalid unicode in path: {:?}", entry_path))?; let uid = file .header() .uid()? @@ -77,6 +133,12 @@ pub async fn unpack(input: R, destination: &Path) -> Resul .gid()? .try_into() .context("GID is too large!")?; + let mode = file.header().mode().ok(); + + if attr_available && is_whiteout(entry_name) { + convert_whiteout(entry_name, &entry_path, uid, gid, mode, destination).await?; + continue; + } file.unpack_in(destination).await?; @@ -89,7 +151,6 @@ pub async fn unpack(input: R, destination: &Path) -> Resul let file_path = path.to_str().expect("must be utf8"); let kind = file.header().entry_type(); - let mode = file.header().mode().ok(); let mtime = file.header().mtime()? as i64; // krata-tokio-tar crate does not provide a way to preserve permissions @@ -195,6 +256,7 @@ async fn set_perms_ownerships( mod tests { use std::os::unix::fs::{chown, lchown, MetadataExt}; + use std::os::unix::fs::FileTypeExt; use tokio::{ fs::{self, File}, io::AsyncWriteExt, @@ -231,12 +293,37 @@ mod tests { .await .unwrap(); + let path = tempdir + .path() + .join(WHITEOUT_PREFIX.to_owned() + "whiteout_file.txt"); + File::create(&path).await.unwrap(); + ar.append_file( + WHITEOUT_PREFIX.to_owned() + "whiteout_file.txt", + &mut File::open(&path).await.unwrap(), + ) + .await + .unwrap(); + let path = tempdir.path().join("dir"); fs::create_dir(&path).await.unwrap(); filetime::set_file_mtime(&path, mtime).unwrap(); ar.append_path_with_name(&path, "dir").await.unwrap(); + let path = tempdir.path().join("whiteout_dir"); + fs::create_dir(&path).await.unwrap(); + ar.append_path_with_name(&path, "whiteout_dir") + .await + .unwrap(); + + let path = tempdir + .path() + .join("whiteout_dir/".to_owned() + WHITEOUT_OPAQUE_DIR); + fs::create_dir(&path).await.unwrap(); + ar.append_path_with_name(&path, "whiteout_dir/".to_owned() + WHITEOUT_OPAQUE_DIR) + .await + .unwrap(); + // TODO: Add more file types like symlink, char, block devices. let data = ar.into_inner().await.unwrap(); tempdir.close().unwrap(); @@ -262,11 +349,24 @@ mod tests { assert_eq!(metadata.gid(), 10); assert_eq!(metadata.uid(), 10); + let attr_available = is_attr_available(destination).unwrap(); + if attr_available { + let path = destination.join("whiteout_file.txt"); + let metadata = fs::metadata(path).await.unwrap(); + assert!(metadata.file_type().is_char_device()); + } + let path = destination.join("dir"); let metadata = fs::metadata(path).await.unwrap(); let new_mtime = filetime::FileTime::from_last_modification_time(&metadata); assert_eq!(mtime, new_mtime); + if attr_available { + let path = destination.join("whiteout_dir"); + let opaque = xattr::get(path, "trusted.overlay.opaque").unwrap().unwrap(); + assert_eq!(opaque, b"y"); + } + // though destination already exists, it will be deleted // and rewrite assert!(unpack(data.as_slice(), destination).await.is_ok()); From 84c966770a9dc464e40b40812661576d2c4de128 Mon Sep 17 00:00:00 2001 From: Silenio Quarti Date: Thu, 29 Aug 2024 16:37:18 -0400 Subject: [PATCH 2/2] review: use nix mknod Signed-off-by: Silenio Quarti --- image-rs/Cargo.toml | 2 +- image-rs/src/unpack.rs | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/image-rs/Cargo.toml b/image-rs/Cargo.toml index 1ab5d1451..c63563b64 100644 --- a/image-rs/Cargo.toml +++ b/image-rs/Cargo.toml @@ -31,7 +31,7 @@ kbc = { path = "../attestation-agent/kbc", default-features = false, optional = lazy_static = { workspace = true, optional = true } log = "0.4.22" loopdev = { git = "https://github.com/mdaffin/loopdev", rev = "c9f91e8f0326ce8a3364ac911e81eb32328a5f27" } -nix = { workspace = true, optional = true, features = ["mount"] } +nix = { workspace = true, optional = true, features = ["mount", "fs"] } oci-client = { version = "0.12", default-features = false, optional = true } oci-spec = "0.6.7" ocicrypt-rs = { path = "../ocicrypt-rs", default-features = false, features = [ diff --git a/image-rs/src/unpack.rs b/image-rs/src/unpack.rs index 1851a601a..3583cfcf5 100644 --- a/image-rs/src/unpack.rs +++ b/image-rs/src/unpack.rs @@ -7,6 +7,8 @@ use filetime::FileTime; use futures::StreamExt; use log::{debug, warn}; use nix::libc::timeval; +use nix::sys::stat::{mknod, Mode, SFlag}; + use std::{ collections::HashMap, convert::TryInto, @@ -80,10 +82,7 @@ async fn convert_whiteout( original_path.display() ))?; - let ret = unsafe { nix::libc::mknod(path.as_ptr(), nix::libc::S_IFCHR, 0) }; - if ret != 0 { - bail!("mknod: {:?} error: {:?}", path, io::Error::last_os_error()); - } + mknod(path.as_c_str(), SFlag::S_IFCHR, Mode::empty(), 0)?; set_perms_ownerships(&path, ChownType::LChown, uid, gid, mode).await }