create_disk: fix UEFI secure boot #1105
Conversation
darkmuggle
added
bug
|
Tested with:
Calling this a draft because I'm not sure how it will play with the $OTHER_ARCHES Confirmed that this works with UEFI in KVM: |
|
OK as far as I can tell, this bug started when someone just did an When I discussed with Andrew he said something about shim being unnecessary, and I didn't pursue it further at the time. However, I did notice that secure boot was broken in qemu, and the previous PR fixed it. We also now have test coverage for all of that via I didn't test on bare metal. So my question is: Concretely what exactly is this fixing? For example, does it fix booting on bare metal (are our qemu tests broken?). What does "compliant EFI boot" mean? AIUI, what shim is doing is ensuring that a system that only has the Microsoft keys enrolled (common for real hardware) can boot kernels signed with the Fedora/RH keys. So I could completely imagine that for example our That would be a real bug. But I'd like to have it spelled out exactly what user-visible scenario we're fixing. |
Same rationale as coreos/fedora-coreos-pipeline#187 Motivated by ensuring we're covering proposed changes like coreos#1105 in CI here too.
Same rationale as coreos/fedora-coreos-pipeline#187 Motivated by ensuring we're covering proposed changes like coreos#1105 in CI here too.
Same rationale as coreos/fedora-coreos-pipeline#187 Motivated by ensuring we're covering proposed changes like coreos#1105 in CI here too.
|
We are missing a couple of notable files:
I reached out to the Red Hat's UEFI SME and the answer is that we got really lucky that the current way works. Further, since there is no The bug here is that our UEFI story is broken on some hardware but not all. If the hardware requires key enrollment then our UEFI images will not boot. What we ship: After my patch: |
darkmuggle
added
WIP
|
Okay, this is ready for review. |
Our current EFI boot layout has the removable media layout, but not support key-registration via the
On first boot with the full layout, the firmware registers the signing keys and then reloads and boots normally. |
There was a problem hiding this comment.
/approve
Thanks, that sounds reasonable. Did you test this on real hardware (with SB enabled ideally?) Do we have any idea of what hardware would be affected by this that we could use to verify - or a way to do so by e.g. changing the qemu firmware setup?
I'm OK merging without, it may be easier to test afterwards.
Though hopefully you verified at least the existing cosa kola --qemu-basic-scenarios works? (One gap we still have is that we should probably change that to pass Ignitition to verify UEFI/lockdown etc.)
| local vendordir="${target_efi}/EFI/${vendor_id}" | ||
|
|
||
| # Some of the files in EFI/BOOT are _symlinks_ to EFI/$VENDOR | ||
| # in the OS tree. We need to make copies here. |
There was a problem hiding this comment.
Seems like it's just grubenv that's a symlink; may be easier to special case that?
Also worth commenting here probably are
coreos/rpm-ostree#969
and
ostreedev/ostree#1873
which may be related to some of this being awkward.
In fact we should probably change coreos-assembler (and/or rpm-ostree) to download the grub/efi bits separately and not commit them to the ostree as discussed. But, definitely that can come later.
| cd "${source_efidir}"/EFI/${t} | ||
| for i in *; do | ||
| /usr/lib/coreos-assembler/cp-reflink -vRL \ | ||
| $(readlink -f $i) \ |
There was a problem hiding this comment.
Having both -L and explicit readlink seems odd?
|
Confirmed that this produces images for both RHCOS and FCOS boot in a secure boot situation and that keys are registered. |
Ensure that all the efi binaries are included from the target. Fixes #1090 Fixes BZ 1799891 Our UEFI boot today is a bit lucky. This fixes a couple problems: 1. The full UEFI/EFI binaries were not copied over. In order to do a compliant EFI boot, BOOT<ARCH>.EFI should be there. 2. /usr/lib/ostree-boot/efi/BOOT/BOOT<ARCH>.EFI is a symlink to /usr/lib/ostree-boot/efi/<VENDOR>/shim<ARCH>.efi. This requires that a copy be made. 3. /boot/efi/EFI/<VENDOR>/grub2.cfg was not complete. It needed to load the /boot/grub2/grub2.cfg file and then boot. Previous images were booting using fallback. 4. Missing mmx64.efi means that secure UEFI could not happen since the the keys are no registered. mmx64.efi is needed to ensure GPL compliance on the shim. Without this file, the shim has is not recorded in the nvram.
|
/approve IMO, it makes sense for RHCOS to operate as closely as RHEL for this feature, so I believe this is the right thing to do. |
|
@ashcrow given that we're late in the game for 4.4 can you git this a sanity check and approval (if so inclined)? |
|
/lgtm |
|
We can address some of the review comments above as followup; merging in favor of shipping a tested patch. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, darkmuggle, miabbott The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Same rationale as coreos/fedora-coreos-pipeline#187 Motivated by ensuring we're covering proposed changes like #1105 in CI here too.
```
Benjamin Gilbert (3):
cmd/ore/aws: let list-regions list disabled regions, or all regions
Merge pull request coreos#1102 from bgilbert/regions
Merge pull request coreos#1104 from jcajka/bcrypt
Jakub Čajka (1):
kola/tests/rpmostree: use NVR instead of NVRA
Jonathan Lebon (5):
auth/aliyun: fix parsing config file
Merge pull request coreos#1105 from jlebon/pr/aliyun-profile-fix
ore/aliyun: add --delete-object and default to true
platform/api/aliyun: sprinkle with some logging
Merge pull request coreos#1107 from jlebon/pr/aliyun-delete-object
```
auth/aliyun: fix parsing config file
Ensure that all the efi binaries are included from the target.
Fixes #1090
We have a couple problems with our UEFI Secure Boot story:
compliant EFI boot, BOOT.EFI should be there.
to /usr/lib/ostree-boot/efi//shim.efi.
This requires that a copy be made.
load the /boot/grub2/grub2.cfg file and then boot.