-
Notifications
You must be signed in to change notification settings - Fork 220
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
Fixes #36833 - Add SecureBoot support for arbitrary operating systems to "Grub2 UEFI" PXE loaders #877
base: develop
Are you sure you want to change the base?
Conversation
Can one of the admins verify this patch? |
I have opened a PR to document this feature in foreman-documentation: |
This PR also requires the changes from Foreman PR #9864 |
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'd advertise this as an optional capability. In https://theforeman.org/2019/04/smart-proxy-capabilities-explained.html I've written about this. The benefit is two fold. On the one hand, it can disabled if bootloader_universe
is not set. Foreman can detect this. It can also help with n-1 support where Foreman x.y runs with a Foreman Proxy x.(y-1).
One example where you can see it in Foreman is https://github.com/theforeman/foreman/blob/66f0c87b26c9b3a125bd216eb26c9a28b06effb3/app/models/concerns/orchestration/dhcp.rb#L84-L86 but Katello also uses it for the various content types.
I encountered an error on a standalone Smart Proxy with the check if the bootloader directory is configured and fixed it. |
Hi there, What's left to do for now:
@sbernhard @ekohl What do you think? Please share your thoughts. I'm looking forward to input and discussion. |
Implemented changes as discussed in the course of the Foreman Community Forum thread. Please let me know, if anything is missing or further changes are required. |
Added deletion of host specific bootloader files in case host leaves build mode. |
[test smart-proxy] |
ok to test |
… to "Grub2 UEFI" PXE loaders In the course of theforeman/foreman#9864 and theforeman/smart-proxy#877, SecureBoot support for arbitrary operating systems has been added to the "Grub2 UEFI" PXE loaders. This patch adds a new parameter 'bootloader_universe' to the TFTP configuration and a directory 'host_config' inside the TFTP root directory, that are both required by the aforementioned PRs.
Squashed and changed commit message to reflect current implementation. => Any objections against merging this? What do you think, @nofaralfasi and @stejskalleos? |
… to "Grub2 UEFI" PXE loaders In the course of theforeman/foreman#9864 and theforeman/smart-proxy#877, SecureBoot support for arbitrary operating systems has been added to the "Grub2 UEFI" PXE loaders. This patch adds a new parameter 'bootloader_universe' to the TFTP configuration and a directory 'host_config' inside the TFTP root directory, that are both required by the aforementioned PRs.
… to "Grub2 UEFI" PXE loaders In the course of theforeman/foreman#9864 and theforeman/smart-proxy#877, SecureBoot support for arbitrary operating systems has been added to the "Grub2 UEFI" PXE loaders. This patch adds the 'bootloader-universe' and 'host-config' directories inside the TFTP root, that are both required by the aforementioned PRs.
… to "Grub2 UEFI" PXE loaders In the course of theforeman/foreman#9864 and theforeman/smart-proxy#877, SecureBoot support for arbitrary operating systems has been added to the "Grub2 UEFI" PXE loaders. This patch adds the 'bootloader-universe' and 'host-config' directories inside the TFTP root, that are both required by the aforementioned PRs.
… to "Grub2 UEFI" PXE loaders In the course of theforeman/foreman#9864 and theforeman/smart-proxy#877, SecureBoot support for arbitrary operating systems has been added to the "Grub2 UEFI" PXE loaders. This patch adds the 'bootloader-universe' and 'host-config' directories inside the TFTP root, that are both required by the aforementioned PRs.
cc589c7
to
7995045
Compare
Thanks @ekohl for your review! I hope I could clarify all points I resolved. I also updated the PR accordingly to reflect the outcome of the above mentioned discussion. This includes:
|
modules/tftp/server.rb
Outdated
symlink_path = File.join(pxeconfig_dir_mac, file_name) | ||
|
||
logger.debug "TFTP: Creating relative symlink: '#{symlink_path}' -> '#{source_file}'" | ||
FileUtils.ln_sr(source_file, symlink_path, force: true) |
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'm using Ruby version 3.0.4, and I don't have access to FileUtils.ln_sr
. This method has been available since FileUtils v1.7.0: FileUtils v1.7.0 release, but I currently have v1.6.0
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.
We should support FileUtils version 1.4.1 and above, as discussed 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.
Hey Nofar, I now headed for a new approach for the relative symlinks. I calculate the relative paths with the Pathname.relative_path_from
method. According to the Ruby API documentation it is available in Ruby 2.7 and newer.
Note: I can't remember if I mentioned it, but I saw that the suggested File.relative_path
method (DM) is provided by the yard
RubyGem. As far as I saw, it is not a dependency of Smart Proxy and also it was not installed on my Test Deployment.
When trying to create a new host, I'm getting the following error:
|
b672a7c
to
9acb90d
Compare
Hey Nofar, I finally made it to address your comments and to test the changes. I ran tests on AlmaLinux 8 and Rocky Linux 8 with the |
f76fe4b
to
7624180
Compare
Hey Nofar, as discussed I had a look at the method length of |
Affects "Grub2 UEFI" PXE loaders * PR in foreman: theforeman/foreman#9864 * PR in smart-proxy: theforeman/smart-proxy#877 * RFC: https://community.theforeman.org/t/add-secureboot-support-for-arbitrary-distributions/32601/1
Affects "Grub2 UEFI" PXE loaders * PR in foreman: theforeman/foreman#9864 * PR in smart-proxy: theforeman/smart-proxy#877 * RFC: https://community.theforeman.org/t/add-secureboot-support-for-arbitrary-distributions/32601/1
Affects "Grub2 UEFI" PXE loaders * PR in foreman: theforeman/foreman#9864 * PR in smart-proxy: theforeman/smart-proxy#877 * RFC: https://community.theforeman.org/t/add-secureboot-support-for-arbitrary-distributions/32601/1
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.
@goarsna Thank you for your efforts on this PR. The code is shaping up well, and I believe we're very close to finalizing it.
modules/tftp/server.rb
Outdated
symlinks = [ | ||
{ source: Pathname.new(File.join(pxeconfig_dir, "grub#{bootfilename_efi}.efi")), symlink: Pathname.new(File.join(pxeconfig_dir_mac, "boot.efi")) }, | ||
{ source: Pathname.new(File.join(pxeconfig_dir, "grub#{bootfilename_efi}.efi")), symlink: Pathname.new(File.join(pxeconfig_dir_mac, "grub#{bootfilename_efi}.efi")) }, | ||
{ source: Pathname.new(File.join(pxeconfig_dir, "shim#{bootfilename_efi}.efi")), symlink: Pathname.new(File.join(pxeconfig_dir_mac, "boot-sb.efi")) }, | ||
{ source: Pathname.new(File.join(pxeconfig_dir, "shim#{bootfilename_efi}.efi")), symlink: Pathname.new(File.join(pxeconfig_dir_mac, "shim#{bootfilename_efi}.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.
Could we avoid repeating the same code twice by extracting the common logic into a reusable method or loop? This will make the code more maintainable and reduce duplication.
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 for that suggestion. I reworked the logic.
modules/tftp/server.rb
Outdated
def bootloader_path(os, release, arch) | ||
bootloader_path = File.join(Proxy::TFTP::Plugin.settings.tftproot, 'bootloader-universe/pxegrub2', os, release, arch) | ||
|
||
logger.debug "TFTP: Checking if bootloader universe is configured for OS '#{os}' version '#{release}' (#{arch})." | ||
logger.debug "TFTP: Checking if directory '#{bootloader_path}' exists." | ||
unless Dir.exist?(bootloader_path) | ||
logger.debug "TFTP: Directory '#{bootloader_path}' does not exist." | ||
|
||
bootloader_path = File.join(Proxy::TFTP::Plugin.settings.tftproot, 'bootloader-universe/pxegrub2', os, 'default', arch) | ||
|
||
logger.debug "TFTP: Checking if bootloader universe is configured for OS '#{os}' (#{arch})." | ||
logger.debug "TFTP: Checking if directory '#{bootloader_path}' exists." | ||
unless Dir.exist?(bootloader_path) | ||
logger.debug "TFTP: Directory '#{bootloader_path}' does not exist." | ||
return | ||
end | ||
end | ||
|
||
bootloader_path | ||
end |
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.
The bootloader_path
method currently has multiple debug messages and nested conditionals that could be streamlined. IMO, combining the logging messages and adopting an early return pattern would simplify the logic and improve readability.
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 reworked the method. Please let me know in case you would like further adjustments. :)
modules/tftp/server.rb
Outdated
if bootloader_path | ||
logger.debug "TFTP: Creating symlinks from bootloader universe." | ||
create_bootloader_universe_symlinks(bootloader_path, pxeconfig_dir_mac) | ||
else | ||
logger.debug "TFTP: Creating symlinks from default bootloader files." | ||
create_default_symlinks(bootfilename_efi, pxeconfig_dir_mac) | ||
end |
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 terms bootloader universe
or default bootloader files
might not be clear to users reading these logs. It would be better to either remove these messages or include the actual directory paths for better context.
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 added these debug messages to have a short one liner to search for before the created symlinks are printed. The next four lines in the log should show the actual symlinks that are created. In my tests, for example for an AlmaLinux host the log states:
2024-09-18T14:24:59 fec247f7 [D] TFTP: Creating symlinks from bootloader universe.
2024-09-18T14:24:59 fec247f7 [D] TFTP: Creating relative symlink: /var/lib/tftpboot/host-config/00-11-22-33-44-55/grub2/grubx64.efi -> ../../../bootloader-universe/pxegrub2/almalinux/default/x86_64/grubx64.efi
2024-09-18T14:24:59 fec247f7 [D] TFTP: Creating relative symlink: /var/lib/tftpboot/host-config/00-11-22-33-44-55/grub2/shimx64.efi -> ../../../bootloader-universe/pxegrub2/almalinux/default/x86_64/shimx64.efi
2024-09-18T14:24:59 fec247f7 [D] TFTP: Creating relative symlink: /var/lib/tftpboot/host-config/00-11-22-33-44-55/grub2/boot.efi -> ../../../bootloader-universe/pxegrub2/almalinux/default/x86_64/boot.efi
2024-09-18T14:24:59 fec247f7 [D] TFTP: Creating relative symlink: /var/lib/tftpboot/host-config/00-11-22-33-44-55/grub2/boot-sb.efi -> ../../../bootloader-universe/pxegrub2/almalinux/default/x86_64/boot-sb.efi
=> Your second suggestion is already implemented. :) Would it then be ok to leave the one-liner as it is?
… to "Grub2 UEFI" PXE loaders This feature consists of four patches, one each for foreman, smart-proxy, foreman-installer, and puppet-foreman_proxy. This patch adds support for individual Network Bootstrap Programs (NBP) in order to enable network based installations of SecureBoot enabled hosts for arbitrary operating systems. SecureBoot expects to follow a chain of trust from the initial boot of the host to the loading of Linux kernel modules. The very first shim that is loaded determines which distribution is allowed to be booted or kexec'ed until next reboot. Currently the "Grub2 UEFI SecureBoot" PXE loaders use NBPs provided by the vendor of the Foreman/Smart Proxy host system. All hosts receive and execute the same binary. On SecureBoot enabled hosts, this limits installations to operating systems by the vendor of the Foreman/ Smart Proxy host system. Providing shim and GRUB2 by the vendor of the operating system to be installed allows Foreman to install any operating system on SecureBoot enabled hosts over network. To achieve this, the host's DHCP filename option is set to a shim/GRUB2 binary in a host specific directory based on their MAC address. Corresponding shim and GRUB2 binaries are copied into that directory along with the generated GRUB2 configuration files. When provisioning a host, the Smart Proxy checks in a dedicated directory inside the TFTP root - the so called "bootloader universe" - if NBPs are present matching the operating system, operating system version, and architecture of the host to be installed. If this is the case, these NBPs are copied from the bootloader universe directory to the host specific directory. If not, as a fallback the default NBPs provided by the vendor of the Foreman/Smart Proxy host system are copied from the `:tftproot:/grub2` directory to the host specific directory. Up to now, shim and GRUB2 binaries have to be retrieved and set up in the bootloader universe directory manually according to the documentation. An automatic way to provide OS dependent NBPs will be added in future. In case there are no NBPs present in the bootloader universe matching the operating system, operating system version, and architecture of the host to be installed, the behaviour of the "Grub2 UEFI" PXE loaders does not change to the behavior prior to this feature. Implementation notes: --------------------- * To be future proof (e.g. to be able to provide NBPs in the bootloader universe for other PXE loaders without running into any filename conflicts) and for better structure, the PXE kind is prepended as a first directory level inside the bootloader universe. * The operating system version inside the bootloader universe consists of the major and minor version (if applicable) of the operating system separated by a dot (`.`). If no NBPs are configured for a specific operating system version the fallback directory `default` is used. * To simplify things on Foreman side in future, symlinks are used for the shim (boot-sb.efi) and GRUB2 (boot.efi) binaries. * Inside the TFTP root directory a new directory `host-config` is created for storing all the host specific directories. * Inside the TFTP root directory a new directory `bootloader-universe` is created for storing all the OS specific boot files. * For storage efficiency the shim and GRUB2 binaries from the bootloader universe or the `:tftproot:/grub2` directory are symlinked to the host specific directory. Full example: ------------- [root@vm ~]# hammer host info --id 241 | grep -E "(MAC address|Operating System)" MAC address: 00:50:56:b4:75:5e Operating System: AlmaLinux 8.9 [root@vm ~]# tree /var/lib/tftpboot/bootloader-universe/ /var/lib/tftpboot/bootloader-universe/ └── pxegrub2 └── almalinux ├── 8.9 │ └── x86_64 │ ├── boot.efi -> grubx64.efi │ ├── boot-sb.efi -> shimx64.efi │ ├── grubx64.efi │ └── shimx64.efi └── default └── x86_64 ├── boot.efi -> grubx64.efi ├── boot-sb.efi -> shimx64.efi ├── grubx64.efi └── shimx64.efi [root@vm ~]# hammer host update --id 241 --build true [root@vm ~]# tree /var/lib/tftpboot/host-config /var/lib/tftpboot/host-config └── 00-50-56-a3-41-a8 └── grub2 ├── boot.efi -> ../../../bootloader-universe/grubx64.efi ├── boot-sb.efi -> ../../../bootloader-universe/shimx64.efi ├── grub.cfg ├── grub.cfg-00:50:56:a3:41:a8 ├── grub.cfg-01-00-50-56-a3-41-a8 ├── grubx64.efi -> ../../../bootloader-universe/grubx64.efi ├── os_info └── shimx64.efi -> ../../../bootloader-universe/shimx64.efi [root@vm ~]# grep -B2 00-50-56-b4-75-5e /var/lib/dhcpd/dhcpd.leases hardware ethernet 00:50:56:b4:75:5e; fixed-address 192.168.145.84; supersede server.filename = "host-config/00-50-56-b4-75-5e/grub2/boot-sb.efi"; [root@vm ~]# pesign -S -i /var/lib/tftpboot/host-config/00-50-56-b4-75-5e/grub2/boot-sb.efi | grep "Microsoft Windows UEFI Driver Publisher" The signer's common name is Microsoft Windows UEFI Driver Publisher
This feature consists of two patches, one for foreman and one for smart-proxy.
This patch introduces a new loader of kind
:PXEGrub2TargetOS
which allows to provide host-specific Network Bootstrap Programs (NPB) in order to enable network based installations for SecureBoot-enabled hosts.SecureBoot expects to follow a chain of trust from the start of the host to the loading of Linux kernel modules. The very first shim that is loaded basically determines which distribution is allowed to be booted or kexec'ed until next reboot.
The existing "Grub2 UEFI SecureBoot" is not sufficiant as it limits the possible installations to the vendor of the Foreman (Smart Proxy) host system.
Providing shim and GRUB2 by the vendor of the to-be-installed operating system allows Foreman to install any operating system on SecureBoot-enabled hosts over network.
To achieve this, the host's DHCP filename option is set to a shim path in a directory that is host-specific (contains MAC address). Corresponding shim and GRUB2 binaries are copied into that directory along with the generated GRUB2 configuration files as we know from "Grub2 UEFI".
The required binaries must be provided once in the so called "bootloader universe". This directory can be configured via the settings file
/etc/foreman-proxy/settings.d/tftp.yml
and defaults to/usr/local/share/bootloader-universe/<os>/
. These binaries can be manually retrieved from the installation media and is not part of this patchset.Full example: