Merge pull request containers#650 from HuijingHei/get-efi-vendor

Add `get_efi_vendor` method to both the bios and efi backends

.github/workflows/ci.yml

@@ -39,7 +39,23 @@ jobs:
           tags: localhost/bootupd:latest
       - name: Copy to podman
         run: sudo skopeo copy docker-daemon:localhost/bootupd:latest containers-storage:localhost/bootupd:latest
-      - name: bootc install
+      - name: bootc install to disk
+        run: |
+          set -xeuo pipefail
+          sudo truncate -s 10G myimage.raw
+          sudo podman run --rm -ti --privileged -v .:/target --pid=host --security-opt label=disable \
+            -v /var/lib/containers:/var/lib/containers \
+            -v /dev:/dev \
+            localhost/bootupd:latest bootc install to-disk --skip-fetch-check \
+            --disable-selinux --generic-image --via-loopback /target/myimage.raw
+          # Verify we installed grub.cfg and shim on the disk
+          sudo losetup -P -f myimage.raw
+          device=$(losetup --list --noheadings --output NAME,BACK-FILE | grep myimage.raw | awk '{print $1}')
+          sudo mount "${device}p2" /mnt/
+          sudo ls /mnt/EFI/centos/{grub.cfg,shimx64.efi}
+          sudo losetup -D "${device}"
+          sudo rm -f myimage.raw
+      - name: bootc install to filesystem
         run: |
           set -xeuo pipefail
           sudo podman run --rm -ti --privileged -v /:/target --pid=host --security-opt label=disable \

Cargo.toml

@@ -38,6 +38,7 @@ serde = { version = "^1.0", features = ["derive"] }
 serde_json = "^1.0"
 tempfile = "^3.10"
 widestring = "1.1.0"
+walkdir = "2.3.2"
 
 [profile.release]
 # We assume we're being delivered via e.g. RPM which supports split debuginfo

src/bios.rs

@@ -155,4 +155,8 @@ impl Component for Bios {
     fn validate(&self, _: &InstalledContent) -> Result<ValidationResult> {
         Ok(ValidationResult::Skip)
     }
+
+    fn get_efi_vendor(&self, _: &openat::Dir) -> Result<Option<String>> {
+        Ok(None)
+    }
 }

src/bootupd.rs

@@ -83,7 +83,7 @@ pub(crate) fn install(
     }
 
     let mut state = SavedState::default();
-    let mut installed_efi = false;
+    let mut installed_efi_vendor = None;
     for &component in target_components.iter() {
         // skip for BIOS if device is empty
         if component.name() == "BIOS" && device.is_empty() {
@@ -100,8 +100,9 @@ pub(crate) fn install(
         log::info!("Installed {} {}", component.name(), meta.meta.version);
         state.installed.insert(component.name().into(), meta);
         // Yes this is a hack...the Component thing just turns out to be too generic.
-        if component.name() == "EFI" {
-            installed_efi = true;
+        if let Some(vendor) = component.get_efi_vendor(&source_root)? {
+            assert!(installed_efi_vendor.is_none());
+            installed_efi_vendor = Some(vendor);
         }
     }
     let sysroot = &openat::Dir::open(dest_root)?;
@@ -115,7 +116,7 @@ pub(crate) fn install(
                 target_arch = "aarch64",
                 target_arch = "powerpc64"
             ))]
-            crate::grubconfigs::install(sysroot, installed_efi, uuid)?;
+            crate::grubconfigs::install(sysroot, installed_efi_vendor.as_deref(), uuid)?;
             // On other architectures, assume that there's nothing to do.
         }
         None => {}

src/component.rs

@@ -72,6 +72,9 @@ pub(crate) trait Component {
 
     /// Used on the client to validate an installed version.
     fn validate(&self, current: &InstalledContent) -> Result<ValidationResult>;
+
+    /// Locating efi vendor dir
+    fn get_efi_vendor(&self, sysroot: &openat::Dir) -> Result<Option<String>>;
 }
 
 /// Given a component name, create an implementation.
@@ -140,3 +143,42 @@ pub(crate) fn get_component_update(
         Ok(None)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_get_efi_vendor() -> Result<()> {
+        let td = tempfile::tempdir()?;
+        let tdp = td.path();
+        let tdp_updates = tdp.join("usr/lib/bootupd/updates");
+        let td = openat::Dir::open(tdp)?;
+        std::fs::create_dir_all(tdp_updates.join("EFI/BOOT"))?;
+        std::fs::create_dir_all(tdp_updates.join("EFI/fedora"))?;
+        std::fs::create_dir_all(tdp_updates.join("EFI/centos"))?;
+        std::fs::write(
+            tdp_updates.join("EFI/fedora").join(crate::efi::SHIM),
+            "shim data",
+        )?;
+        std::fs::write(
+            tdp_updates.join("EFI/centos").join(crate::efi::SHIM),
+            "shim data",
+        )?;
+
+        let all_components = crate::bootupd::get_components();
+        let target_components: Vec<_> = all_components.values().collect();
+        for &component in target_components.iter() {
+            if component.name() == "BIOS" {
+                assert_eq!(component.get_efi_vendor(&td)?, None);
+            }
+            if component.name() == "EFI" {
+                let x = component.get_efi_vendor(&td);
+                assert_eq!(x.is_err(), true);
+                std::fs::remove_dir_all(tdp_updates.join("EFI/centos"))?;
+                assert_eq!(component.get_efi_vendor(&td)?, Some("fedora".to_string()));
+            }
+        }
+        Ok(())
+    }
+}

src/efi.rs

@@ -12,6 +12,7 @@ use std::process::Command;
 use anyhow::{bail, Context, Result};
 use fn_error_context::context;
 use openat_ext::OpenatDirExt;
+use walkdir::WalkDir;
 use widestring::U16CString;
 
 use crate::filetree;
@@ -131,16 +132,11 @@ impl Efi {
     }
 
     #[context("Updating EFI firmware variables")]
-    fn update_firmware(&self, device: &str, espdir: &openat::Dir) -> Result<()> {
+    fn update_firmware(&self, device: &str, espdir: &openat::Dir, vendordir: &str) -> Result<()> {
         if !is_efi_booted()? {
             log::debug!("Not booted via EFI, skipping firmware update");
             return Ok(());
         }
-        let efidir = &espdir.sub_dir("EFI").context("Opening EFI")?;
-        let vendordir = super::grubconfigs::find_efi_vendordir(efidir)?;
-        let vendordir = vendordir
-            .to_str()
-            .ok_or_else(|| anyhow::anyhow!("Invalid non-UTF-8 vendordir"))?;
         clear_efi_current()?;
         set_efi_current(device, espdir, vendordir)
     }
@@ -319,7 +315,9 @@ impl Component for Efi {
             anyhow::bail!("Failed to copy");
         }
         if update_firmware {
-            self.update_firmware(device, destd)?
+            if let Some(vendordir) = self.get_efi_vendor(&src_root)? {
+                self.update_firmware(device, destd, &vendordir)?
+            }
         }
         Ok(InstalledContent {
             meta,
@@ -417,6 +415,28 @@ impl Component for Efi {
             Ok(ValidationResult::Valid)
         }
     }
+
+    fn get_efi_vendor(&self, sysroot: &openat::Dir) -> Result<Option<String>> {
+        let updated = sysroot
+            .sub_dir(&component_updatedirname(self))
+            .context("opening update dir")?;
+        let shim_files = find_file_recursive(updated.recover_path()?, SHIM)?;
+
+        // Does not support multiple shim for efi
+        if shim_files.len() > 1 {
+            anyhow::bail!("Found multiple {SHIM} in the image");
+        }
+        if let Some(p) = shim_files.first() {
+            let p = p
+                .parent()
+                .unwrap()
+                .file_name()
+                .ok_or_else(|| anyhow::anyhow!("No file name found"))?;
+            Ok(Some(p.to_string_lossy().into_owned()))
+        } else {
+            anyhow::bail!("Failed to find {SHIM} in the image")
+        }
+    }
 }
 
 impl Drop for Efi {
@@ -507,3 +527,22 @@ pub(crate) fn set_efi_current(device: &str, espdir: &openat::Dir, vendordir: &st
     }
     anyhow::Ok(())
 }
+
+#[context("Find target file recursively")]
+fn find_file_recursive<P: AsRef<Path>>(dir: P, target_file: &str) -> Result<Vec<PathBuf>> {
+    let mut result = Vec::new();
+
+    for entry in WalkDir::new(dir).into_iter().filter_map(|e| e.ok()) {
+        if entry.file_type().is_file() {
+            if let Some(file_name) = entry.file_name().to_str() {
+                if file_name == target_file {
+                    if let Some(path) = entry.path().to_str() {
+                        result.push(path.into());
+                    }
+                }
+            }
+        }
+    }
+
+    Ok(result)
+}

src/grubconfigs.rs

@@ -5,37 +5,18 @@ use anyhow::{anyhow, Context, Result};
 use fn_error_context::context;
 use openat_ext::OpenatDirExt;
 
-use crate::efi::SHIM;
-
 /// The subdirectory of /boot we use
 const GRUB2DIR: &str = "grub2";
 const CONFIGDIR: &str = "/usr/lib/bootupd/grub2-static";
 const DROPINDIR: &str = "configs.d";
 
-#[context("Locating EFI vendordir")]
-pub(crate) fn find_efi_vendordir(efidir: &openat::Dir) -> Result<PathBuf> {
-    for d in efidir.list_dir(".")? {
-        let d = d?;
-        let meta = efidir.metadata(d.file_name())?;
-        if !meta.is_dir() {
-            continue;
-        }
-        // skip if not find shim under dir
-        let dir = efidir.sub_dir(d.file_name())?;
-        for entry in dir.list_dir(".")? {
-            let entry = entry?;
-            if entry.file_name() != SHIM {
-                continue;
-            }
-            return Ok(d.file_name().into());
-        }
-    }
-    anyhow::bail!("Failed to find EFI vendor dir that contains {SHIM}")
-}
-
 /// Install the static GRUB config files.
 #[context("Installing static GRUB configs")]
-pub(crate) fn install(target_root: &openat::Dir, efi: bool, write_uuid: bool) -> Result<()> {
+pub(crate) fn install(
+    target_root: &openat::Dir,
+    installed_efi_vendor: Option<&str>,
+    write_uuid: bool,
+) -> Result<()> {
     let bootdir = &target_root.sub_dir("boot").context("Opening /boot")?;
     let boot_is_mount = {
         let root_dev = target_root.self_metadata()?.stat().st_dev;
@@ -99,29 +80,26 @@ pub(crate) fn install(target_root: &openat::Dir, efi: bool, write_uuid: bool) ->
         None
     };
 
-    let efidir = efi
-        .then(|| {
-            target_root
-                .sub_dir_optional("boot/efi/EFI")
-                .context("Opening /boot/efi/EFI")
-        })
-        .transpose()?
-        .flatten();
-    if let Some(efidir) = efidir.as_ref() {
-        let vendordir = find_efi_vendordir(efidir)?;
+    if let Some(vendordir) = installed_efi_vendor {
         log::debug!("vendordir={:?}", &vendordir);
-        let target = &vendordir.join("grub.cfg");
-        efidir
-            .copy_file(&Path::new(CONFIGDIR).join("grub-static-efi.cfg"), target)
-            .context("Copying static EFI")?;
-        println!("Installed: {target:?}");
-        if let Some(uuid_path) = uuid_path {
-            // SAFETY: we always have a filename
-            let filename = Path::new(&uuid_path).file_name().unwrap();
-            let target = &vendordir.join(filename);
-            bootdir
-                .copy_file_at(uuid_path, efidir, target)
-                .context("Writing bootuuid.cfg to efi dir")?;
+        let vendor = PathBuf::from(vendordir);
+        let target = &vendor.join("grub.cfg");
+        let dest_efidir = target_root
+            .sub_dir_optional("boot/efi/EFI")
+            .context("Opening /boot/efi/EFI")?;
+        if let Some(efidir) = dest_efidir {
+            efidir
+                .copy_file(&Path::new(CONFIGDIR).join("grub-static-efi.cfg"), target)
+                .context("Copying static EFI")?;
+            println!("Installed: {target:?}");
+            if let Some(uuid_path) = uuid_path {
+                // SAFETY: we always have a filename
+                let filename = Path::new(&uuid_path).file_name().unwrap();
+                let target = &vendor.join(filename);
+                bootdir
+                    .copy_file_at(uuid_path, &efidir, target)
+                    .context("Writing bootuuid.cfg to efi dir")?;
+            }
         }
     }
 
@@ -142,36 +120,10 @@ mod tests {
         std::fs::create_dir_all(tdp.join("boot/grub2"))?;
         std::fs::create_dir_all(tdp.join("boot/efi/EFI/BOOT"))?;
         std::fs::create_dir_all(tdp.join("boot/efi/EFI/fedora"))?;
-        install(&td, true, false).unwrap();
+        install(&td, Some("fedora"), false).unwrap();
 
         assert!(td.exists("boot/grub2/grub.cfg")?);
         assert!(td.exists("boot/efi/EFI/fedora/grub.cfg")?);
         Ok(())
     }
-
-    #[test]
-    fn test_find_efi_vendordir() -> Result<()> {
-        let td = tempfile::tempdir()?;
-        let tdp = td.path();
-        let efidir = tdp.join("EFI");
-        std::fs::create_dir_all(efidir.join("BOOT"))?;
-        std::fs::create_dir_all(efidir.join("dell"))?;
-        std::fs::create_dir_all(efidir.join("fedora"))?;
-        let td = openat::Dir::open(&efidir)?;
-
-        std::fs::write(efidir.join("dell").join("foo"), "foo data")?;
-        std::fs::write(efidir.join("fedora").join("grub.cfg"), "grub config")?;
-        std::fs::write(efidir.join("fedora").join(SHIM), "shim data")?;
-
-        assert!(td.exists("BOOT")?);
-        assert!(td.exists("dell/foo")?);
-        assert!(td.exists("fedora/grub.cfg")?);
-        assert!(td.exists(format!("fedora/{SHIM}"))?);
-        assert_eq!(find_efi_vendordir(&td)?.to_str(), Some("fedora"));
-
-        std::fs::remove_file(efidir.join("fedora").join(SHIM))?;
-        let x = find_efi_vendordir(&td);
-        assert_eq!(x.is_err(), true);
-        Ok(())
-    }
 }