RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
Tdx Virtual Firmware (TDVF) includes one Firmware Volume (FV) known
as the Boot Firmware Volume (BFV). The FV format is defined in the
UEFI Platform Initialization (PI) spec. BFV includes all TDVF components
required during boot.
TDVF also include a configuration firmware volume (CFV) that is separated
from the BFV. The reason is because the CFV is measured in RTMR, while
the BFV is measured in MRTD.
In practice BFV is the code part of Ovmf image. CFV is the vars part of
Ovmf image (exclude the SPARE part).
PcdOvmfImageSizeInKb is added which is used to calculate the offset
of TdxMetadata in ResetVectorVtf0.asm.
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
OvmfPkg/OvmfPkg.dec | 12 ++++++++++++
OvmfPkg/OvmfPkgDefines.fdf.inc | 10 ++++++++++
2 files changed, 22 insertions(+)
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index c37dafad49bb..5216700754db 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -340,6 +340,18 @@
# header definition.
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader|0|UINT32|0x51
+ ## The base address and size of the TDX Cfv base and size.
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvBase|0|UINT32|0x52
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataOffset|0|UINT32|0x53
+ gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize|0|UINT32|0x54
+
+ ## The base address and size of the TDX Bfv base and size.
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvBase|0|UINT32|0x55
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataOffset|0|UINT32|0x56
+ gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataSize|0|UINT32|0x57
+
+ ## Size of the Ovmf image in KB
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfImageSizeInKb|0|UINT32|0x58
[PcdsDynamic, PcdsDynamicEx]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 3b5e45253916..1ed4be84107d 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -9,6 +9,7 @@
##
DEFINE BLOCK_SIZE = 0x1000
+DEFINE VARS_OFFSET = 0
#
# A firmware binary built with FD_SIZE_IN_KB=1024, and a firmware binary built
@@ -66,6 +67,7 @@ DEFINE SECFV_OFFSET = 0x003CC000
DEFINE SECFV_SIZE = 0x34000
!endif
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfImageSizeInKb = $(FD_SIZE_IN_KB)
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFdBaseAddress = $(FW_BASE_ADDRESS)
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize = $(FW_SIZE)
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareBlockSize = $(BLOCK_SIZE)
@@ -88,6 +90,14 @@ SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = $(VARS_SPARE_
# Computing Work Area header defined in the Include/WorkArea.h
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader = 4
+SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvBase = $(FW_BASE_ADDRESS)
+SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataOffset = $(VARS_OFFSET)
+SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize = $(VARS_LIVE_SIZE)
+
+SET gUefiOvmfPkgTokenSpaceGuid.PcdBfvBase = $(CODE_BASE_ADDRESS)
+SET gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataOffset = $(VARS_SIZE)
+SET gUefiOvmfPkgTokenSpaceGuid.PcdBfvRawDataSize = $(CODE_SIZE)
+
!if $(SMM_REQUIRE) == TRUE
SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
--
2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79932): https://edk2.groups.io/g/devel/message/79932
Mute This Topic: https://groups.io/mt/85242567/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi, > In practice BFV is the code part of Ovmf image. CFV is the vars part of > Ovmf image (exclude the SPARE part). Why do you exclude the spare part? From a security point of view I don't think it is a good idea to hard code any assumptions about the layout of the vars volume. > +SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvBase = $(FW_BASE_ADDRESS) > +SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataOffset = $(VARS_OFFSET) > +SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize = $(VARS_LIVE_SIZE) I'd suggest to use $(VARS_SIZE) here. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#79943): https://edk2.groups.io/g/devel/message/79943 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Monday, August 30, 2021 3:04 PM, Gerd Hoffmann wrote: > > Hi, > > > In practice BFV is the code part of Ovmf image. CFV is the vars part > > of Ovmf image (exclude the SPARE part). > > Why do you exclude the spare part? CFV includes all the provisioned data, such as UEFI Secure Boot Variable contents. It will be measured into RTMR by TDVF. So the other parts, such as SPARE part, is excluded because SPARE part should not be measured. Detailed information is in TDVF design guide Section 3.2 https://software.intel.com/content/dam/develop/external/us/en/documents/tdx-virtual-firmware-design-guide-rev-1.pdf > > From a security point of view I don't think it is a good idea to hard code any > assumptions about the layout of the vars volume. Do you mean I cannot assume the layout of VarStore? At least in Ovmf the VarStore.fdf.inc defines the layout of VarStore like below. [VARIABLE_STORE_HEADER]<-- 0 [ VAR 1 ] [ VAR 2 ] [ VAR n ] [ ] <-- VARS_LIVE_SIZE [ NV_EVENT_LOG ] [ NV_FTW_WORKING ] <-- VARS_SIZE > > > +SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvBase = > $(FW_BASE_ADDRESS) > > +SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataOffset = > $(VARS_OFFSET) > > +SET gUefiOvmfPkgTokenSpaceGuid.PcdCfvRawDataSize = > $(VARS_LIVE_SIZE) > > I'd suggest to use $(VARS_SIZE) here. As I explained above CFV only includes the provisioned data. So VARS_LIVE_SIZE is used. VARS_SIZE is the whole size of VarStore. > Thanks! Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#79974): https://edk2.groups.io/g/devel/message/79974 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi, > > From a security point of view I don't think it is a good idea to hard code any > > assumptions about the layout of the vars volume. > Do you mean I cannot assume the layout of VarStore? > At least in Ovmf the VarStore.fdf.inc defines the layout of VarStore like below. What prevents an attacker from creating a varstore with a different layout? Place the variables at the end of the file, which isn't measured (because you assume it is the spare part), then being able to change variables without the guest noticing? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#79977): https://edk2.groups.io/g/devel/message/79977 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On August 31, 2021 1:13 PM, Gerd Hoffmann wrote: > Hi, > > > > From a security point of view I don't think it is a good idea to > > > hard code any assumptions about the layout of the vars volume. > > Do you mean I cannot assume the layout of VarStore? > > At least in Ovmf the VarStore.fdf.inc defines the layout of VarStore like > below. > > What prevents an attacker from creating a varstore with a different layout? > Place the variables at the end of the file, which isn't measured (because you > assume it is the spare part), then being able to change variables without the > guest noticing? If the VarStore does not follow the layout defined in VarStore.fdf.inc, do you mean the current Variable mechanism still works? From the code of InitNonVolatileVariableStore(), the first variable is right after the VarStoreHeader. See GetStartPointer(). > > take care, > Gerd > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#79983): https://edk2.groups.io/g/devel/message/79983 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Tue, Aug 31, 2021 at 06:17:29AM +0000, Xu, Min M wrote: > On August 31, 2021 1:13 PM, Gerd Hoffmann wrote: > > Hi, > > > > > > From a security point of view I don't think it is a good idea to > > > > hard code any assumptions about the layout of the vars volume. > > > Do you mean I cannot assume the layout of VarStore? > > > At least in Ovmf the VarStore.fdf.inc defines the layout of VarStore like > > below. > > > > What prevents an attacker from creating a varstore with a different layout? > > Place the variables at the end of the file, which isn't measured (because you > > assume it is the spare part), then being able to change variables without the > > guest noticing? > If the VarStore does not follow the layout defined in VarStore.fdf.inc, do you mean > the current Variable mechanism still works? From the code of > InitNonVolatileVariableStore(), the first variable is right after the VarStoreHeader. > See GetStartPointer(). I didn't fully investigate what kind of attacks one can do. I'm pretty sure simply making the variable store larger and the spare smaller works, so parts of the variable store are outside the area you are measuring. Not fully sure whenever one can actually reorder the sections to move the varstore completely into the unmeasured area. Or play out other attacks with the same effect, like bloating some header struct. Simply measuring everything (including the spare) will stop all that. Changes wouldn't go unnoticed, period. No ifs and buts. So I'm wondering why you not doing that? Performance? Wouldn't be the first time a performance optimization pokes a hole into a security concept ... take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80002): https://edk2.groups.io/g/devel/message/80002 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On August 31, 2021 6:21 PM, Gerd Hoffmann wrote: > On Tue, Aug 31, 2021 at 06:17:29AM +0000, Xu, Min M wrote: > > On August 31, 2021 1:13 PM, Gerd Hoffmann wrote: > > > Hi, > > > > > > > > From a security point of view I don't think it is a good idea to > > > > > hard code any assumptions about the layout of the vars volume. > > > > Do you mean I cannot assume the layout of VarStore? > > > > At least in Ovmf the VarStore.fdf.inc defines the layout of > > > > VarStore like > > > below. > > > > > > What prevents an attacker from creating a varstore with a different layout? > > > Place the variables at the end of the file, which isn't measured > > > (because you assume it is the spare part), then being able to change > > > variables without the guest noticing? > > If the VarStore does not follow the layout defined in > > VarStore.fdf.inc, do you mean the current Variable mechanism still > > works? From the code of InitNonVolatileVariableStore(), the first variable is > right after the VarStoreHeader. > > See GetStartPointer(). > > I didn't fully investigate what kind of attacks one can do. I'm pretty sure simply > making the variable store larger and the spare smaller works, so parts of the > variable store are outside the area you are measuring. Not fully sure whenever > one can actually reorder the sections to move the varstore completely into the > unmeasured area. Or play out other attacks with the same effect, like bloating > some header struct. > > Simply measuring everything (including the spare) will stop all that. > Changes wouldn't go unnoticed, period. No ifs and buts. So I'm wondering why > you not doing that? Performance? Wouldn't be the first time a performance > optimization pokes a hole into a security concept ... > The measurement value of the CFV (provisioned configuration data) is extended to RTMR registers (similar to TPM PCRs). At the same time it is recorded in the TD Event log. These information will be used by the Attestation server (This is the so-called Attestation). In other words there is a known *good* CFV measurement value. Any changes to the CFV, for example the layout, the order of the variables, the content of the variables will produce a *bad* CFV measurement. Then in the later Attestation phase those changes will be detected. Thanks! Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80047): https://edk2.groups.io/g/devel/message/80047 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi, > > I didn't fully investigate what kind of attacks one can do. I'm pretty sure simply > > making the variable store larger and the spare smaller works, so parts of the > > variable store are outside the area you are measuring. Not fully sure whenever > > one can actually reorder the sections to move the varstore completely into the > > unmeasured area. Or play out other attacks with the same effect, like bloating > > some header struct. > > > > Simply measuring everything (including the spare) will stop all that. > > Changes wouldn't go unnoticed, period. No ifs and buts. So I'm wondering why > > you not doing that? Performance? Wouldn't be the first time a performance > > optimization pokes a hole into a security concept ... > > > The measurement value of the CFV (provisioned configuration data) is extended to > RTMR registers (similar to TPM PCRs). At the same time it is recorded in the TD Event > log. > These information will be used by the Attestation server (This is the so-called Attestation). > In other words there is a known *good* CFV measurement value. Any changes to > the CFV, for example the layout, the order of the variables, the content of the variables > will produce a *bad* CFV measurement. Yes. The attacker would need a varstore with a modified layout being approved by the attestation server as first step, then he would be able to modify variables unnoticed in a second step. So, assuming an attacker isn't able to carry out the first step it should be all fine in theory. When it comes to security it never hurts to have another line of defense though, so I would still strongly recommend to measure the complete varstore (including spare). At the end of the day it is your call, I'm not going to veto the patch. But I'll reserve the right to pull a "told you so" in case someone manages to exploit that some day. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80050): https://edk2.groups.io/g/devel/message/80050 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, 1 Sept 2021 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote: > > Hi, > > > > I didn't fully investigate what kind of attacks one can do. I'm pretty sure simply > > > making the variable store larger and the spare smaller works, so parts of the > > > variable store are outside the area you are measuring. Not fully sure whenever > > > one can actually reorder the sections to move the varstore completely into the > > > unmeasured area. Or play out other attacks with the same effect, like bloating > > > some header struct. > > > > > > Simply measuring everything (including the spare) will stop all that. > > > Changes wouldn't go unnoticed, period. No ifs and buts. So I'm wondering why > > > you not doing that? Performance? Wouldn't be the first time a performance > > > optimization pokes a hole into a security concept ... > > > > > The measurement value of the CFV (provisioned configuration data) is extended to > > RTMR registers (similar to TPM PCRs). At the same time it is recorded in the TD Event > > log. > > These information will be used by the Attestation server (This is the so-called Attestation). > > In other words there is a known *good* CFV measurement value. Any changes to > > the CFV, for example the layout, the order of the variables, the content of the variables > > will produce a *bad* CFV measurement. > > Yes. The attacker would need a varstore with a modified layout being > approved by the attestation server as first step, then he would be able > to modify variables unnoticed in a second step. > > So, assuming an attacker isn't able to carry out the first step it > should be all fine in theory. When it comes to security it never hurts > to have another line of defense though, so I would still strongly > recommend to measure the complete varstore (including spare). > > At the end of the day it is your call, I'm not going to veto the patch. > But I'll reserve the right to pull a "told you so" in case someone > manages to exploit that some day. > Have to agree with Gerd here: if those contents are being interpreted by the code, and may therefore affect its execution, I don't think it should be omitted from the measurement unless there is a compelling reason for it. Omitting it simply because you can doesn't seem sufficient justification to me. -- Ard. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80054): https://edk2.groups.io/g/devel/message/80054 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On September 1, 2021 2:57 PM, Ard Biesheuvel wrote: > On Wed, 1 Sept 2021 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > Hi, > > > > > > I didn't fully investigate what kind of attacks one can do. I'm > > > > pretty sure simply making the variable store larger and the spare > > > > smaller works, so parts of the variable store are outside the area > > > > you are measuring. Not fully sure whenever one can actually > > > > reorder the sections to move the varstore completely into the > > > > unmeasured area. Or play out other attacks with the same effect, like > bloating some header struct. > > > > > > > > Simply measuring everything (including the spare) will stop all that. > > > > Changes wouldn't go unnoticed, period. No ifs and buts. So I'm > > > > wondering why you not doing that? Performance? Wouldn't be the > > > > first time a performance optimization pokes a hole into a security > concept ... > > > > > > > The measurement value of the CFV (provisioned configuration data) is > > > extended to RTMR registers (similar to TPM PCRs). At the same time > > > it is recorded in the TD Event log. > > > These information will be used by the Attestation server (This is the so-called > Attestation). > > > In other words there is a known *good* CFV measurement value. Any > > > changes to the CFV, for example the layout, the order of the > > > variables, the content of the variables will produce a *bad* CFV > measurement. > > > > Yes. The attacker would need a varstore with a modified layout being > > approved by the attestation server as first step, then he would be > > able to modify variables unnoticed in a second step. > > > > So, assuming an attacker isn't able to carry out the first step it > > should be all fine in theory. When it comes to security it never > > hurts to have another line of defense though, so I would still > > strongly recommend to measure the complete varstore (including spare). > > > > At the end of the day it is your call, I'm not going to veto the patch. > > But I'll reserve the right to pull a "told you so" in case someone > > manages to exploit that some day. > > > > Have to agree with Gerd here: if those contents are being interpreted by the > code, and may therefore affect its execution, I don't think it should be omitted > from the measurement unless there is a compelling reason for it. Omitting it > simply because you can doesn't seem sufficient justification to me. CFV (the variables part) is treated as external input. For example, the secure boot variables. From the security perspective external input maybe modified by some malicious users. That's why it is measured so that in the later Attestation can find the modification. This is the same reason why the data downloaded from QEMU (thru fw_cfg) should be measured. As to the spare part in varstore, it is not external input, is it? It's produced and consumed by code itself. From this perspective it should not be measured. If the spare part is included in the measurement, then the *good* measurement is not known anymore. Because no one knows about the content of spare part in advance. > > -- > Ard. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80057): https://edk2.groups.io/g/devel/message/80057 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi, > As to the spare part in varstore, it is not external input, is it? It is part of the VARS file passed by the host to the guest. With normal ovmf its part of the writable flash. I'd consider that external input, although I think nothing actually uses it so it should just be a zero-filled block. take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80062): https://edk2.groups.io/g/devel/message/80062 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Min I agree with Gerd and Ard in this case. It is NOT so obvious that the FTW is produced then consumed in the code. What if the attacker prepares some special configuration to trigger the FTW process at the first boot, the code will do *read* before *write*? That is a potential attack surface. In my view, the design should be so simple that it is obviously no bug. I recommend to measure the whole CFV, to eliminate any potential attack surface, which is always the best way in security. To ensure the developer does configure the PCD correctly, I also recommend to add "ASSERT(CfvSize == Fv->FvLength" MeasureConfigurationVolume(). Thank you Yao Jiewen > -----Original Message----- > From: Xu, Min M <min.m.xu@intel.com> > Sent: Wednesday, September 1, 2021 3:19 PM > To: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io; > kraxel@redhat.com > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L > <jordan.l.justen@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Erdem > Aktas <erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; > Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky > <thomas.lendacky@amd.com> > Subject: RE: [edk2-devel] [PATCH V5 1/2] OvmfPkg: Introduce Tdx BFV/CFV PCDs > and PcdOvmfImageSizeInKb > > On September 1, 2021 2:57 PM, Ard Biesheuvel wrote: > > On Wed, 1 Sept 2021 at 08:10, Gerd Hoffmann <kraxel@redhat.com> wrote: > > > > > > Hi, > > > > > > > > I didn't fully investigate what kind of attacks one can do. I'm > > > > > pretty sure simply making the variable store larger and the spare > > > > > smaller works, so parts of the variable store are outside the area > > > > > you are measuring. Not fully sure whenever one can actually > > > > > reorder the sections to move the varstore completely into the > > > > > unmeasured area. Or play out other attacks with the same effect, like > > bloating some header struct. > > > > > > > > > > Simply measuring everything (including the spare) will stop all that. > > > > > Changes wouldn't go unnoticed, period. No ifs and buts. So I'm > > > > > wondering why you not doing that? Performance? Wouldn't be the > > > > > first time a performance optimization pokes a hole into a security > > concept ... > > > > > > > > > The measurement value of the CFV (provisioned configuration data) is > > > > extended to RTMR registers (similar to TPM PCRs). At the same time > > > > it is recorded in the TD Event log. > > > > These information will be used by the Attestation server (This is the so- > called > > Attestation). > > > > In other words there is a known *good* CFV measurement value. Any > > > > changes to the CFV, for example the layout, the order of the > > > > variables, the content of the variables will produce a *bad* CFV > > measurement. > > > > > > Yes. The attacker would need a varstore with a modified layout being > > > approved by the attestation server as first step, then he would be > > > able to modify variables unnoticed in a second step. > > > > > > So, assuming an attacker isn't able to carry out the first step it > > > should be all fine in theory. When it comes to security it never > > > hurts to have another line of defense though, so I would still > > > strongly recommend to measure the complete varstore (including spare). > > > > > > At the end of the day it is your call, I'm not going to veto the patch. > > > But I'll reserve the right to pull a "told you so" in case someone > > > manages to exploit that some day. > > > > > > > Have to agree with Gerd here: if those contents are being interpreted by the > > code, and may therefore affect its execution, I don't think it should be omitted > > from the measurement unless there is a compelling reason for it. Omitting it > > simply because you can doesn't seem sufficient justification to me. > CFV (the variables part) is treated as external input. For example, the secure > boot > variables. From the security perspective external input maybe modified by some > malicious users. That's why it is measured so that in the later Attestation can > find > the modification. This is the same reason why the data downloaded from QEMU > (thru fw_cfg) should be measured. > As to the spare part in varstore, it is not external input, is it? It's produced and > consumed > by code itself. From this perspective it should not be measured. If the spare part > is included in the measurement, then the *good* measurement is not known > anymore. > Because no one knows about the content of spare part in advance. > > > > -- > > Ard. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80067): https://edk2.groups.io/g/devel/message/80067 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Wed, 2021-09-01 at 08:59 +0000, Yao, Jiewen wrote: > Hi Min > I agree with Gerd and Ard in this case. > > It is NOT so obvious that the FTW is produced then consumed in the > code. What if the attacker prepares some special configuration to > trigger the FTW process at the first boot, the code will do *read* > before *write*? That is a potential attack surface. It's not just that: even if you can ensure nothing in the host changed the variables, how do you know *your* code inside the guest is updating them? In ordinary OVMF we try to ensure that by having the variables SMM protected so the only update path available to the kernel is via the setVariable interface, but we can't do that in the confidential computing case because SMM isn't supported. That means a random kernel attacker in the guest can potentially write to the var store too. At least for the first SEV prototype I had to make the var store part of the first firmware volume firstly so it got measured but secondly so it couldn't be used as a source of configuration attacks. I have a nasty feeling that configuration attacks are going to be the bane of all confidential computing solutions because they give the untrusted VMM a wide attack surface. James -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80107): https://edk2.groups.io/g/devel/message/80107 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
> On Sep 1, 2021, at 9:53 AM, James Bottomley <jejb@linux.ibm.com> wrote: > > On Wed, 2021-09-01 at 08:59 +0000, Yao, Jiewen wrote: >> Hi Min >> I agree with Gerd and Ard in this case. >> >> It is NOT so obvious that the FTW is produced then consumed in the >> code. What if the attacker prepares some special configuration to >> trigger the FTW process at the first boot, the code will do *read* >> before *write*? That is a potential attack surface. > > It's not just that: even if you can ensure nothing in the host changed > the variables, how do you know *your* code inside the guest is updating > them? In ordinary OVMF we try to ensure that by having the variables > SMM protected so the only update path available to the kernel is via > the setVariable interface, but we can't do that in the confidential > computing case because SMM isn't supported. That means a random kernel > attacker in the guest can potentially write to the var store too. > > At least for the first SEV prototype I had to make the var store part > of the first firmware volume firstly so it got measured but secondly so > it couldn't be used as a source of configuration attacks. > > I have a nasty feeling that configuration attacks are going to be the > bane of all confidential computing solutions because they give the > untrusted VMM a wide attack surface. > James, If we take a big step back the requirement for an EFI Runtime Service, like the variable API, is just exclusive access to hardware at OS runtime. The variable store needs to be on a hardware device that has a persistent reliable store. The FTW is really about maintaining the consistency of the store if the power gets yanked at the wrong moment. So the fact that the UEFI Variable Store is in NOR FLASH is a historical artifact more than architecture. Also on physical devices hardware cost money, and you need the NOR FLASH for the firmware so why change it. Thus conceptually the variable store could be backed by a virtual hardware device that was designed with security in mind. Maybe more of message passing interface and the reliability of updates is maintained by the hardware device not the UEFI code. It would also be possible for the hardware device to enforce security policy. You could even have EFI send a one shot message per 1st boot to the hardware to define a security policy. If you wanted the hardware device could even implement the UEFI Secure Boot infrastructure so the UEFI Variable Driver could be untrusted. I guess this hypothetical variable store virtual hardware device could also have hardware access to other security hardware resources (like a TPM) and implement security policies based on that. FYI on Macs with a T2 (security chip) the UEFI variable store lives on the T2. Thanks, Andrew Fish > James > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80110): https://edk2.groups.io/g/devel/message/80110 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
I have few naive questions. Sorry if the answers were obvious. >>TDVF also include a configuration firmware volume (CFV) that is separated >>from the BFV. The reason is because the CFV is measured in RTMR, while >>the BFV is measured in MRTD. If I understand correctly, this means that the BFV is encrypted and measured during TD build time. Since CFV is not included in the MRTD, CFV region is not encrypted with the guest key, is it? >> The measurement value of the CFV (provisioned configuration data) is extended to >> RTMR registers (similar to TPM PCRs). At the same time it is recorded in the TD Event >> log. Even if it is measured at runtime, the content needs to be copied to somewhere else, otherwise what stops VMM to change the content after it is being measured (assuming that it is not encrypted). >> As to the spare part in varstore, it is not external input, is it? It's produced and consumed >> by code itself. From this perspective it should not be measured. If the spare part >> is included in the measurement, then the *good* measurement is not known anymore. >> Because no one knows about the content of spare part in advance. I am confused about how this memory is initialized. If it is encrypted, then no need to measure it but also it becomes useless as the key will change in the next boot. If it is not encrypted, VMM can always modify the content and might cause unexpected behavior at runtime, right? I might be missing something here but if this region is not encrypted: - CFV content needs to be copied into an encrypted buffer after being measured and should never be used again. - Allowing variables to be stored in SPARE part seems like opening an attack surface as no one knows what will be stored in that region. Is this correct understanding? -Erdem On Wed, Sep 1, 2021 at 10:20 PM Andrew Fish <afish@apple.com> wrote: > > > > On Sep 1, 2021, at 9:53 AM, James Bottomley <jejb@linux.ibm.com> wrote: > > > > On Wed, 2021-09-01 at 08:59 +0000, Yao, Jiewen wrote: > >> Hi Min > >> I agree with Gerd and Ard in this case. > >> > >> It is NOT so obvious that the FTW is produced then consumed in the > >> code. What if the attacker prepares some special configuration to > >> trigger the FTW process at the first boot, the code will do *read* > >> before *write*? That is a potential attack surface. > > > > It's not just that: even if you can ensure nothing in the host changed > > the variables, how do you know *your* code inside the guest is updating > > them? In ordinary OVMF we try to ensure that by having the variables > > SMM protected so the only update path available to the kernel is via > > the setVariable interface, but we can't do that in the confidential > > computing case because SMM isn't supported. That means a random kernel > > attacker in the guest can potentially write to the var store too. > > > > At least for the first SEV prototype I had to make the var store part > > of the first firmware volume firstly so it got measured but secondly so > > it couldn't be used as a source of configuration attacks. > > > > I have a nasty feeling that configuration attacks are going to be the > > bane of all confidential computing solutions because they give the > > untrusted VMM a wide attack surface. > > > > James, > > If we take a big step back the requirement for an EFI Runtime Service, like the variable API, is just exclusive access to hardware at OS runtime. The variable store needs to be on a hardware device that has a persistent reliable store. The FTW is really about maintaining the consistency of the store if the power gets yanked at the wrong moment. So the fact that the UEFI Variable Store is in NOR FLASH is a historical artifact more than architecture. Also on physical devices hardware cost money, and you need the NOR FLASH for the firmware so why change it. Thus conceptually the variable store could be backed by a virtual hardware device that was designed with security in mind. Maybe more of message passing interface and the reliability of updates is maintained by the hardware device not the UEFI code. It would also be possible for the hardware device to enforce security policy. You could even have EFI send a one shot message per 1st boot to the hardware to define a security policy. If you wanted the hardware device could even implement the UEFI Secure Boot infrastructure so the UEFI Variable Driver could be untrusted. I guess this hypothetical variable store virtual hardware device could also have hardware access to other security hardware resources (like a TPM) and implement security policies based on that. > > FYI on Macs with a T2 (security chip) the UEFI variable store lives on the T2. > > Thanks, > > Andrew Fish > > > > James > > > > > > > > > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#80500): https://edk2.groups.io/g/devel/message/80500 Mute This Topic: https://groups.io/mt/85242567/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.