Skip to content

Commit

Permalink
Fixes #37566 - Add UEFI Secure Boot Firmware to Libvirt
Browse files Browse the repository at this point in the history
- Added a new firmware type for Secure Boot.
- Enable `enrolled-keys` by default when Secure Boot is activated.
- Added firmware-related methods to the ComputeResource model
  for shared use between VMware and Libvirt.
  • Loading branch information
nofaralfasi committed Nov 25, 2024
1 parent bde4347 commit 995d520
Show file tree
Hide file tree
Showing 9 changed files with 102 additions and 18 deletions.
33 changes: 29 additions & 4 deletions app/models/compute_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -406,23 +406,35 @@ def firmware_types
"automatic" => N_("Automatic"),
"bios" => N_("BIOS"),
"uefi" => N_("UEFI"),
"uefi_secure_boot" => N_("UEFI Secure Boot"),
}.freeze
end

# Converts the firmware type from a VM object to the Foreman-compatible format.
#
# @param firmware [String] The firmware type from the VM object.
# @param secure_boot [Boolean] Indicates if secure boot is enabled for the VM.
# @return [String] The converted firmware type.
def firmware_type(firmware)
firmware == 'efi' ? 'uefi' : firmware
def firmware_type(firmware, secure_boot)
if firmware == 'efi'
secure_boot ? 'uefi_secure_boot' : 'uefi' # Adjust for secure boot
else
firmware
end
end

# Normalizes the firmware type to 'efi' or defaults to 'bios'.
# Converts a firmware type from Foreman format to a CR-compatible format.
# If no specific type is provided, defaults to 'bios'.
#
# @param firmware_type [String] The firmware type in Foreman format.
# @return [String] The converted firmware type.
def normalize_firmware_type(firmware_type)
firmware_type == 'uefi' ? 'efi' : 'bios'
case firmware_type
when 'uefi', 'uefi_secure_boot'
'efi'
else
'bios'
end
end

# Resolves the firmware setting when it is 'automatic' based on the provided firmware_type, or defaults to 'bios'.
Expand All @@ -435,6 +447,19 @@ def resolve_automatic_firmware(firmware, firmware_type)
firmware_type.presence || 'bios'
end

# Processes firmware attributes to configure firmware and secure boot settings.
#
# @param firmware [String] The firmware setting to be processed.
# @param firmware_type [String] The firmware type based on the provided PXE Loader.
# @return [Hash] A hash containing the processed firmware attributes.
def process_firmware_attributes(firmware, firmware_type)
firmware = resolve_automatic_firmware(firmware, firmware_type)

attrs = generate_secure_boot_settings(firmware)
attrs[:firmware] = normalize_firmware_type(firmware)
attrs
end

protected

def memory_gb_to_bytes(memory_size)
Expand Down
21 changes: 18 additions & 3 deletions app/models/compute_resources/foreman/model/libvirt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ def new_vm(attr = { })
opts[:boot_order].unshift 'network' unless attr[:image_id]

firmware_type = opts.delete(:firmware_type).to_s
firmware = resolve_automatic_firmware(opts[:firmware], firmware_type)
opts[:firmware] = normalize_firmware_type(firmware)
opts.merge!(process_firmware_attributes(opts[:firmware], firmware_type))

vm = client.servers.new opts
vm.memory = opts[:memory] if opts[:memory]
Expand Down Expand Up @@ -294,7 +293,8 @@ def vm_instance_defaults
:listen => Setting[:libvirt_default_console_address],
:password => random_password(console_password_length(display_type)),
:port => '-1' },
:firmware => 'automatic'
:firmware => 'automatic',
:firmware_features => { "secure-boot" => "no" }
)
end

Expand Down Expand Up @@ -331,5 +331,20 @@ def validate_volume_capacity(vol)
raise Foreman::Exception.new(N_("Please specify volume size. You may optionally use suffix 'G' to specify volume size in gigabytes."))
end
end

# Generates Secure Boot settings for Libvirt based on the provided firmware type.
# The `secure_boot` setting is used to properly configure and display the Firmware in the `compute_attributes` form.
#
# @param firmware [String] The firmware type.
# @return [Hash] A hash with secure boot settings if applicable.
def generate_secure_boot_settings(firmware)
return {} unless firmware == 'uefi_secure_boot'

{
firmware_features: { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' },
loader_attributes: { 'secure' => 'yes' },
secure_boot: true,
}
end
end
end
2 changes: 2 additions & 0 deletions app/models/concerns/pxe_loader_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ def firmware_type(pxe_loader)
case pxe_loader
when 'None'
:none
when /SecureBoot/
:uefi_secure_boot
when /UEFI/
:uefi
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
<% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:start] || '1' %>
<%= checkbox_f f, :start, { :checked => (checked == '1'), :help_inline => _("Power ON this machine"), :label => _('Start'), :label_size => "col-md-2"} if new_vm && controller_name != "compute_attributes" %>
<% firmware_type = new_vm ? 'automatic' : compute_resource.firmware_type(f.object.firmware) %>
<% firmware_type = new_vm ? 'automatic' : compute_resource.firmware_type(f.object.firmware, f.object.secure_boot) %>
<%= field(f, :firmware, :disabled => !new_vm, :label => _('Firmware'), :label_help => _("Choose 'Automatic' to set the firmware based on the host's PXE Loader. If no PXE Loader is selected, it defaults to BIOS."), :label_size => "col-md-2") do
compute_resource.firmware_types.collect do |type, name|
radio_button_f f, :firmware, {:checked => (firmware_type == type), :disabled => !new_vm, :value => type, :text => _(name) }
Expand Down
4 changes: 1 addition & 3 deletions bundler.d/libvirt.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
group :libvirt do
# gem 'fog-libvirt', '>= 0.12.0'
gem 'fog-libvirt', '>= 0.13.0'
gem 'ruby-libvirt', '~> 0.5', :require => 'libvirt'
# TODO: Remove this line after merging the PR
gem 'fog-libvirt', github: 'nofaralfasi/fog-libvirt', branch: 'sb_libvirt'
end
26 changes: 19 additions & 7 deletions test/models/compute_resource_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -397,14 +397,20 @@ def setup
@cr = compute_resources(:mycompute)
end

test "returns 'uefi' when firmware is 'efi'" do
assert_equal 'uefi', @cr.firmware_type('efi')
test "returns firmware unchanged when firmware is not 'efi'" do
assert_equal 'bios', @cr.firmware_type('bios', true)
assert_equal 'bios', @cr.firmware_type('bios', false)
assert_empty(@cr.firmware_type('', true))
assert_nil(@cr.firmware_type(nil, false))
end

test "returns firmware unchanged when firmware is not 'efi'" do
assert_equal 'bios', @cr.firmware_type('bios')
assert_equal '', @cr.firmware_type('')
assert_equal nil, @cr.firmware_type(nil)
test "returns 'uefi' when firmware is 'efi' and secure boot is not enabled" do
assert_equal 'uefi', @cr.firmware_type('efi', false)
assert_equal 'uefi', @cr.firmware_type('efi', nil)
end

test "returns 'uefi_secure_boot' when firmware is 'efi' and secure boot is enabled" do
assert_equal 'uefi_secure_boot', @cr.firmware_type('efi', true)
end
end

Expand All @@ -417,12 +423,16 @@ def setup
assert_equal 'efi', @cr.normalize_firmware_type('uefi')
end

test "returns 'bios' when firmware is not 'uefi'" do
test "returns 'bios' for non-uefi firmware types" do
assert_equal 'bios', @cr.normalize_firmware_type('bios')
assert_equal 'bios', @cr.normalize_firmware_type('none')
assert_equal 'bios', @cr.normalize_firmware_type('')
assert_equal 'bios', @cr.normalize_firmware_type(nil)
end

test "returns 'efi' when firmware is 'uefi_secure_boot'" do
assert_equal 'efi', @cr.normalize_firmware_type('uefi_secure_boot')
end
end

describe '#resolve_automatic_firmware' do
Expand All @@ -434,13 +444,15 @@ def setup
assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'automatic', 'uefi')
assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', 'bios')
assert_equal 'none', @cr.send(:resolve_automatic_firmware, 'automatic', 'none')
assert_equal 'uefi_secure_boot', @cr.send(:resolve_automatic_firmware, 'automatic', 'uefi_secure_boot')
end

test "returns firmware unchanged when not 'automatic'" do
assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', 'bios')
assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', 'uefi')
assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', false)
assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', '')
assert_equal 'uefi_secure_boot', @cr.send(:resolve_automatic_firmware, 'uefi_secure_boot', '')
end

test "returns 'bios' when firmware is 'automatic' and firmware_type is not present" do
Expand Down
23 changes: 23 additions & 0 deletions test/models/compute_resources/libvirt_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -270,4 +270,27 @@ class Foreman::Model::LibvirtTest < ActiveSupport::TestCase
check_vm_attribute_names(cr)
end
end

describe '#generate_secure_boot_settings' do
before do
@cr = FactoryBot.build_stubbed(:libvirt_cr)
end

test "returns secure boot settings when firmware is 'uefi_secure_boot'" do
expected_sb_settings = {
firmware_features: { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' },
loader_attributes: { 'secure' => 'yes' },
secure_boot: true,
}

assert_equal expected_sb_settings, @cr.send(:generate_secure_boot_settings, 'uefi_secure_boot')
end

test "returns an empty hash for firmware types other than 'uefi_secure_boot'" do
assert_empty @cr.send(:generate_secure_boot_settings, 'uefi')
assert_empty @cr.send(:generate_secure_boot_settings, 'bios')
assert_empty @cr.send(:generate_secure_boot_settings, '')
assert_empty @cr.send(:generate_secure_boot_settings, nil)
end
end
end
4 changes: 4 additions & 0 deletions test/models/concerns/pxe_loader_support_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -200,5 +200,9 @@ def setup
test 'defaults to bios firmware' do
assert_equal :bios, DummyPxeLoader.firmware_type('Anything')
end

test 'detects uefi_secure_boot firmware' do
assert_equal :uefi_secure_boot, DummyPxeLoader.firmware_type('Grub2 UEFI SecureBoot')
end
end
end
5 changes: 5 additions & 0 deletions test/models/host_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3313,6 +3313,11 @@ def to_managed!
host = FactoryBot.build_stubbed(:host, :managed, :pxe_loader => "Grub2 UEFI")
assert_equal :uefi, host.firmware_type
end

test 'should be :uefi_secure_boot for host with uefi_secure_boot loader' do
host = FactoryBot.build_stubbed(:host, :managed, :pxe_loader => "Grub2 UEFI SecureBoot")
assert_equal :uefi_secure_boot, host.firmware_type
end
end

describe '#templates_used' do
Expand Down

0 comments on commit 995d520

Please sign in to comment.