Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shim 15.8 for openSUSE Tumbleweed #394

Closed
8 tasks done
jsegitz opened this issue Mar 1, 2024 · 18 comments
Closed
8 tasks done

Shim 15.8 for openSUSE Tumbleweed #394

jsegitz opened this issue Mar 1, 2024 · 18 comments
Assignees
Labels
accepted Submission is ready for sysdev

Comments

@jsegitz
Copy link

jsegitz commented Mar 1, 2024

Confirm the following are included in your repo, checking each box:

  • completed README.md file with the necessary information
  • shim.efi to be signed
  • public portion of your certificate(s) embedded in shim (the file passed to VENDOR_CERT_FILE)
  • binaries, for which hashes are added to vendor_db ( if you use vendor_db and have hashes allow-listed )
  • any extra patches to shim via your own git tree or as files
  • any extra patches to grub via your own git tree or as files
  • build logs
  • a Dockerfile to reproduce the build of the provided shim EFI binaries

What is the link to your tag in a repo cloned from rhboot/shim-review?


https://github.com/jsegitz/shim-review/tree/SUSE-openSUSE_tumbleweed-shim-15.8-20240301


What is the SHA256 hash of your final SHIM binary?


x86_64:
pesign: 211669e51a5e8c2315afe7a978740a972d721116ab81cbe384f993301ecde884 shim-opensuse_x86_64.efi
sha256sum: be35bac95713a29f10c93a3ab22d1b51f4a8dc67667746e30a668bbe741e2f3f shim-opensuse_x86_64.efi

aarch64:
pesign: 15854cd77be6b61bb6d22b4d448fe9b2d5d06dfa67d8161b6497e10af5b1bfb3 shim-opensuse_aarch64.efi
sha256sum: 3a262eed6a4cc311c1ef05a4216262cc09de052b57bd428d2bdcaa25c07d342a shim-opensuse_aarch64.efi


What is the link to your previous shim review request (if any, otherwise N/A)?


#333

@MuthuvelKuppusamy
Copy link

Able to reproduce the x86_64 build and details as below,
image

@aronowski aronowski self-assigned this Mar 12, 2024
@aronowski
Copy link
Collaborator

Reviewing as much as I can right now. That's not all, but I don't want to keep you waiting.


Both builds reproduce fine. Checksums match. The characteristics are alright!


Minor nitpick: the GRUB2-related entries in the application mention that version 2.06 is used. However, I just downloaded and installed the release having VERSION_ID="20240315" in /etc/os-release and the artifact seems to be different. It presents itself as GRUB2 version 2.12, e.g. in its .sbat section, like here

$ objcopy --only-section .sbat -O binary grub.efi /dev/stdout
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,4,Free Software Foundation,grub,2.12,https://www.gnu.org/software/grub/
grub.opensuse,1,The openSUSE Project,grub2,2.12,mailto:[email protected]

or by editing the GRUB2 commandline before the system boots - there's also a mention of version 2.12. Are these mentions just remnants of those from earlier application or am I missing out on something?


Notes for other reviewers:

The kernel modules ephemeral keys answer has been brought up as part of the SUSE Enterprise Linux application.

While the kernel image has the NX bit set, GRUB2 binaries do not have it:

localhost:/boot # objdump -x /usr/lib/modules/6.7.9-1-default/vmlinuz | grep DllCharacteristics
DllCharacteristics	00000100
localhost:/boot # objdump -x /boot/efi/EFI/opensuse/grubx64.efi | grep DllCharacteristics
DllCharacteristics	00000000
localhost:/boot # objdump -x /boot/efi/EFI/opensuse/grub.efi | grep DllCharacteristics
DllCharacteristics	00000000

Therefore, it's OK that shim binaries also do not have it set.

@aronowski aronowski added the question Reviewer(s) waiting on response label Mar 18, 2024
@jsegitz
Copy link
Author

jsegitz commented Mar 20, 2024

grub2: The reason here is that we use this for Tumbleweed, but also for Leap (stable distribution based on SLES). The answer is correct for Leap, not for Tumbleweed. I'll update the tag with the proper version, thanks for the hint

@jsegitz
Copy link
Author

jsegitz commented Apr 12, 2024

does that answer the question or is more information needed from us?

@aronowski
Copy link
Collaborator

aronowski commented Apr 16, 2024 via email

@jsegitz
Copy link
Author

jsegitz commented Apr 17, 2024

Thanks :)

the tag is already up to date AFAICS.

I could ask @dennis-tseng99 to have a look here. Not sure if that's okay from a separation of duties perspective though, as he's a SUSE employee

@aronowski
Copy link
Collaborator

The tag has been updated. Thank you!

I could ask @dennis-tseng99 to have a look here. Not sure if that's okay from a separation of duties perspective though, as he's a SUSE employee

I myself don't discriminate based on where one is employed.

@aronowski aronowski added extra review wanted and removed question Reviewer(s) waiting on response labels Apr 17, 2024
@aronowski aronowski removed their assignment Apr 17, 2024
@steve-mcintyre steve-mcintyre self-assigned this Apr 29, 2024
@steve-mcintyre
Copy link
Collaborator

Review of Shim 15.8 for openSUSE Tumbleweed: SUSE-openSUSE_tumbleweed-shim-15.8-20240301

OK

  • Contact verification already done before - OK
  • shim builds reproduce here - OK
  • Builds from 15.8 upstream - OK
    • with 5 patches applied:
      • remove_build_id.patch
        trivial, no security impact - OK
      • shim-arch-independent-names.patch
        trivial, no security impact - OK
      • shim-bsc1177315-verify-eku-codesign.patch
        seems fair, why not upstream?
      • shim-change-debug-file-path.patch
        trivial, no security impact - OK
      • shim-disable-export-vendor-dbx.patch
        trivial, no security impact - OK
  • NX bit not set - OK
  • Includes a CA key expiring in 2035 - OK
  • SBAT bumped for shim and GRUB to revoke old binaries, and older
    signing certs are blocked - OK
  • Two different GRUB2 setups:
    • 2.12 with CVE fixes included - OK
    • 2.06 with CVE fixes included - OK
  • Shim loads grub, fwupd and systemd-boot, claims no other binaries. fwupdate?
  • kernel 6.7.x with lockdown patches - OK
  • SBAT data looks fine for shim, grub, - OK

Issues / queries

  • Kernel modules not signed with ephemeral key - this is really
    desirable these days.
  • The list of grub modules is very large - do you really need all
    thse modules??
  • Are you still shipping a signed fwupdate?
  • Not 100% clear on key management:
  • Please upstream the codesign verification patch!
  • Are you planning to ship UKIs in the chain from this shim?

@steve-mcintyre steve-mcintyre added question Reviewer(s) waiting on response and removed extra review wanted labels Apr 29, 2024
@jsegitz
Copy link
Author

jsegitz commented May 2, 2024

Thank you very much for you review. As for you questions:

  • Kernel modules not signed with ephemeral key - this is really desirable these days.
    

I agree. It's unfortunately not possible ATM with the way we distribute kernel modules. We have partners where we can't rebuild the modules with each kernel

  • The list of grub modules is very large - do you really need all thse modules??

It's synced with the SLES modules. I can discuss this with our grub2 maintainer, but usually there's a customer requirement behind those and it's tricky to drop them

  • Are you still shipping a signed fwupdate?

We do for SLES, but not on openSUSE anymore. Thanks for the hint, we can remove this

I think it's equivalent. The key management process is CC EAL 4+ certified and AFAIK RH uses a similar setup. We're currently evaluating moving to a different HSM solution, but performance is an issue for us since this is also used for signing kernel modules etc.

  • Please upstream the codesign verification patch!

I'll ask our shim maintainer to do this

  • Are you planning to ship UKIs in the chain from this shim?

we're currently looking into this, but AFAIK don't have anything yet

@jsegitz
Copy link
Author

jsegitz commented May 3, 2024

I removed the fwupdate entry, we don't use this on openSUSE anymore.

@jsegitz
Copy link
Author

jsegitz commented May 15, 2024

The shim patch will be upstreamed shortly by our shim maintainer.

Does that answer all open question? Really would like to be able to release this one to our users

@verpejas
Copy link

  • Are you still shipping a signed fwupdate?

We do for SLES, but not on openSUSE anymore. Thanks for the hint, we can remove this

Does that mean that even with shim 15.8 on Tumbleweed it will still be necessary to disable secureboot for fwupd firmware updates? This has been a long-lasting issue on OpenSUSE, which I hope gets sorted.

@steve-mcintyre
Copy link
Collaborator

  • Are you still shipping a signed fwupdate?

We do for SLES, but not on openSUSE anymore. Thanks for the hint, we can remove this

Does that mean that even with shim 15.8 on Tumbleweed it will still be necessary to disable secureboot for fwupd firmware updates? This has been a long-lasting issue on OpenSUSE, which I hope gets sorted.

fwupdate is the (very) old progream for doing firmware updates via UEFI. Since then, people have moved to fwupd and then fwupd-efi.

I was just querying the SBAT entry for the old fwupdate code.

@steve-mcintyre steve-mcintyre removed the question Reviewer(s) waiting on response label May 23, 2024
@steve-mcintyre
Copy link
Collaborator

All looks good for me, accepting

@steve-mcintyre steve-mcintyre added the accepted Submission is ready for sysdev label May 23, 2024
@jsegitz
Copy link
Author

jsegitz commented Jun 3, 2024

Thank you very much! I'll sent this to Microsoft tomorrow

@THS-on
Copy link
Collaborator

THS-on commented Jul 29, 2024

@jsegitz did you get a signed shim back?

@jsegitz
Copy link
Author

jsegitz commented Jul 29, 2024

yes, I got a signed shim back. Next release of Tumbleweed should contain it

@jsegitz
Copy link
Author

jsegitz commented Jul 29, 2024

and we can close it. Thank you everyone!

@jsegitz jsegitz closed this as completed Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Submission is ready for sysdev
Projects
None yet
Development

No branches or pull requests

6 participants