rhboot / shim-review

Reviews of shim
66 stars 130 forks source link

Debian GNU/Linux 10 (buster LTS) shim-15.8-1 x64 and ia32 #418

Closed steve-mcintyre closed 5 months ago

steve-mcintyre commented 6 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/steve-mcintyre/shim-review/tree/debian-10-shim-amd64-20240513 for amd64 - https://github.com/steve-mcintyre/shim-review/tree/debian-10-shim-i386-20240513 for i386

The latter simply includes a change to the Dockerfile to request an i386 Docker image for building.


What is the SHA256 hash of your final SHIM binary?


f54982beab2158ec8440112297ca60a2c430151b9530a6b098d54f10ba0b2fa4  shimia32.efi
8df1d5590c44541a31248bade1fa02a6b953248c1f48eba582115e42a623f2ba  shimx64.efi

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


https://github.com/rhboot/shim-review/issues/315 is the last successful shim review. This review is almost identical to the review for Debian 13 at https://github.com/rhboot/shim-review/issues/415 .


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


Pass - we've been submitting shims for years!

THS-on commented 6 months ago

Review for debian-10-shim-amd64-20240513 and debian-10-shim-i386-20240513

Note because this is similar to https://github.com/rhboot/shim-review/issues/415 I'm just reviewing the differences.

Shim

GRUB2 and fwupd

Linux

Questions and Notes

Fabian-Gruenbichler commented 6 months ago

(neither an official reviewer, nor part of the teams submitting this, but I am a DD and submitted here as part of a Debian derivative that is based on Debian's secure boot implementation/stack)

I think the commit title was just forgotten to be updated, the grub variants in buster and bullseye are almost identical modulo toolchain/build environment (and seemingly, one unrelated packaging bug fix that was not backported ;)).

you can use the following (on a Debian based distro) to check that:

$ mkdir /tmp/compare
$ cd /tmp/compare
$ mkdir bullseye
$ mkdir buster
$ dget http://security.debian.org/pool/updates/main/g/grub2/grub2_2.06-3\~deb10u4.dsc
$ mv grub* buster
$  dget http://security.debian.org/pool/updates/main/g/grub2/grub2_2.06-3\~deb11u6.dsc
$ mv grub* bullseye
$ diffoscope buster/grub2-2.06 bullseye/grub2-2.06

Debian mostly doesn't care about git contents or metadata, since uploads are based on signed tarballs+metadata (https://tracker.debian.org/news/1468057/accepted-grub2-206-3deb10u4-source-into-oldoldstable/)

I guess the SBAT data in the submission was wrongly copy-pasted from an outdated source, but I'll leave it for Steve to confirm :)

steve-mcintyre commented 6 months ago

@THS-on thanks for your review and finding my silly mistake in the last of our 4 submissions.

I grabbed the SBAT data from the last version of grub2 uploaded for the normal buster release, which was grub-efi-amd64-signed_1+2.06+3~deb10u1_amd64.deb. The correct package for buster is now the version in the buster-security archive (grub-efi-amd64-signed_1+2.06+3~deb10u4_amd64.deb). The SBAT data from that package is:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,4,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/
grub.debian,4,Debian,grub2,2.06-3~deb10u4,https://tracker.debian.org/pkg/grub2

As you also identified, it looks like @kukrimate made a mistake in the commit message for that build. It's easily done - it looks like just a cut-and-paste error from the Debian 11 (bullseye) build.

I've just updated the submission now to fix the SBAT data error. New tags are

with just the 32-bit/64-bit docker image difference as before

THS-on commented 6 months ago

Now looks good! Can you also update the top comment to the new tags?

NeilHanlon commented 5 months ago

Review for debian-10-shim-amd64-20240528 and debian-10-shim-i386-20240528

Shim

STEP 23/23: RUN sha256sum /shim/shim*.efi /shim-review/$(basename /shim/shim*.efi)
8df1d5590c44541a31248bade1fa02a6b953248c1f48eba582115e42a623f2ba  /shim/shimx64.efi
8df1d5590c44541a31248bade1fa02a6b953248c1f48eba582115e42a623f2ba  /shim-review/shimx64.efi
STEP 23/23: RUN sha256sum /shim/shim*.efi /shim-review/$(basename /shim/shim*.efi)
f54982beab2158ec8440112297ca60a2c430151b9530a6b098d54f10ba0b2fa4  /shim/shimia32.efi
f54982beab2158ec8440112297ca60a2c430151b9530a6b098d54f10ba0b2fa4  /shim-review/shimia32.efi

Grub and fwupd

Linux

Verdict

While I am not an official reviewer, this shim submission appears to be in order. It reproduces on the binary level and the information contained in the submissions are both correct and sufficient to fulfil the requirements.

steve-mcintyre commented 5 months ago

We have signed shims now, closing.