Re: [edk2] [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in AuthVariableLib

2016-06-27 Thread Zeng, Star 

Reviewed-by: Star Zeng 

On 2016/6/27 15:12, Yao, Jiewen wrote:

Reviewed-by: jiewen@intel.com


-Original Message-
From: Zhang, Chao B
Sent: Monday, June 27, 2016 3:06 PM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen ; Zeng, Star ;
Zhang, Chao B 
Subject: [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in
AuthVariableLib

AuthVariableLib is updated to cache the UserPhysicalPresent state to global
variable. This avoids calling PlatformSecureLib during runtime and makes
PhysicalPresent state consistent during one boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 
 SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h | 1 +
 SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 7 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 6e1e284..1d49b6a 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -931,7 +931,7 @@ ProcessVarWithPk (
   // Init state of Del. State may change due to secure check
   //
   Del = FALSE;
-  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode ==
SETUP_MODE && !IsPk)) {
+  if ((InCustomMode() && mUserPhysicalPresent) || (mPlatformMode ==
SETUP_MODE && !IsPk)) {
 Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
 PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
 if (PayloadSize == 0) {
@@ -1049,7 +1049,7 @@ ProcessVarWithKek (
   }

   Status = EFI_SUCCESS;
-  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
UserPhysicalPresent())) {
+  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
mUserPhysicalPresent)) {
 //
 // Time-based, verify against X509 Cert KEK.
 //
@@ -1204,7 +1204,7 @@ ProcessVariable (
  
  );

-  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
UserPhysicalPresent()) {
+  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
mUserPhysicalPresent) {
 //
 // Allow the delete operation of common authenticated variable at
user physical presence.
 //
@@ -1222,7 +1222,7 @@ ProcessVariable (
 return Status;
   }

-  if (NeedPhysicallyPresent (VariableName, VendorGuid)
&& !UserPhysicalPresent()) {
+  if (NeedPhysicallyPresent (VariableName, VendorGuid)
&& !mUserPhysicalPresent) {
 //
 // This variable is protected, only physical present user could modify its
value.
 //
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..ac7ea89 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -128,6 +128,7 @@ extern UINT8*mCertDbStore;
 extern UINT32   mMaxCertDbSize;
 extern UINT32   mPlatformMode;
 extern UINT8mVendorKeyState;
+extern BOOLEAN  mUserPhysicalPresent;

 extern VOID *mHashCtx;

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index c4fbb64..dd35a44 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
@@ -35,6 +35,7 @@ UINT8*mCertDbStore;
 UINT32   mMaxCertDbSize;
 UINT32   mPlatformMode;
 UINT8mVendorKeyState;
+BOOLEAN  mUserPhysicalPresent;

 EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID,
EFI_CERT_SHA256_GUID, EFI_CERT_RSA2048_GUID,
EFI_CERT_X509_GUID};

@@ -435,6 +436,12 @@ AuthVariableLibInitialize (
   AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
   AuthVarLibContextOut->AddressPointerCount = sizeof
(mAuthVarAddressPointer) / sizeof (mAuthVarAddressPointer[0]);

+  //
+  // Cache UserPhysicalPresent State.
+  // Platform should report PhysicalPresent before this point
+  //
+  mUserPhysicalPresent = UserPhysicalPresent();
+
   return Status;
 }

--
1.9.5.msysgit.1

f
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in AuthVariableLib

2016-06-27 Thread Yao, Jiewen 
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zhang, Chao B
> Sent: Monday, June 27, 2016 3:06 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star ;
> Zhang, Chao B 
> Subject: [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in
> AuthVariableLib
> 
> AuthVariableLib is updated to cache the UserPhysicalPresent state to global
> variable. This avoids calling PlatformSecureLib during runtime and makes
> PhysicalPresent state consistent during one boot.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 
>  SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h | 1 +
>  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 7 +++
>  3 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> index 6e1e284..1d49b6a 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> @@ -931,7 +931,7 @@ ProcessVarWithPk (
>// Init state of Del. State may change due to secure check
>//
>Del = FALSE;
> -  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode ==
> SETUP_MODE && !IsPk)) {
> +  if ((InCustomMode() && mUserPhysicalPresent) || (mPlatformMode ==
> SETUP_MODE && !IsPk)) {
>  Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
>  PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
>  if (PayloadSize == 0) {
> @@ -1049,7 +1049,7 @@ ProcessVarWithKek (
>}
> 
>Status = EFI_SUCCESS;
> -  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
> UserPhysicalPresent())) {
> +  if (mPlatformMode == USER_MODE && !(InCustomMode() &&
> mUserPhysicalPresent)) {
>  //
>  // Time-based, verify against X509 Cert KEK.
>  //
> @@ -1204,7 +1204,7 @@ ProcessVariable (
>   
>   );
> 
> -  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
> (OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
> UserPhysicalPresent()) {
> +  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable
> (OrgVariableInfo.Attributes, Data, DataSize, Attributes) &&
> mUserPhysicalPresent) {
>  //
>  // Allow the delete operation of common authenticated variable at
> user physical presence.
>  //
> @@ -1222,7 +1222,7 @@ ProcessVariable (
>  return Status;
>}
> 
> -  if (NeedPhysicallyPresent (VariableName, VendorGuid)
> && !UserPhysicalPresent()) {
> +  if (NeedPhysicallyPresent (VariableName, VendorGuid)
> && !mUserPhysicalPresent) {
>  //
>  // This variable is protected, only physical present user could modify 
> its
> value.
>  //
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> index e7c4bf0..ac7ea89 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
> @@ -128,6 +128,7 @@ extern UINT8*mCertDbStore;
>  extern UINT32   mMaxCertDbSize;
>  extern UINT32   mPlatformMode;
>  extern UINT8mVendorKeyState;
> +extern BOOLEAN  mUserPhysicalPresent;
> 
>  extern VOID *mHashCtx;
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> index c4fbb64..dd35a44 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
> @@ -35,6 +35,7 @@ UINT8*mCertDbStore;
>  UINT32   mMaxCertDbSize;
>  UINT32   mPlatformMode;
>  UINT8mVendorKeyState;
> +BOOLEAN  mUserPhysicalPresent;
> 
>  EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID,
> EFI_CERT_SHA256_GUID, EFI_CERT_RSA2048_GUID,
> EFI_CERT_X509_GUID};
> 
> @@ -435,6 +436,12 @@ AuthVariableLibInitialize (
>AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
>AuthVarLibContextOut->AddressPointerCount = sizeof
> (mAuthVarAddressPointer) / sizeof (mAuthVarAddressPointer[0]);
> 
> +  //
> +  // Cache UserPhysicalPresent State.
> +  // Platform should report PhysicalPresent before this point
> +  //
> +  mUserPhysicalPresent = UserPhysicalPresent();
> +
>return Status;
>  }
> 
> --
> 1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] SecurityPkg: AuthVariableLib: Cache UserPhysicalPresent in AuthVariableLib

2016-06-27 Thread Zhang, Chao B 
AuthVariableLib is updated to cache the UserPhysicalPresent state to global 
variable. This avoids calling PlatformSecureLib during runtime and makes 
PhysicalPresent state consistent during one boot.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 SecurityPkg/Library/AuthVariableLib/AuthService.c | 8 
 SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h | 1 +
 SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c | 7 +++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c 
b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index 6e1e284..1d49b6a 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -931,7 +931,7 @@ ProcessVarWithPk (
   // Init state of Del. State may change due to secure check
   //
   Del = FALSE;
-  if ((InCustomMode() && UserPhysicalPresent()) || (mPlatformMode == 
SETUP_MODE && !IsPk)) {
+  if ((InCustomMode() && mUserPhysicalPresent) || (mPlatformMode == SETUP_MODE 
&& !IsPk)) {
 Payload = (UINT8 *) Data + AUTHINFO2_SIZE (Data);
 PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
 if (PayloadSize == 0) {
@@ -1049,7 +1049,7 @@ ProcessVarWithKek (
   }
 
   Status = EFI_SUCCESS;
-  if (mPlatformMode == USER_MODE && !(InCustomMode() && 
UserPhysicalPresent())) {
+  if (mPlatformMode == USER_MODE && !(InCustomMode() && mUserPhysicalPresent)) 
{
 //
 // Time-based, verify against X509 Cert KEK.
 //
@@ -1204,7 +1204,7 @@ ProcessVariable (
  
  );
 
-  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable 
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) && 
UserPhysicalPresent()) {
+  if ((!EFI_ERROR (Status)) && IsDeleteAuthVariable 
(OrgVariableInfo.Attributes, Data, DataSize, Attributes) && 
mUserPhysicalPresent) {
 //
 // Allow the delete operation of common authenticated variable at user 
physical presence.
 //
@@ -1222,7 +1222,7 @@ ProcessVariable (
 return Status;
   }
 
-  if (NeedPhysicallyPresent (VariableName, VendorGuid) && 
!UserPhysicalPresent()) {
+  if (NeedPhysicallyPresent (VariableName, VendorGuid) && 
!mUserPhysicalPresent) {
 //
 // This variable is protected, only physical present user could modify its 
value.
 //
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h 
b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
index e7c4bf0..ac7ea89 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
+++ b/SecurityPkg/Library/AuthVariableLib/AuthServiceInternal.h
@@ -128,6 +128,7 @@ extern UINT8*mCertDbStore;
 extern UINT32   mMaxCertDbSize;
 extern UINT32   mPlatformMode;
 extern UINT8mVendorKeyState;
+extern BOOLEAN  mUserPhysicalPresent;
 
 extern VOID *mHashCtx;
 
diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c 
b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
index c4fbb64..dd35a44 100644
--- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.c
@@ -35,6 +35,7 @@ UINT8*mCertDbStore;
 UINT32   mMaxCertDbSize;
 UINT32   mPlatformMode;
 UINT8mVendorKeyState;
+BOOLEAN  mUserPhysicalPresent;
 
 EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID, EFI_CERT_SHA256_GUID, 
EFI_CERT_RSA2048_GUID, EFI_CERT_X509_GUID};
 
@@ -435,6 +436,12 @@ AuthVariableLibInitialize (
   AuthVarLibContextOut->AddressPointer = mAuthVarAddressPointer;
   AuthVarLibContextOut->AddressPointerCount = sizeof (mAuthVarAddressPointer) 
/ sizeof (mAuthVarAddressPointer[0]);
 
+  //
+  // Cache UserPhysicalPresent State. 
+  // Platform should report PhysicalPresent before this point
+  //
+  mUserPhysicalPresent = UserPhysicalPresent();
+
   return Status;
 }
 
-- 
1.9.5.msysgit.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel