From 99e5a0dec16698ebc74feb3668c355bf06e19948 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 10 Dec 2021 10:56:04 -0500 Subject: [PATCH] importer: Canonicalize `chkconfig` symlink target This is a build-side fix for `chkconfig` shipping a non-canonical symlink; see: https://github.com/ostreedev/ostree-rs-ext/pull/182 I'll copy/paste that commit message: 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) I verified that this fixes `rpm-ostree ex-container encapsulate` for RHCOS. In fact, it is *just that* one systemd symlink which is non-canonical. --- rust/src/importer.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/rust/src/importer.rs b/rust/src/importer.rs index 4ab9f9638a..adf03ca8b4 100644 --- a/rust/src/importer.rs +++ b/rust/src/importer.rs @@ -9,6 +9,7 @@ use crate::cxxrsutil::{CxxResult, FFIGObjectWrapper}; use crate::utils; use anyhow::{bail, format_err, Result}; +use camino::{Utf8Path, Utf8PathBuf}; use fn_error_context::context; use gio::{FileInfo, FileType}; use ostree::RepoCommitFilterResult; @@ -17,6 +18,23 @@ use std::borrow::Cow; use std::fmt::Write; use std::pin::Pin; +/// Canonicalize a path, e.g. replace `//` with `/` and `././` with `./`. +// For some background behind this, see https://github.com/alexcrichton/tar-rs/pull/274 +// The specific problem case was: +// # rpm -qf /usr/lib/systemd/systemd-sysv-install +// chkconfig-1.13-2.el8.x86_64 +// # ll /usr/lib/systemd/systemd-sysv-install +// lrwxrwxrwx. 2 root root 24 Nov 29 18:08 /usr/lib/systemd/systemd-sysv-install -> ../../..//sbin/chkconfig +// # +fn canonicalize_path(p: &str) -> String { + let p = Utf8Path::new(p); + let mut r = Utf8PathBuf::new(); + for part in p.components() { + r.push(part); + } + r.into_string() +} + /// Adjust mode for specific file entries. pub fn tweak_imported_file_info( mut file_info: Pin<&mut crate::FFIGFileInfo>, @@ -44,6 +62,18 @@ pub fn tweak_imported_file_info( file_info.set_attribute_uint32("unix::mode", mode); } } + + if filetype == FileType::SymbolicLink { + if let Some(target) = file_info.symlink_target() { + // See above, this is a special case hack until + // https://github.com/fedora-sysv/chkconfig/pull/67 propagates everywhere + // and/or https://github.com/ostreedev/ostree-rs-ext/pull/182 merges. + if target.ends_with("//sbin/chkconfig") { + let canonicalized = &canonicalize_path(&target); + file_info.set_symlink_target(canonicalized); + } + } + } } /// Apply filtering and manipulation logic to an RPM file before importing. @@ -186,6 +216,21 @@ fn fix_tmpfiles_path(abs_path: Cow) -> Cow { mod tests { use super::*; + #[test] + fn test_canonicalize_path() { + let canonical = &["/", "/usr", "../usr/share", "../../usr/lib/systemd/system"]; + for &k in canonical { + assert_eq!(k, canonicalize_path(k)); + } + let noncanonical = &[ + ("./././foo", "./foo"), + ("../../..//sbin/chkconfig", "../../../sbin/chkconfig"), + ]; + for k in noncanonical { + assert_eq!(canonicalize_path(k.0), k.1); + } + } + #[test] fn test_path_is_compliant() { let ostree_cases = &["/", "/usr", "/usr/share", "/bin/foo", "/usr/lib/opt/bar"];