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 OL7 (ol7-shim-x86_64-20240215) #382

Closed
8 tasks done
iokomin opened this issue Feb 15, 2024 · 12 comments
Closed
8 tasks done

Shim 15.8 for OL7 (ol7-shim-x86_64-20240215) #382

iokomin opened this issue Feb 15, 2024 · 12 comments
Labels
accepted Submission is ready for sysdev

Comments

@iokomin
Copy link

iokomin commented Feb 15, 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/oracle/shim-review/tree/ol7-shim-x86_64-20240215


What is the SHA256 hash of your final SHIM binary?


26ee414cdf7e900938f7f6120f9f9825b58d45314172a418d29c96d70ba81893 shimx64.efi


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


#306

@realnickel
Copy link

Hi, I'm not an authorized reviewer. I just want to help.
I'll get back as soon as my review will be done.

@iokomin
Copy link
Author

iokomin commented Mar 8, 2024

@realnickel kindly asking if you had a chance to review this request.

@realnickel
Copy link

@realnickel kindly asking if you had a chance to review this request.

@iokomin, sorry for keeping you waiting. The review is still WIP, I'll try to finish it ASAP.

@realnickel
Copy link

While I am not an official reviewer, looking at latest tag: https://github.com/oracle/shim-review/tree/ol7-shim-x86_64-20240215 and additionally guided by discussions in #377 and #378 (same vendor, different distro branch) I can confirm that:

  • SHIM build (x64) is reproducible with the provided dockerfile giving sha256sum hash match;
  • SHIM sources within the SRPM matches the release hash;
  • SHIM only patch in fact does what it is claimed to do: add "-r" opt to D2UFLAGS in Make.defaults only if supported by OL7 DOS2UNIX version;
  • SHIM includes 3 EV certs for x64 (grub2, kernel, vendor), they are valid for about 12 months (expiring from 03-15-2025 to 04-02-2025);
  • SHIM SBAT entries look good;
  • SHIM SBAT entry for the vendor looks good, didn't get bumped up , remains at shim.ol,3;
  • Contacts GPG keys look good (see comment below);
  • Certs protection with HSM story looks good;
  • Grub modules looks good, no NTFS module, list is not changed since last submission;
  • SBAT entry for grub looks good and it's grub,3 since no NTFS patches has been applied to their grub sources;
  • NX is disabled in shim and claimed to not exist in the boot chain.

Previous accepted submission for OL7 was issue #306 (ol7-shim-x86_64-20221129).

git diff origin/ol7-shim-x86_64-20221129..origin/ol7-shim-x86_64-20240215
Submission information diff looks sane, same as current questionary answers.
Transition from shim-15.7 to shim-15.8 is performed.

Source code provided in shim-15.8-2.0.3.el7.src.rpm package matches to upstream shim-15.8.tar.bz2

Vendor is probably going to drop shipping signed ia32 version of shim as neither binary nor its hash provided for review (while it is still built inside the provided container).

No security contact verification since first submission in 2018 (issue #33) has been performed explicitly, but IMHO that's no issue here as vendor had got several accepted submissions before "new vendor" tag was even introduced.

Moreover @iokomin (who first was involved in issue #69 discussion and continuously submits shim for review since then) mentions non-public keybase review for shim 15.6 accepted and signed as part of grub2 embargo cve-2021-3695 work in his comment [1][2]. That fact pretty well confirms security contacts status.

It also might be useful to consider comments on SBAT_AUTOMATIC_DATE=2021030218 in shim.spec [3] and @@Version@@ in grub2 SBAT metadata [4] in advance as they seem to be relevant to this submission either.

Main concerns at this point:

I haven't dig deep into mentioned kernels' lockdown patches and Oracle customization patches [5] on top of RHEL GRUB2 yet.

[1] #306 (comment)
[2] https://blogs.oracle.com/linux/post/an-inside-look-at-cve-2020-10713-aka-the-grub2-boothole
[3] #377 (comment)
[4] #377 (comment)
[5] #377 (comment)

@realnickel
Copy link

One more point worth mentioning on security contacts information: it seems John Haxby's key might need a renewal nowadays?


pub   dsa2048/450BBB7E942FA3C8 2010-03-31 [SCA]
      FEA71BDBD7508559490D48E5450BBB7E942FA3C8
[...]
sub   rsa2048/EDB4CA820F52B75D 2011-06-23 [E]

@iokomin
Copy link
Author

iokomin commented Mar 25, 2024

@realnickel appreciate for reviewing this submission.
John Haxby was informed regarding moving GPG key from DSA, thanks for the hint.

@iokomin
Copy link
Author

iokomin commented Apr 5, 2024

@aronowski please share if there are any blocking issues left with this request to have it accepted.
Based on @realnickel review, the submission was verified, possible concerns already discussed in details in similar review requests for OL8 and OL9 shim15.8 requests which were accepted and already signed by MSFT.
Appreciate your and other reviewers time!

@steve-mcintyre
Copy link
Collaborator

@realnickel 's review looks good, let's get another set of eyes too.

@steve-mcintyre steve-mcintyre self-assigned this Apr 9, 2024
@steve-mcintyre
Copy link
Collaborator

Review of Shim 15.8 for OL7: ol7-shim-x86_64-20240215

OK

  • Contact verification looks ok enough from previous submissions
  • shim build reproduces here - OK
  • shim Builds from 15.8 upstream, with 1 trivial build-time patch
    applied:
    • 1000-Check-if-r-flag-is-supported-for-dos2unix.patch
  • NX bit not set - OK
  • Includes 3 EV code-signing certs from Digicert - OK.
    • Short-ish expiry dates, but good enough - that's expected with EV
      certs.
    • Key management sounds fine, HSM.
  • No systemd-boot in the boot chain - OK.
  • Grub looks OK
    • Old version, but with backported fixes
    • Module list looks fine
  • Kernel sounds OK
    • Modules signed with ephemeral key - OK
    • Backported fixes and SB patches - OK
  • SBAT data looks fine for shim
  • SBAT data looks ok (enough) for grub2 (no update for ntfs)
  • Revocation story looks fine - dbx then change of cert

Minor nits

  • It would be nice to see actual SBAT data for grub2 and fwupd with
    the @@Version@@ macro substituted.
    • Not blocking on that, but please add in future.

@steve-mcintyre steve-mcintyre added accepted Submission is ready for sysdev and removed extra review wanted labels Apr 9, 2024
@steve-mcintyre
Copy link
Collaborator

Accepted!

@iokomin
Copy link
Author

iokomin commented Apr 9, 2024

@steve-mcintyre thanks for quick turnaround, appreciate your time.

It would be nice to see actual SBAT data for grub2 and fwupd with
the @@Version@@ macro substituted.

Ack, will definitely do.

@iokomin
Copy link
Author

iokomin commented Apr 10, 2024

submission id: 13904177012883421
Signed by Microsoft, closing.

@iokomin iokomin closed this as completed Apr 10, 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

3 participants