rhboot/shim-review

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

Closed this issue · 6 comments

steve-mcintyre commented

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

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

  • https://github.com/steve-mcintyre/shim-review/tree/debian-10-shim-amd64-20240528 for amd64
  • https://github.com/steve-mcintyre/shim-review/tree/debian-10-shim-i386-20240528 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)?

#315 is the last successful shim review.
This review is almost identical to the review for Debian 13 at #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

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

Note because this is similar to #415 I'm just reviewing the differences.

Shim

  • Based on 15.8
    • Includes revocation patches, same as #415
    • Includes build patches for aarch64 (ok). Note: there is no intention to submit an aarch64 version to MS for signing
  • NX bit is disabled
  • Builds are reproducible
#24 0.441 8df1d5590c44541a31248bade1fa02a6b953248c1f48eba582115e42a623f2ba  /shim/shimx64.efi
#24 0.456 8df1d5590c44541a31248bade1fa02a6b953248c1f48eba582115e42a623f2ba  /shim-review/shimx64.efi
#24 0.423 f54982beab2158ec8440112297ca60a2c430151b9530a6b098d54f10ba0b2fa4  /shim/shimia32.efi
#24 0.432 f54982beab2158ec8440112297ca60a2c430151b9530a6b098d54f10ba0b2fa4  /shim-review/shimia32.efi

GRUB2 and fwupd

Linux

  • Based on 4.19.249
  • has the usual lockdown patches
  • No ephemeral signing used (still ok)

Questions and Notes

  • Please clarify the SBAT entry for GRUB2 and where the old version in the submission is coming from
Fabian-Gruenbichler commented

(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

@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

  • debian-10-shim-i386-20240528
  • debian-10-shim-amd64-20240528

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

  • 👍1
THS-on commented

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

NeilHanlon commented

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

  • Well known vendor submitting update for existing shim

Shim

  • SBAT entries looks good
  • NX flag not set
  • Build reproduces
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

  • sbat entries for grub are 4, as expected
  • fwupd version and sbat look ok
  • no unexpected grub modules

Linux

  • based on 4.19.249 with lockdown patches applied (confirmed)
  • no ephemeral keys used

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.

  • 👍1
steve-mcintyre commented

We have signed shims now, closing.