search

rhboot / shim-review

Reviews of shim
67 stars 130 forks source link

Shim 15.8 for EuroLinux 9 #375

Closed aronowski closed 4 months ago

aronowski commented 8 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://github.com/EuroLinux/shim-review/tree/eurolinux-shim-x86_64-20240209

What is the SHA256 hash of your final SHIM binary?

c6763bf19239ad8437dde50d8263b6ab776e0ecbb48cab85d55fe3e97771ae79

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

https://github.com/rhboot/shim-review/issues/327

aronowski commented 8 months ago

The key pair of the primary contact has changed since the last application - please, perform a verification.

vden-irm commented 8 months ago

Hi, I'm not an authorized reviewer. I just want to help.

./shimx64.efi: file format pei-x86-64

Contents of section .sbat: d4000 73626174 2c312c53 42415420 56657273 sbat,1,SBAT Vers d4010 696f6e2c 73626174 2c312c68 74747073 ion,sbat,1,https d4020 3a2f2f67 69746875 622e636f 6d2f7268 ://github.com/rh d4030 626f6f74 2f736869 6d2f626c 6f622f6d boot/shim/blob/m d4040 61696e2f 53424154 2e6d640a 7368696d ain/SBAT.md.shim d4050 2c342c55 45464920 7368696d 2c736869 ,4,UEFI shim,shi d4060 6d2c312c 68747470 733a2f2f 67697468 m,1,https://gith d4070 75622e63 6f6d2f72 68626f6f 742f7368 ub.com/rhboot/sh d4080 696d0a73 68696d2e 6575726f 6c696e75 im.shim.eurolinu d4090 782c312c 4575726f 4c696e75 782c7368 x,1,EuroLinux,sh d40a0 696d2c31 352e382c 73656375 72697479 im,15.8,security d40b0 40657572 6f2d6c69 6e75782e 636f6d0a @euro-linux.com.

- Newline at the end of SBAT exists - OK
- .sbatlevel seems OK and there is no binutils bug:

$ objdump -s -j .sbatlevel ./shimx64.efi

./shimx64.efi: file format pei-x86-64

Contents of section .sbatlevel: 86000 00000000 08000000 37000000 73626174 ........7...sbat 86010 2c312c32 30323330 31323930 300a7368 ,1,2023012900.sh 86020 696d2c32 0a677275 622c330a 67727562 im,2.grub,3.grub 86030 2e646562 69616e2c 340a0073 6261742c .debian,4..sbat, 86040 312c3230 32343031 30393030 0a736869 1,2024010900.shi 86050 6d2c340a 67727562 2c330a67 7275622e m,4.grub,3.grub. 86060 64656269 616e2c34 0a00 debian,4.. 

- NX compatibility is disabled and it is OK for now:

$ objdump -p shimx64.efi | grep DllCharacteristics DllCharacteristics 00000000

- Certificate matches the organization:

$ openssl x509 -inform der -in eurolinuxCA.cer -text | grep Subject Subject: C = PL, ST = Poland, L = Cracow, O = EuroLinux Sp. z o.o., CN = EuroLinux Secure Boot CA

- Certificate validity is OK (~19 years):

$ openssl x509 -inform der -in eurolinuxCA.cer -text | grep -A2 Validity Validity Not Before: Feb 7 13:38:07 2024 GMT Not After : Oct 25 13:38:07 2043 GMT

- Certificate is CA:

$ openssl x509 -inform der -in eurolinuxCA.cer -text | grep -A3 "X509v3 Key Usage" X509v3 Key Usage: Digital Signature, Non Repudiation, Key Encipherment, Data Encipherment, Key Agreement, Certificate Sign, CRL Sign X509v3 Basic Constraints: critical CA:TRUE

- GRUB doesn't use ntfs module and is not affected by the October 2023 CVEs. SBAT level 3 is OK in that case.
- SBAT for GRUB2 looks good and Red Hat's SBAT entry is included:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md grub,3,Free Software Foundation,grub,2.06,https//www.gnu.org/software/grub/ grub.rh,2,Red Hat,grub2,2.06-70.el9_3.2,mailto:secalert@redhat.com grub.eurolinux,1,EuroLinux,grub2,2.06-70.el9_3.2,mailto:security@euro-linux.com


- Ephemeral key signing for kernel modules is used 
- The keys are stored on a FIPS 140-2 certified HSM
SherifNagy commented 7 months ago

While I am not an official reviewer, here are my comments "looking at latest tag: https://github.com/EuroLinux/shim-review/tree/eurolinux-shim-x86_64-20240209":

steve-mcintyre commented 7 months ago

Contact verification emails sent

jaromaz commented 7 months ago

@steve-mcintyre

Eugenio lot ventricular humiliated trainer redone Garza Ruchbah belabors galosh

aronowski commented 7 months ago

Chou dominate seasons Deborah fluoridates bossier sum appliances redundancy railings

steve-mcintyre commented 7 months ago

Contact verification done

SherifNagy commented 7 months ago

Just adding label to be easier to track

SherifNagy commented 7 months ago

@aronowski I noticed that the vault do have https://vault.cdn.euro-linux.com/legacy/eurolinux/9/9.3/BaseOS/x86_64/os/Packages/k/kernel-uki-virt-5.14.0-362.18.1.el9_3.x86_64.rpm , and by inspecting the uki image, it is not signed by eurolinux certs.

---------------------------------------------
certificate address is 0x7fbc3bf40dc8
Content was not encrypted.
Content is detached; signature cannot be verified.
The signer's common name is Red Hat Test Certificate
No signer email address.
Signing time: Thu Jan 25, 2024
There were certs or crls included.
---------------------------------------------

and the SBAT:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
systemd,1,The systemd Developers,systemd,252,https://systemd.io/
systemd.eurolinux,1,EuroLinux,systemd,252-18.el9,https://github.com/EuroLinux/eurolinux-distro-bugs-and-rfc/
linux,1,Red Hat,linux,5.14.0-362.18.1.el9_3.x86_64,https://bugzilla.redhat.com/
linux.centos,1,Red Hat,linux,5.14.0-362.18.1.el9_3.x86_64,https://bugzilla.redhat.com/
kernel-uki-virt.centos,1,Red Hat,kernel-uki-virt,5.14.0-362.18.1.el9_3.x86_64,https://bugzilla.redhat.com/
AlexBaranowski commented 7 months ago

@SherifNagy

I'm quite sure that none of our kernels ATM is signed by our key 😅 .

Until we have signed Shim there is no benefit of changing from the vanilla src.rpm. The not signed vanilla src.rpm allows trivial rebuilds and reproducibility (at least on a functional level).

When it comes to UKI, specifically, I think that we will go with the accepted and approved Rocky Linux way -> https://github.com/rocky-linux/shim-review/tree/rockylinux-9-shim-15.8-x86_64-aarch64-20240214

From reviewed and accepted Rocky Linux 9 SHIM.

What changes were made in the distor's secure boot chain since your SHIM was last signed?

As there is no official guideline for extending the SHIM to new kernels flavours (UKI) or even reusing the secure boot key for multiple kernel versions, including one that is different from the reviewed, it's next to impossible for me to address your note.

As SHIM review looks much more like promise and baseline I think that it's only fair to say that we will do it similarly to the recently accepted reviews.

Best, Alex

SherifNagy commented 6 months ago

Thanks for the clarification Alex, even the rocky accepted review among others, will have some tweaks needs to be done for UKI after the community is agreeing on standard way for how they look now. I will take a note of this issue number in the meta issue so we can go back to it when you sign UKI kernels

jsetje commented 6 months ago

I don't see any outstanding issues here. Should this just be approved at this point?

SherifNagy commented 6 months ago

I don't see any outstanding issues here. Should this just be approved at this point?

I think it can be approved, no issues from my end other than the UKI entries "same for other vendors", currently they are shipping the upstream distor "RHEL" UKI sbat entries unsigned, I would request them to update #397 with their final UKI's SBAT entries once they build and sign their kernel

aronowski commented 4 months ago

Signed binary received, closing. Huge thanks to all the great folks, who helped us!