grubconfig: find correct efi vendordir

[HuijingHei]

Refer to Timothée's comment: There are cases where could have multiple shim binaries in the EFI partition and we only want to touch our own, not others.

The logic should be:

• Look at what we have in: /usr/lib/bootupd/updates/EFI/foo/bar
• Find the corresponding file in the EFI partition: /boot/efi/ EFI/foo/bar
• Update the file if needed

See https://github.com/coreos/bootupd/issues/630#issuecomment-2063529216

[travier]

Please don't mention people by their GitHub handle in commit messages, otherwise they'll get a ping everyone pushed a commit with it.

[cgwalters]

There are cases where could have multiple shim binaries in the EFI partition

Let's be a bit more explicit here. Are we trying to support cases where e.g. two different fedora shim versions are installed? That can't work AFAIK since the path isn't versioned.

So let's be clear, this is about different vendors. For example with Ubuntu 22.04 it's $esp/EFI/ubuntu/shimx64.efi vs Fedora $esp/EFI/fedora/shimx64.efi etc.

And what I'd then say is two things:

• We should explicitly error out if we detect existing content matching our same EFI vendor
• Let's change this code to operate on EFI vendors, not full paths; i.e. we need to strengthen our checking not to "find shim" but "find $vendor/shim".

[HuijingHei]

There are cases where could have multiple shim binaries in the EFI partition

Let's be a bit more explicit here. Are we trying to support cases where e.g. two different fedora shim versions are installed? That can't work AFAIK since the path isn't versioned.

No, should error out if existing multiple shim or none.

So let's be clear, this is about different vendors. For example with Ubuntu 22.04 it's $esp/EFI/ubuntu/shimx64.efi vs Fedora $esp/EFI/fedora/shimx64.efi etc.

And what I'd then say is two things:

• We should explicitly error out if we detect existing content matching our same EFI vendor

Does this mean should error if $esp/EFI/ubuntu/shimx64.efi match the content in /usr/lib/bootupd/updates/EFI/fedora/shimx64.efi ?

• Let's change this code to operate on EFI vendors, not full paths; i.e. we need to strengthen our checking not to "find shim" but "find $vendor/shim".

To clarify, check if the path ends with $vendor/shim, and match the content, is this enough?

[cgwalters]

To clarify, check if the path ends with $vendor/shim, and match the content, is this enough?

Right, I think that's enough.

[cgwalters]

OK so I took another look at this (and thank you so much for working on this problem!).

Here's what I think would be a lot simpler...this patch doesn't compile but I think shows the shape of a starting point:

diff --git a/src/bootupd.rs b/src/bootupd.rs
index 0835fa2..50589e1 100644
--- a/src/bootupd.rs
+++ b/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() {
+            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, uuid)?;
             // On other architectures, assume that there's nothing to do.
         }
         None => {}

Basically, we know which efi vendor we used when we installed shim. What we want to do is pass that data down into the grub install process instead of trying to rediscover it.

We'd need to add this get_efi_vendor method to both the bios and efi backends; in bios it'd just return None.

WDYT?

[HuijingHei]

Close this as impl in https://github.com/coreos/bootupd/pull/650