eabdullin1 commented 7 months ago

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

[x] completed README.md file with the necessary information
[x] shim.efi to be signed
[x] public portion of your certificate(s) embedded in shim (the file passed to VENDOR_CERT_FILE)
[x] binaries, for which hashes are added to vendor_db ( if you use vendor_db and have hashes allow-listed )
[x] any extra patches to shim via your own git tree or as files
[x] any extra patches to grub via your own git tree or as files
[x] build logs
[x] 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/AlmaLinux/shim-review/tree/almalinux-8-shim-x64-20240404

What is the SHA256 hash of your final SHIM binary?

a872d4a6b1ae5ed2827825a64b7c4feb792f86d1726cf178f0747e11036b7cf9 shimx64.efi be32ae82e0b75dcee8b79c22531bb908e4ac736636ba648ae835cec8c5e8680f shimia32.efi

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

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

SherifNagy commented 7 months ago

Just a quick scan, seems like the shim SBAT entries are wrong in the issue, can we fix this and grab them from the binary to make sure they are correct?

eabdullin1 commented 7 months ago

@SherifNagy Thank you for your quick response. Issue is updated with correct SBAT entries

SherifNagy commented 7 months ago

I will take a closer look

SherifNagy commented 7 months ago

Review of almalinux-8-shim-x64-20240404

AlmaLinux has their own RHEL like kernel and signed kernels in the paste
Security contacts haven't changed since last submission and they are verified on issue #250 and PGP keys are cross-signed between security contact but not signed with by anyone else
Keys are stored in HSM and different certs are used for the components, however, nothing mentioned if the new CA and future new certs cert are stored / will be stored in HSM " need confirmation from vendor "

Shim

Uses upstream 15.8 and source hashes matches original hashes
SBAT entries from shim looks fine after the fix
Vendor SBAT entry has been increased from almalinux,2 to almalinux,3 from last submission #250

Binaries are reproducible using the container image, however, There is another copy / past error, the read me and this issue doesn't have the sha256sum but the pesign -h output command

STEP 26/26: RUN sha256sum /usr/share/shim/15.8-2.el8.alma.1/x64/shimx64.efi /shimx64.efi /usr/share/shim/15.8-2.el8.alma.1/ia32/shimia32.efi /shimia32.efi
a872d4a6b1ae5ed2827825a64b7c4feb792f86d1726cf178f0747e11036b7cf9  /usr/share/shim/15.8-2.el8.alma.1/x64/shimx64.efi
a872d4a6b1ae5ed2827825a64b7c4feb792f86d1726cf178f0747e11036b7cf9  /shimx64.efi
be32ae82e0b75dcee8b79c22531bb908e4ac736636ba648ae835cec8c5e8680f  /usr/share/shim/15.8-2.el8.alma.1/ia32/shimia32.efi
be32ae82e0b75dcee8b79c22531bb908e4ac736636ba648ae835cec8c5e8680f  /shimia32.efi

I think MSFT do review the sha256sum hashes of the binaries thought " Vendor needs to update the issue and the readme "

NX flag is not set, because the chain is not yet ready
Two EV certs valid for around 9 months with 3072 bits and one new self-signed CA valid for 10 years and 2048 bits

GRUB2

SBAT looks fine (keeps upstream RHEL grub2)
Version currently does not include NTFS patches, but the signed versions also not include the NTFS module
Module list sound fine

Kernel

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

eabdullin1 commented 7 months ago

@SherifNagy Thank you for the review! sha256sum hashes of the binaries updated in readme and issue. We confirm that FIPS-certifed HSM is used for the new CA and will be used for future keys.

SherifNagy commented 7 months ago

LGTM! I will add extra review need and easy to review tags, one more note, I don't see submission for Alma9, and if you are planning to use same shim for Alma9, keep an eye on this issue to track the upcoming UKI revocation once it is in place #397

aronowski commented 7 months ago

Kinda worried about the 8.9 version being used to build the binaries, but hopefully, with the 8.10 release coming soon, the buildroot won't change too much, to make the build non-reproducible.

Accepting!

SherifNagy commented 7 months ago

Kinda worried about the 8.9 version being used to build the binaries, but hopefully, with the 8.10 release coming soon, the buildroot won't change too much, to make the build non-reproducible.

Accepting!

I think it depends a lot on the policies within the vendor building policies, some have to build based on latest releases, some builds on any release and using same shim for other version, I think now Ubuntu and fedora using same shim for all releases and I guess Alma will be using same shim from this submission for alma9, that's why I mentioned the UKI ticket, to keep an eye for.

andrewlukoshko commented 6 months ago

@SherifNagy here is SBAT entry from latest AlmaLinux 9.4 UKI image:

# objcopy -O binary -j .sbat /lib/modules/5.14.0-427.16.1.el9_4.x86_64/vmlinuz-virt.efi /dev/stdout
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
linux,1,Red Hat,linux,5.14.0-427.16.1.el9_4.x86_64,mailto:secalert@redhat.com
linux,1,AlmaLinux,linux,5.14.0-427.16.1.el9_4.x86_64,mailto:security@almalinux.org
linux.rhel,1,Red Hat,linux,5.14.0-427.16.1.el9_4.x86_64,mailto:secalert@redhat.com
linux.almalinux,1,AlmaLinux,linux,5.14.0-427.16.1.el9_4.x86_64,mailto:security@almalinux.org
kernel-uki-virt.rhel,1,Red Hat,kernel-uki-virt,5.14.0-427.16.1.el9_4.x86_64,mailto:secalert@redhat.com
kernel-uki-virt.almalinux,1,AlmaLinux,kernel-uki-virt,5.14.0-427.16.1.el9_4.x86_64,mailto:security@almalinux.org
systemd,1,The systemd Developers,systemd,252,https://systemd.io/
systemd.almalinux,1,AlmaLinux,systemd,252-32.el9_4.alma.1,mailto:security@almalinux.org

andrewlukoshko commented 6 months ago

Signed by Microsoft.

Submission IDs: 13958458479179316 (x64) 13944978356239123 (ia32)

Closing. Thanks everyone.

rhboot / shim-review

Shim 15.8 for AlmaLinux OS 8 #407

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

What is the SHA256 hash of your final SHIM binary?

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

Review of almalinux-8-shim-x64-20240404

Shim

GRUB2

Kernel