introduction of custom iPXE firmware support #429

Rozzii · 2023-08-08T06:59:43Z

This PR adds the following, :

  - Support for detecting iPXE certificates
  - When iPXE certs are detected, the iPXE services
    and conigs will be advertised with https addresses
  - Enforce chainloading of the iPXE firmware provided by
    the user even if the client machine comes with an iPXE
    firmware by default
  - Adds modular iPXE apache2 config file to enable separate
    iPXE specific virtual host (port) where TLS is enforced
  - Adds ipxe builder script that can be run as an entry point
    to standalone container or as a K8s pod init container
  - custom iPXE config file template has been added thus Ironic can
    generate node specific iPXE config files that use the custom iPXE
    firmware
  - a set of environment variable that control the new iPXE related
    features

This PR builds on top of #428 so it will be on hold until that is merged.

IPXE building functionality was originally planned to be hosted in this repository but after extensive community discussion I have decided to move the iPXE firmware builder to it's own repository.

Rozzii · 2023-08-08T07:04:03Z

/test-ubuntu-integration-main

Rozzii · 2023-08-08T07:13:26Z

/hold

Rozzii · 2023-08-08T08:11:40Z

/test-centos-integration-main

tuminoid

Some nits

README.md

ironic-config/httpd.conf.j2

scripts/buildipxe

Rozzii · 2023-08-11T13:20:48Z

/test-ubuntu-integration-main

Rozzii · 2023-08-11T13:26:59Z

/test-ubuntu-integration-main

Rozzii · 2023-08-11T18:35:34Z

/test-ubuntu-integration-main

Rozzii · 2023-08-17T12:13:58Z

/test-ubuntu-integration-main

Rozzii · 2023-09-01T05:48:47Z

/hold cancel

tuminoid · 2023-09-01T09:31:44Z

/cc @lentzi90
/assign @elfosardo

lentzi90

Really nice work from what I can understand. I must confess this is a bit outside of my expertise though

scripts/buildipxe

Rozzii · 2023-11-16T10:42:49Z

/test-ubuntu-integration-main

dtantsur · 2023-11-16T10:48:24Z

ironic-config/apache2-ipxe.conf.j2

+</VirtualHost>
+
+<Location ~ "^/grub.*/">
+    SSLRequireSSL


nit: Why not just everywhere?

I had a few things in mind when I have done this

I want to use the modular functionality of the Apache config files, so config files that are related to a type of boot interface should only restrict files that are also related to the same Ironic boot interface/boot method.

The other thing is that, at the moment firmware loading is not secured by TLS only the image download this I will explain in detail under your other comments.

dtantsur · 2023-11-16T10:50:10Z

ironic-config/dnsmasq.conf.j2

-# Client is PXE booting over EFI without iPXE ROM; send EFI version of iPXE chainloader
+# Client is PXE booting over EFI without iPXE ROM; send EFI version of iPXE chainloader do the same also if iPXE ROM boots but TLS is enabled
+{%- if env.IPXE_TLS_SETUP == "true"  %}
+dhcp-boot=tag:efi,tag:ipxe,snponly.efi


This does not look right. You're saying "if iPXE on UEFI, boot iPXE".

The goal here is to always force the chain loading of our iPXE firmware and not whatever is running on the machine by default. Our firmware will have the ability to exit this loop.

Okay, so it will only work for your custom firmware? :-/

Yes because the firmware has to contain the correct certificate, although by default iPXE supports the Mozzilla cert bundle but in our case we have our own for Ironic.

I could add https address support for loading the boot.ipxe directly but then that would exclude all machines that doesn't have a TLS enabled IPXE with the Mozzilla bundle because I can't enable the two addresses at the same time because the dhcp "tag:"s have to match.

I have looked into the possibility to differentiate our custom firmware from a regular TLS enabled iPXE via a custom dhcp tag but I have not found any documented ways to do it . I am not sure in general whether a custom DHCP tag is a thing or not.

dtantsur · 2023-11-16T10:50:57Z

ironic-config/dnsmasq.conf.j2

@@ -29,13 +29,23 @@ dhcp-option=option{% if ":" in env["DNS_IP"] %}6{% endif %}:dns-server,{{ env["D
 # IPv4 Configuration:
 dhcp-match=ipxe,175
 # Client is already running iPXE; move to next stage of chainloading
+{%- if env.IPXE_TLS_SETUP == "true"  %}
+# iPXE with (U)EFI
+dhcp-boot=tag:efi,tag:ipxe,http://{{ env.IRONIC_URL_HOST }}:{{ env.HTTP_PORT }}/custom-ipxe/snponly.efi


"http://" here sorta defeats the whole point.. if the attacker can intersect the connection, they can provide a different image?

You are right this will load the firmware via http, at the moment the TLS support is only introduced for the later part of the chain loading .There are a simple reason for this, we don't know what will be running on a machine by default, it could be and iPXE firmware that is unable to use https and first we need to provide a firmware that we can use with TLS.

The topic of man in the middle during firmware loading is very important I agree but that will be a followup feature based on all the things that are getting introduced in the ipxe-builder image and here. Securing the firmware loading won't be as easy as turning on TLS as I have mentioned we don't know what the host machine is capable at the initial stage of the loading. I plan to go into the direction of "trusted firmware" which would mean that we would only allow a firmware to get to our images if it can identify itself as a trusted image.

dtantsur · 2023-11-16T10:52:06Z

ironic-config/ipxe_config.template

@@ -0,0 +1,81 @@
+#!ipxe


I know Ironic allows copying this file, but I'd much rather prefer we don't do it. You're going to miss any upstream fixes, and reconciling will be very hard. Can we add the missing functionality in Ironic?

I would like to get this in first, but for sure I can later implement changes from the Ironic side also I agree eventually a natively supported approach should be the way to do it.

ironic-config/ironic.conf.j2

This commit adds the following: - Support for detecting iPXE certificates - When iPXE certs are detected, the iPXE services and configs will be advertised with https addresses - Enforce chainloading of the iPXE firmware provided by the user even if the client machine comes with an iPXE firmware by default - Adds modular iPXE apache2 config file to enable separate iPXE specific virtual host (port) where TLS is enforced - Custom iPXE config file template has been added thus Ironic can generate node specific iPXE config files that use the custom iPXE firmware - A set of environment variable that control the new iPXE related features Signed-off-by: Adam Rozman <adam.rozman@est.tech>

Rozzii · 2023-11-20T18:33:25Z

/test-ubuntu-integration-main

Rozzii · 2023-11-21T07:33:42Z

/test-ubuntu-integration-main

Rozzii · 2023-12-06T14:12:24Z

/test-centos-integration-main

elfosardo · 2023-12-06T14:13:30Z

/approve

metal3-io-bot · 2023-12-06T14:14:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elfosardo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [elfosardo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Rozzii · 2023-12-06T14:41:35Z

/hold

Rozzii · 2023-12-07T07:12:01Z

@lentzi90 could you please lgtm this? it has the "hold" only because we are not sure this should go in the first ironic-image release or not

lentzi90

/lgtm

Rozzii · 2023-12-12T07:49:47Z

/hold cancel
Now that the first release is done I will merge this as agreed @elfosardo .

OCPBUGS-23903: Ironic side of external_http_url (METAL-163) is not wired in correctly

metal3-io deleted a comment from metal3-io-bot Aug 8, 2023

metal3-io-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 8, 2023

Rozzii force-pushed the ipxe_adam branch from db492a8 to 5fdc123 Compare August 8, 2023 07:03

metal3-io deleted a comment from metal3-io-bot Aug 8, 2023

Rozzii changed the title ~~Ipxe adam~~ introduction of custom iPXE firmware support Aug 8, 2023

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 8, 2023

Rozzii mentioned this pull request Aug 9, 2023

🌱 add iPXE TLS support to local ironic deployment metal3-io/baremetal-operator#1326

Merged

tuminoid reviewed Aug 11, 2023

View reviewed changes

Rozzii force-pushed the ipxe_adam branch from 5fdc123 to 608b47c Compare August 11, 2023 13:09

metal3-io deleted a comment from metal3-io-bot Aug 11, 2023

Rozzii force-pushed the ipxe_adam branch from 608b47c to a8d177c Compare August 11, 2023 13:20

metal3-io deleted a comment from metal3-io-bot Aug 11, 2023

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 1, 2023

metal3-io-bot assigned elfosardo Sep 1, 2023

metal3-io-bot requested a review from lentzi90 September 1, 2023 09:31

lentzi90 reviewed Sep 1, 2023

View reviewed changes

scripts/buildipxe Outdated Show resolved Hide resolved

Rozzii force-pushed the ipxe_adam branch from a8d177c to d4061e9 Compare September 4, 2023 06:39

metal3-io-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 4, 2023

metal3-io deleted a comment from metal3-io-bot Sep 4, 2023

Rozzii force-pushed the ipxe_adam branch from f2bbb42 to 4c51bdb Compare November 16, 2023 10:36

metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 16, 2023

dtantsur reviewed Nov 16, 2023

View reviewed changes

ironic-config/ironic.conf.j2 Outdated Show resolved Hide resolved

Rozzii force-pushed the ipxe_adam branch 2 times, most recently from 4c62c59 to 00e3478 Compare November 20, 2023 18:30

Rozzii force-pushed the ipxe_adam branch from 00e3478 to 15149c0 Compare November 20, 2023 18:31

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 6, 2023

elfosardo approved these changes Dec 6, 2023

View reviewed changes

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 6, 2023

lentzi90 reviewed Dec 7, 2023

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2023

metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 12, 2023

metal3-io-bot merged commit 5dffc24 into metal3-io:main Dec 12, 2023
5 checks passed

metal3-io-bot deleted the ipxe_adam branch December 12, 2023 07:51

kashifest mentioned this pull request Dec 18, 2023

WIP: Revert "introduction of custom iPXE firmware support" #462

Closed

dtantsur pushed a commit to dtantsur/ironic-image that referenced this pull request Jan 2, 2024

Merge pull request metal3-io#429 from iurygregory/OCPBUGS-23903-4.14

30c987f

OCPBUGS-23903: Ironic side of external_http_url (METAL-163) is not wired in correctly

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

introduction of custom iPXE firmware support #429

introduction of custom iPXE firmware support #429

Rozzii commented Aug 8, 2023 •

edited

Loading

Rozzii commented Aug 8, 2023

Rozzii commented Aug 8, 2023

Rozzii commented Aug 8, 2023

tuminoid left a comment

Rozzii commented Aug 11, 2023

Rozzii commented Aug 11, 2023

Rozzii commented Aug 11, 2023

Rozzii commented Aug 17, 2023

Rozzii commented Sep 1, 2023

tuminoid commented Sep 1, 2023

lentzi90 left a comment

Rozzii commented Nov 16, 2023

dtantsur Nov 16, 2023

Rozzii Nov 17, 2023 •

edited

Loading

dtantsur Nov 16, 2023

Rozzii Nov 17, 2023

dtantsur Nov 17, 2023

Rozzii Nov 20, 2023 •

edited

Loading

Rozzii Nov 20, 2023

dtantsur Nov 16, 2023

Rozzii Nov 17, 2023

dtantsur Nov 16, 2023

Rozzii Nov 17, 2023

Rozzii commented Nov 20, 2023

Rozzii commented Nov 21, 2023

Rozzii commented Dec 6, 2023

elfosardo commented Dec 6, 2023

metal3-io-bot commented Dec 6, 2023

Rozzii commented Dec 6, 2023

Rozzii commented Dec 7, 2023

lentzi90 left a comment

Rozzii commented Dec 12, 2023

introduction of custom iPXE firmware support #429

introduction of custom iPXE firmware support #429

Conversation

Rozzii commented Aug 8, 2023 • edited Loading

Rozzii commented Aug 8, 2023

Rozzii commented Aug 8, 2023

Rozzii commented Aug 8, 2023

tuminoid left a comment

Choose a reason for hiding this comment

Rozzii commented Aug 11, 2023

Rozzii commented Aug 11, 2023

Rozzii commented Aug 11, 2023

Rozzii commented Aug 17, 2023

Rozzii commented Sep 1, 2023

tuminoid commented Sep 1, 2023

lentzi90 left a comment

Choose a reason for hiding this comment

Rozzii commented Nov 16, 2023

Choose a reason for hiding this comment

Rozzii Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rozzii Nov 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rozzii commented Nov 20, 2023

Rozzii commented Nov 21, 2023

Rozzii commented Dec 6, 2023

elfosardo commented Dec 6, 2023

metal3-io-bot commented Dec 6, 2023

Rozzii commented Dec 6, 2023

Rozzii commented Dec 7, 2023

lentzi90 left a comment

Choose a reason for hiding this comment

Rozzii commented Dec 12, 2023

Rozzii commented Aug 8, 2023 •

edited

Loading

Rozzii Nov 17, 2023 •

edited

Loading

Rozzii Nov 20, 2023 •

edited

Loading