[edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[qinkun Bao via groups.io]

From: Qinkun Bao <qinkun@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.

Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Qinkun Bao <qinkun@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
+  }
+  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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117131): https://edk2.groups.io/g/devel/message/117131
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Dionna Glaze via groups.io]

On Thu, Mar 21, 2024 at 9:59 AM qinkun Bao <qinkun@google.com> wrote:
>
> From: Qinkun Bao <qinkun@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 <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Signed-off-by: Qinkun Bao <qinkun@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 (#117017): https://edk2.groups.io/g/devel/message/117017
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Yao, Jiewen]

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 <dionnaglaze@google.com>
> Sent: Friday, March 22, 2024 1:47 AM
> To: qinkun Bao <qinkun@google.com>
> Cc: devel@edk2.groups.io; linux-coco@lists.linux.dev; Aktas, Erdem
> <erdemaktas@google.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ard
> Biesheuvel <ardb@kernel.org>; Peter Gonda <pgonda@google.com>; James
> Bottomley <jejb@linux.ibm.com>; Gerd Hoffmann <kraxel@redhat.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Michael Roth
> <michael.roth@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 <qinkun@google.com> wrote:
> >
> > From: Qinkun Bao <qinkun@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 <erdemaktas@google.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Michael Roth <michael.roth@amd.com>
> > Signed-off-by: Qinkun Bao <qinkun@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Gerd Hoffmann]

On Fri, Mar 22, 2024 at 02:39:20AM +0000, Yao, Jiewen wrote:
> 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.

As far I know (it's been a while I looked at those patches) shim.efi and
grub.efi have support for EFI_CC_MEASUREMENT_PROTOCOL, but use the same
logic we have in DxeTpm2MeasureBootLib, i.e. they will not measure to
both RTMR and vTPM.

Looking at systemd-boot I see it will likewise not measure to both RTMR
and vTPM, but with reversed priority (use vTPM not RTMR in case both are
present).

Linux kernel appears to not have EFI_CC_MEASUREMENT_PROTOCOL support.

> 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.

Rumors say intel is working on coconut-svsm support for tdx.  That will
most likely allow to use a vTPM with tdx even without depending on the
virtualization host or cloud hyperscaler providing one.  We will see VMs
with both RTMR and vTPM and surely need a strategy how guests should
deal with that situation, consistent across the whole boot stack and
not every component doing something different.

Given that the vTPM might be provided by the hypervisor and thus not be
part of the TCB I can see that guests might want use both vTPM and RTMR.
So, yes, for that case coexistance makes sense.  I'm not convinced it is
a good idea to make that a compile time option though.  That will not
help to promote a consistent story ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117029): https://edk2.groups.io/g/devel/message/117029
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Dionna Glaze via groups.io]

On Fri, Mar 22, 2024 at 1:52 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Fri, Mar 22, 2024 at 02:39:20AM +0000, Yao, Jiewen wrote:
> > 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.
>
> As far I know (it's been a while I looked at those patches) shim.efi and
> grub.efi have support for EFI_CC_MEASUREMENT_PROTOCOL, but use the same
> logic we have in DxeTpm2MeasureBootLib, i.e. they will not measure to
> both RTMR and vTPM.

Shim will measure into CC and then continue to measure into TPM
https://github.com/rhboot/shim/blob/126a07ebc30bbd203b6966465b058da741b2654b/tpm.c#L164

GRUB2 has the same behavior. We can at least get coexistence
supporting the current boot integrity strategy that Confidential Space
is using, which is to depend on a dmverity initramfs whose root hash
is in the kernel_cmdline, and a Linux kernel built with LOADPIN. The
changes to Linux are proposed but not accepted precisely due to this
conversation we're having now.
I recall describing this to another CSP engineer at an IETF meeting
and they claimed they used the same approach, but I can't remember if
that was Oracle or another company.

>
> Looking at systemd-boot I see it will likewise not measure to both RTMR
> and vTPM, but with reversed priority (use vTPM not RTMR in case both are
> present).
>

Interesting. Thanks for this report. We'll push for the changed
semantics here if the spec is indeed changed, and request partner
distros in the CCC to include the updated systemd-boot.
I think that since the current boot integrity story stops at PCR9, we
have time to update this component before the attestation method
evolves to support a less special-purpose system composition.

> Linux kernel appears to not have EFI_CC_MEASUREMENT_PROTOCOL support.
>
> > 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.
>
> Rumors say intel is working on coconut-svsm support for tdx.  That will
> most likely allow to use a vTPM with tdx even without depending on the
> virtualization host or cloud hyperscaler providing one.  We will see VMs
> with both RTMR and vTPM and surely need a strategy how guests should
> deal with that situation, consistent across the whole boot stack and
> not every component doing something different.
>

An ephemeral TPM that Coconut-svsm offers is qualitatively different
than the persistent TPMs in CSPs today. Users of Confidential VMs all
have different threat models that allow for trusting a CSP-managed
vTPM for sealing keys but not for trusting unencrypted data in use.
The boot stack attestation story will not be fully resolved for a long
while, and a smooth transition is better than a jarring one.

> Given that the vTPM might be provided by the hypervisor and thus not be
> part of the TCB I can see that guests might want use both vTPM and RTMR.
> So, yes, for that case coexistance makes sense.  I'm not convinced it is
> a good idea to make that a compile time option though.  That will not
> help to promote a consistent story ...
>

I agree, but it does mean we have to change the event log composition
to describe the configured measurement services like described above.
I think a static Pcd is okay to begin with. The idea for tracking
these qualities is through software supply chain endorsements.
Eventually that would look like the proposed in-toto's SCAI [1] but
until then we'd have a bespoke format that we document and integrate
with in an attestation verification service.

[1] https://arxiv.org/pdf/2210.05813.pdf

-- 
-Dionna Glaze, PhD (she/her)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117055): https://edk2.groups.io/g/devel/message/117055
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[qinkun Bao via groups.io]

On Fri, Mar 22, 2024 at 7:57 AM Dionna Amalie Glaze
<dionnaglaze@google.com> wrote:
>
> On Fri, Mar 22, 2024 at 1:52 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Fri, Mar 22, 2024 at 02:39:20AM +0000, Yao, Jiewen wrote:
> > > 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.
> >

Thank you for the feedback. I believe it is the existing status even
without the patch.
Both EFI_CC_MEASUREMENT_PROTOCOL and EFI_TCG2_PROTOCOL
are exposed by TDVF when CC_MEASUREMENT_ENABLE is set to true even
without the patch. TDVF only measures with EFI_CC_MEASUREMENT_PROTOCOL
protocol. However, since both protocols exist, Shim and Grub2 measure
into both CCand TPM. In the guest, the user can access both the event log for
TPM andvRTMR. Some of the event logs for TPM are missing without the patch.
As Dionna mentioned (CVE-2021-42299), measuring into every device that
is available seems to be a safer choice.  It's a mistake we've
concretely made in the past.

I am thinking your biggest concern is OS. Fortunately, there is no
accepted patch in the kernel that appears to have
EFI_CC_MEASUREMENT_PROTOCOL support. Once this
patch is accepted, we will work with the kernel community to recommend
the following code pattern "for device in measurement_devices:
device.Extend(bla)" for all guest components.





> > As far I know (it's been a while I looked at those patches) shim.efi and
> > grub.efi have support for EFI_CC_MEASUREMENT_PROTOCOL, but use the same
> > logic we have in DxeTpm2MeasureBootLib, i.e. they will not measure to
> > both RTMR and vTPM.
>
> Shim will measure into CC and then continue to measure into TPM
> https://github.com/rhboot/shim/blob/126a07ebc30bbd203b6966465b058da741b2654b/tpm.c#L164
>
> GRUB2 has the same behavior. We can at least get coexistence
> supporting the current boot integrity strategy that Confidential Space
> is using, which is to depend on a dmverity initramfs whose root hash
> is in the kernel_cmdline, and a Linux kernel built with LOADPIN. The
> changes to Linux are proposed but not accepted precisely due to this
> conversation we're having now.
> I recall describing this to another CSP engineer at an IETF meeting
> and they claimed they used the same approach, but I can't remember if
> that was Oracle or another company.
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117132): https://edk2.groups.io/g/devel/message/117132
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Mikko Ylinen]

On Fri, Mar 22, 2024 at 07:56:53AM -0700, Dionna Amalie Glaze wrote:
> On Fri, Mar 22, 2024 at 1:52 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
> >
> > On Fri, Mar 22, 2024 at 02:39:20AM +0000, Yao, Jiewen wrote:
> > > 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.
> >
> > As far I know (it's been a while I looked at those patches) shim.efi and
> > grub.efi have support for EFI_CC_MEASUREMENT_PROTOCOL, but use the same
> > logic we have in DxeTpm2MeasureBootLib, i.e. they will not measure to
> > both RTMR and vTPM.
> 
> Shim will measure into CC and then continue to measure into TPM
> https://github.com/rhboot/shim/blob/126a07ebc30bbd203b6966465b058da741b2654b/tpm.c#L164
> 
> GRUB2 has the same behavior. We can at least get coexistence
> supporting the current boot integrity strategy that Confidential Space
> is using, which is to depend on a dmverity initramfs whose root hash
> is in the kernel_cmdline, and a Linux kernel built with LOADPIN. The
> changes to Linux are proposed but not accepted precisely due to this
> conversation we're having now.
> I recall describing this to another CSP engineer at an IETF meeting
> and they claimed they used the same approach, but I can't remember if
> that was Oracle or another company.
> 
> >
> > Looking at systemd-boot I see it will likewise not measure to both RTMR
> > and vTPM, but with reversed priority (use vTPM not RTMR in case both are
> > present).
> >
> 
> Interesting. Thanks for this report. We'll push for the changed
> semantics here if the spec is indeed changed, and request partner
> distros in the CCC to include the updated systemd-boot.

FWIW, my RTMRs patch to systemd was merged quite recently so it's not
included in any systemd release yet. (It was mainly implemented for the
UKI case that allows TDVF to boot a UKI image directly and then have the
image sections measured separately.)

> I think that since the current boot integrity story stops at PCR9, we
> have time to update this component before the attestation method
> evolves to support a less special-purpose system composition.
> 
> > Linux kernel appears to not have EFI_CC_MEASUREMENT_PROTOCOL support.
> >

Since 6.9-rc1 we have it: ac93cbfc

> > > 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.
> >
> > Rumors say intel is working on coconut-svsm support for tdx.  That will
> > most likely allow to use a vTPM with tdx even without depending on the
> > virtualization host or cloud hyperscaler providing one.  We will see VMs
> > with both RTMR and vTPM and surely need a strategy how guests should
> > deal with that situation, consistent across the whole boot stack and
> > not every component doing something different.
> >
> 
> An ephemeral TPM that Coconut-svsm offers is qualitatively different
> than the persistent TPMs in CSPs today. Users of Confidential VMs all
> have different threat models that allow for trusting a CSP-managed
> vTPM for sealing keys but not for trusting unencrypted data in use.
> The boot stack attestation story will not be fully resolved for a long
> while, and a smooth transition is better than a jarring one.
> 
> > Given that the vTPM might be provided by the hypervisor and thus not be
> > part of the TCB I can see that guests might want use both vTPM and RTMR.
> > So, yes, for that case coexistance makes sense.  I'm not convinced it is
> > a good idea to make that a compile time option though.  That will not
> > help to promote a consistent story ...
> >
> 
> I agree, but it does mean we have to change the event log composition
> to describe the configured measurement services like described above.
> I think a static Pcd is okay to begin with. The idea for tracking
> these qualities is through software supply chain endorsements.
> Eventually that would look like the proposed in-toto's SCAI [1] but
> until then we'd have a bespoke format that we document and integrate
> with in an attestation verification service.
> 
> [1] https://arxiv.org/pdf/2210.05813.pdf

-- Regards, Mikko


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117134): https://edk2.groups.io/g/devel/message/117134
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Dionna Glaze via groups.io]

On Mon, Mar 25, 2024 at 6:07 AM Mikko Ylinen
<mikko.ylinen@linux.intel.com> wrote:
>
> > >
> > > Looking at systemd-boot I see it will likewise not measure to both RTMR
> > > and vTPM, but with reversed priority (use vTPM not RTMR in case both are
> > > present).
> > >
> >
> > Interesting. Thanks for this report. We'll push for the changed
> > semantics here if the spec is indeed changed, and request partner
> > distros in the CCC to include the updated systemd-boot.
>
> FWIW, my RTMRs patch to systemd was merged quite recently so it's not
> included in any systemd release yet. (It was mainly implemented for the
> UKI case that allows TDVF to boot a UKI image directly and then have the
> image sections measured separately.)
>

Thank you, I've proposed a change in
https://github.com/systemd/systemd/pull/31939


-- 
-Dionna Glaze, PhD (she/her)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117087): https://edk2.groups.io/g/devel/message/117087
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [linux-collab] [CCC][tac] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[qinkun Bao via groups.io]

I brought the RFC into the CCC community
(https://github.com/confidential-computing/governance) and received
some comments.

Forward the email into the EDK2 dev and linux-coco.

Thanks,
Qinkun

---------- Forwarded message ---------
From: Mingshen Sun <mingshen.sun@tiktok.com>
Date: Thu, Apr 4, 2024 at 1:43 PM
Subject: Re: [External] Re: [linux-collab] [CCC][tac] [RFC PATCH]
OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and
RTMR.
To: <qinkun@google.com>
Cc: <tac@lists.confidentialcomputing.io>,
<linux-collab@lists.confidentialcomputing.io>


Hi Qinkun,

Thanks for bringing this to the CCC community.

I think the proposal makes sense to me. RTMR and vTPM measurements
shouldn't be mutually exclusive.

Under certain threat models (e.g., workload operator is not trusted),
both of them may be valid.
1. Measurements of RTMR and vTPM can be used for cross validation with
different root-of-trust.
2. Key sealing feature provided by vTPM is not available in the
current TEE ecosystem.

Mingshen


On Thu, Apr 4, 2024 at 12:32 PM qinkun Bao via


lists.confidentialcomputing.io
<qinkun=google.com@lists.confidentialcomputing.io> wrote:
>
> Hello,
>
> The current TDVF implementation does not extend to the vTPM if the
> RTMR attestation is enabled. We are working on proposals to address
> the issue. We would like to get the feedback from the CCC community
> about the proposal.
>
> Thanks,
> Qinkun
>
> On Thu, Apr 4, 2024 at 12:16 PM qinkun Bao via
> lists.confidentialcomputing.io
> <qinkun=google.com@lists.confidentialcomputing.io> wrote:
> >
> >
> >
> > ---------- Forwarded message ---------
> > From: qinkun Bao <qinkun@google.com>
> > Date: Thu, Mar 21, 2024 at 9:59 AM
> > Subject: [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.
> > To: <devel@edk2.groups.io>
> > Cc: <linux-coco@lists.linux.dev>, Erdem Aktas <erdemaktas@google.com>, Jiewen Yao <jiewen.yao@intel.com>, Ard Biesheuvel <ardb@kernel.org>, Peter Gonda <pgonda@google.com>, Dionna Glaze <dionnaglaze@google.com>, Qinkun Bao <qinkun@google.com>, James Bottomley <jejb@linux.ibm.com>, Gerd Hoffmann <kraxel@redhat.com>, Tom Lendacky <thomas.lendacky@amd.com>, Michael Roth <michael.roth@amd.com>
> >
> >
> > From: Qinkun Bao <qinkun@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.
> >
> > Cc: Erdem Aktas <erdemaktas@google.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Cc: Michael Roth <michael.roth@amd.com>
> > Signed-off-by: Qinkun Bao <qinkun@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
> > +  }
> > +  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
> >
> >
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117554): https://edk2.groups.io/g/devel/message/117554
Mute This Topic: https://groups.io/mt/105428501/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Yao, Jiewen]

Hi Dionna/Qinkun
I am not sure if systemd is the last software in guest we need to patch to support coexistence to extend the measurement.
Are you aware of any other Linux guest software needs to be updated? Such as Linux IMA (Integrity Measurement Architecture)?

To move this forward.

In Intel, we had discussed and we did see the potential security risk. As I mentioned in the first email, "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."

At same time, we also respect that it might be a valid use case for Google.
I would like to ask the opinion in the EDKII community, especially the OVMF and CC maintainer and reviewer.


Hi Ard Biesheuvel
Do you think Kernel is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?

Hi Gerd Hoffman
Do you think RedHat is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?

Hi James Bottomley
Do you think IBM is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?

Hi Tom Lendacky/Michael Roth
Do you think AMD is OK with this coexistence proposal?
Are you willing to give "reviewed-by"?


Thank you
Yao, Jiewen


> -----Original Message-----
> From: Dionna Amalie Glaze <dionnaglaze@google.com>
> Sent: Monday, March 25, 2024 11:29 PM
> To: Mikko Ylinen <mikko.ylinen@linux.intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> qinkun Bao <qinkun@google.com>; devel@edk2.groups.io; linux-
> coco@lists.linux.dev; Aktas, Erdem <erdemaktas@google.com>; Ard Biesheuvel
> <ardb@kernel.org>; Peter Gonda <pgonda@google.com>; James Bottomley
> <jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>; Michael
> Roth <michael.roth@amd.com>
> Subject: Re: [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance
> of vTPM and RTMR.
> 
> On Mon, Mar 25, 2024 at 6:07 AM Mikko Ylinen
> <mikko.ylinen@linux.intel.com> wrote:
> >
> > > >
> > > > Looking at systemd-boot I see it will likewise not measure to both RTMR
> > > > and vTPM, but with reversed priority (use vTPM not RTMR in case both are
> > > > present).
> > > >
> > >
> > > Interesting. Thanks for this report. We'll push for the changed
> > > semantics here if the spec is indeed changed, and request partner
> > > distros in the CCC to include the updated systemd-boot.
> >
> > FWIW, my RTMRs patch to systemd was merged quite recently so it's not
> > included in any systemd release yet. (It was mainly implemented for the
> > UKI case that allows TDVF to boot a UKI image directly and then have the
> > image sections measured separately.)
> >
> 
> Thank you, I've proposed a change in
> https://github.com/systemd/systemd/pull/31939
> 
> 
> --
> -Dionna Glaze, PhD (she/her)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117606): https://edk2.groups.io/g/devel/message/117606
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[qinkun Bao via groups.io]

Hi Jiewen,

Thank you!

On Wed, Apr 10, 2024 at 3:20 PM Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Hi Dionna/Qinkun
> I am not sure if systemd is the last software in guest we need to patch to support coexistence to extend the measurement.

The direct boot patch needs to be patched as well. Here is the link.
efi/libstub: Add Confidential Computing (CC) measurement support -
Kuppuswamy Sathyanarayanan (kernel.org)
https://lore.kernel.org/lkml/20240215030002.281456-2-sathyanarayanan.kuppuswamy@linux.intel.com/
Ard is the maintainer for EFI Stub.

> Are you aware of any other Linux guest software needs to be updated? Such as Linux IMA (Integrity Measurement Architecture)?

You are right that Linux IMA needs to support coexistence. However,
the TDX RTMR IMA support has not been merged into the kernel source
code yet. I have never seen the TDX IMA patch in the LKML as well.

I find that Intel's TDX MVP kernel has the TDX RTMR IMA support patch.
Here is the link
https://github.com/intel/tdx-tools/tree/tdx-1.5/build/common
For what I see, the TDX RTMR IMA patches ([PATCH 672/731] ima: support
for boot aggregate and runtime
measurements in TDX RTMR) from TDX MVP kernel support the coexistence.
The patch author is Ruoyu Ying <ruoyu.ying@intel.com>.


>
> To move this forward.
>
> In Intel, we had discussed and we did see the potential security risk. As I mentioned in the first email, "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."
>
> At same time, we also respect that it might be a valid use case for Google.
> I would like to ask the opinion in the EDKII community, especially the OVMF and CC maintainer and reviewer.
>
>
> Hi Ard Biesheuvel
> Do you think Kernel is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
> Hi Gerd Hoffman
> Do you think RedHat is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
> Hi James Bottomley
> Do you think IBM is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
> Hi Tom Lendacky/Michael Roth
> Do you think AMD is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>
>
> Thank you
> Yao, Jiewen
>
>
> > -----Original Message-----
> > From: Dionna Amalie Glaze <dionnaglaze@google.com>
> > Sent: Monday, March 25, 2024 11:29 PM
> > To: Mikko Ylinen <mikko.ylinen@linux.intel.com>
> > Cc: Gerd Hoffmann <kraxel@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > qinkun Bao <qinkun@google.com>; devel@edk2.groups.io; linux-
> > coco@lists.linux.dev; Aktas, Erdem <erdemaktas@google.com>; Ard Biesheuvel
> > <ardb@kernel.org>; Peter Gonda <pgonda@google.com>; James Bottomley
> > <jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>; Michael
> > Roth <michael.roth@amd.com>
> > Subject: Re: [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance
> > of vTPM and RTMR.
> >
> > On Mon, Mar 25, 2024 at 6:07 AM Mikko Ylinen
> > <mikko.ylinen@linux.intel.com> wrote:
> > >
> > > > >
> > > > > Looking at systemd-boot I see it will likewise not measure to both RTMR
> > > > > and vTPM, but with reversed priority (use vTPM not RTMR in case both are
> > > > > present).
> > > > >
> > > >
> > > > Interesting. Thanks for this report. We'll push for the changed
> > > > semantics here if the spec is indeed changed, and request partner
> > > > distros in the CCC to include the updated systemd-boot.
> > >
> > > FWIW, my RTMRs patch to systemd was merged quite recently so it's not
> > > included in any systemd release yet. (It was mainly implemented for the
> > > UKI case that allows TDVF to boot a UKI image directly and then have the
> > > image sections measured separately.)
> > >
> >
> > Thank you, I've proposed a change in
> > https://github.com/systemd/systemd/pull/31939
> >
> >
> > --
> > -Dionna Glaze, PhD (she/her)


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117615): https://edk2.groups.io/g/devel/message/117615
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Ard Biesheuvel]

Hello all,

On Thu, 11 Apr 2024 at 03:20, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Hi Dionna/Qinkun
> I am not sure if systemd is the last software in guest we need to patch to support coexistence to extend the measurement.
> Are you aware of any other Linux guest software needs to be updated? Such as Linux IMA (Integrity Measurement Architecture)?
>
> To move this forward.
>
> In Intel, we had discussed and we did see the potential security risk. As I mentioned in the first email, "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."
>
> At same time, we also respect that it might be a valid use case for Google.
> I would like to ask the opinion in the EDKII community, especially the OVMF and CC maintainer and reviewer.
>
>
> Hi Ard Biesheuvel
> Do you think Kernel is OK with this coexistence proposal?
> Are you willing to give "reviewed-by"?
>

I think it is a bad idea to go and apply changes all across the boot
software ecosystem to measure the same assets into different
measurement protocols. I'mm afraid it creates technical debt that will
come and bite us in the future.

Given that RTMR is a proper subset of vTPM (modulo the PCR/RTMR index
conversion), I feel that it should be the CoCo firmware's
responsibility to either:
- expose RTMR and not vTPM
- expose vTPM, and duplicate each measurement into RTMR as they are taken

However, I understand that this is only viable for execution under the
UEFI boot services, and after that, the vTPM and RTMR are exposed in
different ways to the OS.

Could someone explain how that piece of the puzzle is supposed to
work? Do we measure into RTMR after ExitBootServices()?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117616): https://edk2.groups.io/g/devel/message/117616
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Gerd Hoffmann]

  Hi,
 
> Given that RTMR is a proper subset of vTPM (modulo the PCR/RTMR index
> conversion), I feel that it should be the CoCo firmware's
> responsibility to either:
> - expose RTMR and not vTPM
> - expose vTPM, and duplicate each measurement into RTMR as they are taken

That approach looks good to me.  It will make sure vTPM and RTMR
measurements are consistent and it also solves the event log issue
(we don't need separate vTPM and RTMR entries then).

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117619): https://edk2.groups.io/g/devel/message/117619
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Yao, Jiewen]

Please allow me to clarify what you are proposing:
Do you mean in vTPM case, we extend both, but we only need TCG event log, NOT CC event log?




> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Thursday, April 11, 2024 4:08 PM
> To: Ard Biesheuvel <ardb@kernel.org>
> Cc: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Dionna Amalie
> Glaze <dionnaglaze@google.com>; Mikko Ylinen <mikko.ylinen@linux.intel.com>;
> James Bottomley <jejb@linux.ibm.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Michael Roth <michael.roth@amd.com>; qinkun
> Bao <qinkun@google.com>; linux-coco@lists.linux.dev; Aktas, Erdem
> <erdemaktas@google.com>; Peter Gonda <pgonda@google.com>; Johnson,
> Simon P <simon.p.johnson@intel.com>; Xiang, Qinglan
> <qinglan.xiang@intel.com>
> Subject: Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for
> coexistance of vTPM and RTMR.
> 
>   Hi,
> 
> > Given that RTMR is a proper subset of vTPM (modulo the PCR/RTMR index
> > conversion), I feel that it should be the CoCo firmware's
> > responsibility to either:
> > - expose RTMR and not vTPM
> > - expose vTPM, and duplicate each measurement into RTMR as they are taken
> 
> That approach looks good to me.  It will make sure vTPM and RTMR
> measurements are consistent and it also solves the event log issue
> (we don't need separate vTPM and RTMR entries then).
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117628): https://edk2.groups.io/g/devel/message/117628
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Gerd Hoffmann]

On Thu, Apr 11, 2024 at 09:56:48AM +0000, Yao, Jiewen wrote:
> Please allow me to clarify what you are proposing:
> Do you mean in vTPM case, we extend both, but we only need TCG event log, NOT CC event log?

Elsewhere in this thread it was mentioned that writing both vTPM and
RTMR events to the event log is problematic because the event log format
has no field to specify whenever a given event was measured to vTPM or
RTMR.

If the firmware can make sure all events are measured to both vTPM and
RTMR the need to trace them separately goes away.

So, yes, in case a vTPM is present the firmware would:
  (a) expose EFI_TCG2_PROTOCOL, measure to both vTPM + RTMR
  (b) not expose EFI_CC_MEASUREMENT_PROTOCOL
  (c) log measurements to TCG event log

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117629): https://edk2.groups.io/g/devel/message/117629
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Ard Biesheuvel]

On Thu, 11 Apr 2024 at 12:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Thu, Apr 11, 2024 at 09:56:48AM +0000, Yao, Jiewen wrote:
> > Please allow me to clarify what you are proposing:
> > Do you mean in vTPM case, we extend both, but we only need TCG event log, NOT CC event log?
>
> Elsewhere in this thread it was mentioned that writing both vTPM and
> RTMR events to the event log is problematic because the event log format
> has no field to specify whenever a given event was measured to vTPM or
> RTMR.
>
> If the firmware can make sure all events are measured to both vTPM and
> RTMR the need to trace them separately goes away.
>
> So, yes, in case a vTPM is present the firmware would:
>   (a) expose EFI_TCG2_PROTOCOL, measure to both vTPM + RTMR
>   (b) not expose EFI_CC_MEASUREMENT_PROTOCOL
>   (c) log measurements to TCG event log
>

A TDX attestation would require the PCR to RTMR mapping used by the
firmware in order to reconstruct the RTMR values from the TCG event
log, but that seems feasible to me.

In any case, I think it should be the guest firmware's job to abstract
away the difference.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117630): https://edk2.groups.io/g/devel/message/117630
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Lendacky, Thomas via groups.io]

On 4/11/24 05:33, Ard Biesheuvel wrote:
> On Thu, 11 Apr 2024 at 12:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>> On Thu, Apr 11, 2024 at 09:56:48AM +0000, Yao, Jiewen wrote:
>>> Please allow me to clarify what you are proposing:
>>> Do you mean in vTPM case, we extend both, but we only need TCG event log, NOT CC event log?
>>
>> Elsewhere in this thread it was mentioned that writing both vTPM and
>> RTMR events to the event log is problematic because the event log format
>> has no field to specify whenever a given event was measured to vTPM or
>> RTMR.
>>
>> If the firmware can make sure all events are measured to both vTPM and
>> RTMR the need to trace them separately goes away.
>>
>> So, yes, in case a vTPM is present the firmware would:
>>    (a) expose EFI_TCG2_PROTOCOL, measure to both vTPM + RTMR
>>    (b) not expose EFI_CC_MEASUREMENT_PROTOCOL
>>    (c) log measurements to TCG event log
>>
> 
> A TDX attestation would require the PCR to RTMR mapping used by the
> firmware in order to reconstruct the RTMR values from the TCG event
> log, but that seems feasible to me.
> 
> In any case, I think it should be the guest firmware's job to abstract
> away the difference.

Agreed, this approach seems to be the best way forward.

Thanks,
Tom


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117633): https://edk2.groups.io/g/devel/message/117633
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Xiang, Qinglan]

+ Cfir, Vishal and Ranga for awareness. 

-----Original Message-----
From: Tom Lendacky <thomas.lendacky@amd.com> 
Sent: Thursday, April 11, 2024 7:08 AM
To: Ard Biesheuvel <ardb@kernel.org>; devel@edk2.groups.io; kraxel@redhat.com
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dionna Amalie Glaze <dionnaglaze@google.com>; Mikko Ylinen <mikko.ylinen@linux.intel.com>; James Bottomley <jejb@linux.ibm.com>; Michael Roth <michael.roth@amd.com>; qinkun Bao <qinkun@google.com>; linux-coco@lists.linux.dev; Aktas, Erdem <erdemaktas@google.com>; Peter Gonda <pgonda@google.com>; Johnson, Simon P <simon.p.johnson@intel.com>; Xiang, Qinglan <qinglan.xiang@intel.com>
Subject: Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

On 4/11/24 05:33, Ard Biesheuvel wrote:
> On Thu, 11 Apr 2024 at 12:29, Gerd Hoffmann <kraxel@redhat.com> wrote:
>>
>> On Thu, Apr 11, 2024 at 09:56:48AM +0000, Yao, Jiewen wrote:
>>> Please allow me to clarify what you are proposing:
>>> Do you mean in vTPM case, we extend both, but we only need TCG event log, NOT CC event log?
>>
>> Elsewhere in this thread it was mentioned that writing both vTPM and 
>> RTMR events to the event log is problematic because the event log 
>> format has no field to specify whenever a given event was measured to 
>> vTPM or RTMR.
>>
>> If the firmware can make sure all events are measured to both vTPM 
>> and RTMR the need to trace them separately goes away.
>>
>> So, yes, in case a vTPM is present the firmware would:
>>    (a) expose EFI_TCG2_PROTOCOL, measure to both vTPM + RTMR
>>    (b) not expose EFI_CC_MEASUREMENT_PROTOCOL
>>    (c) log measurements to TCG event log
>>
> 
> A TDX attestation would require the PCR to RTMR mapping used by the 
> firmware in order to reconstruct the RTMR values from the TCG event 
> log, but that seems feasible to me.
> 
> In any case, I think it should be the guest firmware's job to abstract 
> away the difference.

Agreed, this approach seems to be the best way forward.

Thanks,
Tom


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117636): https://edk2.groups.io/g/devel/message/117636
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[qinkun Bao via groups.io]

Hi all,

Thank you all for the feedback.

> > In Intel, we had discussed and we did see the potential security risk. As I mentioned in the first email, "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."
> >

Thank you for bringing it up. Unfortunately, it is the current status
even without this patch.
On a TDX VM with Grub boot:
TDVF, Shim, Grub extend their measurements into RTMR.
Shim, Grub extend their measurements into TPM.
We are seeking to add a non-default, build-time option that makes TDVF
also extend its measurement into TPM.

The measurement skew should not be a huge security concern.  Yes, some
environments won't be able to successfully attest. Again, we are
talking about TDX VMs. The security property of a TEE should be based
on the attestation results. The attestation verifier/service (e.g.,
Intel ITA) should examine the quote and check if the chain of trust is
broken. And the end user should only be trusting attestations that
contain full boot chains that are known to correctly measure every
step into the relevant device.


>
> I think it is a bad idea to go and apply changes all across the boot
> software ecosystem to measure the same assets into different
> measurement protocols. I'mm afraid it creates technical debt that will
> come and bite us in the future.

Could you shed some lights on why it creates technical debts?

>
> Given that RTMR is a proper subset of vTPM (modulo the PCR/RTMR index
> conversion), I feel that it should be the CoCo firmware's
> responsibility to either:
> - expose RTMR and not vTPM
> - expose vTPM, and duplicate each measurement into RTMR as they are taken
>
> However, I understand that this is only viable for execution under the
> UEFI boot services, and after that, the vTPM and RTMR are exposed in
> different ways to the OS.

Yes, they are exposed in different ways. In Linux, the TPM driver uses
the mmio interface rather than the EFI service. Even if
EFI_TCG2_PROTOCOL is not installed, the TPM as a device is still
visible to the guest. The RTMR values are included in the TD report
and could be extended through a TDCALL. The security concern caused by
not measuring into every device that is available is a concern. Please
see CVE-2021-42299.

>
> Could someone explain how that piece of the puzzle is supposed to
> work? Do we measure into RTMR after ExitBootServices()?

Yes, we still measure into RTMR after ExitBootServices() [1]. One
example is measuring container images into RTMR2 during the loading
[2].

Link:
[1] https://lore.kernel.org/lkml/20240128212532.2754325-1-sameo@rivosinc.com/
[2] https://github.com/confidential-containers/guest-components/pull/467/


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117719): https://edk2.groups.io/g/devel/message/117719
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH] OvmfPkg/SecurityPkg: Add build option for coexistance of vTPM and RTMR.

[Ard Biesheuvel]

On Sat, 13 Apr 2024 at 11:37, Qinkun Bao <qinkun@google.com> wrote:
>
...
> >
> > I think it is a bad idea to go and apply changes all across the boot
> > software ecosystem to measure the same assets into different
> > measurement protocols. I'mm afraid it creates technical debt that will
> > come and bite us in the future.
>
> Could you shed some lights on why it creates technical debts?
>

If it is so vitally important that measurements are taken into both
the TPM PCRs and the RTMRs if both are available, can we really trust
all those boot components to do the right thing? And do we really want
to have to reason about that?

Given that the guest system firmware exposes both protocols anyway,
wouldn't it make more sense to make it the guest firmware's job to
duplicate measurements into the RTMRs and only expose the TCG protocol
to the guest OS stack?

The hybrid model of trusting the host (and using a vTPM) and not
trusting the host (and therefore relying on TDX/RTMRs) at the same
time seems a bit odd to me in any case: under which circumstances
would a guest distrust the host but still rely on the vTPM?


> >
> > Given that RTMR is a proper subset of vTPM (modulo the PCR/RTMR index
> > conversion), I feel that it should be the CoCo firmware's
> > responsibility to either:
> > - expose RTMR and not vTPM
> > - expose vTPM, and duplicate each measurement into RTMR as they are taken
> >
> > However, I understand that this is only viable for execution under the
> > UEFI boot services, and after that, the vTPM and RTMR are exposed in
> > different ways to the OS.
>
> Yes, they are exposed in different ways. In Linux, the TPM driver uses
> the mmio interface rather than the EFI service. Even if
> EFI_TCG2_PROTOCOL is not installed, the TPM as a device is still
> visible to the guest. The RTMR values are included in the TD report
> and could be extended through a TDCALL. The security concern caused by
> not measuring into every device that is available is a concern.

That does not imply that each and every component should be
responsible for taking both measurements.

> Please
> see CVE-2021-42299.
>
> >
> > Could someone explain how that piece of the puzzle is supposed to
> > work? Do we measure into RTMR after ExitBootServices()?
>
> Yes, we still measure into RTMR after ExitBootServices() [1]. One
> example is measuring container images into RTMR2 during the loading
> [2].
>

Fair enough. So keeping RTMRs and PCRs in sync after EBS() is going to
be problematic :-(


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117788): https://edk2.groups.io/g/devel/message/117788
Mute This Topic: https://groups.io/mt/105070442/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-