ronnystandtke commented 5 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/Lernstick/shim-review/tree/lernstick-shim-amd64-20240731

What is the SHA256 hash of your final SHIM binary?

6544e9cee3a3308c9090875a8edb40be648b222db7c17f09ab4801c5b4ef5268  shimx64.efi

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

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

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

Ronny Standtke was verified in https://github.com/rhboot/shim-review/issues/196#issuecomment-974085573
Jörg Berkel still has to be verified

steve-mcintyre commented 5 months ago

Verification mail sent to Jörg

joberkel commented 5 months ago

thanks & verified! electioneers satyrs centrifuging combative expedients Christopher oddity strolling flammable pulsars

MuthuvelKuppusamy commented 5 months ago

Getting some build errors as below, could you please verify.

GPG error: http://deb.debian.org/debian bookworm InRelease: The following signatures couldn't be verified because the public key is not available: NO_PUBKEY 0E98404D386FA1D9 NO_PUBKEY 6ED0E7B82643E131 NO_PUBKEY F8D2585B8783D481 The repository 'http://deb.debian.org/debian bookworm InRelease' is not signed.

THS-on commented 5 months ago

@MuthuvelKuppusamy that's an interesting error, because we use the official Debian docker image.

Just retried with docker build . --progress plain --no-cache using docker version 26.1.4 and it worked fine, can you try again?

The image used to build was docker.io/library/debian:bookworm@sha256:a92ed51e0996d8e9de041ca05ce623d2c491444df6a535a566dabd5cb8336946

THS-on commented 4 months ago

We updated to a shim with the automatic revocations to the upstream 15.8 ones, instead of the Debian ones which already revoke every shim with SBAT level < 4.

jclab-joseph commented 3 months ago

Build failed with default dockerfile:

20] RUN hexdump -Cv /shim/shim*.efi > build
#20 DONE 0.6s

#21 [17/20] RUN hexdump -Cv /shim-review/$(basename /shim/shim*.efi) > orig
#21 DONE 0.7s

#22 [18/20] RUN diff -u orig build
#22 0.395 --- orig  2024-07-30 03:23:29.672566221 +0000
#22 0.395 +++ build 2024-07-30 03:23:28.988602703 +0000
#22 0.395 @@ -11,7 +11,7 @@
#22 0.395  000000a0  00 7e 06 00 00 00 00 00  00 30 02 00 00 30 02 00  |.~.......0...0..|
#22 0.395  000000b0  00 00 00 00 00 00 00 00  00 10 00 00 00 02 00 00  |................|
#22 0.395  000000c0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395 -000000d0  00 30 0d 00 00 04 00 00  7b 52 0f 00 0a 00 00 00  |.0......{R......|
#22 0.395 +000000d0  00 30 0d 00 00 04 00 00  7c 52 0f 00 0a 00 00 00  |.0......|R......|
#22 0.395  000000e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395  000000f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395  00000100  00 00 00 00 10 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395 @@ -32063,8 +32063,8 @@
#22 0.395  0007d3e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395  0007d3f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395  0007d400  00 00 00 00 08 00 00 00  37 00 00 00 73 62 61 74  |........7...sbat|
#22 0.395 -0007d410  2c 31 2c 32 30 32 33 30  31 32 39 30 30 0a 73 68  |,1,2023012900.sh|
#22 0.395 -0007d420  69 6d 2c 32 0a 67 72 75  62 2c 33 0a 67 72 75 62  |im,2.grub,3.grub|
#22 0.395 +0007d410  2c 31 2c 32 30 32 34 30  31 30 39 30 30 0a 73 68  |,1,2024010900.sh|
#22 0.395 +0007d420  69 6d 2c 34 0a 67 72 75  62 2c 33 0a 67 72 75 62  |im,4.grub,3.grub|
#22 0.395  0007d430  2e 64 65 62 69 61 6e 2c  34 0a 00 73 62 61 74 2c  |.debian,4..sbat,|
#22 0.395  0007d440  31 2c 32 30 32 34 30 34  30 39 30 30 0a 73 68 69  |1,2024040900.shi|
#22 0.395  0007d450  6d 2c 34 0a 67 72 75 62  2c 34 0a 67 72 75 62 2e  |m,4.grub,4.grub.|
#22 ERROR: executor failed running [/bin/sh -c diff -u orig build]: exit code: 1
------
 > [18/20] RUN diff -u orig build:
#22 0.395  0007d3e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395  0007d3f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
#22 0.395  0007d400  00 00 00 00 08 00 00 00  37 00 00 00 73 62 61 74  |........7...sbat|
#22 0.395 -0007d410  2c 31 2c 32 30 32 33 30  31 32 39 30 30 0a 73 68  |,1,2023012900.sh|
#22 0.395 -0007d420  69 6d 2c 32 0a 67 72 75  62 2c 33 0a 67 72 75 62  |im,2.grub,3.grub|
#22 0.395 +0007d410  2c 31 2c 32 30 32 34 30  31 30 39 30 30 0a 73 68  |,1,2024010900.sh|
#22 0.395 +0007d420  69 6d 2c 34 0a 67 72 75  62 2c 33 0a 67 72 75 62  |im,4.grub,3.grub|
#22 0.395  0007d430  2e 64 65 62 69 61 6e 2c  34 0a 00 73 62 61 74 2c  |.debian,4..sbat,|
#22 0.395  0007d440  31 2c 32 30 32 34 30 34  30 39 30 30 0a 73 68 69  |1,2024040900.shi|
#22 0.395  0007d450  6d 2c 34 0a 67 72 75 62  2c 34 0a 67 72 75 62 2e  |m,4.grub,4.grub.|

review helper: https://github.com/jclab-joseph/other-shim-reviews/tree/master/20240730-lernstick-shim-amd64-20240721

hash also different:

review: 6544e9cee3a3308c9090875a8edb40be648b222db7c17f09ab4801c5b4ef5268
reproduced: 1043721c968fd18578ab8b66a022e0c105cdd8c0a700bb5b2d94cd7981dd997f

THS-on commented 3 months ago

@jclab-joseph thanks for catching it. I missed updating the Dockerfile to the new tag. Can you try again with https://github.com/Lernstick/shim-review/tree/lernstick-shim-amd64-20240730?

jclab-joseph commented 3 months ago

Review of reproducibility for lernstick-shim-amd64-20240730

review helper : https://github.com/jclab-joseph/other-shim-reviews/tree/master/20240730-lernstick-shim-amd64-20240730

shim

[X] Version : 15.8
[X] Reproducible
[X] SBAT of the built file matches the review request
[X] BAT entries from shim looks fine : shim.lernstick,1,Lerntsick,shim,15.8,https://github.com/Lernstick/shim
NX flag not set

Patches:

$ git clone -b lernstick_15.8-2-lernstick https://github.com/Lernstick/shim.git shim-dest
$ diff -urN shim-15.8 shim-dest/ | grep -E '^--- |^\+\+\+ ' | grep -v -E '/.git/|/debian/'
--- shim-15.8/commit    2024-01-23 04:18:05.000000000 +0900
+++ shim-dest/commit    1970-01-01 09:00:00.000000000 +0900
--- shim-15.8/gnu-efi/lib/Makefile.orig 2024-01-23 04:18:05.000000000 +0900
+++ shim-dest/gnu-efi/lib/Makefile.orig 1970-01-01 09:00:00.000000000 +0900

=> not changed actually.

Patch files for debian packaging:

0001-sbat-Add-grub.peimage-2-to-latest-CVE-2024-2312.patch : same as https://salsa.debian.org/efi-team/shim/-/blob/master/debian/patches/0001-sbat-Add-grub.peimage-2-to-latest-CVE-2024-2312.patch
0002-sbat-Also-bump-latest-for-grub-4-and-to-todays-date.patch : same as https://salsa.debian.org/efi-team/shim/-/blob/master/debian/patches/0002-sbat-Also-bump-latest-for-grub-4-and-to-todays-date.patch?ref_type=heads

certificate

Not After: Apr 22 11:06:04 2031 GMT
self-signed 2048 cert and valid for almost 7 years
[X] The keys are stored on a FIPS 140-2 certified SmartCard (YubiKey FIPS Model 0010). Only Ronny Standtke has access to this SmartCard.

grub

debian's 2.12-2~deb13u1 based.

$ git clone -b debian/2.12-2_deb13u1 https://salsa.debian.org/grub-team/grub.git grub-src
$ git clone -b lernstick/2.12 https://github.com/Lernstick/grub.git grub-dest
$ diff -urN grub-src grub-dest/ | grep -E '^--- |^\+\+\+ ' | grep -v -E '/.git/'
--- grub-src/debian/build-efi-images    2024-07-30 13:51:27.032002336 +0900
+++ grub-dest/debian/build-efi-images   2024-07-30 13:50:55.189784241 +0900
--- grub-src/debian/changelog   2024-07-30 13:51:27.032002336 +0900
+++ grub-dest/debian/changelog  2024-07-30 13:50:55.189784241 +0900
--- grub-src/debian/sbat.lernstick.csv.in   1970-01-01 09:00:00.000000000 +0900
+++ grub-dest/debian/sbat.lernstick.csv.in  2024-07-30 13:50:55.265779988 +0900

...

$ diff -urN grub-src/debian/build-efi-images grub-dest/debian/build-efi-images
--- grub-src/debian/build-efi-images    2024-07-30 13:51:27.032002336 +0900
+++ grub-dest/debian/build-efi-images   2024-07-30 13:50:55.189784241 +0900
@@ -142,6 +142,7 @@
    peimage
    png
    probe
+   read
    reboot
    regexp
    search

--- grub-src/debian/sbat.debian.csv.in  2024-07-30 13:51:27.111997860 +0900
+++ grub-dest/debian/sbat.lernstick.csv.in  2024-07-30 13:50:55.265779988 +0900
@@ -2,4 +2,5 @@
 grub,4,Free Software Foundation,grub,@UPSTREAM_VERSION@,https://www.gnu.org/software/grub/
 grub.debian,5,Debian,grub2,@DEB_VERSION@,https://tracker.debian.org/pkg/grub2
 grub.debian13,1,Debian,grub2,@DEB_VERSION@,https://tracker.debian.org/pkg/grub2
+grub.lernstick,1,Debian,grub2,@DEB_VERSION@,https://github.com/Lernstick/grub
 grub.peimage,2,Canonical,grub2,@DEB_VERSION@,https://salsa.debian.org/grub-team/grub/-/blob/master/debian/patches/secure-boot/efi-use-peimage-shim.patch

SherifNagy commented 3 months ago

Review of lernstick-shim-amd64-20240730

Security contacts looks good, keys cross signed
Keys are stored in FIPS HSM including 3rd party kernel modules

Shim

Uses upstream 15.8 and source hashes matches original hashes
SBAT entries from shim looks fine
Patched cherry picked from upstream for Debian distros "see note"
Vendor SBAT entry is at 1 same as previous submissions

Binaries are reproducible using the container image

STEP 20/20: RUN sha256sum /shim/shim*.efi /shim-review/$(basename /shim/shim*.efi)
6544e9cee3a3308c9090875a8edb40be648b222db7c17f09ab4801c5b4ef5268  /shim/shimx64.efi
6544e9cee3a3308c9090875a8edb40be648b222db7c17f09ab4801c5b4ef5268  /shim-review/shimx64.efi
COMMIT
--> 13e149ed9e9c
13e149ed9e9c35de2dbfecf991d8931cda10cae369b4b48890dc0a4717b63abf

NX flag is not set, because the chain is not yet ready
Self signed 2048 bit cert and valid for almost 7 years "see note"

GRUB2

SBAT looks fine (keeps upstream Debian grub2)
Version currently does include NTFS patches, NTFS module being signed and shipped so grub sbat entry set to grub,4
Module list sound fine

Kernel

Ephemeral keys are used for signing kernel modules and HSM for 3rd party modules
Lockdown patches are included from Debian and configurations enabled

Note

The issue mention the usage of SBAT_AUTOMATIC_DATE=2024010900 however this isn't correct for latest tag, it is set to 2023012900

7sbat,1,2023012900
shim,2
grub,3
grub.debian,4
sbat,1,2024040900
shim,4
grub,4
grub.peimage,2

and from the provided build logs

dh_auto_build -- INSTALL=install RELEASE=15.8 COMMIT_ID=657b2483ca6e9fcf2ad8ac7ee577ff546d24c3aa MAKELEVEL=0 ENABLE_HTTPBOOT=true VENDOR_CERT_FILE=debian/lernstick-uefi-ca.der VENDOR_DBX_FILE=dbx.esl EFIDIR=lernstick CROSS_COMPILE=x86_64-linux-gnu- CC=x86_64-linux-gnu-gcc-12  SBAT_AUTOMATIC_DATE=2023012900

The CA size is 2048 not 4096

@THS-on can you confirm that my notes are correct? other than this, LGTM

THS-on commented 3 months ago

@SherifNagy

The issue mention the usage of SBAT_AUTOMATIC_DATE=2024010900 however this isn't correct for latest tag, it is set to 2023012900

Yes, we set it to 2023012900. Added clarification and created a new tag: https://github.com/Lernstick/shim-review/tree/lernstick-shim-amd64-20240731

The CA size is 2048 not 4096

Correct our CA is 2048bit