rhboot / shim-review

Reviews of shim
65 stars 124 forks source link

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

Closed jason-rodri closed 4 months ago

jason-rodri commented 5 months ago

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


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


https://bitbucket.org/ciqinc/ciq-shim-build/src/ctrliq-shim-x64-ia32-20240131/ Orginal repo migrated to https://github.com/ctrliq/ciq-shim-build/tree/ctrliq-shim-x64-ia32-20240131


What is the SHA256 hash of your final SHIM binary?


SHA256 (shimx64.efi) = 654d8efe248cd113f7ecb5a1f4fc9c309cc0d65a66b4bb8d9b2991f57f2dbcf6 SHA256 (shimia32.efi) = b739423471c03d32f2918906286076ea73c1385ced3f175a60ceeb8fadf009de


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


Ctrl IQ, Inc Shim 15.7 for x64 & ia32 #339

SherifNagy commented 5 months ago

While I am not an official reviewer, here are my comments "looking at latest tag: https://bitbucket.org/ciqinc/ciq-shim-build/src/ctrliq-shim-x64-ia32-20240131/":

Regarding certwrapper(mule), it's great tool, however based on my understanding, it's still in early stages, while you can sign the .EFI with your certs and package it to your users, you must know it is still not fully tested and might causes some issues

I think we need two more official reviewer to look at this submission, best of luck with the submission :)

aronowski commented 5 months ago

The build does reproduce, checksums match. No NX compatibility bit, as the whole chain is not NX-compatible. No binutils bug. OK.


*******************************************************************************
### If your boot chain of trust includes a Linux kernel:
[...]
### Is upstream commit [eadb2f47a3ced5c64b23b90fd2a3463f63726066 "lockdown: also lock down previous kgdb use"](https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eadb2f47a3ced5c64b23b90fd2a3463f63726066) applied?
*******************************************************************************
Yes, all of these patches are already in the Rocky/RHEL 8 + 9 kernels we plan to base on.

:+1:

Yes, this is a thing I got aware of recently - the Enterprise Linux 8.9 kernel does have the CVE-2022-21499 fix implemented by the commit eadb2f47a3ced5c64b23b90fd2a3463f63726066 ported by disabling writing to kernel memory in lockdown mode.

Furthermore, debugging appears to be enabled only in aarch64 debug config:

$ grep -r CONFIG_KDB_DEFAULT_ENABLE kernel-*.config
kernel-aarch64-debug.config:CONFIG_KDB_DEFAULT_ENABLE=0x1
kernel-aarch64.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-ppc64le-debug.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-ppc64le.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-s390x-debug.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-s390x-zfcpdump.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-s390x.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-x86_64-debug.config:CONFIG_KDB_DEFAULT_ENABLE=0x0
kernel-x86_64.config:CONFIG_KDB_DEFAULT_ENABLE=0x0

LGTM! Let's wait for another official review.

kukrimate commented 5 months ago

(Not an official reviewer, but here we go)

Basics

CA

Shim

GRUB

sd-boot

Kernel

kukrimate commented 5 months ago

Shim point I missed:

jason-rodri commented 4 months ago

Signed binaries returned, closing