1. Correct the counter-based hash algorithm according to UEFI spec.

2. Check the reserverd bit in variable attribute.
3. Return EFI_OUT_OF_RESOURCE instead of EFI_SECURITY_VIOLATION if there is not enough speace to store the public key.
4. Fix a bug when deleting a non-existent time-based auth variable, we store the certificate into cert DB incorrectly.
5. Fix a bug that time-based auth variable can't been updated again after append operation.

Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
Reviewed-by: Ye Ting  <ting.ye@intel.com>
Reviewed-by: Dong Guo <guo.dong@intel.com>
git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@13957 6f19259b-4bc3-4df7-8a09-765794883524

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 6576e681c3b6ba812c15b3b0c377b4cb37864d2c..64ce968ac10859599e9508e00cedef814489e644 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -526,7 +526,9 @@ VerifyCounterBasedPayload (
   EFI_CERT_BLOCK_RSA_2048_SHA256  *CertBlock;\r
   UINT8                           Digest[SHA256_DIGEST_SIZE];\r
   VOID                            *Rsa;\r
-\r
+  UINTN                           PayloadSize;\r
+  \r
+  PayloadSize = DataSize - AUTHINFO_SIZE;\r
   Rsa         = NULL;\r
   CertData    = NULL;\r
   CertBlock   = NULL;\r
@@ -558,7 +560,14 @@ VerifyCounterBasedPayload (
   if (!Status) {\r
     goto Done;\r
   }\r
-  Status  = Sha256Update (mHashCtx, Data + AUTHINFO_SIZE, (UINTN) (DataSize - AUTHINFO_SIZE));\r
+  Status  = Sha256Update (mHashCtx, Data + AUTHINFO_SIZE, PayloadSize);\r
+  if (!Status) {\r
+    goto Done;\r
+  }\r
+  //\r
+  // Hash Size.\r
+  //\r
+  Status  = Sha256Update (mHashCtx, &PayloadSize, sizeof (UINTN));\r
   if (!Status) {\r
     goto Done;\r
   }\r
@@ -1099,6 +1108,7 @@ ProcessVarWithKek (
   @return EFI_INVALID_PARAMETER           Invalid parameter.\r
   @return EFI_WRITE_PROTECTED             Variable is write-protected and needs authentication with\r
                                           EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS set.\r
+  @return EFI_OUT_OF_RESOURCES            The Database to save the public key is full.\r
   @return EFI_SECURITY_VIOLATION          The variable is with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS\r
                                           set, but the AuthInfo does NOT pass the validation\r
                                           check carried out by the firmware.\r
@@ -1253,7 +1263,7 @@ ProcessVariable (
     //\r
     KeyIndex = AddPubKeyInStore (PubKey);\r
     if (KeyIndex == 0) {\r
-      return EFI_SECURITY_VIOLATION;\r
+      return EFI_OUT_OF_RESOURCES;\r
     }\r
   }\r
 \r
@@ -2155,13 +2165,13 @@ VerifyTimeBasedPayload (
     //\r
     // Delete signer's certificates when delete the common authenticated variable.\r
     //\r
-    if ((PayloadSize == 0) && (Variable->CurrPtr != NULL)) {\r
+    if ((PayloadSize == 0) && (Variable->CurrPtr != NULL) && ((Attributes & EFI_VARIABLE_APPEND_WRITE) == 0)) {\r
       Status = DeleteCertsFromDb (VariableName, VendorGuid);\r
       if (EFI_ERROR (Status)) {\r
         VerifyStatus = FALSE;\r
         goto Exit;\r
       }\r
-    } else if (Variable->CurrPtr == NULL) {\r
+    } else if (Variable->CurrPtr == NULL && PayloadSize != 0) {\r
       //\r
       // Insert signer's certificates when adding a new common authenticated variable.\r
       //\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
index 07fe99bee7e142bde76d258b8955a939ab641f19..e683783fa58ff4feaebb3c11d88c38011915b172 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.c
@@ -2280,6 +2280,13 @@ VariableServiceSetVariable (
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
+  //\r
+  // Check for reserverd bit in variable attribute.\r
+  //\r
+  if ((Attributes & (~EFI_VARIABLE_ATTRIBUTES_MASK)) != 0) {\r
+    return EFI_INVALID_PARAMETER;\r
+  }\r
+\r
   //\r
   //  Make sure if runtime bit is set, boot service bit is set also.\r
   //\r

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
index 14a0744a57ed39cdc174c253f5ef0839b0bf225e..563485f9293587f9493a56c800e5e9426d0e9ec6 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/Variable.h
@@ -43,6 +43,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Guid/HardwareErrorVariable.h>\r
 \r
 #define VARIABLE_RECLAIM_THRESHOLD (1024)\r
+#define EFI_VARIABLE_ATTRIBUTES_MASK (EFI_VARIABLE_NON_VOLATILE | \\r
+                                      EFI_VARIABLE_BOOTSERVICE_ACCESS | \\r
+                                      EFI_VARIABLE_RUNTIME_ACCESS | \\r
+                                      EFI_VARIABLE_HARDWARE_ERROR_RECORD | \\r
+                                      EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \\r
+                                      EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \\r
+                                      EFI_VARIABLE_APPEND_WRITE)\r
 \r
 ///\r
 /// The size of a 3 character ISO639 language code.\r