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

update grub and shim #3775

Merged
merged 4 commits into from
Feb 22, 2024
Merged

Conversation

bcressey
Copy link
Contributor

@bcressey bcressey commented Feb 14, 2024

Issue number:
N/A

Description of changes:
Update shim to 15.8 which includes recent CVE fixes.

Update grub to the latest version from AL23, and revert two patches that aren't required for Bottlerocket's Secure Boot implementation. This includes fixes for CVE-2023-4692 and CVE-2023-4693, which don't apply to Bottlerocket since the NTFS module isn't built. Overall, this update is a bit of a no-op but I wanted to get it in to record the decision to revert the patches, rather than leaving that for a future update.

I also added a patch from Red Hat that fixes CVE-2023-4001. That doesn't apply to Bottlerocket because the search directive isn't used to locate grub.cfg, so there is no ability to bypass a password that might be set. On variants that support Secure Boot there's also no way to modify grub.cfg to set a password. However, the functionality is useful to ensure that we read the expected boot config file for extending the kernel command line.

Testing done:
Confirmed that Secure Boot works for the following variants:

  • aws-k8s-1.28 - x86_64, aarch64
  • vmware-dev - x86_64
  • metal-dev - x86_64 (under QEMU)

I also tested legacy BIOS boot on c3.large to confirm the "uefi-preferred" functionality works as expected.

Verified that in-place upgrades and downgrades worked for aws-k8s-1.28 (x86_64) and aws-k8s-1.28-nvidia (aarch64).

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Switch to the release archive, which bundles the gnu-efi sources.

Add `-N` to the PE post-process flags, to disable the NX compat flag
by default. GRUB and the Linux kernel are not yet ready for this.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
Revert two Amazon Linux patches that aren't needed for Bottlerocket's
Secure Boot implementation, which uses shim and verifies the grub.cfg
signature.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
The private data partition is assumed to be on the same device as the
root filesystem. Enforce that assumption by passing `--root-dev-only`
in the embedded grub.cfg.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
"grub,4" denotes a build that includes fixes for CVE-2023-4692 and
CVE-2023-4693, so it is correct even though our build never enabled
the vulnerable NTFS module.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
Comment on lines +23 to +31
+ GRUB_FILE_FILTER_VERIFY,
GRUB_FILE_FILTER_GZIO,
GRUB_FILE_FILTER_XZIO,
GRUB_FILE_FILTER_LZOPIO,
+ GRUB_FILE_FILTER_MAX,
GRUB_FILE_FILTER_COMPRESSION_FIRST = GRUB_FILE_FILTER_GZIO,
GRUB_FILE_FILTER_COMPRESSION_LAST = GRUB_FILE_FILTER_LZOPIO,
- GRUB_FILE_FILTER_VERIFY,
- GRUB_FILE_FILTER_MAX,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary sort?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the order does matter here. Enumerations in C are numeric, not symbolic, i.e. the order changes their assigned values. It is idiomatic to loop e.g. from 0 to some sentinel value (like GRUB_FILE_FILTER_MAX here). For this particular case, the ordering of filters in the code determines their numeric value and therefore in which order they're applied. In any case, those two patches are reverts of downstream patches packaged in the SRPM and hence should match what's in there.

Comment on lines +124 to +125
+ *r = '\0';
+ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The termination of r should probably be outside the conditional to guarantee sane output in case there's nothing following the delimiter. That's not a problem for the function's use case in this patch for now, since the remainder is ignored anyway, but curious whether you know the proper process to get this fixed in Fedora?

@markusboehme
Copy link
Member

Confirmed patches match their upstream counterparts and the shim update included revokes the previous grub generation number. Ready to approve once test results are in. 🐎

This was referenced Feb 20, 2024
@bcressey bcressey marked this pull request as ready for review February 21, 2024 23:53
Copy link
Contributor

@yeazelm yeazelm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to me. 🚢

Copy link
Member

@markusboehme markusboehme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 ✔️

@bcressey bcressey merged commit 03d436e into bottlerocket-os:develop Feb 22, 2024
50 checks passed
@bcressey bcressey deleted the grub-and-shim branch February 22, 2024 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants