Review request of shim-15+dev76.f60ba43-13 for Endless OS

https://phabricator.endlessm.com/T30651

BUILDING.txt

@@ -1,9 +1,9 @@
-To reproduce the build in the attached container defined by the attached
-Dockerfile run:
+To reproduce the build in a container defined by the attached Dockerfile, run:
 
 ```
-sudo docker build .
+./build.sh
 ```
 
-At the end of the process the SHA256 checksum of shimx64.efi that was just
-built will be printed.
+This will print the SHA256 checksum of the shimx64.efi binary that was just
+built, as well as copy the file to the current directory and verify its
+checksum against the attached shimx64.efi.sha256sum at the end of the process.

Dockerfile

@@ -4,16 +4,16 @@ RUN echo "deb-src http://deb.debian.org/debian buster main" > /etc/apt/sources.l
 RUN apt-get update -y
 RUN DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends build-essential devscripts git
 RUN apt-get build-dep -y shim
-RUN git clone https://github.com/endlessm/shim.git shim-15+dev16.b59aab6
-WORKDIR /shim-15+dev16.b59aab6
+RUN git clone https://github.com/endlessm/shim.git shim-15+dev76.f60ba43-13
+WORKDIR /shim-15+dev76.f60ba43-13
 RUN git config user.email "root@example.com"
 RUN git merge --allow-unrelated-histories -m "Import the packaging bits into master" origin/debian-master
 RUN echo "1.0" > debian/source/format
 RUN echo "--compression=gzip" > debian/source/options
-RUN dch -v 15+dev16.b59aab6-5 -D eos --force-distribution 'Automatic release from git (15+dev16.b59aab6-5)'
+RUN dch -v 15+dev76.f60ba43-13 -D eos --force-distribution 'Automatic release from git (15+dev76.f60ba43-13)'
 RUN rm -rf .git
 RUN DEB_VENDOR=endless dpkg-buildpackage -us -uc
 WORKDIR /
-RUN dpkg-deb -x shim-efi-image_15+dev16.b59aab6-5_amd64.deb shim-efi-image
+RUN dpkg-deb -x shim-efi-image_15+dev76.f60ba43-13_amd64.deb shim-efi-image
 RUN cp shim-efi-image/boot/efi/EFI/endless/shimx64.efi .
 RUN sha256sum shimx64.efi

ISSUE_TEMPLATE.md

@@ -1,35 +1,36 @@
 Make sure you have provided the following information:
 
  - [X] link to your code branch cloned from rhboot/shim-review in the form user/repo@tag
-   https://github.com/endlessm/shim-review/tree/endless-shim-x64-20190410
+   https://github.com/endlessm/shim-review/tree/endless-shim-x64-20200819
  - [X] completed README.md file with the necessary information
-   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20190410/README.md
+   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20200819/README.md
  - [X] shim.efi to be signed
-   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20190410/shimx64.efi
+   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20200819/shimx64.efi
  - [X] public portion of your certificate embedded in shim (the file passed to VENDOR_CERT_FILE)
-   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20190410/endless-uefi-ca.der
+   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20200819/endless-uefi-ca.der
  - [X] any extra patches to shim via your own git tree or as files
-   The last 16 commits on https://github.com/endlessm/shim/commits/master.
+   The last 21 commits on https://github.com/endlessm/shim/commits/master.
  - [X] any extra patches to grub via your own git tree or as files
    The grub source code we use can be found on the master branch of
-   https://github.com/endlessm/grub. We are based on tag grub-2.02 from
-   upstream.
+   https://github.com/endlessm/grub. We are based on tag grub-2.04 from
+   upstream, and Debian's package version 2.04-9.
  - [X] build logs
-   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20190410/logs.txt
+   https://github.com/endlessm/shim-review/blob/endless-shim-x64-20200819/logs.txt
 
 
 ###### What organization or people are asking to have this signed:
-Endless Mobile, Inc.
-https://endlessos.com/
+Endless OS Foundation LLC
+https://endlessos.org/
 
 ###### What product or service is this for:
 Endless OS.
 
 ###### What is the origin and full version number of your shim?
-Our code is based on upstream tag `15` with a few changes to the fallback
-program. The packaging bits are based on the packaging from Debian Buster,
-`version 0.9+1474479173.6c180c6-1`. The full version of our package is
-`15+dev16.b56aab6-7bem1`.
+Our code is based on upstream commit `a4a1fbe7`, plus the same patches shipped
+by Ubuntu on version `15+1552672080.a4a1fbe-0ubuntu2` and a few
+Endless-specific changes to the fallback program. The packaging bits are based
+on the packaging from Ubuntu, version `15+1552672080.a4a1fbe-0ubuntu2`. The
+full version of our package is `15+dev76.f60ba43-13`.
 
 ###### What's the justification that this really does need to be signed for the whole world to be able to boot it:
 Endless OS is a Linux distribution available for anyone to download on
@@ -49,10 +50,12 @@ https://fedoraproject.org/wiki/User:Pjones/SecureBootSmartCardDeployment
 No.
 
 ###### What is the origin and full version number of your bootloader (GRUB or other)?
-We use GRUB as the bootloader, with the Secure Boot patchset from Red Hat /
-Fedora which adds integration between GRUB and shim. The grub source code we
-use can be found on the master branch of https://github.com/endlessm/grub. We
-are based on tag grub-2.02 from upstream.
+We use GRUB2 as the bootloader, with Debian's package version 2.04-9 as base,
+including their downstream patches.
+
+The GRUB2 source code we use can be found at https://github.com/endlessm/grub,
+on branches master (for code) and debian-master (for packaging). Our master
+branch is based on tag grub-2.04 from upstream.
 
 ###### If your SHIM launches any other components, please provide further details on what is launched
 Our shim does not load any other components.
@@ -61,15 +64,19 @@ Our shim does not load any other components.
 N/A.
 
 ###### Does your SHIM load any loaders that support loading unsigned kernels (e.g. GRUB)?
-No, our grub does not allow loading unsigned kernels when secure boot is
-enabled. Our grub EFI binary only includes the `linuxefi` module, which
-verifies the kernel via shim, and does not include the `linux` module.
+No, the GRUB version we ship does not allow loading unsigned kernels under
+secure boot. The `linux` loader in our grub EFI binary always hands-off loading
+to the `linuxefi` module, which verifies the kernel via the shim protocol under
+secure boot.
 
 ###### What kernel are you using? Which patches does it includes to enforce Secure Boot?
-Linux 5.0 with secure boot support.
+We are based on Ubuntu's kernel 5.4.0-42.46, which in includes the most recent
+fixes for secure boot enforcement.
 
 ###### What changes were made since your SHIM was last signed?
-Rebased on a newer upstream version.
+Rebased on a newer upstream version. All signing certificates used to sign
+previous versions of GRUB or the Linux kernel are being included in shim's
+internal `vendor_dbx`, via the build-time macro `VENDOR_DBX_FILE`.
 
 ###### What is the hash of your final SHIM binary?
-e592a7f77ed424c30c7afa4f79a66346b86ea2246bf1394d386fd180a9aa5f6b  shimx64.efi
+4a2f9d9c740c0b7b3c3e0450281bdd024e7a1e24323b42458dadb0443b88a36f  shimx64.efi

README.md

@@ -18,8 +18,8 @@ Here's the template:
 -------------------------------------------------------------------------------
 What organization or people are asking to have this signed:
 -------------------------------------------------------------------------------
-Endless Mobile, Inc.
-https://endlessos.com/
+Endless OS Foundation LLC
+https://endlessos.org/
 
 -------------------------------------------------------------------------------
 What product or service is this for:
@@ -56,66 +56,78 @@ Who is the secondary contact for security updates, etc.
 -------------------------------------------------------------------------------
 What upstream shim tag is this starting from:
 -------------------------------------------------------------------------------
-Upstream tag `15`, commit hash `51413d1deb0df0debdf1d208723131ff0e36d3a3`.
-https://github.com/rhboot/shim/tree/15
+Upstream commit `a4a1fbe7`.
 
 -------------------------------------------------------------------------------
 URL for a repo that contains the exact code which was built to get this binary:
 -------------------------------------------------------------------------------
-https://github.com/endlessm/shim/commits/master
-Branch `master`, commit hash `b59aab6e1e011e02bcc0cbbe53e050a010244797`.
+https://github.com/endlessm/shim/, branch `master`, commit hash `f60ba438`.
 
 -------------------------------------------------------------------------------
 What patches are being applied and why:
 -------------------------------------------------------------------------------
-There are no patches that change the behavior of the shim binary.
-
-There is one patch to remove the use of `git` in the build process because our
-build servers start from a distribuition tarball which does not contain the
-`.git` directory: `af37fc2 Makefile: Don't use git for cleaning`.
-
-All other patches only affect the fallback binary.
-
-- Two patches to make fallback silent by default so we get a flicker-free
-  experience on the fallback path as well.
-  `cc162f1 fallback: Only use VerbosePrint for debug messages`
-  `025982a fallback: Be silent by default`
-
-- Four patches to make the process of debugging the fallback binary easier.
-  `5cc4d65 fallback: Allow defining FALLBACK_VERBOSE at build time`
-  `3ea2c3a fallback: Wait before chainloading as well in verbose mode`
-  `3455285 fallback: Make verbose mode's wait time configurable`
-  `bf2915d fallback: Print original BootOrder value in verbose mode`
-
-- One patch to make fallback check all boot variables instead of only the
-  numbers listed in BootOrder when looking for duplicates, to address a problem
-  where the firmware re-generates BootOrder every boot to only including its
-  pre-determined boot entries, but still leaving user-created boot entries
-  intact (only removing them from BootOrder).
-  `d3a2489 fallback: Consider all Boot* vars when checking for duplicates`
-
-- One patch to always chain-load the newly created boot entry instead of
-  rebooting the system to avoid a reboot loop on firmwares that re-generate
-  BootOrder every boot to only including their pre-determined boot entries.
-  `e3513c8 [Endless] fallback: Always try to chain-load the first entry`
-
-- One small code improvement that removes an un-necessary call to StrLen.
-  `e26c5b1 fallback: Store label size instead of calculating on every use`
-
-- A set of changes to have fallback treat boot entries with the same label as
-  duplicates, and remove any entries that are a duplicate of the entry we are
-  about to create from the CSV file in the fallback path. This is necessary for
-  Endless OS because randomize the partition ids during the first boot, since
-  the partition layout is created by the server at image build time and the
-  image in simply dd'ed to the disk during installation. We explicitly do not
-  support have two Endless OS installations side-by-side on the same machine,
-  even on different disks, so the added restriction of not allowing to boot
-  entries with the same name is not a problem for us.
-  `3e70ad8 [Endless] Partially revert "fallback: work around the issue of boot option creation with AMI BIOS"`
-  `f7c0d9b [Endless] fallback: Clean-up duplicate boot entries`
-  `a81b858 [Endless] fallback: Use a dynamic buffer when list var names`
-  `cb0b03b [Endless] fallback: Print info on GetNextVariableName errors`
-  `f6267f1 [Endless] fallback: Do not remove vars while listing var names`
+We are applying the same patches applied by Ubuntu on version
+`15+1552672080.a4a1fbe-0ubuntu2` plus a few Endless-specific patches to the
+fallback program. No Endless-specific patches change the shim binary.
+
+The full list of commits can be found below:
+
+From Ubuntu's `15+1552672080.a4a1fbe-0ubuntu2`:
+
+- `13631c19 [UBUNTU] Fix OBJ_create() to tolerate a NULL sn and ln`
+- `36170184 [UBUNTU] MokManager: avoid -Werror=address-of-packed-member`
+- `f3b3734d [UBUNTU] tpm: Define EFI_VARIABLE_DATA_TREE as packed`
+- `02c285b8 [UBUNTU] tpm: Fix off-by-one error when calculating event size`
+- `9a5c7e0e [UBUNTU] tpm: Don't log duplicate identical events`
+- `21598b93 [UBUNTU] MokManager: console mode modification for hi-dpi screen devices`
+- `a5c0630a [UBUNTU] Fix a use of strlen() instead of Strlen()`
+
+One small code improvement that removes an un-necessary call to StrLen:
+
+- `b71ce459 fallback: Store label size instead of calculating on every use`
+
+One patch to make fallback check all boot variables instead of only the numbers
+listed in BootOrder when looking for duplicates, to address a problem where the
+firmware re-generates BootOrder every boot to only including its pre-determined
+boot entries, but still leaving user-created boot entries intact (only removing
+them from BootOrder):
+
+- `393df2f4 fallback: Consider all Boot* vars when checking for duplicates`
+
+Two patches to make fallback silent by default so we get a flicker-free
+experience on the fallback path as well:
+
+- `117a7291 fallback: Only use VerbosePrint for debug messages`
+- `3d3327d0 fallback: Be silent by default`
+
+Four patches to improve the debug output from the fallback binary:
+
+- `0e06758c fallback: Print original BootOrder value in verbose mode`
+- `6cd6a597 fallback: Wait before chainloading as well in verbose mode`
+- `5cce553a fallback: Make verbose mode's wait time configurable`
+- `81a7cffe fallback: Allow defining FALLBACK_VERBOSE at build time`
+
+One patch to always chain-load the newly created boot entry instead of
+rebooting the system to avoid a reboot loop on firmwares that re-generate
+BootOrder every boot to only including their pre-determined boot entries:
+
+- `af56d403 [Endless] fallback: Always try to chain-load the first entry`
+
+A set of changes to have fallback treat boot entries with the same label as
+duplicates, and remove any entries that are a duplicate of the entry we are
+about to create from the CSV file in the fallback path. This is necessary for
+Endless OS because randomize the partition ids during the first boot, since the
+partition layout is created by the server at image build time and the image in
+simply dd'ed to the disk during installation. We explicitly do not support have
+two Endless OS installations side-by-side on the same machine, even on
+different disks, so the added restriction of not allowing to boot entries with
+the same name is not a problem for us:
+
+- `acb37f45 [Endless] Partially revert "fallback: work around the issue of boot option creation with AMI BIOS"`
+- `681aa66f [Endless] fallback: Clean-up duplicate boot entries`
+- `e1e66a48 [Endless] fallback: Use a dynamic buffer when list var names`
+- `be188ca9 [Endless] fallback: Print info on GetNextVariableName errors`
+- `f60ba438 [Endless] fallback: Do not remove vars while listing var names`
 
 -------------------------------------------------------------------------------
 What OS and toolchain must we use to reproduce this build?  Include where to find it, etc.  We're going to try to reproduce your build as close as possible to verify that it's really a build of the source tree you tell us it is, so these need to be fairly thorough. At the very least include the specific versions of gcc, binutils, and gnu-efi which were used, and where to find those binaries.
@@ -124,26 +136,30 @@ This was manually built on Debian Buster, to make it possible to reproduce,
 since we currently don't make development tools publicly available on Endless
 OS. The versions of gcc, binutils and gnu-efi are listed bellow.
 
-gcc 8.3.0-4
-binutils 2.31.1-15
-gnu-efi 3.0.9-1
+- gcc 8.3.0-6
+- binutils 2.31.1-16
+- gnu-efi 3.0.9-1
 
-Inspired by Debian's own submission, we are providing a Dockerfile in this repo
+Inspired by Debian's submission, we are providing a Dockerfile in this repo
 that can be used to reproduce the build pulling all dependencies from the
 Debian repositories -- instructions are availabled in BUILDING.txt.
 
 -------------------------------------------------------------------------------
 Which files in this repo are the logs for your build?   This should include logs for creating the buildroots, applying patches, doing the build, creating the archives, etc.
 -------------------------------------------------------------------------------
-https://github.com/endlessm/shim-review/blob/endless-shim-x64-20190410/logs.txt
+https://github.com/endlessm/shim-review/blob/endless-shim-x64-20200819/logs.txt
 
 -------------------------------------------------------------------------------
 Add any additional information you think we may need to validate this shim
 -------------------------------------------------------------------------------
 The `debian` directory with the package building recipes and scripts, vendor
-certificate included in the shim image etc, is available at
-https://github.com/endlessm/shim/commits/debian-master, commit id
-`0ca72677098e173e4d48e67d3d7e9344d25f408c`.
+certificate, vendor DBX contents etc, is available at
+https://github.com/endlessm/shim/commits/debian-master, commit `ce6ec5cd`.
+
+The code and packaging up for review here are based on the same code up for
+review at https://github.com/rhboot/shim-review/issues/92. The Endless-specific
+patches listed here are also present on shim images previously signed by
+Microsoft and shipped with our current releases.
 
-We already ship a previous version of shim with same patches listed above,
-signed by Microsoft, in our currently available releases.
+Our previous community review request can be found at
+https://github.com/rhboot/shim-review/issues/61.

build.sh

@@ -0,0 +1,11 @@
+#!/bin/bash -e
+
+sudo docker build -t endless-shim-review .
+
+id=$(sudo docker create endless-shim-review)
+sudo docker cp $id:shimx64.efi .
+sudo docker rm -v $id
+
+sha256sum -c shimx64.efi.sha256sum
+
+echo "To remove the container image used for this build: sudo docker image rm endless-shim-review"