BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
DxeTpmMeasurementLib supports TPM based measurement in DXE phase.
After CcMeasurementProtocol is introduced, CC based measurement needs
to be supported in DxeTpmMeasurementLib as well.
In TpmMeasureAndLogData, CC based measurement will be first called.
If it failed, TPM based measurement will be called sequentially.
Currently there is an assumption that CC based measurement and
TPM based measurement won't be exist at the same time.If the
assumption is not true in the future, we will revisit here then.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Min Xu <min.m.xu@intel.com>
---
.../DxeTpmMeasurementLib.c | 91 ++++++++++++++++++-
.../DxeTpmMeasurementLib.inf | 9 +-
2 files changed, 92 insertions(+), 8 deletions(-)
diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
index 061136ee7860..2ddb9033a0d5 100644
--- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
@@ -1,5 +1,6 @@
/** @file
- This library is used by other modules to measure data to TPM.
+ This library is used by other modules to measure data to TPM and Confidential
+ Computing (CC) measure registers.
Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. <BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -19,8 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Guid/Acpi.h>
#include <IndustryStandard/Acpi.h>
-
-
+#include <Protocol/CcMeasurement.h>
/**
Tpm12 measure and log data, and extend the measurement result into a specific PCR.
@@ -149,6 +149,73 @@ Tpm20MeasureAndLogData (
return Status;
}
+/**
+ Cc measure and log data, and extend the measurement result into a
+ specific CC MR.
+
+ @param[in] PcrIndex PCR Index.
+ @param[in] EventType Event type.
+ @param[in] EventLog Measurement event log.
+ @param[in] LogLen Event log length in bytes.
+ @param[in] HashData The start of the data buffer to be hashed, extended.
+ @param[in] HashDataLen The length, in bytes, of the buffer referenced by HashData
+
+ @retval EFI_SUCCESS Operation completed successfully.
+ @retval EFI_UNSUPPORTED Tdx device not available.
+ @retval EFI_OUT_OF_RESOURCES Out of memory.
+ @retval EFI_DEVICE_ERROR The operation was unsuccessful.
+**/
+EFI_STATUS
+EFIAPI
+CcMeasureAndLogData (
+ IN UINT32 PcrIndex,
+ IN UINT32 EventType,
+ IN VOID *EventLog,
+ IN UINT32 LogLen,
+ IN VOID *HashData,
+ IN UINT64 HashDataLen
+ )
+{
+ EFI_STATUS Status;
+ EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol;
+ EFI_CC_EVENT *EfiCcEvent;
+ UINT32 MrIndex;
+
+ Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ EfiCcEvent = (EFI_CC_EVENT *) AllocateZeroPool (LogLen + sizeof (EFI_CC_EVENT));
+ if(EfiCcEvent == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ EfiCcEvent->Size = (UINT32) LogLen + sizeof (EFI_CC_EVENT) - sizeof (EfiCcEvent->Event);
+ EfiCcEvent->Header.HeaderSize = sizeof (EFI_CC_EVENT_HEADER);
+ EfiCcEvent->Header.HeaderVersion = EFI_CC_EVENT_HEADER_VERSION;
+ EfiCcEvent->Header.MrIndex = MrIndex;
+ EfiCcEvent->Header.EventType = EventType;
+ CopyMem (&EfiCcEvent->Event[0], EventLog, LogLen);
+
+ Status = CcProtocol->HashLogExtendEvent (
+ CcProtocol,
+ 0,
+ (EFI_PHYSICAL_ADDRESS) (UINTN) HashData,
+ HashDataLen,
+ EfiCcEvent
+ );
+ FreePool (EfiCcEvent);
+
+ return Status;
+}
+
+
/**
Tpm measure and log data, and extend the measurement result into a specific PCR.
@@ -178,9 +245,9 @@ TpmMeasureAndLogData (
EFI_STATUS Status;
//
- // Try to measure using Tpm20 protocol
+ // Try to measure using Cc measurement protocol
//
- Status = Tpm20MeasureAndLogData(
+ Status = CcMeasureAndLogData (
PcrIndex,
EventType,
EventLog,
@@ -189,6 +256,20 @@ TpmMeasureAndLogData (
HashDataLen
);
+ if (EFI_ERROR (Status)) {
+ //
+ // Try to measure using Tpm20 protocol
+ //
+ Status = Tpm20MeasureAndLogData(
+ PcrIndex,
+ EventType,
+ EventLog,
+ LogLen,
+ HashData,
+ HashDataLen
+ );
+ }
+
if (EFI_ERROR (Status)) {
//
// Try to measure using Tpm1.2 protocol
diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
index 7d41bc41f95d..3af3d4e33b25 100644
--- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
@@ -1,5 +1,7 @@
## @file
-# Provides TPM measurement functions for TPM1.2 and TPM 2.0
+# Provides below measurement functions:
+# 1. TPM measurement functions for TPM1.2 and TPM 2.0
+# 2. Confidential Computing (CC) measurement functions
#
# This library provides TpmMeasureAndLogData() to measure and log data, and
# extend the measurement result into a specific PCR.
@@ -40,5 +42,6 @@
UefiBootServicesTableLib
[Protocols]
- gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES
- gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
+ gEfiCcMeasurementProtocolGuid ## SOMETIMES_CONSUMES
--
2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83086): https://edk2.groups.io/g/devel/message/83086
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
May I know which platform you have run the test?
I think we need cover both TD and TPM in real platform.
> -----Original Message-----
> From: Xu, Min M <min.m.xu@intel.com>
> Sent: Tuesday, November 2, 2021 10:51 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>; Sami Mujawar
> <sami.mujawar@arm.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in
> DxeTpmMeasurementLib
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
>
> DxeTpmMeasurementLib supports TPM based measurement in DXE phase.
> After CcMeasurementProtocol is introduced, CC based measurement needs
> to be supported in DxeTpmMeasurementLib as well.
>
> In TpmMeasureAndLogData, CC based measurement will be first called.
> If it failed, TPM based measurement will be called sequentially.
> Currently there is an assumption that CC based measurement and
> TPM based measurement won't be exist at the same time.If the
> assumption is not true in the future, we will revisit here then.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> .../DxeTpmMeasurementLib.c | 91 ++++++++++++++++++-
> .../DxeTpmMeasurementLib.inf | 9 +-
> 2 files changed, 92 insertions(+), 8 deletions(-)
>
> diff --git
> a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> index 061136ee7860..2ddb9033a0d5 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> @@ -1,5 +1,6 @@
> /** @file
> - This library is used by other modules to measure data to TPM.
> + This library is used by other modules to measure data to TPM and Confidential
> + Computing (CC) measure registers.
>
> Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. <BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -19,8 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include <Guid/Acpi.h>
> #include <IndustryStandard/Acpi.h>
> -
> -
> +#include <Protocol/CcMeasurement.h>
>
> /**
> Tpm12 measure and log data, and extend the measurement result into a
> specific PCR.
> @@ -149,6 +149,73 @@ Tpm20MeasureAndLogData (
> return Status;
> }
>
> +/**
> + Cc measure and log data, and extend the measurement result into a
> + specific CC MR.
> +
> + @param[in] PcrIndex PCR Index.
> + @param[in] EventType Event type.
> + @param[in] EventLog Measurement event log.
> + @param[in] LogLen Event log length in bytes.
> + @param[in] HashData The start of the data buffer to be hashed,
> extended.
> + @param[in] HashDataLen The length, in bytes, of the buffer referenced by
> HashData
> +
> + @retval EFI_SUCCESS Operation completed successfully.
> + @retval EFI_UNSUPPORTED Tdx device not available.
> + @retval EFI_OUT_OF_RESOURCES Out of memory.
> + @retval EFI_DEVICE_ERROR The operation was unsuccessful.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CcMeasureAndLogData (
> + IN UINT32 PcrIndex,
> + IN UINT32 EventType,
> + IN VOID *EventLog,
> + IN UINT32 LogLen,
> + IN VOID *HashData,
> + IN UINT64 HashDataLen
> + )
> +{
> + EFI_STATUS Status;
> + EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol;
> + EFI_CC_EVENT *EfiCcEvent;
> + UINT32 MrIndex;
> +
> + Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL,
> (VOID **) &CcProtocol);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
> + if (EFI_ERROR (Status)) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + EfiCcEvent = (EFI_CC_EVENT *) AllocateZeroPool (LogLen + sizeof
> (EFI_CC_EVENT));
> + if(EfiCcEvent == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + EfiCcEvent->Size = (UINT32) LogLen + sizeof (EFI_CC_EVENT) - sizeof
> (EfiCcEvent->Event);
> + EfiCcEvent->Header.HeaderSize = sizeof (EFI_CC_EVENT_HEADER);
> + EfiCcEvent->Header.HeaderVersion = EFI_CC_EVENT_HEADER_VERSION;
> + EfiCcEvent->Header.MrIndex = MrIndex;
> + EfiCcEvent->Header.EventType = EventType;
> + CopyMem (&EfiCcEvent->Event[0], EventLog, LogLen);
> +
> + Status = CcProtocol->HashLogExtendEvent (
> + CcProtocol,
> + 0,
> + (EFI_PHYSICAL_ADDRESS) (UINTN) HashData,
> + HashDataLen,
> + EfiCcEvent
> + );
> + FreePool (EfiCcEvent);
> +
> + return Status;
> +}
> +
> +
> /**
> Tpm measure and log data, and extend the measurement result into a specific
> PCR.
>
> @@ -178,9 +245,9 @@ TpmMeasureAndLogData (
> EFI_STATUS Status;
>
> //
> - // Try to measure using Tpm20 protocol
> + // Try to measure using Cc measurement protocol
> //
> - Status = Tpm20MeasureAndLogData(
> + Status = CcMeasureAndLogData (
> PcrIndex,
> EventType,
> EventLog,
> @@ -189,6 +256,20 @@ TpmMeasureAndLogData (
> HashDataLen
> );
>
> + if (EFI_ERROR (Status)) {
> + //
> + // Try to measure using Tpm20 protocol
> + //
> + Status = Tpm20MeasureAndLogData(
> + PcrIndex,
> + EventType,
> + EventLog,
> + LogLen,
> + HashData,
> + HashDataLen
> + );
> + }
> +
> if (EFI_ERROR (Status)) {
> //
> // Try to measure using Tpm1.2 protocol
> diff --git
> a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> index 7d41bc41f95d..3af3d4e33b25 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +++
> b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> @@ -1,5 +1,7 @@
> ## @file
> -# Provides TPM measurement functions for TPM1.2 and TPM 2.0
> +# Provides below measurement functions:
> +# 1. TPM measurement functions for TPM1.2 and TPM 2.0
> +# 2. Confidential Computing (CC) measurement functions
> #
> # This library provides TpmMeasureAndLogData() to measure and log data, and
> # extend the measurement result into a specific PCR.
> @@ -40,5 +42,6 @@
> UefiBootServicesTableLib
>
> [Protocols]
> - gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES
> - gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiCcMeasurementProtocolGuid ## SOMETIMES_CONSUMES
> --
> 2.29.2.windows.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83093): https://edk2.groups.io/g/devel/message/83093
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On November 2, 2021 2:25 PM, Jiewen Yao wrote: > May I know which platform you have run the test? > > I think we need cover both TD and TPM in real platform. > I have run the test in Intel's internal hardware platform (covering both TD and TPM). The test all pass. Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83204): https://edk2.groups.io/g/devel/message/83204 Mute This Topic: https://groups.io/mt/86758672/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Min,
Thank you for this patch.
Please find my feedback inline marked [SAMI].
Regards,
Sami Mujawar
On 02/11/2021 02:50 AM, Min Xu wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3625
>
> DxeTpmMeasurementLib supports TPM based measurement in DXE phase.
> After CcMeasurementProtocol is introduced, CC based measurement needs
> to be supported in DxeTpmMeasurementLib as well.
>
> In TpmMeasureAndLogData, CC based measurement will be first called.
> If it failed, TPM based measurement will be called sequentially.
> Currently there is an assumption that CC based measurement and
> TPM based measurement won't be exist at the same time.If the
> assumption is not true in the future, we will revisit here then.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Min Xu <min.m.xu@intel.com>
> ---
> .../DxeTpmMeasurementLib.c | 91 ++++++++++++++++++-
> .../DxeTpmMeasurementLib.inf | 9 +-
> 2 files changed, 92 insertions(+), 8 deletions(-)
>
> diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> index 061136ee7860..2ddb9033a0d5 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.c
> @@ -1,5 +1,6 @@
> /** @file
> - This library is used by other modules to measure data to TPM.
> + This library is used by other modules to measure data to TPM and Confidential
> + Computing (CC) measure registers.
>
> Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved. <BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -19,8 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>
> #include <Guid/Acpi.h>
> #include <IndustryStandard/Acpi.h>
> -
> -
> +#include <Protocol/CcMeasurement.h>
>
> /**
> Tpm12 measure and log data, and extend the measurement result into a specific PCR.
> @@ -149,6 +149,73 @@ Tpm20MeasureAndLogData (
> return Status;
> }
>
> +/**
> + Cc measure and log data, and extend the measurement result into a
> + specific CC MR.
> +
> + @param[in] PcrIndex PCR Index.
> + @param[in] EventType Event type.
> + @param[in] EventLog Measurement event log.
> + @param[in] LogLen Event log length in bytes.
> + @param[in] HashData The start of the data buffer to be hashed, extended.
> + @param[in] HashDataLen The length, in bytes, of the buffer referenced by HashData
> +
> + @retval EFI_SUCCESS Operation completed successfully.
> + @retval EFI_UNSUPPORTED Tdx device not available.
> + @retval EFI_OUT_OF_RESOURCES Out of memory.
> + @retval EFI_DEVICE_ERROR The operation was unsuccessful.
> +**/
> +EFI_STATUS
> +EFIAPI
> +CcMeasureAndLogData (
> + IN UINT32 PcrIndex,
> + IN UINT32 EventType,
> + IN VOID *EventLog,
> + IN UINT32 LogLen,
> + IN VOID *HashData,
> + IN UINT64 HashDataLen
> + )
> +{
> + EFI_STATUS Status;
> + EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol;
> + EFI_CC_EVENT *EfiCcEvent;
> + UINT32 MrIndex;
[SAMI] Same comment as in patch 2/3. Is it possible to use the typedef
for the measurment register index here, please?
> +
> + Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
> + if (EFI_ERROR (Status)) {
> + return EFI_INVALID_PARAMETER;
[SAMI] Is it possible to return the error code returned by
CcProtocol->MapPcrToMrIndex(), please?
> + }
> +
> + EfiCcEvent = (EFI_CC_EVENT *) AllocateZeroPool (LogLen + sizeof (EFI_CC_EVENT));
> + if(EfiCcEvent == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + EfiCcEvent->Size = (UINT32) LogLen + sizeof (EFI_CC_EVENT) - sizeof (EfiCcEvent->Event);
> + EfiCcEvent->Header.HeaderSize = sizeof (EFI_CC_EVENT_HEADER);
> + EfiCcEvent->Header.HeaderVersion = EFI_CC_EVENT_HEADER_VERSION;
> + EfiCcEvent->Header.MrIndex = MrIndex;
> + EfiCcEvent->Header.EventType = EventType;
> + CopyMem (&EfiCcEvent->Event[0], EventLog, LogLen);
> +
> + Status = CcProtocol->HashLogExtendEvent (
> + CcProtocol,
> + 0,
> + (EFI_PHYSICAL_ADDRESS) (UINTN) HashData,
> + HashDataLen,
> + EfiCcEvent
> + );
> + FreePool (EfiCcEvent);
> +
> + return Status;
> +}
> +
> +
> /**
> Tpm measure and log data, and extend the measurement result into a specific PCR.
>
> @@ -178,9 +245,9 @@ TpmMeasureAndLogData (
> EFI_STATUS Status;
>
> //
> - // Try to measure using Tpm20 protocol
> + // Try to measure using Cc measurement protocol
> //
> - Status = Tpm20MeasureAndLogData(
> + Status = CcMeasureAndLogData (
> PcrIndex,
> EventType,
> EventLog,
> @@ -189,6 +256,20 @@ TpmMeasureAndLogData (
> HashDataLen
> );
>
> + if (EFI_ERROR (Status)) {
> + //
> + // Try to measure using Tpm20 protocol
> + //
> + Status = Tpm20MeasureAndLogData(
> + PcrIndex,
> + EventType,
> + EventLog,
> + LogLen,
> + HashData,
> + HashDataLen
> + );
> + }
[SAMI] Apologies, I missed this in my previous review. I think the
behaviour if both the TCG2 and CC measurement protocols are installed
would be inconsistent between DxeTpmMeasurementLib and
DxeTpm2MeasureBootLib. The main difference being in the later, the
TCG2 protocol takes precedence for extending the measurement.
I think it would be good to modify DxeTpm2MeasureBootLib so that the CC
measurement protocol is used if both protocols are installed. What do
you think?
There is another subtle difference in this patch. Consider the case
where LocateProtocol(gEfiCcMeasurementProtocolGuid, ... ) succeeds but
subsequently
MapPcrToMrIndex() or HashLogExtendEvent() fail then
Tpm20MeasureAndLogData() will be called. This is not the case with
DxeTpm2MeasureBootLib.
I think it would be good to have a consistent behaviour.
[/SAMI]
> +
> if (EFI_ERROR (Status)) {
> //
> // Try to measure using Tpm1.2 protocol
> diff --git a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> index 7d41bc41f95d..3af3d4e33b25 100644
> --- a/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +++ b/SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> @@ -1,5 +1,7 @@
> ## @file
> -# Provides TPM measurement functions for TPM1.2 and TPM 2.0
> +# Provides below measurement functions:
> +# 1. TPM measurement functions for TPM1.2 and TPM 2.0
> +# 2. Confidential Computing (CC) measurement functions
> #
> # This library provides TpmMeasureAndLogData() to measure and log data, and
> # extend the measurement result into a specific PCR.
> @@ -40,5 +42,6 @@
> UefiBootServicesTableLib
>
> [Protocols]
> - gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES
> - gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiTcgProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
> + gEfiCcMeasurementProtocolGuid ## SOMETIMES_CONSUMES
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83114): https://edk2.groups.io/g/devel/message/83114
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi, > [SAMI] Apologies, I missed this in my previous review. I think the behaviour > if both the TCG2 and CC measurement protocols are installed > would be inconsistent between DxeTpmMeasurementLib and > DxeTpm2MeasureBootLib. The main difference being in the later, the > TCG2 protocol takes precedence for extending the measurement. Yes, we should have consistent behavior in both cases. > I think it would be good to modify DxeTpm2MeasureBootLib so that the CC > measurement protocol is used if both protocols are installed. What do you > think? Does it makes sense to use both protocols? take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83326): https://edk2.groups.io/g/devel/message/83326 Mute This Topic: https://groups.io/mt/86758672/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On November 4, 2021 4:21 PM, Gerd Hoffmann wrote: > Hi, > > > [SAMI] Apologies, I missed this in my previous review. I think the > > behaviour if both the TCG2 and CC measurement protocols are installed > > would be inconsistent between DxeTpmMeasurementLib and > > DxeTpm2MeasureBootLib. The main difference being in the later, the > > TCG2 protocol takes precedence for extending the measurement. > > Yes, we should have consistent behavior in both cases. In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If it fails, then it try to measure with TCG2 / TCG protocol in turn. In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails, CC measurement protocol is tried in turn. Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc measurement protocol first, then try TCG2 protocol if Cc measurement protocol fails. In this way, only one protocol will be called to do the measurement. But TCG2 protocol is the first try, CC measurement protocol is the second try. > > > I think it would be good to modify DxeTpm2MeasureBootLib so that the > > CC measurement protocol is used if both protocols are installed. What > > do you think? > > Does it makes sense to use both protocols? Agree with Gerd. I don't think we should use both protocols to do the measurement. My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just as I explained above. Sami, what's your thought? Thanks Min -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#83336): https://edk2.groups.io/g/devel/message/83336 Mute This Topic: https://groups.io/mt/86758672/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On November 4, 2021 9:35 PM, Xu Min wrote:
> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
> > Hi,
> >
> > > [SAMI] Apologies, I missed this in my previous review. I think the
> > > behaviour if both the TCG2 and CC measurement protocols are
> > > installed would be inconsistent between DxeTpmMeasurementLib and
> > > DxeTpm2MeasureBootLib. The main difference being in the later, the
> > > TCG2 protocol takes precedence for extending the measurement.
> >
> > Yes, we should have consistent behavior in both cases.
> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If
> it fails, then it try to measure with TCG2 / TCG protocol in turn.
> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails,
> CC measurement protocol is tried in turn.
> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
> measurement protocol first, then try TCG2 protocol if Cc measurement protocol
> fails. In this way, only one protocol will be called to do the measurement. But
> TCG2 protocol is the first try, CC measurement protocol is the second try.
>
> >
> > > I think it would be good to modify DxeTpm2MeasureBootLib so that the
> > > CC measurement protocol is used if both protocols are installed.
> > > What do you think?
> >
> > Does it makes sense to use both protocols?
> Agree with Gerd. I don't think we should use both protocols to do the
> measurement.
> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just
> as I explained above.
Another option will be that:
In DxeTpmMeasurementLib the pseudo would look like:
If (CC Protocol is installed) {
Status = CcMeasureAndLogData (...)
} else { // below is the original code
Status = Tpm20MeasureAndLogData (...)
If (EFI_ERROR (Status)) {
Status = Tpm12MeasureAndLogData (...)
}
}
In DxeTpm2MeasureBootLib, the pseudo would look like:
If (CC Protocol is installed) {
Status = DoCcMeasureBoot(...)
} else if (TCG2 protocol is installed) {
Status = DoTcg2MeasureBoot(...)
}
Sami & Gerd
What's your thougth?
Thanks
Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83340): https://edk2.groups.io/g/devel/message/83340
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Min,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 04/11/2021 01:49 PM, Xu, Min M wrote:
> On November 4, 2021 9:35 PM, Xu Min wrote:
>> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
>>> Hi,
>>>
>>>> [SAMI] Apologies, I missed this in my previous review. I think the
>>>> behaviour if both the TCG2 and CC measurement protocols are
>>>> installed would be inconsistent between DxeTpmMeasurementLib and
>>>> DxeTpm2MeasureBootLib. The main difference being in the later, the
>>>> TCG2 protocol takes precedence for extending the measurement.
>>> Yes, we should have consistent behavior in both cases.
>> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If
>> it fails, then it try to measure with TCG2 / TCG protocol in turn.
>> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails,
>> CC measurement protocol is tried in turn.
>> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
>> measurement protocol first, then try TCG2 protocol if Cc measurement protocol
>> fails. In this way, only one protocol will be called to do the measurement. But
>> TCG2 protocol is the first try, CC measurement protocol is the second try.
>>
>>>> I think it would be good to modify DxeTpm2MeasureBootLib so that the
>>>> CC measurement protocol is used if both protocols are installed.
>>>> What do you think?
>>> Does it makes sense to use both protocols?
>> Agree with Gerd. I don't think we should use both protocols to do the
>> measurement.
>> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just
>> as I explained above.
> Another option will be that:
> In DxeTpmMeasurementLib the pseudo would look like:
> If (CC Protocol is installed) {
> Status = CcMeasureAndLogData (...)
> } else { // below is the original code
> Status = Tpm20MeasureAndLogData (...)
> If (EFI_ERROR (Status)) {
> Status = Tpm12MeasureAndLogData (...)
> }
> }
>
> In DxeTpm2MeasureBootLib, the pseudo would look like:
> If (CC Protocol is installed) {
> Status = DoCcMeasureBoot(...)
> } else if (TCG2 protocol is installed) {
> Status = DoTcg2MeasureBoot(...)
> }
[SAMI] Your pseudo code looks good to me. It makes the measurement logic
much clearer.
Also, I am not aware if there is a use-case for both the CC Protocol
and the TCG2 protocols to be installed at the same time.
[/SAMI]
> Sami & Gerd
> What's your thougth?
>
> Thanks
> Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83345): https://edk2.groups.io/g/devel/message/83345
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
I believe a platform should have only one RTS/RTR.
Only one of (virtual)TPM1.2, (virtual)TPM2.0 and CC MR exists. Then only one TCG_SERVICE_PROTOCOL, TCG2_PROTOCOL, CC_MEASUREMENT_PROTOCOL is exposed.
In the case that, a vTPM is present to emulate the CC MR, then a TDVF should only expose TCG2_PROTOCOL. Otherwise, there will be confusing on the final event log.
Thank you
Yao Jiewen
> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, November 4, 2021 10:18 PM
> To: Xu, Min M <min.m.xu@intel.com>; devel@edk2.groups.io;
> kraxel@redhat.com
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; nd
> <nd@arm.com>
> Subject: Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support
> CcMeasurementProtocol in DxeTpmMeasurementLib
>
> Hi Min,
>
> Please find my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
>
> On 04/11/2021 01:49 PM, Xu, Min M wrote:
> > On November 4, 2021 9:35 PM, Xu Min wrote:
> >> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
> >>> Hi,
> >>>
> >>>> [SAMI] Apologies, I missed this in my previous review. I think the
> >>>> behaviour if both the TCG2 and CC measurement protocols are
> >>>> installed would be inconsistent between DxeTpmMeasurementLib and
> >>>> DxeTpm2MeasureBootLib. The main difference being in the later, the
> >>>> TCG2 protocol takes precedence for extending the measurement.
> >>> Yes, we should have consistent behavior in both cases.
> >> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try.
> If
> >> it fails, then it try to measure with TCG2 / TCG protocol in turn.
> >> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it
> fails,
> >> CC measurement protocol is tried in turn.
> >> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
> >> measurement protocol first, then try TCG2 protocol if Cc measurement
> protocol
> >> fails. In this way, only one protocol will be called to do the measurement. But
> >> TCG2 protocol is the first try, CC measurement protocol is the second try.
> >>
> >>>> I think it would be good to modify DxeTpm2MeasureBootLib so that the
> >>>> CC measurement protocol is used if both protocols are installed.
> >>>> What do you think?
> >>> Does it makes sense to use both protocols?
> >> Agree with Gerd. I don't think we should use both protocols to do the
> >> measurement.
> >> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol.
> Just
> >> as I explained above.
> > Another option will be that:
> > In DxeTpmMeasurementLib the pseudo would look like:
> > If (CC Protocol is installed) {
> > Status = CcMeasureAndLogData (...)
> > } else { // below is the original code
> > Status = Tpm20MeasureAndLogData (...)
> > If (EFI_ERROR (Status)) {
> > Status = Tpm12MeasureAndLogData (...)
> > }
> > }
> >
> > In DxeTpm2MeasureBootLib, the pseudo would look like:
> > If (CC Protocol is installed) {
> > Status = DoCcMeasureBoot(...)
> > } else if (TCG2 protocol is installed) {
> > Status = DoTcg2MeasureBoot(...)
> > }
> [SAMI] Your pseudo code looks good to me. It makes the measurement logic
> much clearer.
> Also, I am not aware if there is a use-case for both the CC Protocol
> and the TCG2 protocols to be installed at the same time.
> [/SAMI]
> > Sami & Gerd
> > What's your thougth?
> >
> > Thanks
> > Min
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83346): https://edk2.groups.io/g/devel/message/83346
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi, Sami
Please see my comments inline.
+**/
+EFI_STATUS
+EFIAPI
+CcMeasureAndLogData (
+ IN UINT32 PcrIndex,
+ IN UINT32 EventType,
+ IN VOID *EventLog,
+ IN UINT32 LogLen,
+ IN VOID *HashData,
+ IN UINT64 HashDataLen
+ )
+{
+ EFI_STATUS Status;
+ EFI_CC_MEASUREMENT_PROTOCOL *CcProtocol;
+ EFI_CC_EVENT *EfiCcEvent;
+ UINT32 MrIndex;
[SAMI] Same comment as in patch 2/3. Is it possible to use the typedef for the measurment register index here, please?
[Min] Thanks for reminder. It will be fixed.
+
+ Status = gBS->LocateProtocol (&gEfiCcMeasurementProtocolGuid, NULL, (VOID **) &CcProtocol);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = CcProtocol->MapPcrToMrIndex (CcProtocol, PcrIndex, &MrIndex);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
[SAMI] Is it possible to return the error code returned by CcProtocol->MapPcrToMrIndex(), please?
[Min] Sure. It will be updated in the next version.
Thanks
Min_._,_._,_
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#83361): https://edk2.groups.io/g/devel/message/83361
Mute This Topic: https://groups.io/mt/86758672/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.