Skip to content

Conversation

dinhngtu
Copy link
Member

@dinhngtu dinhngtu commented Mar 3, 2025

Before submitting the pull request, you must agree with the following statements by checking both boxes with a 'x'.

  • "I accept that my contribution is placed under the CC BY-SA 2.0 license [1]."
  • "My contribution complies with the Developer Certificate of Origin [2]."

[1] https://creativecommons.org/licenses/by-sa/2.0/
[2] https://docs.xcp-ng.org/project/contributing/#developer-certificate-of-origin-dco

@dinhngtu dinhngtu force-pushed the secureboot-changes branch from 8cc41b0 to 3300fd5 Compare March 13, 2025 11:46
@dinhngtu dinhngtu marked this pull request as ready for review March 13, 2025 11:46
* If you followed our previous guides and used `secureboot-certs install` to install the default Secure Boot variables into your pool, these variables will not be changed.

The only VMs affected by these changes are **newly created VMs** with Secure Boot enabled, running on pools where `secureboot-certs install` have not been executed.
Previously, these VMs will execute all UEFI binaries even with Secure Boot enabled (due to an empty dbx variable); however, going forward, revoked UEFI binaries (e.g. from an outdated media) will no longer boot on such VMs with Secure Boot enabled.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small conjugation fix, but actually I think the whole sentence is wrong. We put in a lot of work during 8.3 development in order to avoid having VMs that boot despite secure boot is enabled for them. And a missing dbx is not enough to let Secure Boot checks pass, if db is present.

Suggested change
Previously, these VMs will execute all UEFI binaries even with Secure Boot enabled (due to an empty dbx variable); however, going forward, revoked UEFI binaries (e.g. from an outdated media) will no longer boot on such VMs with Secure Boot enabled.
Previously, these VMs would execute all UEFI binaries even with Secure Boot enabled (due to an empty dbx variable); however, going forward, revoked UEFI binaries (e.g. from an outdated media) will no longer boot on such VMs with Secure Boot enabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"would accept to execute"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understand. Why would missing dbx cause VMs to not boot?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"these VMs will execute all UEFI binaries even with Secure Boot enabled (due to an empty dbx variable)" this can be interpreted as "if dbx is empty, any UEFI binary, signed or not, would get executed, and thus allow to bypass Secure Boot checks".

That's just a phrasing issue. I think the whole section overall is not

Here's a suggestion of topics to address to improve the whole "What this change means for you" section. Giving just the main points:

  • For pools

    • Existing pools
      • The pool was already configured for Secure Boot: nothing changes for the pool. The certificates that you installed in the pool are still the ones used to provision new VMs. However, if you would like to sync with the default certificates we now provide (which include a more up to date exclusion database, dbx), you can remove the certificates you installed (TODO explain how) in order to let the pool use our defaults.
      • New pools: they are now ready for guest secure boot without any extra action from administrators, as we now provide the required certificates.
  • For VMs

    • Existing VMs, that have booted at least once
      • Nothing changes for them. Each VM has its own UEFI variable store, unaffected by any changes on the default certificates in the pool.
      • This also applies to copies of existing VMs. If the original VM has booted once, then any subsequent copy has its own copy of the UEFI variable store.
    • New VMs just get their certificates from the pool. If the pool uses the new defaults, then the main notable differences are:
      • (updated KEK/db that do not expire soon so you're good for a long time?)
      • updated dbx, which will reject boot binaries and installation media that were signed with an excluded certificate (the part you covered)

That is, if I understood the whole change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added more details. But I don't think it's necessary to repeat the behaviors that didn't change (existing VMs etc.)

Comment on lines 45 to 57
## 8.3 with varstored >= 1.2.0-2.4

Secure Boot is ready to use without extra configuration. Simply activate Secure Boot on your VMs, and they will be provided with an appropriate set of default Secure Boot variables.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with varstored including certs, certs will still need to be manually propagated to existing VMs that are not in setup mode, that is any VM which already booted once and was not explicitly put in setup mode.

So parts of the Quick start guide below, and of the rest of the guide, still apply.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that should be mentioned here.

@@ -91,24 +120,46 @@ For custom certificates (advanced use), see [Install Custom UEFI Certificates](#

### Install the Default UEFI Certificates

:::info
This procedure is not necessary if you're using varstored 1.2.0-2.4 and newer. However, the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence is not complete

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@dinhngtu dinhngtu force-pushed the secureboot-changes branch 3 times, most recently from 9d722a9 to 0a9f2df Compare August 7, 2025 10:40
@dinhngtu dinhngtu force-pushed the secureboot-changes branch from 0a9f2df to b2c9acd Compare August 7, 2025 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants