From 39d4b938cbf703ec3ad7074b72b374ca1cba272a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 18 Nov 2021 16:54:39 -0500 Subject: [PATCH 1/2] Use new `append_link()` API to handle long symlinks I hit this when exporting Fedora Silverblue, there are some long symlinks in there. Depends: https://github.com/alexcrichton/tar-rs/pull/273 Closes: https://github.com/ostreedev/ostree-rs-ext/issues/162 --- lib/Cargo.toml | 2 +- lib/src/tar/export.rs | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/Cargo.toml b/lib/Cargo.toml index bc928136..ba4d88dd 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -35,7 +35,7 @@ pin-project = "1.0" serde = { features = ["derive"], version = "1.0.125" } serde_json = "1.0.64" structopt = "0.3.21" -tar = "0.4.33" +tar = "0.4.38" tempfile = "3.2.0" tokio = { features = ["full"], version = "1" } tokio-util = { features = ["io-util"], version = "0.6.9" } diff --git a/lib/src/tar/export.rs b/lib/src/tar/export.rs index 2cfadda9..ff9721a6 100644 --- a/lib/src/tar/export.rs +++ b/lib/src/tar/export.rs @@ -231,13 +231,11 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { .append_data(&mut h, &path, &mut instream) .with_context(|| format!("Writing regfile {}", checksum))?; } else { - h.set_size(0); - h.set_entry_type(tar::EntryType::Symlink); let context = || format!("Writing content symlink: {}", checksum); - h.set_link_name(meta.symlink_target().unwrap().as_str()) - .with_context(context)?; + h.set_entry_type(tar::EntryType::Symlink); + h.set_size(0); self.out - .append_data(&mut h, &path, &mut std::io::empty()) + .append_link(&mut h, &path, meta.symlink_target().unwrap().as_str()) .with_context(context)?; } } From 05f11ec7e2c6172d3ae8f8794753ce9a2c14db39 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 10 Dec 2021 09:08:20 -0500 Subject: [PATCH 2/2] tar/export: Write symlink targets literally Requires: https://github.com/alexcrichton/tar-rs/pull/274 And I'll just copy/paste the commit message from there, lightly edited: In https://github.com/ostreedev/ostree we generate a cryptographic checksum over files and symlinks, and directories. ostree does not currently perform any canonicalization on symlinks; we'll respect and honor whatever bytes we're provided as input, and replicate that on the target. We're using the Rust tar crate to do tar serialization, which has so far worked fine...except, I hit this corner case: ``` [root@cosa-devsh ~]# rpm -qf /usr/lib/systemd/systemd-sysv-install chkconfig-1.13-2.el8.x86_64 [root@cosa-devsh ~]# ll /usr/lib/systemd/systemd-sysv-install lrwxrwxrwx. 2 root root 24 Nov 29 18:08 /usr/lib/systemd/systemd-sysv-install -> ../../..//sbin/chkconfig [root@cosa-devsh ~]# ``` But, using `set_link_name` to write the tarball, we end up with the canonicalized path `../../../sbin/chkconfig` - i.e. without the double `//`. This breaks the checksum. Now, I am a bit tempted to change ostree to do canonicalization. But even if we did, I'd need to *exactly* match what tar-rs is doing. (I may of course also try to change the rhel8 systemd package, but that's going to take a while to propagate and this corner case isn't the only one I'm sure) --- lib/src/tar/export.rs | 44 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/src/tar/export.rs b/lib/src/tar/export.rs index ff9721a6..e7a15bd2 100644 --- a/lib/src/tar/export.rs +++ b/lib/src/tar/export.rs @@ -56,6 +56,17 @@ fn xattrs_path(checksum: &str) -> Utf8PathBuf { format!("{}/repo/xattrs/{}", OSTREEDIR, checksum).into() } +/// Check for "denormal" symlinks which contain "//" +/// See https://github.com/fedora-sysv/chkconfig/pull/67 +/// [root@cosa-devsh ~]# rpm -qf /usr/lib/systemd/systemd-sysv-install +/// chkconfig-1.13-2.el8.x86_64 +/// [root@cosa-devsh ~]# ll /usr/lib/systemd/systemd-sysv-install +/// lrwxrwxrwx. 2 root root 24 Nov 29 18:08 /usr/lib/systemd/systemd-sysv-install -> ../../..//sbin/chkconfig +/// [root@cosa-devsh ~]# +fn symlink_is_denormal(target: &str) -> bool { + target.contains("//") +} + impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { fn new(repo: &'a ostree::Repo, out: &'a mut tar::Builder) -> Self { Self { @@ -231,12 +242,23 @@ impl<'a, W: std::io::Write> OstreeTarWriter<'a, W> { .append_data(&mut h, &path, &mut instream) .with_context(|| format!("Writing regfile {}", checksum))?; } else { + let target = meta.symlink_target().unwrap(); + let target = target.as_str(); let context = || format!("Writing content symlink: {}", checksum); - h.set_entry_type(tar::EntryType::Symlink); - h.set_size(0); - self.out - .append_link(&mut h, &path, meta.symlink_target().unwrap().as_str()) - .with_context(context)?; + // Handle //chkconfig, see above + if symlink_is_denormal(target) { + h.set_link_name_literal(meta.symlink_target().unwrap().as_str()) + .with_context(context)?; + self.out + .append_data(&mut h, &path, &mut std::io::empty()) + .with_context(context)?; + } else { + h.set_entry_type(tar::EntryType::Symlink); + h.set_size(0); + self.out + .append_link(&mut h, &path, target) + .with_context(context)?; + } } } @@ -338,4 +360,16 @@ mod tests { Utf8Path::new("./etc/blah") ); } + + #[test] + fn test_denormal_symlink() { + let normal = ["/", "/usr", "../usr/bin/blah"]; + let denormal = ["../../usr/sbin//chkconfig", "foo//bar/baz"]; + for path in normal { + assert!(!symlink_is_denormal(path)); + } + for path in denormal { + assert!(symlink_is_denormal(path)); + } + } }