Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shim 15.4 for CBL-D #203

Closed
9 tasks done
mbearup opened this issue Sep 10, 2021 · 2 comments
Closed
9 tasks done

Shim 15.4 for CBL-D #203

mbearup opened this issue Sep 10, 2021 · 2 comments
Labels
bug Problem with the review that must be fixed before it will be accepted new vendor This is a new vendor

Comments

@mbearup
Copy link

mbearup commented Sep 10, 2021

Make sure you have provided the following information:

  • link to your code branch cloned from rhboot/shim-review in the form user/repo@tag
  • 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 do 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
    https://github.com/mbearup/shim
  • any extra patches to grub via your own git tree or as files
    https://github.com/mbearup/grub2
  • build logs
  • a Dockerfile to reproduce the build of the provided shim EFI binaries
What organization or people are asking to have this signed:

Microsoft Azure

What product or service is this for:

Common Base Linux Delridge (CBL-D)

Please create your shim binaries starting with the 15.4 shim release tar file:
https://github.com/rhboot/shim/releases/download/15.4/shim-15.4.tar.bz2
This matches https://github.com/rhboot/shim/releases/tag/15.4 and contains
the appropriate gnu-efi source.
Please confirm this as the origin your shim.

Yes, we use the shim 15.4 source

What's the justification that this really does need to be signed for the whole world to be able to boot it:

This is for use on Azure VM workloads

How do you manage and protect the keys used in your SHIM?

Keys are stored in a hardware HSM with RBAC permission for signing operations (the key itself is never exposed)

Do you use EV certificates as embedded certificates in the SHIM?

No, we do not use EV certs at this time.

If you use new vendor_db functionality, are any hashes allow-listed, and if yes: for what binaries ?

We don't use vendor_db

Is kernel upstream commit 75b0cea7bf307f362057cc778efe89af4c615354 present in your kernel, if you boot chain includes a Linux kernel ?

Yes

if SHIM is loading GRUB2 bootloader, are CVEs CVE-2020-14372,
CVE-2020-25632, CVE-2020-25647, CVE-2020-27749, CVE-2020-27779,
CVE-2021-20225, CVE-2021-20233, CVE-2020-10713, CVE-2020-14308,
CVE-2020-14309, CVE-2020-14310, CVE-2020-14311, CVE-2020-15705,
( July 2020 grub2 CVE list + March 2021 grub2 CVE list )
and if you are shipping the shim_lock module CVE-2021-3418
fixed ?

Yes, the above CVE's are patched, except CVE-2020-15705 and CVE-2021-3418 which do not apply to our implementation.

"Please specifically confirm that you add a vendor specific SBAT entry for SBAT header in each binary that supports SBAT metadata
( grub2, fwupd, fwupdate, shim + all child shim binaries )" to shim review doc ?
Please provide exact SBAT entries for all SBAT binaries you are booting or planning to boot directly through shim

Yes, we use a vendor-specific SBAT as follows:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,1,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.cbld,1,CBLD,shim,15.4,https://aka.ms/cbld

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,1,Free Software Foundation,grub,2.02,https://www.gnu.org/software/grub/
grub.cbld,1,CBLD,grub2,2.02+dfsg1-20+deb10u4,https://aka.ms/cbld
Were your old SHIM hashes provided to Microsoft ?

We have not previously generated or signed a shim

Did you change your certificate strategy, so that affected by CVE-2020-14372, CVE-2020-25632, CVE-2020-25647, CVE-2020-27749,
CVE-2020-27779, CVE-2021-20225, CVE-2021-20233, CVE-2020-10713,
CVE-2020-14308, CVE-2020-14309, CVE-2020-14310, CVE-2020-14311, CVE-2020-15705 ( July 2020 grub2 CVE list + March 2021 grub2 CVE list )
grub2 bootloaders can not be verified ?

We have not previously signed a shim/grub, so we do not have a previous certificate to account for

What exact implementation of Secureboot in grub2 ( if this is your bootloader ) you have ?
* Upstream grub2 shim_lock verifier or * Downstream RHEL/Fedora/Debian/Canonical like implementation ?

We use the Debian implementation

What is the origin and full version number of your bootloader (GRUB or other)?

Using Grub 2.02 plus Debian patches (2.02+dfsg1-20+deb10u4)

If your SHIM launches any other components, please provide further details on what is launched

The shim only launches grub

If your GRUB2 launches any other binaries that are not Linux kernel in SecureBoot mode,
please provide further details on what is launched and how it enforces Secureboot lockdown

Grub only launches the kernel in SecureBoot mode

If you are re-using a previously used (CA) certificate, you
will need to add the hashes of the previous GRUB2 binaries
exposed to the CVEs to vendor_dbx in shim in order to prevent
GRUB2 from being able to chainload those older GRUB2 binaries. If
you are changing to a new (CA) certificate, this does not
apply. Please describe your strategy.

We have not previously used this CA certificate, nor have we
previously signed grub2. If/when we do so in the future, we will add the hashes.

How do the launched components prevent execution of unauthenticated code?

Signed grub2 includes common secure boot patches that only load signed binaries.
Signed kernel has a common set of lockdown patches.

Does your SHIM load any loaders that support loading unsigned kernels (e.g. GRUB)?

No

What kernel are you using? Which patches does it includes to enforce Secure Boot?

5.4.134

What changes were made since your SHIM was last signed?

Shim has never been signed

What is the SHA256 hash of your final SHIM binary?

185f66b70a01929676ab62ac1cae4cf1b579ac3d22a20e76baec2817cc2e41b8 shimx64.efi

@frozencemetery
Copy link
Member

Hi, the lines at the start of the template are intended as a checklist. It appears you haven't filled any of them out - could you please do so? (Change from - [ ] to - [x] when done.)

@julian-klode julian-klode added the new vendor This is a new vendor label Sep 14, 2021
@julian-klode julian-klode added the bug Problem with the review that must be fixed before it will be accepted label Oct 6, 2021
@julian-klode
Copy link
Collaborator

  • shim-review repository and tag to review have not been specified
  • specified shim and grub repositories are hard to match to upstream, given they've been built from scratch
  • it is probably worth preserving the .debian entries for the grub SBAT, if you just rebuild it, such that you get revocations automatically when Debian does in case someone does miss revoking this one.
  • I cannot comment if the kernel is adequate due to lack of kernel side knowledge, and interaction with patch set
  • You may want to pick additional patches from [tracking] shim 15.4 critical regressions #165

@mbearup mbearup closed this as completed Oct 15, 2021
tedbrandston added a commit to tedbrandston/shim-review that referenced this issue Mar 21, 2022
* Change "Make sure you have provided the following information" to
  "Confirm the following are included in your repo, checking each box".
  "Make sure" makes it seem like the checklist is provided for your
  convenience, if you want to use it. [I don't think that's the case](rhboot#203 (comment)).
* Move the "link to your branch" to a full question
  - Remove the "in the form user/repo@tag". It seems okay
    [when people don't use that format](rhboot#233 (comment)).
    That also doesn't work for non-github repos.
  - Add an example github url, to help communicate precisely what's
    wanted.
* Add a question about the SHA256 to make sure that submitters changing
  the tag can't change the binary without setting off flags.
tedbrandston added a commit to tedbrandston/shim-review that referenced this issue Mar 23, 2022
* Change "Make sure you have provided the following information" to
  "Confirm the following are included in your repo, checking each box".
  "Make sure" makes it seem like the checklist is provided for your
  convenience, if you want to use it. [I don't think that's the case](rhboot#203 (comment)).
* Move the "link to your branch" to a full question
  - Remove the "in the form user/repo@tag". It seems okay
    [when people don't use that format](rhboot#233 (comment)).
    That also doesn't work for non-github repos.
  - Add an example github url, to help communicate precisely what's
    wanted.
* Add a question about the SHA256 to make sure that submitters changing
  the tag can't change the binary without setting off flags.
tedbrandston added a commit to tedbrandston/shim-review that referenced this issue Mar 24, 2022
* Change "Make sure you have provided the following information" to
  "Confirm the following are included in your repo, checking each box".
  "Make sure" makes it seem like the checklist is provided for your
  convenience, if you want to use it. [I don't think that's the case](rhboot#203 (comment)).
* Move the "link to your branch" to a full question
  - Remove the "in the form user/repo@tag". It seems okay
    [when people don't use that format](rhboot#233 (comment)).
    That also doesn't work for non-github repos.
  - Add an example github url, to help communicate precisely what's
    wanted.
* Add a question about the SHA256 to make sure that submitters changing
  the tag can't change the binary without setting off flags.
MarkusSpier pushed a commit to baramundisoftware/ShimReview_2024 that referenced this issue May 24, 2024
Squashed commit:

[e369477] Update build.log

[72f8ffc] Add new cert

[0a36833] Use corporate website in SBAT.csv

[cf373e0] Mention the SafeNet eToken 5110

[8de7371] Rename shimx86.efi -> shimia32.efi

[dbe72dd] Fix README shim_lock verifier implementation

[7618266] Update Readme.md

[644248a] Prepare Shim 15.8 review

[203ba5a] Add clarifications suggested in review 1986775225

Signed-off-by: Kamil Aronowski <kamil.aronowski@yahoo.com>

[576db28] Request a link to an application with verified contacts

If security contacts have already been verified in an earlier
application and haven't changed since the current one, let's point to
that earlier application as part of the current one.

Signed-off-by: Kamil Aronowski <kamil.aronowski@yahoo.com>

[7c3f89e] Add a note to explain that certificates must be in DER format

[5bb7de6] Add UKI-specific line to expect .sbat section of UKIs

Allows to revoke a family of UKIs from a vendor, independently
of the systemd-stub generation numbers.

[64cb767] #103680 Add missing parts to Shim Review

[c1e41f6] #103680 Add Viktor to contacts

[f0c6b23] #103680 Adding some missing parts for RedHat Shim Review Readme.md

[13452cc] Fix README shim_lock verifier implementation

[faa0786] Update Readme.md

[72b9c32] Update Readme.md

[6af4e87] Prepare Shim 15.8 review

[4e08b6d] Fixing spelling error #391

[662e57c] Clarify README question asking for all SBAT entries.

Currently, the wording isn't clear (to me, at least) if it's asking
for the shim SBAT or not; this clarifies that.

[f84b68c] Guidelines: add examples for UKI .sbat section

The .sbat identifier of systemd-boot was split from the identifier of
systemd-stub (which is used by UKI/kernel.efi binaries) after the previous
release, so clarify this with concrete examples.

Signed-off-by: Luca Boccassi <bluca@debian.org>

[6f19b78] Ask about the NX bit and point to NX signing exception

[dac8e76] docs/submitting.md formatting fixes and updates

Since the files in the docs/ directory were migrated from shim-review
Wiki, some formatting errors remained. These have been fixed for
the Markdown version, as well the text got some updates to reflect the
current state of this initiative.

Signed-off-by: Kamil Aronowski <kamil.aronowski@yahoo.com>

[86049d8] Adding examples for What changes since last SHIM submission question

[1dea2ab] Add questions and instructions regarding systemd-boot

Signed-off-by: Luca Boccassi <bluca@debian.org>

[518760b] Update template to new shim version

[15c328e] Add new GRUB2 CVEs, SBAT level and clarifications

This also adds more details about the CVEs and unifies the spelling of GRUB2.

[dc7aebd] Minor spelling fixes

[442daae] Merge docs from the wiki

[1f85d85] Add question about how kernel modules are signed

Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>

[d0ba08c] Use unambiguous character for horizontal lines

In commonmark, `---` and `===` can be used to mark either [setext
headings] or [thematic breaks] (aka horizontal lines). Headings take
precendence, so if you aren't careful with line breaks you can make a
heading where you meant to have a horizontal line. See [example] for a
case of this happening.

Fortunately, `***` is unambiguous: it will always create a horizontal
line instead of a heading. Switch all the separators to that format so
that we never have to worry about accidental headings again.

[setext headings]: https://spec.commonmark.org/0.30/#setext-headings
[thematic breaks]: https://spec.commonmark.org/0.30/#thematic-breaks
[example]: https://github.com/rhboot/shim-review/blob/b8ebe98d7198174e95d9e62e4522c145ee9caa5b/README.md#this-should-include-logs-for-creating-the-buildroots-applying-patches-doing-the-build-creating-the-archives-etc

[b8ebe98] README: make formatting more consistent

On a few questions the `---` separators were missing or placed differently.

[1c638f5] Update to shim 15.7

Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>

[b7cbb92] Update requirements for GRUB2 November 15th 2022 security update

[julian: fix typo]
Signed-off-by: Julian Andres Klode <julian.klode@canonical.com>

[2dc1336] Add question about existing shim reuse

Signed-off-by: Robbie Harwood <rharwood@redhat.com>

[2174fa8] Add an extra question about local kernel patches

If people have arbitrary extra kernel patches, they could well break
SB. Let's check?

[cd3c4a4] Add link to previous review in issue template

Signed-off-by: Robbie Harwood <rharwood@redhat.com>

[98b1e4e] Add documentation for contact verification

[193381b] Fix some minor nits

Signed-off-by: Peter Jones <pjones@redhat.com>

[b79349c] Update to 15.6

Also update list of GRUB2 CVEs and add one more lockdown bypass fix.

[04a5fe4] Update to shim 15.5

[d00e20e] Clarify ISSUE_TEMPLATE, ensure SHA is recorded

* Change "Make sure you have provided the following information" to
  "Confirm the following are included in your repo, checking each box".
  "Make sure" makes it seem like the checklist is provided for your
  convenience, if you want to use it. [I don't think that's the case](rhboot/shim-review#203 (comment)).
* Move the "link to your branch" to a full question
  - Remove the "in the form user/repo@tag". It seems okay
    [when people don't use that format](rhboot/shim-review#233 (comment)).
    That also doesn't work for non-github repos.
  - Add an example github url, to help communicate precisely what's
    wanted.
* Add a question about the SHA256 to make sure that submitters changing
  the tag can't change the binary without setting off flags.

[591a508] Make process slightly clearer

Update the process described in README.md to be slightly clearer.
* The checklist in the ISSUE_TEMPLATE asks for your tag, not your branch
  so we should match that.
* "when you have accepted tag" might be ambiguous in this context.
  We're talking about git tags and issue tags/labels. Acceptance is
  indicated with a github label, so let's try to clearly state that.

[25247e7] Edit questions in README for clarity and consistency

Changes to:
* Formatting
* Capitalization
* Sentence structure, where appropriate
* Question-ifying (please confirm [...]. -> Do you [...]?)

I had a hard time understanding a few of the questions, and spent some
time looking through the history to understand when they were added and
how they evolved. Some of them were phased differently between
ISSUE_TEMPLATE and README, so when in doubt I've erred on the side of
keeping more detailed versions of questions.

[9c5d46b] Move all questions from ISSUE_TEMPLATE to README

This is a bit of a workflow change. Based on the conversation in
rhboot/shim-review#207, seems like the README
should be the source of truth for submissions.

I've tried to remove duplicates. When in doubt I've used the history to
see what questions were added at the same time and considered
similar-but-different phrasing to be "duplicated".

For now all added questions have been tacked on the end. Grouping by
subject can come later.

[28f9f9f] Improve grammar/consistency in README and ISSUE_TEMPLATE

This is almost entirely changes to capitalization, spacing, etc. There
are a few places where I've added words where I felt they'd be
uncontroversial.

[a82f9fe] Make formatting of README and ISSUE_TEMPLATE match

This changes the headers and horizontal rules to be the same style in
both documents. This makes it a little easier for submitters to copy
answers from one to the other, and hopefully easier for maintainers to
update the questions (only one format to manage).

[f186371] Include question about signed grub binary composition

[63371b7] README: clarify PGP requirements

This attempts to fix two problems: first, that pgp.mit.edu isn't
reliable enough to regularly use, and second that we're getting shim
review requests are not providing the information we need to verify
emails.

Signed-off-by: Robbie Harwood <rharwood@redhat.com>

[ec960e3] Ask submissions to preserve upstream SBAT entries in derivatives

[6ed51d7] Add code of conduct

This is the standard Contributor Covenant.

See-also: https://www.contributor-covenant.org/
Signed-off-by: Robbie Harwood <rharwood@redhat.com>

[c2b5650] Minor typos

[d2f40a4] Require a Dockerfile to reproduce the provided shim EFI binaries

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

[60a1014] Point to shim-15.4 source

Shim-15.3 should not be used. Point to shim-15.4 release instead.

Signed-off-by: Chris Co <chrco@microsoft.com>

[095b458] point to tarball of 15.3 shim source
Require vendor_dbx listing or key rolling to secure chainloading

[62de662] CVE-20210-3418 only impacts products that ship shim_lock module

[6d4592c] add CVE-2021-3418 to the list

[2b71aea] One more update of requirements

Add sbat requirements
Add non linux kernel, add request to explain chain of trust

[da3ecfd] update requirements

[b748116] Add Dockerfile recommendation, and example from Ubuntu

[81e350b] "Include a Dockerfile to build it unless you have very good reasons it's not possible."

[87c0eca] Drop question: "Were your old SHIM hashes provided to Microsoft ?"

This was a safety check at beginning of boothole.

[fe5386f] Merge new questions from embargoed BootHole repository

[6477f2f] Fix typo

[a472ea8] Update README, ISSUE_TEMPLATE with submissions questions from Microsoft HDC

[973865d] Add a commonly omitted piece of information to the issue template.

[3c91a04] Create ISSUE_TEMPLATE.md

[af3cb5d] Fix some formatting

[02042f8] Make the reviews branch.

Signed-off-by: Peter Jones <pjones@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem with the review that must be fixed before it will be accepted new vendor This is a new vendor
Projects
None yet
Development

No branches or pull requests

3 participants