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.

Sign up for GitHub

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

fix: Secure Boot does not work for Linux guests #1579

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

fix: Secure Boot does not work for Linux guests #1579

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7706a5a
fix: Enable SMM for Linux guests on Linux hosts when Secure Boot is e…
alexhaydock Feb 2, 2025
0bc3466
fix: Select OVMF_VARS file with preloaded MS Platform Keys (Fedora/RH…
alexhaydock Feb 2, 2025
003e8ea
fix: Select OVMF_VARS file with preloaded MS Platform Keys (Debian/Ub…
alexhaydock Feb 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions quickemu
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,16 @@ function configure_cpu() {
fi
fi

# SMM is also required for Linux guests when Secure Boot is enabled
if [ "${secureboot}" == "on" ]; then
if [ "${guest_os}" == "linux" ]; then
# SMM is not available on QEMU for macOS via Homebrew
if [ "${OS_KERNEL}" == "Linux" ]; then
SMM="on"
fi
fi
fi

case ${guest_os} in
batocera|freedos|haiku|solaris) MACHINE_TYPE="pc";;
kolibrios|reactos)
Expand Down Expand Up @@ -663,8 +673,8 @@ function configure_bios() {
if [ -n "${EFI_CODE}" ] || [ ! -e "${EFI_CODE}" ]; then
case ${secureboot} in
on) # shellcheck disable=SC2054,SC2140
ovmfs=("${SHARE_PATH}/OVMF/OVMF_CODE_4M.secboot.fd","${SHARE_PATH}/OVMF/OVMF_VARS_4M.fd" \
"${SHARE_PATH}/edk2/ovmf/OVMF_CODE.secboot.fd","${SHARE_PATH}/edk2/ovmf/OVMF_VARS.fd" \
ovmfs=("${SHARE_PATH}/OVMF/OVMF_CODE_4M.secboot.fd","${SHARE_PATH}/OVMF/OVMF_VARS_4M.ms.fd" \
"${SHARE_PATH}/edk2/ovmf/OVMF_CODE.secboot.fd","${SHARE_PATH}/edk2/ovmf/OVMF_VARS.secboot.fd" \
Copy link
Member

Choose a reason for hiding this comment

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

Why is this here? It won't do anything. Quickemu searches the OVMF code paths (even indices in the array) and selects the vars (code index+1) based on it. You've added an identical OVMF code path to a prior entry, which will never be matched. Also, which distro has this OVMF_VARS.secboot.fd file? I can't find it in Ubuntu or Arch's edk packages.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see. It's supposed to resolve a different issue. It won't work, though. Please address the above concerns though, we can't have quickemu pointing to nonexistent files on any distro.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is this here? It won't do anything.

This includes the Microsoft PK keys. Without using the file pre-seeded with these keys, Secure Boot isn't actually doing anything when enabled. It just sits in Platform Setup mode, doing no validation.

Quickemu searches the OVMF code paths (even indices in the array) and selects the vars (code index+1) based on it. You've added an identical OVMF code path to a prior entry, which will never be matched.

I could be mistaken but, based on my testing, it seemed like this list gets evaluated in reverse-order so this ought to take precedence over the entry in the line above it. I think this is the desirable behaviour.

Also, which distro has this OVMF_VARS.secboot.fd file?

This path, as well as the one in the line above it come from edk2-ovmf shipped by Fedora (and EL8, EL9, EL10): https://src.fedoraproject.org/rpms/edk2/blob/f41/f/edk2.spec#_664

I can't find it in Ubuntu or Arch's edk packages.

From what I can tell, Arch doesn't ship the MS keys. Ubuntu seems to use /usr/share/OVMF/OVMF_VARS_4M.ms.fd as the path to the VARS pre-seeded with MS keys.

we can't have quickemu pointing to nonexistent files on any distro.

Why not? Most of these files don't exist on most of the distros that a user will run Quickemu on. Surely that's the whole point of iterating through the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking closer, I think I see what you mean.

The if [ -e "${1}" ] loop checks only the existence of the CODE file, making the existence of the VARS file irrelevant?

I had initially read this as if it checked both the CODE file and VARS file.

This fix does actually produce the desired behaviour but presumably what is happening is that because there's no early abort/return on the for loop, this code will always match the last path to a CODE file in the list that exists, in a situation where multiple exist. This explains why I thought it was being evaluated in reverse-order.

I added this as a new line rather than updating the line in-place because although my view is that it will always be preferable to use the VARS file with pre-seeded keys if it exists, I didn't want to cause a regression by updating the path reference in-place, in case there were distros that shipped /usr/share/edk2/ovmf/OVMF_VARS.fd but not the equivalent /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd.

Based on some searching though, it appears this only applies to one distro (Amazon Linux 2, for some reason), and users are unlikely to be running Quickemu on that:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FWIW, updating the equivalent path in-place for Debian/Ubuntu hosts would appear to be safe, as there are no major distros I can find that ship the non-PK-seeded file but don't ship the PK-seeded one.

See:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on some searching:

Distros which ship the MS PK keys in a separate VARS file:

  • Debian - /usr/share/OVMF/OVMF_VARS_4M.ms.fd
  • Ubuntu - /usr/share/OVMF/OVMF_VARS_4M.ms.fd
  • Fedora - /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd
  • CentOS / RHEL / etc - /usr/share/edk2/ovmf/OVMF_VARS.secboot.fd

Distros which do not seem to ship the MS keys at all

  • Alpine - /usr/share/OVMF/OVMF_VARS.fd
  • Arch - /usr/share/edk2/x64/OVMF_VARS.4m.fd
  • Void - /usr/share/edk2/x64/OVMF_VARS.4m.fd

Distros which only ship one VARS file (which always has the MS keys in it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use a replace-in-place method based on the above validation.

"${SHARE_PATH}/OVMF/x64/OVMF_CODE.secboot.fd","${SHARE_PATH}/OVMF/x64/OVMF_VARS.fd" \
"${SHARE_PATH}/edk2-ovmf/OVMF_CODE.secboot.fd","${SHARE_PATH}/edk2-ovmf/OVMF_VARS.fd" \
"${SHARE_PATH}/qemu/ovmf-x86_64-smm-ms-code.bin","${SHARE_PATH}/qemu/ovmf-x86_64-smm-ms-vars.bin" \
Expand Down