Merge pull request #648 from cgwalters/etc-hardlinked

tar: Handle hardlinks into /etc

lib/src/tar/write.rs

@@ -18,6 +18,7 @@ use cap_std_ext::{cap_std, cap_tempfile};
 use fn_error_context::context;
 use ostree::gio;
 use ostree::prelude::FileExt;
+use std::borrow::Cow;
 use std::collections::{BTreeMap, HashMap};
 use std::io::{BufWriter, Seek, Write};
 use std::path::Path;
@@ -49,9 +50,19 @@ pub(crate) fn copy_entry(
     // api as the header api does not handle long paths:
     // https://github.com/alexcrichton/tar-rs/issues/192
     match entry.header().entry_type() {
-        tar::EntryType::Link | tar::EntryType::Symlink => {
+        tar::EntryType::Symlink => {
             let target = entry.link_name()?.ok_or_else(|| anyhow!("Invalid link"))?;
-            dest.append_link(&mut header, path, target)
+            // Sanity check UTF-8 here too.
+            let target: &Utf8Path = (&*target).try_into()?;
+            dest.append_link(&mut header, path, &*target)
+        }
+        tar::EntryType::Link => {
+            let target = entry.link_name()?.ok_or_else(|| anyhow!("Invalid link"))?;
+            let target: &Utf8Path = (&*target).try_into()?;
+            // We need to also normalize the target in order to handle hardlinked files in /etc
+            // where we remap /etc to /usr/etc.
+            let target = remap_etc_path(target);
+            dest.append_link(&mut header, path, &*target)
         }
         _ => dest.append_data(&mut header, path, entry),
     }
@@ -119,6 +130,34 @@ pub(crate) struct TarImportConfig {
     remap_factory_var: bool,
 }
 
+// If a path starts with /etc or ./etc or etc, remap it to be usr/etc.
+fn remap_etc_path(path: &Utf8Path) -> Cow<Utf8Path> {
+    let mut components = path.components();
+    let Some(prefix) = components.next() else {
+        return Cow::Borrowed(path);
+    };
+    let (prefix, first) = if matches!(prefix, Utf8Component::CurDir | Utf8Component::RootDir) {
+        let Some(next) = components.next() else {
+            return Cow::Borrowed(path);
+        };
+        (Some(prefix), next)
+    } else {
+        (None, prefix)
+    };
+    if first.as_str() == "etc" {
+        let usr = Utf8Component::Normal("usr");
+        Cow::Owned(
+            prefix
+                .into_iter()
+                .chain([usr, first])
+                .chain(components)
+                .collect(),
+        )
+    } else {
+        Cow::Borrowed(path)
+    }
+}
+
 fn normalize_validate_path<'a>(
     path: &'a Utf8Path,
     config: &'_ TarImportConfig,
@@ -438,6 +477,25 @@ mod tests {
     use super::*;
     use std::io::Cursor;
 
+    #[test]
+    fn test_remap_etc() {
+        // These shouldn't change. Test etcc to verify we're not doing string matching.
+        let unchanged = ["", "foo", "/etcc/foo", "../etc/baz"];
+        for x in unchanged {
+            similar_asserts::assert_eq!(x, remap_etc_path(x.into()).as_str());
+        }
+        // Verify all 3 forms of "./etc", "/etc" and "etc", and also test usage of
+        // ".."" (should be unchanged) and "//" (will be normalized).
+        for (p, expected) in [
+            ("/etc/foo/../bar/baz", "/usr/etc/foo/../bar/baz"),
+            ("etc/foo//bar", "usr/etc/foo/bar"),
+            ("./etc/foo", "./usr/etc/foo"),
+            ("etc", "usr/etc"),
+        ] {
+            similar_asserts::assert_eq!(remap_etc_path(p.into()).as_str(), expected);
+        }
+    }
+
     #[test]
     fn test_normalize_path() {
         let imp_default = &TarImportConfig {

lib/tests/it/main.rs

@@ -1058,6 +1058,98 @@ async fn test_container_var_content() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test]
+async fn test_container_etc_hardlinked() -> Result<()> {
+    let fixture = Fixture::new_v1()?;
+
+    let imgref = fixture.export_container().await.unwrap().0;
+    let imgref = OstreeImageReference {
+        sigverify: SignatureSource::ContainerPolicyAllowInsecure,
+        imgref,
+    };
+
+    // Build a derived image
+    let derived_path = &fixture.path.join("derived.oci");
+    let srcpath = imgref.imgref.name.as_str();
+    oci_clone(srcpath, derived_path).await.unwrap();
+    ostree_ext::integrationtest::generate_derived_oci_from_tar(
+        derived_path,
+        |w| {
+            let mut layer_tar = tar::Builder::new(w);
+            // Create a simple hardlinked file /etc/foo and /etc/bar in the tar stream, which
+            // needs usr/etc processing.
+            let mut h = tar::Header::new_gnu();
+            h.set_uid(0);
+            h.set_gid(0);
+            h.set_size(0);
+            h.set_mode(0o755);
+            h.set_entry_type(tar::EntryType::Directory);
+            layer_tar.append_data(&mut h.clone(), "etc", &mut std::io::empty())?;
+            let testdata = "hardlinked test data";
+            h.set_mode(0o644);
+            h.set_size(testdata.len().try_into().unwrap());
+            h.set_entry_type(tar::EntryType::Regular);
+            layer_tar.append_data(
+                &mut h.clone(),
+                "etc/foo",
+                std::io::Cursor::new(testdata.as_bytes()),
+            )?;
+            h.set_entry_type(tar::EntryType::Link);
+            h.set_size(0);
+            layer_tar.append_link(&mut h.clone(), "etc/bar", "etc/foo")?;
+
+            // Another case where we have /etc/dnf.conf and a hardlinked /ostree/repo/objects
+            // link into it - in this case we should ignore the hardlinked one.
+            let testdata = "hardlinked into object store";
+            h.set_mode(0o644);
+            h.set_mtime(42);
+            h.set_size(testdata.len().try_into().unwrap());
+            h.set_entry_type(tar::EntryType::Regular);
+            layer_tar.append_data(
+                &mut h.clone(),
+                "etc/dnf.conf",
+                std::io::Cursor::new(testdata.as_bytes()),
+            )?;
+            h.set_entry_type(tar::EntryType::Link);
+            h.set_mtime(42);
+            h.set_size(0);
+            layer_tar.append_link(&mut h.clone(), "sysroot/ostree/repo/objects/45/7279b28b541ca20358bec8487c81baac6a3d5ed3cea019aee675137fab53cb.file", "etc/dnf.conf")?;
+            layer_tar.finish()?;
+            Ok(())
+        },
+        None,
+    )?;
+
+    let derived_imgref = OstreeImageReference {
+        sigverify: SignatureSource::ContainerPolicyAllowInsecure,
+        imgref: ImageReference {
+            transport: Transport::OciDir,
+            name: derived_path.to_string(),
+        },
+    };
+    let mut imp =
+        store::ImageImporter::new(fixture.destrepo(), &derived_imgref, Default::default()).await?;
+    imp.set_ostree_version(2023, 11);
+    let prep = match imp.prepare().await.unwrap() {
+        store::PrepareResult::AlreadyPresent(_) => panic!("should not be already imported"),
+        store::PrepareResult::Ready(r) => r,
+    };
+    let import = imp.import(prep).await.unwrap();
+    let r = fixture
+        .destrepo()
+        .read_commit(import.get_commit(), gio::Cancellable::NONE)?
+        .0;
+    let foo = r.resolve_relative_path("usr/etc/foo");
+    let foo = foo.downcast_ref::<ostree::RepoFile>().unwrap();
+    foo.ensure_resolved()?;
+    let bar = r.resolve_relative_path("usr/etc/bar");
+    let bar = bar.downcast_ref::<ostree::RepoFile>().unwrap();
+    bar.ensure_resolved()?;
+    assert_eq!(foo.checksum(), bar.checksum());
+
+    Ok(())
+}
+
 /// Copy an OCI directory.
 async fn oci_clone(src: impl AsRef<Utf8Path>, dest: impl AsRef<Utf8Path>) -> Result<()> {
     let src = src.as_ref();