* [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7]
2017-01-19 5:13 [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic Zhang, Chao B
@ 2017-01-19 5:13 ` Zhang, Chao B
2017-01-22 1:22 ` Zeng, Star
2017-01-22 2:03 ` Yao, Jiewen
2017-01-19 5:13 ` [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec Zhang, Chao B
` (2 subsequent siblings)
3 siblings, 2 replies; 9+ messages in thread
From: Zhang, Chao B @ 2017-01-19 5:13 UTC (permalink / raw)
To: edk2-devel; +Cc: yao.jiewen, star.zeng, Yao Jiewen, Chao Zhang
Measure DBT into PCR[7] in initial measurement phase according
to TCG PC Client PFP 00.37. http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Cc: Star Zeng <star.zeng@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 556ef33..3534fd1 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -1,7 +1,7 @@
/** @file
This module implements Tcg2 Protocol.
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -115,6 +115,7 @@ VARIABLE_TYPE mVariableType[] = {
{EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid},
{EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid},
{EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
+ {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid},
};
EFI_HANDLE mImageHandle;
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7]
2017-01-19 5:13 ` [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7] Zhang, Chao B
@ 2017-01-22 1:22 ` Zeng, Star
2017-01-22 2:03 ` Yao, Jiewen
1 sibling, 0 replies; 9+ messages in thread
From: Zeng, Star @ 2017-01-22 1:22 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, Zeng, Star
Chao,
I have one minor comment.
Please remember to add " ## SOMETIMES_CONSUMES ## Variable:L"dbt" " in Tcg2Dxe.inf.
With the comment covered, Reviewed-by: Star Zeng <star.zeng@intel.com>
Thanks,
Star
-----Original Message-----
From: Zhang, Chao B
Sent: Thursday, January 19, 2017 1:14 PM
To: edk2-devel@lists.01.org
Cc: yao.jiewen@intel.com; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7]
Measure DBT into PCR[7] in initial measurement phase according to TCG PC Client PFP 00.37. http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Cc: Star Zeng <star.zeng@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 556ef33..3534fd1 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -1,7 +1,7 @@
/** @file
This module implements Tcg2 Protocol.
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -115,6 +115,7 @@ VARIABLE_TYPE mVariableType[] = {
{EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid},
{EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid},
{EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
+ {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid},
};
EFI_HANDLE mImageHandle;
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7]
2017-01-19 5:13 ` [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7] Zhang, Chao B
2017-01-22 1:22 ` Zeng, Star
@ 2017-01-22 2:03 ` Yao, Jiewen
1 sibling, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2017-01-22 2:03 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: yao.jiewen@intel.com, Zeng, Star
Reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: Zhang, Chao B
> Sent: Thursday, January 19, 2017 1:14 PM
> To: edk2-devel@lists.01.org
> Cc: yao.jiewen@intel.com; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7]
>
> Measure DBT into PCR[7] in initial measurement phase according
> to TCG PC Client PFP 00.37.
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
> SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> index 556ef33..3534fd1 100644
> --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
> @@ -1,7 +1,7 @@
> /** @file
> This module implements Tcg2 Protocol.
>
> -Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -115,6 +115,7 @@ VARIABLE_TYPE mVariableType[] = {
> {EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid},
> {EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid},
> {EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
> + {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid},
> };
>
> EFI_HANDLE mImageHandle;
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec
2017-01-19 5:13 [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic Zhang, Chao B
2017-01-19 5:13 ` [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7] Zhang, Chao B
@ 2017-01-19 5:13 ` Zhang, Chao B
2017-01-22 1:19 ` Zeng, Star
2017-01-22 2:20 ` Yao, Jiewen
2017-01-22 1:25 ` [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic Zeng, Star
2017-01-22 2:04 ` Yao, Jiewen
3 siblings, 2 replies; 9+ messages in thread
From: Zhang, Chao B @ 2017-01-19 5:13 UTC (permalink / raw)
To: edk2-devel; +Cc: yao.jiewen, star.zeng, Yao Jiewen, Chao Zhang
Measure DBT into PCR[7] when it is updated between initial measure and
ExitBootService. Measure "SecureBoot" change after PK update.
Spec version : TCG PC Client PFP 00.37. http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Cc: Star Zeng <star.zeng@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
.../Universal/Variable/RuntimeDxe/Measurement.c | 88 +++++++++++++++++++++-
.../Universal/Variable/RuntimeDxe/VariableDxe.c | 17 +++++
.../Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 17 +++++
3 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
index 2f92fae..707f988 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
@@ -1,7 +1,7 @@
/** @file
Measure TrEE required variable.
-Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -36,6 +36,24 @@ VARIABLE_TYPE mVariableType[] = {
{EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid},
{EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid},
{EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
+ {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid},
+};
+
+typedef struct {
+ CHAR16 *VariableName;
+ EFI_GUID *VendorGuid;
+ UINT8 *VarData;
+ UINTN VarDataSize;
+} VARIABLE_FOLLOW_TYPE;
+
+//
+// "SecureBoot" may update following PK Del/Add
+//
+static VARIABLE_FOLLOW_TYPE SecureBootFollowUpdate = {
+ EFI_SECURE_BOOT_MODE_NAME,
+ &gEfiGlobalVariableGuid,
+ NULL,
+ 0,
};
/**
@@ -251,5 +269,73 @@ SecureBootHook (
FreePool (VariableData);
}
+ //
+ // "SecureBoot" is 8bit & read-only. It can only be changed according to PK update
+ //
+ if ((StrCmp (VariableName, EFI_PLATFORM_KEY_NAME) == 0) &&
+ CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) {
+ Status = InternalGetVariable (
+ SecureBootFollowUpdate.VariableName,
+ SecureBootFollowUpdate.VendorGuid,
+ &VariableData,
+ &VariableDataSize
+ );
+ if (EFI_ERROR (Status)) {
+ return;
+ }
+
+ if ((SecureBootFollowUpdate.VarData != NULL) &&
+ (CompareMem(SecureBootFollowUpdate.VarData, VariableData, VariableDataSize) != 0)) {
+ FreePool(SecureBootFollowUpdate.VarData);
+ SecureBootFollowUpdate.VarData = VariableData;
+ SecureBootFollowUpdate.VarDataSize = VariableDataSize;
+
+ DEBUG((DEBUG_INFO, "%s variable updated according to PK change. Remeasure the value!\n", SecureBootFollowUpdate.VariableName));
+ Status = MeasureVariable (
+ SecureBootFollowUpdate.VariableName,
+ SecureBootFollowUpdate.VendorGuid,
+ SecureBootFollowUpdate.VarData,
+ SecureBootFollowUpdate.VarDataSize
+ );
+ DEBUG ((DEBUG_INFO, "MeasureBootPolicyVariable - %r\n", Status));
+ } else {
+ //
+ // "SecureBoot" variable is not changed
+ //
+ FreePool(VariableData);
+ }
+ }
+
return ;
}
+
+/**
+ Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
+ Record their initial State when variable write service is ready.
+
+**/
+VOID
+EFIAPI
+RecordSecureBootPolicyVarFollow(
+ VOID
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Record initial "SecureBoot" variable value.
+ // It is used to detect SecureBoot variable change in SecureBootHook.
+ //
+ Status = InternalGetVariable (
+ SecureBootFollowUpdate.VariableName,
+ SecureBootFollowUpdate.VendorGuid,
+ (VOID **)&SecureBootFollowUpdate.VarData,
+ &SecureBootFollowUpdate.VarDataSize
+ );
+ if (EFI_ERROR(Status)) {
+ //
+ // Read could fail when Auth Variable solution is not supported
+ //
+ DEBUG((DEBUG_INFO, "RecordSecureBootPolicyVarFollow GetVariable %s Status %x\n", SecureBootFollowUpdate.VariableName, Status));
+ }
+}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 3d3cd24..5d81f87 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -32,6 +32,17 @@ EDKII_VAR_CHECK_PROTOCOL mVarCheck = { VarCheckRegis
VarCheckVariablePropertyGet };
/**
+ Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
+ Record their initial State when variable write service is ready.
+
+**/
+VOID
+EFIAPI
+RecordSecureBootPolicyVarFollow(
+ VOID
+ );
+
+/**
Return TRUE if ExitBootServices () has been called.
@retval TRUE If ExitBootServices () has been called.
@@ -415,6 +426,12 @@ FtwNotificationEvent (
}
//
+ // Some Secure Boot Policy Var (SecureBoot, etc) updates following other
+ // Secure Boot Policy Variable change. Record their initial value.
+ //
+ RecordSecureBootPolicyVarFollow();
+
+ //
// Install the Variable Write Architectural protocol.
//
Status = gBS->InstallProtocolInterface (
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 0a076ae..3d0925d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -71,6 +71,17 @@ SecureBootHook (
);
/**
+ Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
+ Record their initial State when variable write service is ready.
+
+**/
+VOID
+EFIAPI
+RecordSecureBootPolicyVarFollow(
+ VOID
+ );
+
+/**
Acquires lock only at boot time. Simply returns at runtime.
This is a temperary function that will be removed when
@@ -1079,6 +1090,12 @@ SmmVariableWriteReady (
return;
}
+ //
+ // Some Secure Boot Policy Var (SecureBoot, etc) updates following other
+ // Secure Boot Policy Variable change. Record their initial value.
+ //
+ RecordSecureBootPolicyVarFollow();
+
Status = gBS->InstallProtocolInterface (
&mHandle,
&gEfiVariableWriteArchProtocolGuid,
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec
2017-01-19 5:13 ` [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec Zhang, Chao B
@ 2017-01-22 1:19 ` Zeng, Star
2017-01-22 2:20 ` Yao, Jiewen
1 sibling, 0 replies; 9+ messages in thread
From: Zeng, Star @ 2017-01-22 1:19 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, Zeng, Star
Chao,
I have some minor comments.
1. Suggest to add mSecureBootVarData and mSecureBootVarDataSize and remove VARIABLE_FOLLOW_TYPE and SecureBootFollowUpdate.
2. Suggest to update function name RecordSecureBootPolicyVarFollow to RecordSecureBootPolicyVarData.
3. Please remember to add " ## SOMETIMES_CONSUMES ## Variable:L"DBT" " in VariableRuntimeDxe.inf and VariableSmmRuntimeDxe.inf.
With those comments covered, Reviewed-by: Star Zeng <star.zeng@intel.com>
Thanks,
Star
-----Original Message-----
From: Zhang, Chao B
Sent: Thursday, January 19, 2017 1:14 PM
To: edk2-devel@lists.01.org
Cc: yao.jiewen@intel.com; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec
Measure DBT into PCR[7] when it is updated between initial measure and ExitBootService. Measure "SecureBoot" change after PK update.
Spec version : TCG PC Client PFP 00.37. http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Cc: Star Zeng <star.zeng@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
.../Universal/Variable/RuntimeDxe/Measurement.c | 88 +++++++++++++++++++++-
.../Universal/Variable/RuntimeDxe/VariableDxe.c | 17 +++++
.../Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 17 +++++
3 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
index 2f92fae..707f988 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
@@ -1,7 +1,7 @@
/** @file
Measure TrEE required variable.
-Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -36,6 +36,24 @@ VARIABLE_TYPE mVariableType[] = {
{EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid},
{EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid},
{EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
+ {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid}, };
+
+typedef struct {
+ CHAR16 *VariableName;
+ EFI_GUID *VendorGuid;
+ UINT8 *VarData;
+ UINTN VarDataSize;
+} VARIABLE_FOLLOW_TYPE;
+
+//
+// "SecureBoot" may update following PK Del/Add // static
+VARIABLE_FOLLOW_TYPE SecureBootFollowUpdate = {
+ EFI_SECURE_BOOT_MODE_NAME,
+ &gEfiGlobalVariableGuid,
+ NULL,
+ 0,
};
/**
@@ -251,5 +269,73 @@ SecureBootHook (
FreePool (VariableData);
}
+ //
+ // "SecureBoot" is 8bit & read-only. It can only be changed according
+ to PK update // if ((StrCmp (VariableName, EFI_PLATFORM_KEY_NAME) ==
+ 0) &&
+ CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) {
+ Status = InternalGetVariable (
+ SecureBootFollowUpdate.VariableName,
+ SecureBootFollowUpdate.VendorGuid,
+ &VariableData,
+ &VariableDataSize
+ );
+ if (EFI_ERROR (Status)) {
+ return;
+ }
+
+ if ((SecureBootFollowUpdate.VarData != NULL) &&
+ (CompareMem(SecureBootFollowUpdate.VarData, VariableData, VariableDataSize) != 0)) {
+ FreePool(SecureBootFollowUpdate.VarData);
+ SecureBootFollowUpdate.VarData = VariableData;
+ SecureBootFollowUpdate.VarDataSize = VariableDataSize;
+
+ DEBUG((DEBUG_INFO, "%s variable updated according to PK change. Remeasure the value!\n", SecureBootFollowUpdate.VariableName));
+ Status = MeasureVariable (
+ SecureBootFollowUpdate.VariableName,
+ SecureBootFollowUpdate.VendorGuid,
+ SecureBootFollowUpdate.VarData,
+ SecureBootFollowUpdate.VarDataSize
+ );
+ DEBUG ((DEBUG_INFO, "MeasureBootPolicyVariable - %r\n", Status));
+ } else {
+ //
+ // "SecureBoot" variable is not changed
+ //
+ FreePool(VariableData);
+ }
+ }
+
return ;
}
+
+/**
+ Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
+ Record their initial State when variable write service is ready.
+
+**/
+VOID
+EFIAPI
+RecordSecureBootPolicyVarFollow(
+ VOID
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Record initial "SecureBoot" variable value.
+ // It is used to detect SecureBoot variable change in SecureBootHook.
+ //
+ Status = InternalGetVariable (
+ SecureBootFollowUpdate.VariableName,
+ SecureBootFollowUpdate.VendorGuid,
+ (VOID **)&SecureBootFollowUpdate.VarData,
+ &SecureBootFollowUpdate.VarDataSize
+ );
+ if (EFI_ERROR(Status)) {
+ //
+ // Read could fail when Auth Variable solution is not supported
+ //
+ DEBUG((DEBUG_INFO, "RecordSecureBootPolicyVarFollow GetVariable %s
+Status %x\n", SecureBootFollowUpdate.VariableName, Status));
+ }
+}
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
index 3d3cd24..5d81f87 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
@@ -32,6 +32,17 @@ EDKII_VAR_CHECK_PROTOCOL mVarCheck = { VarCheckRegis
VarCheckVariablePropertyGet };
/**
+ Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
+ Record their initial State when variable write service is ready.
+
+**/
+VOID
+EFIAPI
+RecordSecureBootPolicyVarFollow(
+ VOID
+ );
+
+/**
Return TRUE if ExitBootServices () has been called.
@retval TRUE If ExitBootServices () has been called.
@@ -415,6 +426,12 @@ FtwNotificationEvent (
}
//
+ // Some Secure Boot Policy Var (SecureBoot, etc) updates following
+ other // Secure Boot Policy Variable change. Record their initial value.
+ //
+ RecordSecureBootPolicyVarFollow();
+
+ //
// Install the Variable Write Architectural protocol.
//
Status = gBS->InstallProtocolInterface ( diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
index 0a076ae..3d0925d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
@@ -71,6 +71,17 @@ SecureBootHook (
);
/**
+ Some Secure Boot Policy Variable may update following other variable changes(SecureBoot follows PK change, etc).
+ Record their initial State when variable write service is ready.
+
+**/
+VOID
+EFIAPI
+RecordSecureBootPolicyVarFollow(
+ VOID
+ );
+
+/**
Acquires lock only at boot time. Simply returns at runtime.
This is a temperary function that will be removed when @@ -1079,6 +1090,12 @@ SmmVariableWriteReady (
return;
}
+ //
+ // Some Secure Boot Policy Var (SecureBoot, etc) updates following
+ other // Secure Boot Policy Variable change. Record their initial value.
+ //
+ RecordSecureBootPolicyVarFollow();
+
Status = gBS->InstallProtocolInterface (
&mHandle,
&gEfiVariableWriteArchProtocolGuid,
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec
2017-01-19 5:13 ` [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec Zhang, Chao B
2017-01-22 1:19 ` Zeng, Star
@ 2017-01-22 2:20 ` Yao, Jiewen
1 sibling, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2017-01-22 2:20 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Zeng, Star
I think below check is unnecessary, because if we can set PK, secure boot variable must exist.
"if ((SecureBootFollowUpdate.VarData != NULL)"
I believe we can use "ASSERT (SecureBootFollowUpdate.VarData != NULL);"
With that change, reviewed-by: Jiewen.yao@intel.com
Thank you
Yao Jiewen
> -----Original Message-----
> From: Zhang, Chao B
> Sent: Thursday, January 19, 2017 1:14 PM
> To: edk2-devel@lists.01.org
> Cc: yao.jiewen@intel.com; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for
> new TCG spec
>
> Measure DBT into PCR[7] when it is updated between initial measure and
> ExitBootService. Measure "SecureBoot" change after PK update.
> Spec version : TCG PC Client PFP 00.37.
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
> .../Universal/Variable/RuntimeDxe/Measurement.c | 88
> +++++++++++++++++++++-
> .../Universal/Variable/RuntimeDxe/VariableDxe.c | 17 +++++
> .../Variable/RuntimeDxe/VariableSmmRuntimeDxe.c | 17 +++++
> 3 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> index 2f92fae..707f988 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
> @@ -1,7 +1,7 @@
> /** @file
> Measure TrEE required variable.
>
> -Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2013 - 2017, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be found
> at
> @@ -36,6 +36,24 @@ VARIABLE_TYPE mVariableType[] = {
> {EFI_KEY_EXCHANGE_KEY_NAME, &gEfiGlobalVariableGuid},
> {EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid},
> {EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid},
> + {EFI_IMAGE_SECURITY_DATABASE2, &gEfiImageSecurityDatabaseGuid},
> +};
> +
> +typedef struct {
> + CHAR16 *VariableName;
> + EFI_GUID *VendorGuid;
> + UINT8 *VarData;
> + UINTN VarDataSize;
> +} VARIABLE_FOLLOW_TYPE;
> +
> +//
> +// "SecureBoot" may update following PK Del/Add
> +//
> +static VARIABLE_FOLLOW_TYPE SecureBootFollowUpdate = {
> + EFI_SECURE_BOOT_MODE_NAME,
> + &gEfiGlobalVariableGuid,
> + NULL,
> + 0,
> };
>
> /**
> @@ -251,5 +269,73 @@ SecureBootHook (
> FreePool (VariableData);
> }
>
> + //
> + // "SecureBoot" is 8bit & read-only. It can only be changed according to PK
> update
> + //
> + if ((StrCmp (VariableName, EFI_PLATFORM_KEY_NAME) == 0) &&
> + CompareGuid (VendorGuid, &gEfiGlobalVariableGuid)) {
> + Status = InternalGetVariable (
> + SecureBootFollowUpdate.VariableName,
> + SecureBootFollowUpdate.VendorGuid,
> + &VariableData,
> + &VariableDataSize
> + );
> + if (EFI_ERROR (Status)) {
> + return;
> + }
> +
> + if ((SecureBootFollowUpdate.VarData != NULL) &&
> + (CompareMem(SecureBootFollowUpdate.VarData, VariableData,
> VariableDataSize) != 0)) {
> + FreePool(SecureBootFollowUpdate.VarData);
> + SecureBootFollowUpdate.VarData = VariableData;
> + SecureBootFollowUpdate.VarDataSize = VariableDataSize;
> +
> + DEBUG((DEBUG_INFO, "%s variable updated according to PK change.
> Remeasure the value!\n", SecureBootFollowUpdate.VariableName));
> + Status = MeasureVariable (
> + SecureBootFollowUpdate.VariableName,
> + SecureBootFollowUpdate.VendorGuid,
> + SecureBootFollowUpdate.VarData,
> + SecureBootFollowUpdate.VarDataSize
> + );
> + DEBUG ((DEBUG_INFO, "MeasureBootPolicyVariable - %r\n", Status));
> + } else {
> + //
> + // "SecureBoot" variable is not changed
> + //
> + FreePool(VariableData);
> + }
> + }
> +
> return ;
> }
> +
> +/**
> + Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> + Record their initial State when variable write service is ready.
> +
> +**/
> +VOID
> +EFIAPI
> +RecordSecureBootPolicyVarFollow(
> + VOID
> + )
> +{
> + EFI_STATUS Status;
> +
> + //
> + // Record initial "SecureBoot" variable value.
> + // It is used to detect SecureBoot variable change in SecureBootHook.
> + //
> + Status = InternalGetVariable (
> + SecureBootFollowUpdate.VariableName,
> + SecureBootFollowUpdate.VendorGuid,
> + (VOID **)&SecureBootFollowUpdate.VarData,
> + &SecureBootFollowUpdate.VarDataSize
> + );
> + if (EFI_ERROR(Status)) {
> + //
> + // Read could fail when Auth Variable solution is not supported
> + //
> + DEBUG((DEBUG_INFO, "RecordSecureBootPolicyVarFollow GetVariable %s
> Status %x\n", SecureBootFollowUpdate.VariableName, Status));
> + }
> +}
> diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> index 3d3cd24..5d81f87 100644
> --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c
> @@ -32,6 +32,17 @@ EDKII_VAR_CHECK_PROTOCOL mVarCheck
> = { VarCheckRegis
>
> VarCheckVariablePropertyGet };
>
> /**
> + Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> + Record their initial State when variable write service is ready.
> +
> +**/
> +VOID
> +EFIAPI
> +RecordSecureBootPolicyVarFollow(
> + VOID
> + );
> +
> +/**
> Return TRUE if ExitBootServices () has been called.
>
> @retval TRUE If ExitBootServices () has been called.
> @@ -415,6 +426,12 @@ FtwNotificationEvent (
> }
>
> //
> + // Some Secure Boot Policy Var (SecureBoot, etc) updates following other
> + // Secure Boot Policy Variable change. Record their initial value.
> + //
> + RecordSecureBootPolicyVarFollow();
> +
> + //
> // Install the Variable Write Architectural protocol.
> //
> Status = gBS->InstallProtocolInterface (
> diff --git
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> index 0a076ae..3d0925d 100644
> ---
> a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
> @@ -71,6 +71,17 @@ SecureBootHook (
> );
>
> /**
> + Some Secure Boot Policy Variable may update following other variable
> changes(SecureBoot follows PK change, etc).
> + Record their initial State when variable write service is ready.
> +
> +**/
> +VOID
> +EFIAPI
> +RecordSecureBootPolicyVarFollow(
> + VOID
> + );
> +
> +/**
> Acquires lock only at boot time. Simply returns at runtime.
>
> This is a temperary function that will be removed when
> @@ -1079,6 +1090,12 @@ SmmVariableWriteReady (
> return;
> }
>
> + //
> + // Some Secure Boot Policy Var (SecureBoot, etc) updates following other
> + // Secure Boot Policy Variable change. Record their initial value.
> + //
> + RecordSecureBootPolicyVarFollow();
> +
> Status = gBS->InstallProtocolInterface (
> &mHandle,
> &gEfiVariableWriteArchProtocolGuid,
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic
2017-01-19 5:13 [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic Zhang, Chao B
2017-01-19 5:13 ` [PATCH V2 2/3] SecurityPkg: Tcg2Dxe: Measure DBT into PCR[7] Zhang, Chao B
2017-01-19 5:13 ` [PATCH V2 3/3] MdeModulePkg: Variable: Update PCR[7] measure for new TCG spec Zhang, Chao B
@ 2017-01-22 1:25 ` Zeng, Star
2017-01-22 2:04 ` Yao, Jiewen
3 siblings, 0 replies; 9+ messages in thread
From: Zeng, Star @ 2017-01-22 1:25 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, Zeng, Star
Chao,
I will leave the review for this patch to Jiewen since I am not so familiar with the code logic.
Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zhang, Chao B
Sent: Thursday, January 19, 2017 1:14 PM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; yao.jiewen@intel.com
Subject: [edk2] [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic
Update PCR[7] measure logic according to TCG PC Client PFP 00.37.
Only entries in DB that is used for image authentication need to be measured.
http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific_Platform_Profile_for_TPM_2p0_Systems_v21.pdf
Cc: Star Zeng <star.zeng@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
.../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 7b7e6af..e28e106 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi
+++ b.c
@@ -12,7 +12,7 @@
DxeImageVerificationHandler(), HashPeImageByType(), HashPeImage() function will accept
untrusted PE/COFF image and validate its data structure within this image buffer before use.
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License @@ -1026,7 +1026,12 @@ IsSignatureFoundInDatabase (
// Find the signature in database.
//
IsFound = TRUE;
- SecureBootHook (VariableName, &gEfiImageSecurityDatabaseGuid, CertList->SignatureSize, Cert);
+ //
+ // Entries in UEFI_IMAGE_SECURITY_DATABASE that are used to validate image should be measured
+ //
+ if (StrCmp(VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) {
+ SecureBootHook (VariableName, &gEfiImageSecurityDatabaseGuid, CertList->SignatureSize, Cert);
+ }
break;
}
@@ -1309,7 +1314,6 @@ IsForbiddenByDbx (
mImageDigestSize
);
if (IsForbidden) {
- SecureBootHook (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, CertList->SignatureSize, CertData);
DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature is forbidden by DBX.\n"));
goto Done;
}
--
1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic
2017-01-19 5:13 [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic Zhang, Chao B
` (2 preceding siblings ...)
2017-01-22 1:25 ` [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update PCR[7] measure logic Zeng, Star
@ 2017-01-22 2:04 ` Yao, Jiewen
3 siblings, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2017-01-22 2:04 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org
Cc: Zhang, Chao B, Zeng, Star, yao.jiewen@intel.com
Reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zhang,
> Chao B
> Sent: Thursday, January 19, 2017 1:14 PM
> To: edk2-devel@lists.01.org
> Cc: Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>;
> yao.jiewen@intel.com
> Subject: [edk2] [PATCH V2 1/3] SecurityPkg: DxeImageVerificationLib: Update
> PCR[7] measure logic
>
> Update PCR[7] measure logic according to TCG PC Client PFP 00.37.
> Only entries in DB that is used for image authentication need to be
> measured.
> http://www.trustedcomputinggroup.org/wp-content/uploads/PC-ClientSpecific
> _Platform_Profile_for_TPM_2p0_Systems_v21.pdf
>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
> .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 10
> +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 7b7e6af..e28e106 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -12,7 +12,7 @@
> DxeImageVerificationHandler(), HashPeImageByType(), HashPeImage()
> function will accept
> untrusted PE/COFF image and validate its data structure within this image
> buffer before use.
>
> -Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> @@ -1026,7 +1026,12 @@ IsSignatureFoundInDatabase (
> // Find the signature in database.
> //
> IsFound = TRUE;
> - SecureBootHook (VariableName, &gEfiImageSecurityDatabaseGuid,
> CertList->SignatureSize, Cert);
> + //
> + // Entries in UEFI_IMAGE_SECURITY_DATABASE that are used to
> validate image should be measured
> + //
> + if (StrCmp(VariableName, EFI_IMAGE_SECURITY_DATABASE) == 0) {
> + SecureBootHook (VariableName,
> &gEfiImageSecurityDatabaseGuid, CertList->SignatureSize, Cert);
> + }
> break;
> }
>
> @@ -1309,7 +1314,6 @@ IsForbiddenByDbx (
> mImageDigestSize
> );
> if (IsForbidden) {
> - SecureBootHook (EFI_IMAGE_SECURITY_DATABASE1,
> &gEfiImageSecurityDatabaseGuid, CertList->SignatureSize, CertData);
> DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed
> but signature is forbidden by DBX.\n"));
> goto Done;
> }
> --
> 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 9+ messages in thread