EFI & Secure Boot #155
EFI & Secure Boot #155
Conversation
|
I encountered an issue with the changes in this PR. For VMs not created by Foreman (either manually or by another tool), the firmware type is not always set, making it unclear what the firmware type is. |
There was a problem hiding this comment.
Please look at my os_loader implementation around https://github.com/fog/fog-libvirt/pull/134/files#r1686901783. I'm not sure why @stejskalleos dropped the os_loader, but from reading https://libvirt.org/kbase/secureboot.html and seeing libvirt 8.0.0 is in EL8 then I suspect you can't use EL8 libvirt & secureboot this way.
Technically we an probe for the version (look for libversion in the code). It would be nice to throw a clean error if we detect too old versions like EL7 hypervisors.
You are right; their documentation can be confusing. I was reading from https://libvirt.org/formatdomain.html#bios-bootloader: But I guess that's not true for older versions. |
|
I agree it's often confusing. I'd recommend setting up an EL8 hypervisor (Foreman can connect to libvirt using SSH) and try it out. Often the best way |
| .select { |feature| feature[:enabled] == 'yes' } | ||
| .map { |feature| feature[:name] } | ||
|
|
||
| required_features = ['secure-boot', 'enrolled-keys'] |
There was a problem hiding this comment.
AFAIK you don't need enrolled-keys for secure-boot. That's just the default set of keys, but users can define their own keys and sign their own kernels.
There was a problem hiding this comment.
According to their documentation, enrolled-keys are needed for SB. Is it necessary to also provide an option for the user to add their own keys?
There was a problem hiding this comment.
https://libvirt.org/kbase/secureboot.html#additional-information says that without enrolled-keys it allows running unsigned code, but I wonder if that's really true. This last bit clarifies with how I think it's supposed to work:
The main difference between using a build of EDKII that has Secure Boot support but without keys enrolled and one that doesn't have Secure Boot support at all is that, with the former, you could enroll your own keys and securely run an operating system that you've built and signed yourself. If you are only planning to run existing, off-the-shelf operating system images, then the two configurations are functionally equivalent.
So the idea is that enrolled-keys contains the default keys, but if you don't have or want those you can enroll your own keys.
|
I have another idea on how to implement this. Moving it to WIP until it's ready. |
Co-authored-by: Ewoud Kohl van Wijngaarden <[email protected]>
nofaralfasi
force-pushed
the
sb_libvirt
branch
2 times, most recently
from
5a05cd7
to
1f88090
Compare
nofaralfasi
force-pushed
the
sb_libvirt
branch
2 times, most recently
from
534c382
to
ba8b95c
Compare
There was a problem hiding this comment.
🍏 LGTM
Tested together with theforeman/foreman#10321, BIOS, UEFI & Secure Boot works fine.
| attribute :firmware | ||
| attribute :firmware_features | ||
| attribute :secure_boot | ||
| attribute :loader |
There was a problem hiding this comment.
Note to myself:
The loader element points to a specific boot binary (BIOS/UEFI), while firmware refers to the type or particular features used to configure or load the guest VM.
There was a problem hiding this comment.
I think those could be good comments around the attributes.
To me loader implies it's singular, but when I look at the code I see it's a list, so should it be
| attribute :loader | |
| attribute :loaders |
There was a problem hiding this comment.
The loader is not a list, it's a single tag with multiple attributes.
There was a problem hiding this comment.
And since we're using the same terminology as Libvirt, I believe there's no need to define "firmware" or "loader" as they should already be familiar to users. See https://libvirt.org/formatdomain.html#bios-bootloader.
There was a problem hiding this comment.
But in the code I see it's iterated over which made me think it was a list.
loader&.each do |key, value|
xml.loader(key => value)
endTechnically I guess it's a hash, but loader still made me expect a string.
There was a problem hiding this comment.
It's a hash, e.g., loader: { "secure" => "yes" }.
|
@ekohl, can we get this so the theforeman/foreman#10321 can be included in the next Foreman's release? |
|
@ekohl @nofaralfasi, any progress here? Can we merge? It's blocking our other work, which is waiting for this. |
- Renamed firmware-related attributes to align with VMware conventions. - Added the `loader` attribute to determine if SB is enabled.
nofaralfasi
force-pushed
the
sb_libvirt
branch
from
ba8b95c
to
a561fed
Compare
|
I'm merging both commits for proper attribution. |
EFI and Secure Boot support for Libvirt.
Required for: theforeman/foreman#10321