| Message ID | 20231115151242.184645-1-kraxel@redhat.com |
|---|---|
| Headers | show |
| Series | hw/uefi: add uefi variable service | expand |
Hey Gerd! On 15.11.23 16:12, Gerd Hoffmann wrote: > This patch adds a virtual device to qemu which the uefi firmware can use > to store variables. This moves the UEFI variable management from > privileged guest code (managing vars in pflash) to the host. Main > advantage is that the need to have privilege separation in the guest > goes away. > > On x86 privileged guest code runs in SMM. It's supported by kvm, but > not liked much by various stakeholders in cloud space due to the > complexity SMM emulation brings. > > On arm privileged guest code runs in el3 (aka secure world). This is > not supported by kvm, which is unlikely to change anytime soon given > that even el2 support (nested virt) is being worked on for years and is > not yet in mainline. > > The design idea is to reuse the request serialization protocol edk2 uses > for communication between SMM and non-SMM code, so large chunks of the > edk2 variable driver stack can be used unmodified. Only the driver > which traps into SMM mode must be replaced by a driver which talks to > qemu instead. I'm not sure I like the split :). If we cut things off at the SMM communication layer, we still have a lot of code inside the Runtime Services (RTS) code that is edk2 specific which means we're tying ourselves tightly to the edk2 data format. It also means we can not easily expose UEFI variables that QEMU owns, which can come in very handy when implementing MOR - another feature that depends on SMM today. In EC2, we are simply serializing all variable RTS calls to the hypervisor, similar to the Xen implementation (https://www.youtube.com/watch?v=jiR8khaECEk). The edk2 side code we have built is here: https://github.com/aws/uefi/tree/main/edk2-stable202211 (see anything with VarStore in the name). For the vmm side, we currently have an AWS-internal C++ implementation that I can convert into QEMU code and send as patch if there is real interest. Given that we deal with untrusted input, I would strongly prefer if we could move it to a Rust implementation instead though. We started a Rust reimplementation of it that interface that can build as a library with C bindings which QEMU could then link against: https://github.com/Mimoja/rs-uefi-varstore/tree/for_main The code never went beyond the initial stages, but if we're pulling the variable store to QEMU, this would be the best path forward IMHO. If instead, we just want something we can quickly integrate while eating up the additional security risk, I think we should just reuse the Xen implementation. Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
On Mon, Nov 20, 2023 at 12:53:45PM +0100, Alexander Graf wrote: > Hey Gerd! > > On 15.11.23 16:12, Gerd Hoffmann wrote: > > This patch adds a virtual device to qemu which the uefi firmware can use > > to store variables. This moves the UEFI variable management from > > privileged guest code (managing vars in pflash) to the host. Main > > advantage is that the need to have privilege separation in the guest > > goes away. > > > > On x86 privileged guest code runs in SMM. It's supported by kvm, but > > not liked much by various stakeholders in cloud space due to the > > complexity SMM emulation brings. > > > > On arm privileged guest code runs in el3 (aka secure world). This is > > not supported by kvm, which is unlikely to change anytime soon given > > that even el2 support (nested virt) is being worked on for years and is > > not yet in mainline. > > > > The design idea is to reuse the request serialization protocol edk2 uses > > for communication between SMM and non-SMM code, so large chunks of the > > edk2 variable driver stack can be used unmodified. Only the driver > > which traps into SMM mode must be replaced by a driver which talks to > > qemu instead. > > > I'm not sure I like the split :). If we cut things off at the SMM > communication layer, we still have a lot of code inside the Runtime Services > (RTS) code that is edk2 specific which means we're tying ourselves tightly > to the edk2 data format. Which data format? Request serialization format? Yes. I can't see what is wrong with that. We need one anyway, and I don't see why inventing a new one is any better than the one we already have in edk2. Variable storage format? Nope, that is not the case. The variable driver supports a cache, which I think is a read-only mapping of the variable store, so using that might imply we have to use edk2 storage format. Didn't check in detail through. The cache is optional, can be disabled at compile time using PcdEnableVariableRuntimeCache=FALSE, and I intentionally do not use the cache feature, exactly to avoid unwanted constrains to the host side implementation. > It also means we can not easily expose UEFI > variables that QEMU owns, Qemu owning variables should be no problem. Adding monitor commands to read/write UEFI variables should be possible too. > which can come in very handy when implementing MOR > - another feature that depends on SMM today. Have a pointer for me? Google finds me https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/device-guard-requirements, which describes the variable behavior (which I think should be no problem to implement), but doesn't say a word about what exactly gets locked when enabled ... > In EC2, we are simply serializing all variable RTS calls to the hypervisor, The edk2 code effectively does the same (with PcdEnableVariableRuntimeCache=FALSE). > similar to the Xen implementation > (https://www.youtube.com/watch?v=jiR8khaECEk). Is the Xen implementation upstream? Can't see a xen variable driver in OvmfPkg. The video is from 2019. What is the state of this? > The edk2 side code we have built is here: > https://github.com/aws/uefi/tree/main/edk2-stable202211 (see anything with > VarStore in the name). Ok, so it's just the variables. The edk2 variant also sends variable policy requests (see Edk2VariablePolicyProtocol, https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/VariablePolicyLib/ReadMe.md). And it can easily be extended should the need arise in the future (all requests are tagged with a protocol/event guid). > Given that we deal with untrusted input, I would strongly prefer if we could > move it to a Rust implementation instead though. Valid point. I've started in C because I have next no to experience with rust (yet), so getting a test-able / demo-able implementation done was much easier for me. Also I think we don't have any rust infrastructure in qemu (yet?). Being able to use the qapi / qobject support to read/write the variable store in json format is nice too. But I'm open to discuss other options. > We started a Rust > reimplementation of it that interface that can build as a library with C > bindings which QEMU could then link against: > > https://github.com/Mimoja/rs-uefi-varstore/tree/for_main > > The code never went beyond the initial stages, Hmm. Why not? > but if we're pulling the variable store to QEMU, this would be the > best path forward IMHO. Ok, so you are trying to sell me a prototype as solution? /me looks a bit skeptical ... take care, Gerd
On 11/20/23 17:50, Gerd Hoffmann wrote: > On Mon, Nov 20, 2023 at 12:53:45PM +0100, Alexander Graf wrote: >> Hey Gerd! >> >> On 15.11.23 16:12, Gerd Hoffmann wrote: >>> This patch adds a virtual device to qemu which the uefi firmware can use >>> to store variables. This moves the UEFI variable management from >>> privileged guest code (managing vars in pflash) to the host. Main >>> advantage is that the need to have privilege separation in the guest >>> goes away. >>> >>> On x86 privileged guest code runs in SMM. It's supported by kvm, but >>> not liked much by various stakeholders in cloud space due to the >>> complexity SMM emulation brings. >>> >>> On arm privileged guest code runs in el3 (aka secure world). This is >>> not supported by kvm, which is unlikely to change anytime soon given >>> that even el2 support (nested virt) is being worked on for years and is >>> not yet in mainline. >>> >>> The design idea is to reuse the request serialization protocol edk2 uses >>> for communication between SMM and non-SMM code, so large chunks of the >>> edk2 variable driver stack can be used unmodified. Only the driver >>> which traps into SMM mode must be replaced by a driver which talks to >>> qemu instead. >> >> >> I'm not sure I like the split :). If we cut things off at the SMM >> communication layer, we still have a lot of code inside the Runtime Services >> (RTS) code that is edk2 specific which means we're tying ourselves tightly >> to the edk2 data format. > > Which data format? > > Request serialization format? Yes. I can't see what is wrong with > that. ... I can't even see what's wrong with *that*. Realistically / practically, I think only edk2 has been considered as guest UEFI firmware for QEMU/KVM virtual machines, as far as upstream projects go. It's certainly edk2 that's bundled with QEMU. My understanding is that firmware is just considered a part of the virtualization platform, so teaching edk2 specifics to QEMU doesn't seem wrong. (As long as we have the personpower to maintain interoperability.) > We need one anyway, and I don't see why inventing a new one is > any better than the one we already have in edk2. > > Variable storage format? Nope, that is not the case. The variable > driver supports a cache, which I think is a read-only mapping of the > variable store, so using that might imply we have to use edk2 storage > format. Didn't check in detail through. The cache is optional, can be > disabled at compile time using PcdEnableVariableRuntimeCache=FALSE, and > I intentionally do not use the cache feature, exactly to avoid unwanted > constrains to the host side implementation. > >> It also means we can not easily expose UEFI >> variables that QEMU owns, > > Qemu owning variables should be no problem. Adding monitor commands to > read/write UEFI variables should be possible too. This patch set is actually an improvement towards QEMU-owned variables. Right now, all variables are internal to the guest; QEMU only has a pflash-level view. > >> which can come in very handy when implementing MOR >> - another feature that depends on SMM today. > > Have a pointer for me? Google finds me > https://learn.microsoft.com/en-us/windows-hardware/drivers/bringup/device-guard-requirements, > which describes the variable behavior (which I think should be no > problem to implement), but doesn't say a word about what exactly gets > locked when enabled ... See: TCG PC Client Platform Reset Attack Mitigation Specification My copy is Family “2.0” Version 1.10 Revision 17 January 21, 2019 Published You should find it somewhere in the download area of <https://trustedcomputinggroup.org>. E.g.... <https://www.trustedcomputinggroup.org/wp-content/uploads/Platform-Reset-Attack-Mitigation-Specification.pdf> In the past we've had a bunch of discussions / patches around this. Some examples: - [edk2] multiple levels of support for MOR / MORLock http://mid.mail-archive.com/039cf353-80fb-9f20-6ad2-f52517ab4de7@redhat.com - https://bugzilla.tianocore.org/show_bug.cgi?id=727 (see edk2 commit range listed there, too) - commit 704b71d7e11f115a3b5b03471d6420a7a70f1585 - commit d20ae95a13e851d56c6618108b18c93526505ca2 - https://bugzilla.redhat.com/show_bug.cgi?id=1854212 - https://bugzilla.redhat.com/show_bug.cgi?id=1498159 > >> In EC2, we are simply serializing all variable RTS calls to the hypervisor, > > The edk2 code effectively does the same (with PcdEnableVariableRuntimeCache=FALSE). > >> similar to the Xen implementation >> (https://www.youtube.com/watch?v=jiR8khaECEk). > > Is the Xen implementation upstream? Can't see a xen variable driver in > OvmfPkg. The video is from 2019. What is the state of this? Not sure about the current state, but when that presentation came out, we discussed it briefly internally. I don't have time to review that old discussion now for the sake of potentially publishing it inside this discussion, but for interested Red Hatters, I can provide a pointer: [virt-devel] Securing Secure Boot on Xen | FOSDEM'19 Date: Fri, 8 Feb 2019 20:46:46 +0100 msgid: <37382312-986c-4ce4-1ba3-942697738d65@redhat.com> > >> The edk2 side code we have built is here: >> https://github.com/aws/uefi/tree/main/edk2-stable202211 (see anything with >> VarStore in the name). Well... why was this never upstreamed to edk2? (Side question, of course.) > > Ok, so it's just the variables. The edk2 variant also sends variable > policy requests (see Edk2VariablePolicyProtocol, > https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/VariablePolicyLib/ReadMe.md). > And it can easily be extended should the need arise in the future (all > requests are tagged with a protocol/event guid). > >> Given that we deal with untrusted input, I would strongly prefer if we could >> move it to a Rust implementation instead though. > > Valid point. Agree that the point is valid. (And this was one of my concerns, if not my main concern, in the above internal discussion, about Xen varstored.) Especially due to the heavy crypto that the final version is supposed to have. Even during the present patch review, while going through only the headers thus far, I've already said at least twice that we're going to have to be super careful about integer overflows and buffer overflows. Any such problem is no longer a guest<->guest privilege boundary breach but a guest<->host one. Not sure if the suggested remedy ("write it in Rust") is practical. > > I've started in C because I have next no to experience with rust (yet), > so getting a test-able / demo-able implementation done was much easier > for me. Also I think we don't have any rust infrastructure in qemu > (yet?). Being able to use the qapi / qobject support to read/write the > variable store in json format is nice too. > > But I'm open to discuss other options. A selfish aspect: given that I've been reviewing this set, should I consider it a proof of concept / prototype, or something we might want to build upon, i.e., should I assume we'd put these foundations into production at some point? I've been reviewing the series with the latter in mind, but if that's not correct, I should probably adjust my pedantry knob. > >> We started a Rust >> reimplementation of it that interface that can build as a library with C >> bindings which QEMU could then link against: >> >> https://github.com/Mimoja/rs-uefi-varstore/tree/for_main >> >> The code never went beyond the initial stages, > > Hmm. Why not? And I'm a bit annoyed that you had to write ~2500 lines of QEMU code (not counting edk2) for qemu-devel to learn about these initiatives. :/ > >> but if we're pulling the variable store to QEMU, this would be the >> best path forward IMHO. > > Ok, so you are trying to sell me a prototype as solution? > /me looks a bit skeptical ... at least with virtiofsd, we had gone with a C impl first, and only then with a Rust impl... Laszlo
On Tue, Nov 21, 2023 at 04:58:44PM +0100, Laszlo Ersek wrote: > On 11/20/23 17:50, Gerd Hoffmann wrote: > > On Mon, Nov 20, 2023 at 12:53:45PM +0100, Alexander Graf wrote: > >> Hey Gerd! > >> > >> On 15.11.23 16:12, Gerd Hoffmann wrote: > >>> This patch adds a virtual device to qemu which the uefi firmware can use > >>> to store variables. This moves the UEFI variable management from > >>> privileged guest code (managing vars in pflash) to the host. Main > >>> advantage is that the need to have privilege separation in the guest > >>> goes away. > >>> > >>> On x86 privileged guest code runs in SMM. It's supported by kvm, but > >>> not liked much by various stakeholders in cloud space due to the > >>> complexity SMM emulation brings. > >>> > >>> On arm privileged guest code runs in el3 (aka secure world). This is > >>> not supported by kvm, which is unlikely to change anytime soon given > >>> that even el2 support (nested virt) is being worked on for years and is > >>> not yet in mainline. > >>> > >>> The design idea is to reuse the request serialization protocol edk2 uses > >>> for communication between SMM and non-SMM code, so large chunks of the > >>> edk2 variable driver stack can be used unmodified. Only the driver > >>> which traps into SMM mode must be replaced by a driver which talks to > >>> qemu instead. > >> > >> > >> I'm not sure I like the split :). If we cut things off at the SMM > >> communication layer, we still have a lot of code inside the Runtime Services > >> (RTS) code that is edk2 specific which means we're tying ourselves tightly > >> to the edk2 data format. > > > > Which data format? > > > > Request serialization format? Yes. I can't see what is wrong with > > that. > > ... I can't even see what's wrong with *that*. Realistically / > practically, I think only edk2 has been considered as guest UEFI > firmware for QEMU/KVM virtual machines, as far as upstream projects go. > It's certainly edk2 that's bundled with QEMU. > > My understanding is that firmware is just considered a part of the > virtualization platform, so teaching edk2 specifics to QEMU doesn't seem > wrong. (As long as we have the personpower to maintain interoperability.) > > > We need one anyway, and I don't see why inventing a new one is > > any better than the one we already have in edk2. > > > > Variable storage format? Nope, that is not the case. The variable > > driver supports a cache, which I think is a read-only mapping of the > > variable store, so using that might imply we have to use edk2 storage > > format. Didn't check in detail through. The cache is optional, can be > > disabled at compile time using PcdEnableVariableRuntimeCache=FALSE, and > > I intentionally do not use the cache feature, exactly to avoid unwanted > > constrains to the host side implementation. > > > >> It also means we can not easily expose UEFI > >> variables that QEMU owns, > > > > Qemu owning variables should be no problem. Adding monitor commands to > > read/write UEFI variables should be possible too. > > This patch set is actually an improvement towards QEMU-owned variables. > Right now, all variables are internal to the guest; QEMU only has a > pflash-level view. To throw confidental computing into the mix.... Right now for SEV-SNP/TDX, the EDK2 builds are stateless so that variables are thrown away at poweroff. Long term though there's interest in having the ability to (optionally) offer persistence of variables to confidential computing too. The key issue is that the host QEMU is not trusted so it would not be viable to let QEMU receive the variables in plain text. One option I've illustrated before is that have SVSM (or equiv) expose an encrypted storage service to EDK2. Given the proposed EDK2 side protocol/modifications for variable storage, I wonder if it is viable for SVSM (or equiv) to replace QEMU in providing the backend storage impl ? IOW, host QEMU would expose a pflash to the guest, to which SVSM would apply full disk encryption, and then store the EFI variables in it With regards, Daniel
Hi, > Even during the present patch review, while going through only the > headers thus far, I've already said at least twice that we're going to > have to be super careful about integer overflows and buffer overflows. > Any such problem is no longer a guest<->guest privilege boundary breach > but a guest<->host one. > > Not sure if the suggested remedy ("write it in Rust") is practical. It should prevent certain classes of bugs such as buffer overflows. Not sure how much compromises you have to make (i.e. 'unsafe' code sections) for a C library interface, so you can link the lib into qemu. And of course it wouldn't automatically stop logic errors. > > But I'm open to discuss other options. > > A selfish aspect: given that I've been reviewing this set, should I > consider it a proof of concept / prototype, or something we might want > to build upon, i.e., should I assume we'd put these foundations into > production at some point? I've been reviewing the series with the latter > in mind, but if that's not correct, I should probably adjust my pedantry > knob. In case we continue the C route I certainly expect that this patch set will turn into something production-ready, and I've tried to code things up accordingly. Copy buffers so the guest can't modify them while qemu processes them, carefully check length fields, ... > at least with virtiofsd, we had gone with a C impl first, and only then > with a Rust impl... And virtiofsd was easier because it is a completely separate process, not something you want link into qemu ... take care, Gerd
Hi, > One option I've illustrated before is that have SVSM (or equiv) > expose an encrypted storage service to EDK2. Given the proposed EDK2 > side protocol/modifications for variable storage, I wonder if it is > viable for SVSM (or equiv) to replace QEMU in providing the backend > storage impl ? IOW, host QEMU would expose a pflash to the guest, > to which SVSM would apply full disk encryption, and then store the > EFI variables in it Yes. IIRC (it's been a while I've looked at the spec) the SVSM can request that access to specific pages trap, so it could use that to emulate the mmio/sysbus variant of the device. Not sure if that could work for io/isa too. Another option would be to add a third interface variant which uses SVSM service calls. And one advantage of a rust implementation would be that integrating with coconut-svsm (which is written in rust too) might be easier. On the other hand the SVSM is a very different environment, the rust stdlib is not available for example, and the way persistence is implemented will probably look very different too. Another difference is crypto support, qemu uses nettle / gnutls whereas svsm will probably use openssl. So not sure how big the opportunity to share the code between qemu and svsm on the backend side actually is. take care, Gerd