diff --git a/src/efi.rs b/src/efi.rs index 2aec115e..f600fb45 100644 --- a/src/efi.rs +++ b/src/efi.rs @@ -55,6 +55,9 @@ const EFIVARFS: &str = "/sys/firmware/efi/efivars"; const LOADER_INFO_VAR_STR: &str = "LoaderInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; const STUB_INFO_VAR_STR: &str = "StubInfo-4a67b082-0a4c-41cf-b6c7-440b29bb8c4f"; +/// The options of cp command for installation +const OPTIONS: &[&str] = &["-rp", "--reflink=auto"]; + /// Return `true` if the system is booted via EFI pub(crate) fn is_efi_booted() -> Result { Path::new("/sys/firmware/efi") @@ -319,10 +322,19 @@ impl Component for Efi { return Ok(None); }; + let updated_path = { + let efilib_path = rootcxt.path.join(EFILIB); + if efilib_path.exists() && get_efi_component_from_usr(&rootcxt.path, EFILIB)?.is_some() + { + PathBuf::from(EFILIB) + } else { + component_updatedirname(self) + } + }; let updated = rootcxt .sysroot - .sub_dir(&component_updatedirname(self)) - .context("opening update dir")?; + .sub_dir(&updated_path) + .with_context(|| format!("opening update dir {}", updated_path.display()))?; let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?; let esp_devices = esp_devices.unwrap_or_default(); @@ -376,8 +388,6 @@ impl Component for Efi { anyhow::bail!("No update metadata for component {} found", self.name()); }; log::debug!("Found metadata {}", meta.version); - let srcdir_name = component_updatedirname(self); - let ft = crate::filetree::FileTree::new_from_dir(&src_dir.sub_dir(&srcdir_name)?)?; // Let's attempt to use an already mounted ESP at the target // dest_root if one is already mounted there in a known ESP location. @@ -398,16 +408,37 @@ impl Component for Efi { .with_context(|| format!("opening dest dir {}", destpath.display()))?; validate_esp_fstype(destd)?; - // TODO - add some sort of API that allows directly setting the working - // directory to a file descriptor. - std::process::Command::new("cp") - .args(["-rp", "--reflink=auto"]) - .arg(&srcdir_name) - .arg(destpath) - .current_dir(format!("/proc/self/fd/{}", src_dir.as_raw_fd())) - .run()?; + let src_path = Utf8Path::new(src_root); + let efi_comps = if src_path.join(EFILIB).exists() { + get_efi_component_from_usr(&src_path, EFILIB)? + } else { + None + }; + let dest = destpath.to_str().with_context(|| { + format!( + "Include invalid UTF-8 characters in dest {}", + destpath.display() + ) + })?; + + let efi_path = if let Some(efi_components) = efi_comps { + for efi in efi_components { + filetree::copy_dir_with_args(&src_dir, efi.path.as_str(), dest, OPTIONS)?; + } + EFILIB + } else { + let updates = component_updatedirname(self); + let src = updates + .to_str() + .context("Include invalid UTF-8 characters in path")?; + filetree::copy_dir_with_args(&src_dir, src, dest, OPTIONS)?; + &src.to_owned() + }; + + // Get filetree from efi path + let ft = crate::filetree::FileTree::new_from_dir(&src_dir.sub_dir(efi_path)?)?; if update_firmware { - if let Some(vendordir) = self.get_efi_vendor(&Path::new(src_root))? { + if let Some(vendordir) = self.get_efi_vendor(src_path.join(efi_path).as_std_path())? { self.update_firmware(device, destd, &vendordir)? } } @@ -429,9 +460,20 @@ impl Component for Efi { .ok_or_else(|| anyhow::anyhow!("No filetree for installed EFI found!"))?; let sysroot_dir = &rootcxt.sysroot; let updatemeta = self.query_update(sysroot_dir)?.expect("update available"); - let updated = sysroot_dir - .sub_dir(&component_updatedirname(self)) - .context("opening update dir")?; + let updated_path = { + let efilib_path = rootcxt.path.join(EFILIB); + if efilib_path.exists() && get_efi_component_from_usr(&rootcxt.path, EFILIB)?.is_some() + { + PathBuf::from(EFILIB) + } else { + component_updatedirname(self) + } + }; + + let updated = rootcxt + .sysroot + .sub_dir(&updated_path) + .with_context(|| format!("opening update dir {}", updated_path.display()))?; let updatef = filetree::FileTree::new_from_dir(&updated).context("reading update dir")?; let diff = currentf.diff(&updatef)?; @@ -480,7 +522,6 @@ impl Component for Efi { ostreeboot.remove_all_optional("efi/EFI")?; } - // copy EFI files from usr/lib/efi to updates dir if let Some(efi_components) = get_efi_component_from_usr(Utf8Path::from_path(sysroot_path).unwrap(), EFILIB)? { @@ -491,12 +532,6 @@ impl Component for Efi { let mut packages = Vec::new(); let mut modules_vec: Vec = vec![]; for efi in efi_components { - Command::new("cp") - .args(["-rp", "--reflink=auto"]) - .arg(&efi.path) - .arg(crate::model::BOOTUPD_UPDATES_DIR) - .current_dir(format!("/proc/self/fd/{}", sysroot_dir.as_raw_fd())) - .run()?; packages.push(format!("{}-{}", efi.name, efi.version)); modules_vec.push(Module { name: efi.name, diff --git a/src/filetree.rs b/src/filetree.rs index f84c18a6..ddd55a5d 100644 --- a/src/filetree.rs +++ b/src/filetree.rs @@ -68,9 +68,11 @@ const DEFAULT_FILE_MODE: u32 = 0o700; use crate::sha512string::SHA512String; /// Metadata for a single file -#[derive(Clone, Serialize, Deserialize, Debug, Hash, PartialEq)] +#[derive(Clone, Serialize, Deserialize, Debug)] #[serde(rename_all = "kebab-case")] pub(crate) struct FileMetadata { + /// File source path + pub(crate) source: Option, /// File size in bytes pub(crate) size: u64, /// Content checksum; chose SHA-512 because there are not a lot of files here @@ -78,6 +80,13 @@ pub(crate) struct FileMetadata { pub(crate) sha512: SHA512String, } +impl PartialEq for FileMetadata { + fn eq(&self, other: &Self) -> bool { + // Skip the source + self.size == other.size && self.sha512 == other.sha512 + } +} + #[derive(Clone, Serialize, Deserialize, Debug, PartialEq)] #[serde(rename_all = "kebab-case")] pub(crate) struct FileTree { @@ -127,6 +136,7 @@ impl FileMetadata { let _ = std::io::copy(&mut r, &mut hasher)?; let digest = SHA512String::from_hasher(&mut hasher); Ok(FileMetadata { + source: None, size: meta.len(), sha512: digest, }) @@ -183,8 +193,10 @@ impl FileTree { ))] pub(crate) fn new_from_dir(dir: &openat::Dir) -> Result { let mut children = BTreeMap::new(); - for (k, v) in Self::unsorted_from_dir(dir)?.drain() { - children.insert(k, v); + for (k, mut v) in Self::unsorted_from_dir(dir)?.drain() { + let k_path = get_dest_efi_path(Utf8Path::new(&k)).to_string(); + v.source = Some(k); + children.insert(k_path, v); } Ok(Self { children }) @@ -228,18 +240,20 @@ impl FileTree { for (k, v1) in self.children.iter() { if let Some(v2) = updated.children.get(k) { if v1 != v2 { - changes.insert(k.clone()); + // Save the source path for changes + changes.insert(v2.source.as_ref().unwrap_or(k).clone()); } } else { removals.insert(k.clone()); } } if check_additions { - for k in updated.children.keys() { + for (k, v) in updated.children.iter() { if self.children.contains_key(k) { continue; } - additions.insert(k.clone()); + // Save the source path for additions + additions.insert(v.source.as_ref().unwrap_or(k).clone()); } } Ok(FileTreeDiff { @@ -268,12 +282,13 @@ impl FileTree { openat::SimpleType::File => { let target_info = FileMetadata::new_from_path(dir, path)?; if info != &target_info { - changes.insert(path.clone()); + // Save the source path for changes + changes.insert(info.source.as_ref().unwrap_or(path).clone()); } } _ => { // If a file became a directory - changes.insert(path.clone()); + changes.insert(info.source.as_ref().unwrap_or(path).clone()); } } } else { @@ -334,13 +349,32 @@ pub(crate) struct ApplyUpdateOptions { pub(crate) skip_sync: bool, } -/// Copy from src to dst at root dir +/// Copy from src to dst at root dir with default option '-a' +#[cfg(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "riscv64" +))] +pub(crate) fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> { + copy_dir_with_args(root, src, dst, ["-a"]) +} + +/// Copy from src to dst at root dir with args #[cfg(any( target_arch = "x86_64", target_arch = "aarch64", target_arch = "riscv64" ))] -fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> { +pub(crate) fn copy_dir_with_args( + root: &openat::Dir, + src: &str, + dst: &str, + args: I, +) -> Result<()> +where + I: IntoIterator, + S: AsRef, +{ use bootc_internal_utils::CommandRunExt; use std::os::unix::process::CommandExt; use std::process::Command; @@ -348,7 +382,7 @@ fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> { let rootfd = unsafe { BorrowedFd::borrow_raw(root.as_raw_fd()) }; unsafe { Command::new("cp") - .args(["-a"]) + .args(args) .arg(src) .arg(dst) .pre_exec(move || rustix::process::fchdir(rootfd).map_err(Into::into)) @@ -366,7 +400,7 @@ fn copy_dir(root: &openat::Dir, src: &str, dst: &str) -> Result<()> { target_arch = "aarch64", target_arch = "riscv64" ))] -fn get_first_dir(path: &Utf8Path) -> Result<(&Utf8Path, String)> { +fn get_first_dir(path: &Utf8Path) -> Result<(Utf8PathBuf, String)> { let first = path .iter() .next() @@ -376,6 +410,20 @@ fn get_first_dir(path: &Utf8Path) -> Result<(&Utf8Path, String)> { Ok((first.into(), tmp)) } +/// Get dest efi path "shim//EFI/fedora/shim.efi" -> "fedora/shim.efi" +#[cfg(any( + target_arch = "x86_64", + target_arch = "aarch64", + target_arch = "riscv64" +))] +fn get_dest_efi_path(path: &Utf8Path) -> Utf8PathBuf { + let parts: Vec<_> = path.iter().collect(); + if parts.get(2).map(|c| *c == "EFI").unwrap_or(false) { + return parts.iter().skip(3).collect(); + } + path.to_path_buf() +} + /// Given two directories, apply a diff generated from srcdir to destdir #[cfg(any( target_arch = "x86_64", @@ -421,8 +469,9 @@ pub(crate) fn apply_diff( } // Write changed or new files to temp dir or temp file for pathstr in diff.changes.iter().chain(diff.additions.iter()) { - let path = Utf8Path::new(pathstr); - let (first_dir, first_dir_tmp) = get_first_dir(path)?; + let src_path = Utf8Path::new(pathstr); + let path = get_dest_efi_path(src_path); + let (first_dir, first_dir_tmp) = get_first_dir(&path)?; let mut path_tmp = Utf8PathBuf::from(&first_dir_tmp); if first_dir != path { if !destdir.exists(&first_dir_tmp)? && destdir.exists(first_dir.as_std_path())? { @@ -443,8 +492,8 @@ pub(crate) fn apply_diff( } updates.insert(first_dir, first_dir_tmp); srcdir - .copy_file_at(path.as_std_path(), destdir, path_tmp.as_std_path()) - .with_context(|| format!("copying {:?} to {:?}", path, path_tmp))?; + .copy_file_at(src_path.as_std_path(), destdir, path_tmp.as_std_path()) + .with_context(|| format!("copying {:?} to {:?}", src_path, path_tmp))?; } // do local exchange or rename @@ -647,6 +696,37 @@ mod tests { "newgrub data" ); assert!(!a.join(relp).join("shim.x64").exists()); + // test apply from foo/1.0 and bar/2.0 + { + let c = tmpdp.join("c"); + let foo = "foo/1.0/EFI/fedora"; + let bar = "bar/2.0/EFI/new"; + for p in [foo, bar] { + fs::create_dir_all(c.join(p))?; + } + // change: "foo/1.0/EFI/fedora/grub.x64" + fs::write(c.join(foo).join("grub.x64"), "grub data 3")?; + // addition: "bar/2.0/EFI/new/newfile" + fs::write(c.join(bar).join("newfile"), "filedata")?; + let a = openat::Dir::open(&a.join("EFI"))?; + let c = openat::Dir::open(&c)?; + let ta = FileTree::new_from_dir(&a)?; + let tc = FileTree::new_from_dir(&c)?; + let diff = ta.diff(&tc)?; + assert_eq!(diff.changes.len(), 1); + assert_eq!(diff.additions.len(), 1); + assert_eq!(diff.count(), 3); + super::apply_diff(&c, &a, &diff, None)?; + } + assert_eq!( + String::from_utf8(std::fs::read(a.join(relp).join("grub.x64"))?)?, + "grub data 3" + ); + assert_eq!( + String::from_utf8(std::fs::read(a.join("EFI/new").join("newfile"))?)?, + "filedata" + ); + assert!(!a.join(newsubp).join("newgrub.x64").exists()); Ok(()) } #[test] @@ -664,6 +744,24 @@ mod tests { Ok(()) } #[test] + fn test_get_dest_efi_path() -> Result<()> { + let test_cases = [ + ("foo/1.0/EFI/vendor/test.efi", "vendor/test.efi"), + ("vendor/test.efi", "vendor/test.efi"), + ("EFI/vendor/test.efi", "EFI/vendor/test.efi"), + ( + "bar/foo/1.0/EFI/vendor/test.efi", + "bar/foo/1.0/EFI/vendor/test.efi", + ), + ]; + + for (input, expected) in test_cases { + let path = Utf8Path::new(input); + assert_eq!(get_dest_efi_path(path), Utf8Path::new(expected)); + } + Ok(()) + } + #[test] fn test_cleanup_tmp() -> Result<()> { let tmpd = tempfile::tempdir()?; let p = tmpd.path(); @@ -763,9 +861,13 @@ mod tests { b.remove_file(testfile)?; let ta = FileTree::new_from_dir(&a)?; let diff = ta.relative_diff_to(&b)?; + assert_eq!(diff.count(), 1); assert_eq!(diff.removals.len(), 1); apply_diff(&a, &b, &diff, None).context("test removed files with relative_diff")?; assert_eq!(b.exists(testfile)?, false); + // creation time is not changed for unchanged file + let b_btime_foo_new = fs::metadata(pb.join(foo))?.created()?; + assert_eq!(b_btime_foo_new, b_btime_foo); } { a.remove_file(bar)?; diff --git a/tests/kola/test-bootupd b/tests/kola/test-bootupd index 7a799ee9..d13a87c6 100755 --- a/tests/kola/test-bootupd +++ b/tests/kola/test-bootupd @@ -24,26 +24,25 @@ tmpefimount=$(mount_tmp_efi) bootmount=/boot tmpefidir=${tmpefimount}/EFI bootupdir=/usr/lib/bootupd/updates -efiupdir=${bootupdir}/EFI -ostbaseefi=/usr/lib/ostree-boot/efi/EFI efisubdir=fedora -efidir=${efiupdir}/${efisubdir} -ostefi=${ostbaseefi}/${efisubdir} -shim=shimx64.efi +shimx64=shimx64.efi +grubx64=grubx64.efi efilib=/usr/lib/efi - -test -f "${efidir}/${shim}" +tmpefisubdir=${tmpefidir}/${efisubdir} prepare_efi_update() { test -w /usr - # Do not need to create ostree-boot dir after transfered - if [ ! -d ${efilib} ]; then - mkdir -p ${ostbaseefi} - cp -a ${efiupdir}.orig/* ${ostbaseefi}/ - fi - rm -rf ${efiupdir} ${bootupdir}/EFI.json + rm -rf ${bootupdir}/EFI.json } +# Find the files under usr/lib/efi after transferred +shim_path=$(find ${efilib} -name shim.efi) +shimx64_path=$(find ${efilib} -name ${shimx64}) +grubx64_path=$(find ${efilib} -name ${grubx64}) + +test -n "${shimx64_path}" && test -n "${grubx64_path}" +! test -d "/usr/lib/bootupd/updates/EFI" + bootupctl status > out.txt evr=$(rpm -q grub2-common --qf '%{EVR}') assert_file_has_content_literal out.txt 'Component EFI' @@ -62,24 +61,11 @@ assert_file_has_content err.txt 'error: This command requires root privileges' # From here we'll fake updates test -w /usr || rpm-ostree usroverlay -# Save a backup copy of the update dir -cp -a ${efiupdir} ${efiupdir}.orig prepare_efi_update -shim= -grubx64= -# Change the files under usr/lib/efi after transferred -if [ -d ${efilib} ]; then - shim=$(find ${efilib} -name shim.efi) - grubx64=$(find ${efilib} -name grubx64.efi) -else - # FIXME need to synthesize an RPM for this - # echo somenewfile > ${ostefi}/somenew.efi - shim=${ostefi}/shim.efi - grubx64=${ostefi}/grubx64.efi -fi -rm -v ${shim} -echo bootupd-test-changes >> ${grubx64} + +rm -v ${shim_path} +echo bootupd-test-changes >> ${grubx64_path} /usr/libexec/bootupd generate-update-metadata / ver=$(jq -r .version < ${bootupdir}/EFI.json) vers=$(jq -r .versions < ${bootupdir}/EFI.json) @@ -116,24 +102,24 @@ ok validate after update # FIXME see above # assert_file_has_content ${tmpefidir}/${efisubdir}/somenew.efi 'somenewfile' -if test -f ${tmpefidir}/${efisubdir}/shim.efi; then +if test -f ${tmpefisubdir}/shim.efi; then fatal "failed to remove file" fi -if ! grep -q 'bootupd-test-changes' ${tmpefidir}/${efisubdir}/grubx64.efi; then +if ! grep -q 'bootupd-test-changes' ${tmpefisubdir}/${grubx64}; then fatal "failed to update modified file" fi -cmp ${tmpefidir}/${efisubdir}/shimx64.efi ${efiupdir}/${efisubdir}/shimx64.efi +cmp ${tmpefisubdir}/${shimx64} ${shimx64_path} ok filesystem changes bootupctl update | tee out.txt assert_file_has_content_literal out.txt 'No update available for any component' assert_not_file_has_content_literal out.txt 'Updated EFI' -echo "some additions" >> ${tmpefidir}/${efisubdir}/shimx64.efi +echo "some additions" >> ${tmpefisubdir}/${shimx64} if bootupctl validate 2>err.txt; then fatal "unexpectedly passed validation" fi -assert_file_has_content err.txt "Changed: ${efisubdir}/shimx64.efi" +assert_file_has_content err.txt "Changed: .*${efisubdir}/${shimx64}" test "$(grep -cEe '^Changed:' err.txt)" = "1" ok validate detected changes