Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
22 Support shim fallback bootchain #106
base: 22
Are you sure you want to change the base?
22 Support shim fallback bootchain #106
Changes from 4 commits
348f49539cdb820071be6faf0d0e1b0e1683077f1da6086ea0176fdc13369342596013efb4ecfFile filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is right.
I was a bit confused on why we still need this. But shim is configured to boot
\grubx64.efi(DEFAULT_LOADER). Andgenerate_path_from_image_pathfrom shim considers paths that do not start with\EFI\to be relative.I wonder if it would be better to sort these content entries by target directory. So it is easier to see what goes in
EFI\BOOTand what goes inEFI\ubuntu.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good to know. I totally agree about sorting, I'll fix that in a moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alfonsosanchezbeato do you think
grub${arch}.efiis required inEFI/BOOT/because of this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaict if the fallback binary is in
EFI\boot, it will get run instead of grub, and even if it does not find any CSV file,grub*.efiwill not be invoked, so I don't see it necessary. @valentindavid please correct if I am getting something wrong here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. This is how it seems documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the mok manager should also go in
EFI\ubuntu.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I don't really know what the mok manager is used for, and there was some consternation about adding it in the past: #56 (comment)
It's not one of the boot assets added by the snapd PR #13205 (https://github.com/snapcore/snapd/pull/13205/files#diff-dca334c34d6acf0ce9fb49a95fb836caafe064f7fd2eda43a2185d6d9c7e20e5R464-R476), so I'm not sure if it's something we actually need to add.
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love to have it. The mok keys can be used to sign your own kernels. But it can also be used by anything that uses the secondary trusted keyring in the kernel: kernel modules, signed dm-verity, etc.
Also there is no security argument here, because there is a signed efi that anyone can install manually. We are just making it more annoying to install it for legitimate users.
If it causes some issues and would delay this PR, you do not have to add it.
But for sure, if we add it in
EFI\BOOT, it should also go inEFI\ubuntu. It feels very weird to have it in only one. That would be, you can install machine owner keys, but only if in the bios, you force to boot the default efi bootloader (EFI\BOOT\bootx64.efi) instead of the configuredBootOrder.To sum up. Either add
EFI\ubuntu\mmx64.efior removeEFI\BOOT\mmx64.efi.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably we can live with MokManager only in
EFI\ubuntu, as we have seen that grub will not ever run fromEFI\bootif we have fbx64.efi. As MM is called either when shim tries to run grub but it has not a valid signature or when the OS modifies EFI variables related to the MoK - and the OS is expected to have been started by grub inEFI\ubuntu.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I've removed it now, thanks!