Fixes #37345 - Improve "EFI local chainloading" on SecureBoot enabled hosts #10126
Conversation
|
Can one of the admins verify this patch? |
2 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
@goarsna feedback on this? |
To be clear: if we use this approach, we can enable efibootmgr_netboot by default (currently only if But this needs to be tested first. |
|
Any objections against merging this? Is there anything left to do, @jloeser? |
|
@jloeser can you squash the commits? After that it should be all green and ready for merge |
jloeser
force-pushed
the
feature/chainload_sb
branch
from
92f8423
to
bae4167
Compare
jloeser
force-pushed
the
feature/chainload_sb
branch
from
bae4167
to
71dd21d
Compare
… hosts Chainloading is not supported when SecureBoot is enabled [1]. Currently, this issue is tried to be tackled by changing the boot order during installation to boot from disk by default. But this disturbs the "always boot from network" workflow which might result in broken attempts for the user to re-provision a host (see theforeman#9123). What we can do is to exit network booted GRUB2 with `exit 1` resulting in the boot of the next boot device, which is probably the boot file from disk. The use of efibootmgr_netboot is still possible (if desired). The proposed solution would also work when SecureBoot is disabled, however to avoid side effects I propose to only boot next device if SecureBoot is enabled (GRUB2 variable `lockdown=y` [2]). [1]: https://www.gnu.org/software/grub/manual/grub/grub.html#UEFI-secure-boot-and-shim [2]: https://www.gnu.org/software/grub/manual/grub/grub.html#Lockdown
jloeser
force-pushed
the
feature/chainload_sb
branch
from
71dd21d
to
4975273
Compare
app/views/unattended/provisioning_templates/snippet/pxegrub2_chainload.erb
Show resolved
Hide resolved
| set default=next_bootdevice | ||
| fi | ||
|
|
||
| menuentry 'Booting from next boot device' --id next_bootdevice { |
There was a problem hiding this comment.
Can you fix the indentation (2 spaces) of menuentry and the lines below as it will only exist if lockdown=y?
There was a problem hiding this comment.
You mean to insert whole menuentry block by two spaces?
What is the rule of thumb for rendered files which do not care about indentation? Using indentation like you would write code or a script resulting in "unreadable" rendered files with wild indentations?
Or to ignore any indentations and try to produce readable rendered files with sane indentations?
Or a mix of both, where if- and loop-conditions are indented but not the actual lines which will be rendered?
| @@ -56,6 +56,19 @@ echo | |||
| "connectefi #{connectefi_option}" if connectefi_option | |||
| %> | |||
|
|
|||
| if [ "${lockdown}" == "y" ]; then | |||
|
|
|||
There was a problem hiding this comment.
I would remove this blank line
Chainloading is not supported when SecureBoot is enabled 1.
Currently, this issue is tried to be tackled by changing the boot order during installation to boot from disk by default. But this disturbs the "always boot from network" workflow which might result in broken attempts for the user to re-provision a host (see
#9123).
What we can do is to exit network booted GRUB2 with
exit 1resulting in the boot of the next boot device, which is probably the boot file from disk.The use of efibootmgr_netboot is still possible (if desired). The proposed solution would also work when SecureBoot is disabled, however to avoid side effects I propose to only boot next device if SecureBoot is enabled (GRUB2 variable
lockdown=y2).