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

Ctrl IQ, Inc EL9 Shim 15.8 for x64 #420

Open
8 tasks done
jason-rodri opened this issue May 19, 2024 · 12 comments
Open
8 tasks done

Ctrl IQ, Inc EL9 Shim 15.8 for x64 #420

jason-rodri opened this issue May 19, 2024 · 12 comments
Assignees
Labels
accepted Submission is ready for sysdev contacts verified OK Contact verification is complete here (or in an earlier submission) easy to review This submission might be a good place to start for an inexperienced reviewer

Comments

@jason-rodri
Copy link

jason-rodri commented May 19, 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/ctrliq/ciq-shim-build/releases/tag/ciqliq-shim-EL9-x64-20240705


What is the SHA256 hash of your final SHIM binary?


SHA256 (shimx64.efi) = f67bf3bb333d1e8ecfbb372f93ad7056e12c43c4eedc335e235c66b7af9fa940


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


Ctrl IQ, Inc Shim 15.8 for x64 & ia32 #366


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)?


Jason Rodriguez
Michael Young

@SherifNagy
Copy link
Collaborator

Can we get the UKI's SBAT, or you aren't providing UKI kernel?

@steve-mcintyre steve-mcintyre added the contacts verified OK Contact verification is complete here (or in an earlier submission) label May 29, 2024
@jason-rodri
Copy link
Author

Can we get the UKI's SBAT, or you aren't providing UKI kernel?

We are not moving forward with signing UKI build at this time.

@aronowski aronowski self-assigned this Jun 4, 2024
@jason-rodri
Copy link
Author

@aronowski @SherifNagy Please let me know if I can provide additional information to help the review process.

@SherifNagy
Copy link
Collaborator

@jason-rodri nothing at the moment, we are working through the queue

@aronowski
Copy link
Collaborator

The application looks alright, apart from one minor nitpick:

*******************************************************************************
### Does your SHIM load any loaders that support loading unsigned kernels (e.g. GRUB2)?
*******************************************************************************
Grub2 will only load unsigned code if the secureboot feature is turned off  load unsigned kernels, but only with secureboot mode turned off on an end-user's system.

The duplicated explanation in the answer hasn't been fixed yet, just like in the EL8 application. ;-)

@aronowski aronowski added extra review wanted Initial review(s) look good, another review desired easy to review This submission might be a good place to start for an inexperienced reviewer labels Jun 19, 2024
@SherifNagy
Copy link
Collaborator

Review of ciqliq-shim-EL9-x64-20240518

  • Security contacts looks good, didn't change since last successful submission
  • Keys are stored in FIPS HSM

Shim

  • Uses upstream 15.8 and source hashes matches original hashes
  • SBAT entries from shim looks fine
  • No patches added on top of upstream shim
  • Vendor SBAT entry is at 1
  • Binaries are reproducible using the container image
STEP 14/14: RUN chmod 0755 /root/shim-compare.sh;  /root/shim-compare.sh

Shim Comparison, original binary vs. freshly built binaries:

SHA256 sums ::
f67bf3bb333d1e8ecfbb372f93ad7056e12c43c4eedc335e235c66b7af9fa940  /shimx64.efi
f67bf3bb333d1e8ecfbb372f93ad7056e12c43c4eedc335e235c66b7af9fa940  /shim_result/usr/share/shim/15.8-0.el9/x64/shimx64.efi
  • NX flag is not set, because the chain is not yet ready
  • Self signed 4096 bit cert and valid for almost 24 years

GRUB2

  • SBAT looks fine (keeps upstream RHEL/rocky grub2)
  • Version currently does not include NTFS patches, but the signed versions also not include the NTFS module so sbat grub,3
  • Module list sound fine

Kernel

  • Ephemeral keys are used for signing kernel modules
  • Lockdown patches are included (keeps upstream RHEL kernel)

Notes

Along side the note from @aronowski

sbat,1,UEFI shim,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
fwupd-efi,1,Firmware update daemon,fwupd-efi,1.4,https://github.com/fwupd/fwupd-efi
fwupd-efi.rhel,1,Red Hat Enterprise Linux,fwupd,1.9.13,mail:secalert@redhat.com
fwupd-efi.rocky,1,Rocky Linux,fwupd,1.9.13,mail:security@rockylinux.org
  • You aren't keeping any upstream fwupd entries for sbat, I assume this is acceptable Cc: @steve-mcintyre for input

Other than those few notes, LGTM, we will need one more reviewer

@aronowski aronowski removed their assignment Jul 11, 2024
@THS-on
Copy link
Collaborator

THS-on commented Jul 29, 2024

Review of ciqliq-shim-EL9-x64-20240705

  • Contacts are already verified
  • Linux vendor for support and products based on Rocky Linux
  • Keys are stored in a HSM

Shim

  • Based on upstream 15.8 without patches
  • NX flag is disabled
  • Embedded self-signed CA is valid till 2048 and uses 4096bit RSA, Digital Signature attribute is set
  • SBAT looks fine
  • Is reproducible using Dockerfile:
#17 0.422 f67bf3bb333d1e8ecfbb372f93ad7056e12c43c4eedc335e235c66b7af9fa940  /shimx64.efi
#17 0.422 f67bf3bb333d1e8ecfbb372f93ad7056e12c43c4eedc335e235c66b7af9fa940  /shim_result/usr/share/shim/15.8-0.el9/x64/shimx64.efi

GRUB2 and fwupd

  • Based on Rocky 9
  • SBAT level still on 3, because NTFS patches are not included (fine for now as NTFS modules were not signed)
  • Module list looks fine
  • upstream SBAT entries are preserved
  • fwupd entries now look good

Kernel

  • Lockdown patches from Rocky/RHEL are used
  • ephemeral key signing is used
  • Mainly 5.14, but also other LT version and latest kernel are build

LGTM! I'll raise the certmule/certwrapper question during today's call and will get back to you

@THS-on THS-on added accepted Submission is ready for sysdev and removed extra review wanted Initial review(s) look good, another review desired labels Jul 29, 2024
@THS-on
Copy link
Collaborator

THS-on commented Jul 30, 2024

As discussed yesterday using certwrapper to import the Rocky Linux CA is likely fine. We just need confirmation that certwrapper is ready for production use and can be signed. @jsetje do you know the status of certwrapper?

@jason-rodri
Copy link
Author

Signed binaries returned from MSFT.

I understand the that there is still a question around certwrapper. Do we still want to keep this open until @jsetje replies?

@THS-on
Copy link
Collaborator

THS-on commented Aug 2, 2024

I understand the that there is still a question around certwrapper. Do we still want to keep this open until @jsetje replies?

Yeah let's keep this open until there is a definite answer if you can sign certwrapper

@THS-on
Copy link
Collaborator

THS-on commented Sep 11, 2024

@jsetje is certwrapper now ready to sign for people or should they wait?

@THS-on
Copy link
Collaborator

THS-on commented Oct 21, 2024

As discussed with @steve-mcintyre, the certwrapper is ready to sign.

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 contacts verified OK Contact verification is complete here (or in an earlier submission) easy to review This submission might be a good place to start for an inexperienced reviewer
Projects
None yet
Development

No branches or pull requests

5 participants