Please aware that this option will cause potential security risk.
In case that any the guest component only knows one of vTPM or RTMR, and only
extends one of vTPM or RTMR, but the other one only verifies the other, then
the chain of trust is broken.
This solution is secure if and only if all guest components aware of
coexistence, and can ensure all measurements are extended to both vTPM and RTMR.
But I am not sure if all guest components are ready today.
Since this option caused a potential risk / misuse breaking the chain of trust,
I recommend we have at least one more company to endorse the runtime
co-existence of vTPM and RTMR.
Also, I would like to hear the opinions from other companies.
BTW: A small comment: In EDKII, we don’t use MACRO. Please change to PCD
(default false), after you get endorsement from other compony.
Thank you
Yao, Jiewen
> -----Original Message-----
> From: Dionna Amalie Glaze <dionnagl...@google.com>
> Sent: Friday, March 22, 2024 1:47 AM
> To: qinkun Bao <qin...@google.com>
> Cc: devel@edk2.groups.io; linux-c...@lists.linux.dev; Aktas, Erdem
> <erdemak...@google.com>; Yao, Jiewen <jiewen....@intel.com>; Ard
> Biesheuvel <a...@kernel.org>; Peter Gonda <pgo...@google.com>; James
> Bottomley <j...@linux.ibm.com>; Gerd Hoffmann <kra...@redhat.com>; Tom
> Lendacky <thomas.lenda...@amd.com>; Michael Roth
> <michael.r...@amd.com>
> Subject: Re: [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance
> of vTPM and RTMR.
>
> On Thu, Mar 21, 2024 at 9:59 AM qinkun Bao <qin...@google.com> wrote:
> >
> > From: Qinkun Bao <qin...@google.com>
> >
> > The UEFI v2.10 spec defines the protocol EFI_CC_MEASUREMENT_PROTOCOL
> > to enable (for example) RTMR-based boot measurement for TDX VMs.
> > With the current UEFI spec’s “should not” wording and EDK2
> > implementation, TPM measurement in TDVF is disabled when
> > RTMR measurement is enabled.
> >
> > Mutual exclusion of the CC measurement protocol and TCG measurement
> > protocol breaks backwards compatibility, which makes adoption of RTMRs
> > challenging. A virtualized TPM device (vTPM) managed by the host VMM
> > makes boot measurements visible to the VMM operator, but this is an
> > oft-requested feature that users can choose to accept.
> >
> > The TPM has been a standard for over a decade and many existing
> > applications rely on the TPM. Both inside and outside Google,
> > we have many users that require vTPM, including features that are
> > not easily available via RTMRs (e.g. sealing using keys that the
> > guest OS cannot access).
> >
> > This patch adds a non-default build option to allow the coexistence
> > of both the CC measurement and TCG protocols. Not included is a
> > vendor-specific measured event in the CC event log that indicates
> > whether a vTPM is attached or not.
>
> Thank you very much for starting this conversation. I'll carry forward
> more context from our more senior engineers.
>
> '
> Not measuring into all possible measurement banks led to
> (CVE-2021-42299) with TPM PCR banks. Let's not repeat this problem.
> Requiring a monumental shift of all attestation-based applications to
> CC_MEASUREMENT_PROTOCOL and CEL in one step is going to make the
> technology very difficult to adopt.
> '
>
> The combination of these requirements means careful rollout of
> attestation verification policies to match the updated behavior of the
> firmware.
> The measured boot components have to be known to correctly measure
> into all available measurement protocols and register banks.
> The firmware has to be known specifically which protocols it makes available.
>
> Now, how this is done is a vendor matter for now. If there is strong
> demand for making vTPM attachment status known for folks who really
> don't want a TPM and don't trust the VMM to not attach one anyway,
> we'll need to agree that the CEL should have an entry for an RTMR0
> update detailing the combination of measurement protocols in use.
> Likewise PCR1 should have an event detailing the protocols in use if
> we want to make CC_MEASUREMENT_PROTOCOL usage configurable.
>
> Philosophizing aside that both protocols should be allowed to be
> active and that the spec should be updated to say something along the
> lines of "all measurement protocols that are active (i.e., any
> combination of EFI_CC_MEASUREMENT_PROTOCOL, EFI_TCG_PROTOCOL,
> EFI_TCG2_PROTOCOL) should have comparable events measured if any one
> protocol receives measurements. All measurement protocols that are
> active MUST measure comparable separator events if any protocol
> receives a separator event." This is crossing a spec boundary between
> USWG and TCG, so I don't know where specifically the language needs to
> go, but we need some language that implementers can use as
> justification for measuring into any found protocol and not just at
> most one.
>
> It's not lost on me that it is similarly difficult to say that all
> protocols that are discoverable need to have comparable events
> measured into them since that _also_ introduces an all-or-nothing
> migration problem, but at least that's more controllable from the
> attestation verification policy side than from the UEFI spec side.
> We're not adding new measurement protocols frequently, so we can get
> the entire boot chain ready for a new measurement protocol before it's
> made discoverable.
>
> >
> > Cc: Erdem Aktas <erdemak...@google.com>
> > Cc: James Bottomley <j...@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen....@intel.com>
> > Cc: Gerd Hoffmann <kra...@redhat.com>
> > Cc: Tom Lendacky <thomas.lenda...@amd.com>
> > Cc: Michael Roth <michael.r...@amd.com>
> > Signed-off-by: Qinkun Bao <qin...@google.com>
> > ---
> > OvmfPkg/OvmfPkgX64.dsc | 9 ++++++++-
> > .../DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c | 12 +++++++++++-
> > .../DxeTpmMeasurementLib/DxeTpmMeasurementLib.c | 6 ++++++
> > 3 files changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index 56c920168d..9bcee45047 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -32,7 +32,8 @@
> > DEFINE SECURE_BOOT_ENABLE = FALSE
> > DEFINE SMM_REQUIRE = FALSE
> > DEFINE SOURCE_DEBUG_ENABLE = FALSE
> > - DEFINE CC_MEASUREMENT_ENABLE = FALSE
> > + DEFINE CC_MEASUREMENT_ENABLE = TRUE
> > + DEFINE CC_MEASUREMENT_AND_TCG2_COEXIST = FASLE
> >
> > !include OvmfPkg/Include/Dsc/OvmfTpmDefines.dsc.inc
> >
> > @@ -99,6 +100,11 @@
> > INTEL:*_*_X64_GENFW_FLAGS = --keepexceptiontable
> > !endif
> > RELEASE_*_*_GENFW_FLAGS = --zero
> > +!if $(CC_MEASUREMENT_ENABLE) == TRUE &&
> $(CC_MEASUREMENT_AND_TCG2_COEXIST) == TRUE
> > + MSFT:*_*_*_CC_FLAGS = /D
> CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
> > + INTEL:*_*_*_CC_FLAGS = /D
> CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
> > + GCC:*_*_*_CC_FLAGS = -D
> CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
> > +!endif
> >
> > #
> > # Disable deprecated APIs.
> > @@ -1045,6 +1051,7 @@
> > }
> > !endif
> >
> > +
> > #
> > # TPM support
> > #
> > diff --git
> a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> > index 73719f3b96..4c9bc8ab4a 100644
> > ---
> a/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> > +++
> b/SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c
> > @@ -325,7 +325,12 @@ Tcg2MeasureGptTable (
> > }
> >
> > DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc
> MeasureGptTable - %r\n", Status));
> > +#ifdef CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
>
> On to the less philosophical. Thank you for starting this
> conversation. I do think that this implementation is unideal. These
> kinds of configuration decisions should be implemented in the form of
> a Pcd. Whether that's a static or dynamic Pcd is a matter of whether
> you want to do all the event design of the configuration measurement
> to MR0~PCR1 at the same time.
>
> > + }
> > + if (Tcg2Protocol != NULL) {
> > +#else
> > } else if (Tcg2Protocol != NULL) {
> > +#endif
> > //
> > // If Tcg2Protocol is installed, then Measure GPT data with this
> > protocol.
> > //
> > @@ -493,7 +498,12 @@ Tcg2MeasurePeImage (
> > CcEvent
> > );
> > DEBUG ((DEBUG_INFO, "DxeTpm2MeasureBootHandler - Cc
> MeasurePeImage - %r\n", Status));
> > - } else if (Tcg2Protocol != NULL) {
> > +#ifdef CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
> > + }
> > + if (Tcg2Protocol != NULL) {
> > +#else
> > + } else if (Tcg2Protocol != NULL) {
> > +#endif
> > Status = Tcg2Protocol->HashLogExtendEvent (
> > Tcg2Protocol,
> > PE_COFF_IMAGE,
> > diff --git
> a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> > index 6f287b31fc..b1c6198b4b 100644
> > --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> > +++
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> > @@ -261,7 +261,11 @@ TpmMeasureAndLogData (
> > HashData,
> > HashDataLen
> > );
> > +#ifdef CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
> > + }
> > +#else
> > } else {
> > +#endif
> > //
> > // Try to measure using Tpm20 protocol
> > //
> > @@ -287,7 +291,9 @@ TpmMeasureAndLogData (
> > HashDataLen
> > );
> > }
> > +#ifndef CC_MEASUREMENT_AND_TCG2_COEXIST_FEATURE
> > }
> > +#endif
> >
> > return Status;
> > }
> > --
> > 2.44.0.291.gc1ea87d7ee-goog
> >
>
>
> --
> -Dionna Glaze, PhD (she/her)
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117022): https://edk2.groups.io/g/devel/message/117022
Mute This Topic: https://groups.io/mt/105070442/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
