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 MIRACLE LINUX 9 #421

Open
8 tasks done
tSU-RooT opened this issue May 24, 2024 · 10 comments
Open
8 tasks done

shim 15.8 for MIRACLE LINUX 9 #421

tSU-RooT opened this issue May 24, 2024 · 10 comments
Labels
1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer bug Problem with the review that must be fixed before it will be accepted contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response

Comments

@tSU-RooT
Copy link

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/miraclelinux/shim-review/tree/miraclelinux-x64-20240524


What is the SHA256 hash of your final SHIM binary?


60485ece0fa7d8a01975e65d6845b9c084018e4c15d40e2d2d1bbe0bbbdca5d9 shimx64.efi


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


#264 is previous shim-review request (accepted)


If no security contacts have changed since verification, what is the link to your request, where they've been verified (if any, otherwise N/A)?


Security contacts are changed, Primary contact is updated to my colleague.
Second contact is me(verified).
#266 (comment)

@steve-mcintyre
Copy link
Collaborator

@tSU-RooT has had contact verification done before (hi!)
Sending a message to Masayuki Moriyama now...

@steve-mcintyre steve-mcintyre added the contact verification pending Contact verification emails have been sent, waiting on response label May 29, 2024
@moriyama
Copy link

I received the following words:

Chaldean darkening alienation defenses ea keybinding Skye cupid skillful co=
llectivism

@steve-mcintyre steve-mcintyre added contacts verified OK Contact verification is complete here (or in an earlier submission) and removed contact verification pending Contact verification emails have been sent, waiting on response labels May 30, 2024
@aronowski aronowski self-assigned this Jun 4, 2024
@aronowski
Copy link
Collaborator

Seems OK to me. I'll submit the appropriate UKI/systemd-boot SBAT entries to this issue.

Just one positive review (might be from a non-accredited reviewer) remaining and the application can be accepted.

@aronowski aronowski added extra review wanted easy to review This submission might be a good place to start for an inexperienced reviewer labels Jul 11, 2024
@aronowski aronowski removed their assignment Jul 11, 2024
@akodanev
Copy link

Shim review of "miraclelinux-x64-20240524" (by a non-accredited reviewer):

  1. Now includes one more certificate, valid until 2029.
  2. There are no additional patches for the 15.8 release.
  3. NX bit is not set.
  4. The grub policy is set to `grub,3' (NTFS module not included)
  5. .sbat section: why is shim.miracle now set to 2?
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.miracle,2,Cybertrust Japan,shim,15.8,[email protected]
  1. Reproducible build:
#25 [21/21] RUN sha256sum /usr/share/shim/15.8-2.el9.ML.1/x64/shimx64.efi /shimx64.efi
#25 0.283 60485ece0fa7d8a01975e65d6845b9c084018e4c15d40e2d2d1bbe0bbbdca5d9  /usr/share/shim/15.8-2.el9.ML.1/x64/shimx64.efi
#25 0.289 60485ece0fa7d8a01975e65d6845b9c084018e4c15d40e2d2d1bbe0bbbdca5d9  /shimx64.efi
#25 DONE 0.3s

GRUB, Linux kernel

Based on RHEL9.


So there is a question about the generation number bump of shim.miracle, otherwise the rest looks good to me.

Also minor comments about SBAT sections consistency:

  • The last part of the component name changes for UKI, for all other entries it is .miracle. Just out of curiosity, is there a reason for this?
  • Some vendor_url entries with [email protected] address have mail:, mailto: prefixes or nothing.

@aronowski aronowski added question Reviewer(s) waiting on response and removed extra review wanted labels Jul 23, 2024
@steve-mcintyre
Copy link
Collaborator

Review of shim 15.8 for MIRACLE LINUX 9

Good

General

  • Contact verification completed - OK
  • key management using an HSM: Yubikey - OK
  • Switched to a new embedded cert for a new root of trust - OK

Shim

  • Builds from 15.8 upstream (ish - see issues below), with no
    patches - OK
  • shim builds reproduce here for x64 and ia32 with the Dockerfile - OK
60485ece0fa7d8a01975e65d6845b9c084018e4c15d40e2d2d1bbe0bbbdca5d9  /usr/share/shim/15.8-2.el9.ML.1/x64/shimx64.efi
60485ece0fa7d8a01975e65d6845b9c084018e4c15d40e2d2d1bbe0bbbdca5d9  /shimx64.efi
  • NX bit not set on shim binaries - OK

Issues / queries

  • I don't see why you're grabbing Peter's
    shim-unsigned-x64-15.8-2.el9.src.rpm rather than just using the
    upstream shim tarball directly. This isn't a blocker for this review
    as it's possible to check the tarball checksum from the
    src.rpm. Please don't do this again, though.
  • You say "We don't use vendor_db functionality in this build." but
    you quite clearly do. From the build:
make TOPDIR=.. -f ../Makefile COMMIT_ID=5914984a1ffeab841f482c791426d7ca9935a5e6 EFIDIR=almalinux PKGNAME=shim RELEASE=2.el9.ML.1 ENABLE_SHIM_HASH=true SBAT_AUTOMATIC_DATE=2023012900 VENDOR_DB_FILE=/builddir/build/SOURCES/vendordb.esl 'DEFAULT_LOADER=\\\\grubx64.efi' all

I can see three .der files in your submission, and maybe those
three keys in your vendordb.esl. As requested in the README.ms
template: "If you use vendor_db functionality of providing multiple
certificates and/or hashes please briefly describe your certificate
setup."

I'm stopping here until this is resolved.

@steve-mcintyre steve-mcintyre added bug Problem with the review that must be fixed before it will be accepted and removed easy to review This submission might be a good place to start for an inexperienced reviewer labels Sep 8, 2024
@moriyama
Copy link

I can see three .der files in your submission, and maybe those
three keys in your vendordb.esl. As requested in the README.ms
template: "If you use vendor_db functionality of providing multiple
certificates and/or hashes please briefly describe your certificate
setup."

Thanks for your review.

2 certificates enrolled in vendor_db.
vendor_db contains ml9secureboot001.der and ml9secureboot002.der, which are included in this repository.

@moriyama
Copy link

moriyama commented Oct 1, 2024

I'll answer some of your questions since they were left unanswered.

akodanev commented on Jul 12:

5..sbat section: why is shim.miracle now set to 2?

Following the example of RHEL 9 [1] and AlmaLinux OS 8 [2], we increased the value of shim.miracle.

The last part of the component name changes for UKI, for all other entries it is .miracle. Just out of curiosity, is there a reason for this?

This is because the different people in charge of the packages each decided on their own the last part of the component name.

Some vendor_url entries with [email protected] address have mail:, mailto: prefixes or nothing.

I will add mailto: in the next update.

[1] #373
[2] #407

@akodanev
Copy link

@moriyama Thanks for the update!

Following the example of RHEL 9 [1] and AlmaLinux OS 8 [2], we increased the value of shim.miracle.

OK, I just thought there was some Miracle Linux specific issue not mentioned here that needs product-specific bump in shim, good to know there really is nothing.

BTW, looks like there is still no fixed tag with vendor_db functionality: https://github.com/miraclelinux/shim-review/tree/miraclelinux-x64-20240524.

@moriyama
Copy link

@akodanev
We updated README.
New tag is: https://github.com/miraclelinux/shim-review/tree/miraclelinux-x64-20241017

@tSU-RooT
Copy link
Author

tSU-RooT commented Nov 6, 2024

@steve-mcintyre
Hi, concerns are answered I think.
Would you re-review it?

@steve-mcintyre steve-mcintyre added 1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer labels Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer bug Problem with the review that must be fixed before it will be accepted contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response
Projects
None yet
Development

No branches or pull requests

5 participants