-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
fix: Secure Boot does not work for Linux guests #1579
base: master
Are you sure you want to change the base?
Conversation
@@ -665,6 +675,7 @@ function configure_bios() { | |||
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" \ | |||
"${SHARE_PATH}/edk2/ovmf/OVMF_CODE.secboot.fd","${SHARE_PATH}/edk2/ovmf/OVMF_VARS.secboot.fd" \ |
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.
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.
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.
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.
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.
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?
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.
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:
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.
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:
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.
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)
- NixOS
- Based on what I can tell from the nixpkgs entry for ovmf
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.
Updated to use a replace-in-place method based on the above validation.
8add318
to
b9c4633
Compare
b9c4633
to
003e8ea
Compare
Description
This change contains two fixes which ought to take the functionality of Secure Booting Linux guests in Quickemu from non-functional to a working state.
SMM fix
Commit b973580 should stand alone and resolve #1578. This enables SMM for Linux guests on Linux hosts when Secure Boot has been enabled.
Microsoft Platform Key fix (partial)
Commit 8add318 is an example of an initial commit which will probably need wider expansion and partially resolves #413. This configures Quickemu to prefer the
*.secboot.*
version of theOVMF_VARS
.fd
file, which is the one which has the Microsoft Platform Keys preloaded into it.I'm honestly less sure about this second fix as although it makes sense to me (the
*.secboot.*
file is needed to have full Secure Boot working, rather than just Secure Boot stuck in Platform Setup Mode), I note that Quickemu also supports Windows 11 which makes Secure Boot mandatory, but I couldn't work out how that support was being provided.I'll do some digging, but it's possible that the Secure Boot config for Windows 11 also hasn't been using the Microsoft Platform Keys up until now, meaning it is effectively doing nothing in Quickemu's current state.
This commit will also need expanding. Currently not every distribution ships the Microsoft PK enrolled VARS files, and the ones that do seem to put them in different places. I'm happy to go hunting around the common host distros that people might be using for Quickemu if this commit is reviewed as sensible and expand the search list for these files with pre-enrolled keys, but for now this fix only applies to Fedora as a host distro (or anything else that uses the exact same path on disk as Fedora does for these files).
Type of change
Checklist: