azurelinux icon indicating copy to clipboard operation
azurelinux copied to clipboard

Update shim to 15.8

Open ddstreetmicrosoft opened this issue 1 year ago • 15 comments

Rework our 3.0 shim, as well as updating it to 15.8.

Upstream review opened: https://github.com/rhboot/shim-review/issues/427

ddstreetmicrosoft avatar Mar 14 '24 21:03 ddstreetmicrosoft

@christopherco @adithyaj

ddstreetmicrosoft avatar Mar 14 '24 21:03 ddstreetmicrosoft

note that I got rid of the shim-unsigned package as it didn't make sense to me...but if we should keep it i'm open to discussion

ddstreetmicrosoft avatar Mar 14 '24 21:03 ddstreetmicrosoft

note that I got rid of the shim-unsigned package as it didn't make sense to me...but if we should keep it i'm open to discussion

Supportive of removing shim-unsigned.

Also, I noticed you are renaming shim-unsigned-aarch64 to shim-unsigned-aa64. Any concerns on keeping shim-unsigned-aarch64? The main benefit is that it aligns with familiar Fedora package naming.

christopherco avatar Mar 21 '24 07:03 christopherco

note that I got rid of the shim-unsigned package as it didn't make sense to me...but if we should keep it i'm open to discussion

Supportive of removing shim-unsigned.

Also, I noticed you are renaming shim-unsigned-aarch64 to shim-unsigned-aa64. Any concerns on keeping shim-unsigned-aarch64? The main benefit is that it aligns with familiar Fedora package naming.

yeah migth as well just keep the naming the same, i adjusted to remove the rename in latest push

ddstreetmicrosoft avatar Apr 11 '24 12:04 ddstreetmicrosoft

@christopherco @gjswalling one q for this pr; I currently have the new shim building with EFIDIR=azurelinux, while our Mariner 2 shim doesn't have any EFIDIR= set at all.

What that means is that for Mariner 2, our shim will look in the same directory (i.e. /boot/efi/EFI/BOOT) for its default boot loader (i.e. grubx64.efi), while in Azure Linux 3, our shim will look in the EFIDIR (i.e. /boot/efi/EFI/azurelinux). That then will require our grub2 package to place its EFI into /boot/efi/EFI/azurelinux instead of /boot/efi/EFI/BOOT. I don't know if there is any "right" approach (e.g. Fedora appears to use EFIDIR= while OpenSUSE appears to not).

I don't have a strong opinion either direction, though I'm inclined to shift over to using EFIDIR= like Fedora does. I'll include an update to the grub2 package in this PR to adjust the grubx64.efi location, but I can remove it if you think it would be better to not use EFIDIR=.

ddstreetmicrosoft avatar Apr 16 '24 18:04 ddstreetmicrosoft

I don't have a strong opinion either direction, though I'm inclined to shift over to using EFIDIR= like Fedora does. I'll include an update to the grub2 package in this PR to adjust the grubx64.efi location, but I can remove it if you think it would be better to not use EFIDIR=.

Actually on second thought, I'm just gonna leave the grub2 package unchanged (i.e. install to /boot/efi/EFI/BOOT/grubx64.efi) and remove the use of EFIDIR= from the azl3 shim package. That will keep everything inside the /boot/efi/EFI/BOOT/ dir, as was the case with Mariner 2.

I'm open to changing it to use a EFIDIR= (and I think that's probably "better") but only if either of you think we should.

ddstreetmicrosoft avatar Apr 16 '24 19:04 ddstreetmicrosoft

I don't have a strong opinion either direction, though I'm inclined to shift over to using EFIDIR= like Fedora does. I'll include an update to the grub2 package in this PR to adjust the grubx64.efi location, but I can remove it if you think it would be better to not use EFIDIR=.

Actually on second thought, I'm just gonna leave the grub2 package unchanged (i.e. install to /boot/efi/EFI/BOOT/grubx64.efi) and remove the use of EFIDIR= from the azl3 shim package. That will keep everything inside the /boot/efi/EFI/BOOT/ dir, as was the case with Mariner 2.

I'm open to changing it to use a EFIDIR= (and I think that's probably "better") but only if either of you think we should.

hmm actually the later shim version might require EFIDIR=, as it doesn't appear to be booting at all without it. I'll look into this more.

ddstreetmicrosoft avatar Apr 16 '24 21:04 ddstreetmicrosoft

@christopherco @gjswalling @adithyaj i think this is ready for another round of reviews...still needed is:

  1. wait for PMC for 3.0 to get the pesign package published
  2. update the shim-review submission to correct the Dockerfile and add docker logs (using the PMC pesign instead of building from the Dockerfile)
  3. work with @adithyaj to update the signing pipeline(s) to sign the mm/fb binaries from the shim-unsigned packages and provide them to the shim package
  4. do secure boot test (following TESTING doc in this pr)

and then of course submit the shim-review upstream and wait for that process to complete (and get the shims signed), and add the signed shim efi binaries into the shim package source

ddstreetmicrosoft avatar Apr 18 '24 17:04 ddstreetmicrosoft

Also note that I added a 'spec entanglement' thingie so that the shim-unsigned-x64, shim-unsigned-aarch64, and shim packages all must be at exactly the same version-release. That might mean that we can't make any changes to the shim-unsigned-* packages without going through a new upstream shim-review, but that is probably what we actually want. Only reason I mention it is if e.g. we want to patch the mmx64.efi to fix something in mokmanager...that might not actually need to get the shim reviewed, if we're only changing mokmanager. it's possible the shim binary would remain binary-identical in which case there wouldn't be a problem (we could just re-use the existing signed shim binary). In any case, I think it's a low probability of it being a problem and even if it is we can change the build process later if needed, so TL;DR is I think having the specs 'entangled' is ok.

ddstreetmicrosoft avatar Apr 18 '24 18:04 ddstreetmicrosoft

I added a 'spec entanglement' thingie

also just to clarify, I added the 'spec entanglement' PR check thingie (which only is effective inside a PR), but also I added a real 'entanglement' by setting the shim package to BuildRequires: shim-unsigned-%{efiarch} = %{version}-%{release}

ddstreetmicrosoft avatar Apr 18 '24 18:04 ddstreetmicrosoft

Just an fyi: our tools are happy to have multiple .spec files share a folder (ie llvm, clang, nodejs etc.).

Your call on if this simplifies your life or makes it more complicated, just wanted to offer the option.

dmcilvaney avatar Apr 18 '24 19:04 dmcilvaney

Just an fyi: our tools are happy to have multiple .spec files share a folder (ie llvm, clang, nodejs etc.).

Your call on if this simplifies your life or makes it more complicated, just wanted to offer the option.

thanks, I'll keep that in mind, though I think in this case it probably isn't needed.

ddstreetmicrosoft avatar Apr 19 '24 14:04 ddstreetmicrosoft

@adithyaj @christopherco to handle the 'must sign after building all packages' design requirement i just created a symlink so the source spec files will be identical (i.e. SPECS-SIGNED/shim-signed/shim.spec -> ../../SPECS/shim/shim.spec). I think this is the best signing design that still fits into the limitations of the 'build from scratch' design; do you think this will work with our signing pipeline?

ddstreetmicrosoft avatar Apr 25 '24 10:04 ddstreetmicrosoft

@adithyaj @christopherco to handle the 'must sign after building all packages' design requirement i just created a symlink so the source spec files will be identical (i.e. SPECS-SIGNED/shim-signed/shim.spec -> ../../SPECS/shim/shim.spec). I think this is the best signing design that still fits into the limitations of the 'build from scratch' design; do you think this will work with our signing pipeline?

minor update - on thinking about it, more than just the spec file is needed so i changed the symlink to the entire dir, i.e. SPECS-SIGNED/shim -> ../SPECS/shim

ddstreetmicrosoft avatar Apr 25 '24 16:04 ddstreetmicrosoft

rebased

ddstreetmicrosoft avatar Jun 12 '24 17:06 ddstreetmicrosoft