Skip to content
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

Auth: Add support to make KEK and DB files optional #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tescande
Copy link

@tescande tescande commented Apr 5, 2024

If the host doesn't have the authentication files correctly configured for secure boot, the VM NVRAM state is always in setup mode and allows the VM to boot even if it has SecureBoot enabled.

This change allows varstored and varstore-sb-state to copy only the PK file (which is always present) and switch the VM to user mode. This will prevent the VM to boot if it has SecureBoot enabled, which is fine. Otherwise, the VM is stuck in setup mode allowing it to boot but with SecureBoot disabled, giving a false impression of security.

It's opt-out by default so DB and KEK files are set to not required only if the build macro AUTH_ONLY_PK_REQUIRED is defined.

If the host doesn't have the authentication files correctly configured
for secure boot, the VM NVRAM state is always in setup mode and allows
the VM to boot even if it has SecureBoot enabled.

This change allows varstored and varstore-sb-state to copy only the PK
file (which is always present) and switch the VM to user mode. This will
prevent the VM to boot if it has SecureBoot enabled, which is fine.
Otherwise, the VM is stuck in setup mode allowing it to boot but with
SecureBoot disabled, giving a false impression of security.

It's opt-out by default so DB and KEK files are set to not required only
if the build macro AUTH_ONLY_PK_REQUIRED is defined.

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
This patch allows passing of extra compilation flags from command line
using 'make EXTRA_CFLAGS=-DFOO'.

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
@stormi
Copy link
Contributor

stormi commented Apr 5, 2024

A bit of context about this PR:

  • XCP-ng doesn't have Microsoft's UEFI certificates by default, only PK. Legal reasons.
  • varstored will only ever propagate the pool certificates from disk to VM when the VM first starts, or when varstore-sb-state UUID user is called.
  • The combination of both these factors makes many VMs end up in setup mode, without the users knowing it, and then when they "enable" SecureBoot in XAPI for the VM, they believe it worked when it was in fact just ignored by varstored due to setup mode. Even once the pool is setup for SecureBoot by installing the required certificates to it, the VM will remain in setup mode, because XAPI and varstored together don't differenciate "accidental setup mode" from "on-purpose setup mode".

We understand that varstored was intentionnally made not to propagate certs if KEK or db are missing, to please Windows Update who wants to update the dbx and can't if they aren't defined correctly. However, we accept the risk of such issues (and know how to help user fix them now, by helping them install the correct certificates) when varstored will be built with these new optional flags, and prefer this over the false sense of security that setup mode induces.

Complementary to this change, we'll enhance XAPI so that clients such as Xen Orchestra can display an accurate status about the VM certs and offer a simple way for users to propagate certs from disk to VM for VMs which have already been booted once.

tescande added a commit to xcp-ng-rpms/varstored that referenced this pull request Apr 17, 2024
This commit adds the patches to support the compilation flag
AUTH_ONLY_PK_REQUIRED used to make KEK and DB authentication files
optional, making only the PK file required.

A pull request on the upstream repository has been submitted [1].

[1] xapi-project/varstored#23

Signed-off-by: Thierry Escande <thierry.escande@vates.tech>
@rosslagerwall rosslagerwall self-requested a review May 7, 2024 10:29
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.

None yet

3 participants