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

SUSE Liberty Linux 7 #450

Open
8 tasks done
jsegitz opened this issue Nov 12, 2024 · 9 comments
Open
8 tasks done

SUSE Liberty Linux 7 #450

jsegitz opened this issue Nov 12, 2024 · 9 comments
Labels
1 review needed Needs 1 (additional) successful review before being accepted contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response

Comments

@jsegitz
Copy link

jsegitz commented Nov 12, 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-nonfork/tree/SUSE-SLL7-x64-ia32-20241112


What is the SHA256 hash of your final SHIM binary?


Output from sha256sum:

$ sha256sum shimia32.efi

7620c859572a45524f36ef89bedaa05b05f0f3a9daefca4566b19516cef4ae9c ./shimia32.efi

$ sha256sum shimx64.efi

36b17db9300b8e90c11e86ec646b5b3975bfaf834ec10ecb2954bd49136fa8c1 ./shimx64.efi

Output from pesign:

$ pesign --hash --padding --in=shimia32.efi

hash: 800377346f8461fa5f85a41aebc1c41f4dab86f7356f3b43f82277f26a4c622d

$ pesign --hash --padding --in=shimx64.efi

hash: 8ed0ca1a25fd7bf38f03a9cdb350673f3dc62ea9e18daee365c9024df381abf1


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


#183


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


#419

@jsegitz
Copy link
Author

jsegitz commented Nov 12, 2024

The repository isn't a direct for fork because LFS doesn't work for forks.

@jsegitz jsegitz changed the title SUSE Liberty Linux 9 SUSE Liberty Linux 7 Nov 12, 2024
@steve-mcintyre steve-mcintyre added the contacts verified OK Contact verification is complete here (or in an earlier submission) label Nov 12, 2024
@steve-mcintyre
Copy link
Collaborator

Contact verification already completed previously

@steve-mcintyre steve-mcintyre added 2 reviews needed Needs 2 (additional) successful reviews before being accepted Accredited review needed Needs a successful review by an accredited reviewer labels Nov 13, 2024
@doccaz
Copy link

doccaz commented Dec 3, 2024

Sorry to bump this issue, but is there anything missing that we need to provide from our side?

@THS-on
Copy link
Collaborator

THS-on commented Dec 16, 2024

Review of SUSE-SLL7-x64-ia32-20241112

Shim

  • Based on upstream 15.8
  • Includes one patch:
    • 0001-dos2unix-fix-flags-for-RHEL-7.patch: removes build flag that does not exit in CentOS 7 (fine)
  • Build is reproducible:
36b17db9300b8e90c11e86ec646b5b3975bfaf834ec10ecb2954bd49136fa8c1  /usr/share/shim/x64-15.8-3.el7/shimx64.efi
36b17db9300b8e90c11e86ec646b5b3975bfaf834ec10ecb2954bd49136fa8c1  /shimx64.efi
7620c859572a45524f36ef89bedaa05b05f0f3a9daefca4566b19516cef4ae9c  /usr/share/shim/ia32-15.8-3.el7/shimia32.efi
7620c859572a45524f36ef89bedaa05b05f0f3a9daefca4566b19516cef4ae9c  /shimia32.efi
  • NX is disabled
  • SBAT looks fine
  • Embedded CA hasn't changed (2048 bit, valid till 2037)

GRUB

  • SBAT level on 3, but no NTFS modules were signed (fine, but will likely cause the GRUB not booting in the future)
  • SBAT looks fine (upstream SBAT is reserved)
  • patches are from CentOS 7 + branding patch

Kernel

  • Ephemeral key signing is used
  • Kernel 3.10 is very old
    • Lockdown seems to be done via EFI_SECURE_BOOT_SECURELEVEL and CONFIG_SECURITY_SECURELEVEL, but I'm not familiar with that.
  • Kernel 5.4 looks fine

Comments & Questions

  • Providing the chroot is not ideal. Is there any reason you can't use a snapshot of e.g. the public CentOS 7 one?
  • For other reviewers: the linked branch of SUSE is the US one, they use the German one for their embedded CA. This is fine.
  • If you are launching fwupd/fwupdate please include the SBAT entry for it in the submission
  • Who maintains the patches for 3.10, as LTS from upstream ended 2017 and from CentOS presumably in April 2024?
  • I would like to have other people that maintain LTS support based on CentOS 7 to have a look here.

@THS-on THS-on added the question Reviewer(s) waiting on response label Dec 16, 2024
@jsegitz
Copy link
Author

jsegitz commented Dec 19, 2024

Thank you for your review!

Comments & Questions

* Providing the chroot is not ideal. Is there any reason you can't use a snapshot of e.g. the public CentOS 7 one?

CentOS7 is EOL now and won't receive any updates, publicly available container images of it had URLs to CentOS repos broken (as it had been moved to the vault). That's why we decided to use our own chroot inside podman.

* If you are launching `fwupd/fwupdate` please include the SBAT entry for it in the submission

Versions of fwupd/fwupdate currently used in SLL7 match CentOS7 and are legacy pre-SBAT ones

* Who maintains the patches for 3.10, as LTS from upstream ended 2017 and from CentOS presumably in April 2024?

SUSE is maintaining this kernel in the scope of the LTSS program for SUSE Liberty Linux 7.

@THS-on THS-on added 1 review needed Needs 1 (additional) successful review before being accepted and removed 2 reviews needed Needs 2 (additional) successful reviews before being accepted Accredited review needed Needs a successful review by an accredited reviewer question Reviewer(s) waiting on response labels Dec 19, 2024
@THS-on
Copy link
Collaborator

THS-on commented Dec 19, 2024

@jsegitz thanks for the clarifications.

Versions of fwupd/fwupdate currently used in SLL7 match CentOS7 and are legacy pre-SBAT ones

Are you currently signing those? Do you have a strategy to revoke them easily, i.e. separate signing certificate?

@THS-on
Copy link
Collaborator

THS-on commented Dec 19, 2024

@jason-rodri @iokomin can maybe you or one from your team have a look at this, as you are probably the ones most familiar with this?

@THS-on THS-on added the question Reviewer(s) waiting on response label Dec 19, 2024
@jsegitz
Copy link
Author

jsegitz commented Dec 19, 2024

Are you currently signing those? Do you have a strategy to revoke them easily, i.e. separate signing certificate?

Unfortunately, SLL7 is using the single certificate for everything UEFI related. Thus, for now we may either ban the binaries via DBX or rotate the signatures.

It's LTS for an old distro, so some of the choices are not in line with current best practices anymore

@iokomin
Copy link

iokomin commented Dec 19, 2024

Unfortunately, SLL7 is using the single certificate for everything UEFI related. Thus, for now we may either ban the binaries via DBX or rotate the signatures.

@jsegitz I'm not sure I understand your concern and why DBX/certificate rotation would be needed.
shim under review supports SBAT revocation. sbat metadata is required for binaries loaded by shim, which should cover fwupd as well. Your fwupd binary supposed to have .sbat metadata and if revocation is needed, new patched fwupd version will get sbat metadata version update, while vulnerable version supposed to be added to your shim SBAT level to block these binaries. Please let me know if I'm missing anything.

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