Shim 15.8 for Endless OS

[dbnicholson]

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

• completed README.md file with the necessary information
  https://github.com/endlessm/shim-review/blob/endless-shim-x64-20240912/README.md
• shim.efi to be signed
  https://github.com/endlessm/shim-review/blob/endless-shim-x64-20240912/shimx64.efi
• public portion of your certificate(s) embedded in shim (the file passed to VENDOR_CERT_FILE)
  https://github.com/endlessm/shim-review/blob/endless-shim-x64-20240912/endless-uefi-ca.der
• binaries, for which hashes are added to vendor_db ( if you use vendor_db and have hashes allow-listed )
  We do not use the vendor_db functionality.
• any extra patches to shim via your own git tree or as files
  https://github.com/endlessm/shim/tree/endless/15.8-1_deb12u1endless2/debian/patches
• any extra patches to grub via your own git tree or as files
  The full set of patches relative to 2.06 can be viewed at
  endlessm/grub@grub-2.06...Release_6.0.2 The
  commits with [DEB] prefix come from Debian's 2.06-13+deb12u1 release.
  All other commits are Endless specific.
• build logs
  https://github.com/endlessm/shim-review/blob/endless-shim-x64-20240912/buildlog.txt
• a Dockerfile to reproduce the build of the provided shim EFI binaries
  https://github.com/endlessm/shim-review/blob/endless-shim-x64-20240912/Dockerfile

---

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

---

https://github.com/endlessm/shim-review/releases/tag/endless-shim-x64-20240912

---

What is the SHA256 hash of your final SHIM binary?

---

7859e02e1fc6dff8e2b221dfcbfaffcb6d1e95e2acb65403e5db7c849f9221cd

---

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

---

#176

---

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

---

The security contacts have not changed, however I can't find any indication of
verification in any previous reviews. Those are:

#176
#105
#61

[es-fabricemarie]

• build is reproducible (sha256sum 7859e02e1fc6dff8e2b221dfcbfaffcb6d1e95e2acb65403e5db7c849f9221cd shimx64.efi)
• revoked certs in dbx:
  • cert 1:
    • Issuer/Subject: CN=Endless Secure Boot CA
    • Not After: Nov 1 23:41:42 2016 GMT
    • SHA1 Fingerprint 2B:F1:FA:0D:1D:0C:AB:5E:EB:B1:B8:95:09:94:EF:4A:4B:9C:5C:00
  • cert 2:
    • Issuer/Subject: CN=Endless Secure Boot CA
    • Not After: Dec 1 17:06:09 2019 GMT
    • SHA1 Fingerprint DA:E6:9E:A6:CA:A2:E8:13:59:FF:ED:44:FE:7D:CF:5B:2A:84:5E:2B
  • cert 3:
    • Issuer/Subject: CN=Endless Secure Boot CA
    • Not After: Dec 3 17:52:41 2029 GMT
    • SHA1 Fingerprint 63:FC:D2:3A:E3:17:14:81:0B:71:9A:95:5E:53:27:E1:A9:F6:0A:E9
• embedded cert is a CA:
  • Subject/Issue :CN=Endless Secure Boot CA
  • valid until May 30 03:15:05 2052 GMT (roughly 28 years)
  • 2048 bit RSA key
  • key in HSM
• NX bit disabled (DllCharacteristics 00000000)
• sbat sections look ok

Issue/question:

• build docker pulls shim from https://github.com/endlessm/shim/tree/endless/15.8-1_deb12u1endless1
  it should pull the official tarball from https://github.com/rhboot/shim/releases/tag/15.8 and apply patches atop of that.
• additional shim patches present will need more eyeballs/reviews.

[dbnicholson]

@es-fabricemarie thanks for the review!

• build docker pulls shim from https://github.com/endlessm/shim/tree/endless/15.8-1_deb12u1endless1
  it should pull the official tarball from https://github.com/rhboot/shim/releases/tag/15.8 and apply patches atop of that.

It actually does use the official tarball, but it restores it using pristine-tar from this data. I can update the Dockerfile so it shows the actual tarball used during the build. https://deb.endlessos.org/debian/pool/core/s/shim/shim_15.8-1~deb12u1endless1.dsc shows the data from the generated source package including the checksum of the tarball:

Checksums-Sha256:
 a79f0a9b89f3681ab384865b1a46ab3f79d88b11b4ca59aa040ab03fffae80a9 2315201 shim_15.8.orig.tar.bz2
 fcbd64a83973fdcec6a0f98aef07ba5e288ff2d07f1050bc6f9c3bdd875caed3 228 shim_15.8.orig.tar.bz2.asc
 baf1852fba47e2bd401d81169b6899b0b18b8fa93af6b7d1680b54dfe4c967fd 65700 shim_15.8-1~deb12u1endless1.debian.tar.xz

[steve-mcintyre]

The security contacts have not changed, however I can't find any indication of verification in any previous reviews. Those are:

#176 #105 #61

Let's do this from scratch then...

Mails on the way.

[ramcq]

I got: distention knuckling unexceptionable hydras tourneys comeliness fraternize Villa cosmetologists domains

[wjt]

craws showbiz switch Dramamine unannounced internists babysitters tared quantum jouncing

The verb “to jounce” is new to me.

Thanks!

[dbnicholson]

• additional shim patches present will need more eyeballs/reviews.

In case this is holding up the review, I'd like to discuss these. There are 4 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
• 0003-Revert-fallback-work-around-the-issue-of-boot-option.patch
• 0004-fallback-Clean-up-duplicate-boot-entries.patch

The first 2 are backports from upstream post-15.8 and inherited from Debian. They simply make the newer SBAT policies available for use. The second 2 are our real downstream patches. They're both against fallback and are trying to address the same issue. 0003-Revert-fallback-work-around-the-issue-of-boot-option.patch is really an optimization once 0004-fallback-Clean-up-duplicate-boot-entries.patch is applied. So, really 0004-fallback-Clean-up-duplicate-boot-entries.patch is the downstream change we make.

Endless OS is always installed by flashing a raw disk image containing filesystems. By definition, the filesystems in the disk image contain UUIDs. During first boot the UUIDs are randomized since they'd no longer be unique if every Endless OS installation had the same partition UUIDs. However, changing the ESP UUID means that existing EFI load options referencing that partition are now invalid. What currently happens is this:

1. User boots Endless OS for the first time.
2. Shim finds no valid load options and loads fallback.
3. Fallback finds endless/BOOT<arch>.csv, creates a new load option referencing the current ESP UUID and loads it.
4. In the first boot initramfs, the disk partitions are updated. The UUIDs are replaced and the main partition is expanded to fill the disk. At this point, the load option just created by fallback has become invalid since it's referencing the old ESP UUID.
5. On the second boot, shim again finds no valid load options and loads fallback.
6. Fallback creates a new load option as in step 3, but now it's referencing the new ESP UUID.
7. In the second boot initramfs, the disk partitions are left as is since they've already been changed.
8. On the third and subsequent boot, shim finds a valid load option and loads it.

The problem 0004-fallback-Clean-up-duplicate-boot-entries.patch is trying to address is after step 4. If the invalid load option isn't removed, then there will be 2 entries for Endless OS, one of which is invalid. That's a poor user experience, so what the patch does is add a routine to delete any existing options with the desired title before adding the new load option. That means you can only have one load option with a given title created by fallback. If your system had another ESP or there was another vendor directory and either provided a load option with the same title, they would be deleted.

This works OK for Endless OS since that type of advanced functionality isn't supported, but it means the patch isn't really upstreamable. I've been working on a tool that updates the partition UUID in an existing load option, which we'd run at step 4 when the UUID is changed. I'm actually surprised that kind of tool doesn't exist (the closest I've seen is to manually do it with efibootmgr), but maybe people changing their ESP UUID is rare enough that the problem is ignored. Anyways, if it would speed up review to drop that patch, I can try to finish up my tool and get it integrated in Endless OS.

[dbnicholson]

I was able to finish up the work that handles updating the UUID in the load option after the ESP partition UUID is changed. The program is here if anyone cares. With that I was able to update our shim package to drop our 2 downstream fallback patches (0003-Revert-fallback-work-around-the-issue-of-boot-option.patch and 0004-fallback-Clean-up-duplicate-boot-entries.patch).

The shim binary is exactly the same as before since the patches were against fallback rather than shim. Still, I've updated our shim-review repo with the current build log. The new tag is https://github.com/endlessm/shim-review/releases/tag/endless-shim-x64-20240912. @es-fabricemarie would you mind reviewing again? Everything should be the same as before except for the 2 dropped patches.

[es-fabricemarie]

Sorry for the delay @dbnicholson

• My earlier comments stand, my questions are resolved.
• Build reproduces fine (7859e02e1fc6dff8e2b221dfcbfaffcb6d1e95e2acb65403e5db7c849f9221cd)
• Built from original 15.8 tarball
• Apply 2 patches inherited from Debian

This shim looks good to go to me.
You'll need an official approver to move this along, maybe @steve-mcintyre if you have time? (This is a Debian based shim)

[evilteq]

I really went mad looking up and down for the 0003 and 0004 patches :P

Built using gpb's black magic against a github repo fork, and the generated orig tarball matches the mainstream one.
I was able to reproduce it it 7859e02e1fc6dff8e2b221dfcbfaffcb6d1e95e2acb65403e5db7c849f9221cd.

Nothing out of the ordinary, looks good to me!

[THS-on]

Review of endless-shim-x64-20240912

• Endless OS had a Shim signed before and needs it to include their Linux and GRUB patches
• Security contacts have been verified

Shim

• Embedded CA is valid till 2052 (RSA 2024) (ok)
• SBAT looks good
• Debian's patches are included
• NX bit not set (still fine)
• Some old signing keys are revoked by vendor_dbx. See: endlessm/shim@5c46ed1
• Keys are stored in a HSM or offline with physical access protection
• Is reproducible using Dockerfile:

#8 0.274 7859e02e1fc6dff8e2b221dfcbfaffcb6d1e95e2acb65403e5db7c849f9221cd  /shim-review/shimx64.efi
#8 0.277 7859e02e1fc6dff8e2b221dfcbfaffcb6d1e95e2acb65403e5db7c849f9221cd  /shim-build/shimx64.efi

GRUB2

• Based on Debian 2.06-13+deb12u1 version
• SBAT entry looks fine
• SBAT level set to 4, Looks like NTFS patches are included
• Module list looks fine (ntfs is included, so the patches are also required)

Linux

• Kernel is based on the Ubuntu 6.5 kernel + custom patches
• Lockdown patches are included and enabled: https://github.com/endlessm/linux/blob/7da2e9bf2b166c74c80f8e64275a09d6c488f8d8/debian.master/config/annotations#L7214

Questions

• Is your main CA key also in an HSM or differently?
• Can you provide a easy way to compare your GRUB2 patches against the Debian package or upstream GRUB2? Mostly interested in that all the CVE patches are applied and what custom modifications are made to GRUB2 that are not packaging/config related
• Note because you apply the Debian patches, you are going to automatically revoke your old Shim when booting a new one. (should be the same revocation entries as via Windows upates)

[dbnicholson]

Questions

• Is your main CA key also in an HSM or differently?

The CA key is kept on an offline encrypted disk that only a couple people have access to and can decrypt.

• Can you provide a easy way to compare your GRUB2 patches against the Debian package or upstream GRUB2? Mostly interested in that all the CVE patches are applied and what custom modifications are made to GRUB2 that are not packaging/config related

The way we generate the grub source package is kinda unusual. We take all of Debian's patches and apply them directly as commits. Then we add our own changes on top so that we can treat it as a normal git repo instead of using patch files. There are better ways to do that nowadays, but nobody has taken the time to rework our build system.

Anyway, the full set of commits on top of grub 2.06 as of today is here. All of Debian's patches are prefixed with [DEB] and then ours start after that. This is based on Debian's 2.06-13+deb12u1 version. You can see it in their format here with their patches in particular here.

We haven't added any additional security patches on top of what Debian has done. The main thing we add is the blscfg module from RedHat. That and a couple other command tweaks allows us to use a fixed grub.cfg with upgrades managed by ostree.

• Note because you apply the Debian patches, you are going to automatically revoke your old Shim when booting a new one. (should be the same revocation entries as via Windows upates)

Yeah, that should be fine. We don't plan to support rolling back to an earlier shim. The people that need to do something like that can also just change the SBAT policy. Not coincidentally, Windows Update erroneously setting the SBAT policy in our dual boot cases is what spurred getting shim updated.

[dbnicholson]

@THS-on anything else I can provide to answer your questions?

[THS-on]

@dbnicholson had a look at the GRUB2 patches and they match the ones that are from Debian and required for SBAT level 4.

Regarding the custom ones:

• Are you planning to upstream the additional options that you added? I.e (endlessm/grub@90a1722, endlessm/grub@dae987d, endlessm/grub@6ad3637)
• I would like to have another set of eyes on endlessm/grub@fc161b3. (It looks fine for me though)

The rest LGTM

[dbnicholson]

I didn't develop those patches but I agree it would be good to upstream them. I can start an effort to do that. We're usually good about upstreaming our patches for no other reason than it eased our work when upgrading. I don't know why we haven't made an effort with grub. I would guess that at the time most of those were developed, grub upstream was in a different state.

[steve-mcintyre]

Looks good here with enough positive reviews, marking accepted

[dbnicholson]

Thank you! @THS-on thanks for raising the flag on grub. For next time I hope that patch queue can be much smaller.