Add comments to clarify mPubKeyStore buffer MemCopy. There is no memory overflow...

[mirror_edk2.git] / SecurityPkg / VariableAuthenticated / RuntimeDxe / AuthService.c

diff --git a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
index 07a41b06f8ad01a52d5e232d63cc4357d0986d66..566c5e29a7866cc4248f01fc56df13fed3e08816 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
+++ b/SecurityPkg/VariableAuthenticated/RuntimeDxe/AuthService.c
@@ -7,6 +7,10 @@
   This external input must be validated carefully to avoid security issue like\r
   buffer overflow, integer overflow.\r
   Variable attribute should also be checked to avoid authentication bypass.\r
+     The whole SMM authentication variable design relies on the integrity of flash part and SMM.\r
+  which is assumed to be protected by platform.  All variable code and metadata in flash/SMM Memory\r
+  may not be modified without authorization. If platform fails to protect these resources, \r
+  the authentication service provided in this driver will be broken, and the behavior is undefined.\r
 \r
   ProcessVarWithPk(), ProcessVarWithKek() and ProcessVariable() are the function to do\r
   variable authentication.\r
@@ -32,9 +36,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 ///\r
 /// Global database array for scratch\r
 ///\r
-UINT8    mPubKeyStore[MAX_KEYDB_SIZE];\r
+UINT8    *mPubKeyStore;\r
 UINT32   mPubKeyNumber;\r
-UINT8    mCertDbStore[MAX_CERTDB_SIZE];\r
+UINT32   mMaxKeyNumber;\r
+UINT32   mMaxKeyDbSize;\r
+UINT8    *mCertDbStore;\r
+UINT32   mMaxCertDbSize;\r
 UINT32   mPlatformMode;\r
 UINT8    mVendorKeyState;\r
 \r
@@ -183,6 +190,25 @@ AutenticatedVariableServiceInitialize (
     return EFI_OUT_OF_RESOURCES;\r
   }\r
 \r
+  //\r
+  // Reserve runtime buffer for public key database. The size excludes variable header and name size.\r
+  //\r
+  mMaxKeyDbSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - sizeof (AUTHVAR_KEYDB_NAME);\r
+  mMaxKeyNumber = mMaxKeyDbSize / EFI_CERT_TYPE_RSA2048_SIZE;\r
+  mPubKeyStore  = AllocateRuntimePool (mMaxKeyDbSize);\r
+  if (mPubKeyStore == NULL) {\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+\r
+  //\r
+  // Reserve runtime buffer for certificate database. The size excludes variable header and name size.\r
+  //\r
+  mMaxCertDbSize = PcdGet32 (PcdMaxVariableSize) - sizeof (VARIABLE_HEADER) - sizeof (EFI_CERT_DB_NAME);\r
+  mCertDbStore   = AllocateRuntimePool (mMaxCertDbSize);\r
+  if (mCertDbStore == NULL) {\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+\r
   //\r
   // Prepare runtime buffer for serialized data of time-based authenticated\r
   // Variable, i.e. (VariableName, VendorGuid, Attributes, TimeStamp, Data).\r
@@ -229,6 +255,10 @@ AutenticatedVariableServiceInitialize (
     DataSize  = DataSizeOfVariable (Variable.CurrPtr);\r
     Data      = GetVariableDataPtr (Variable.CurrPtr);\r
     ASSERT ((DataSize != 0) && (Data != NULL));\r
+    //\r
+    // "AuthVarKeyDatabase" is an internal variable. Its DataSize is always ensured not to exceed mPubKeyStore buffer size(See definition before) \r
+    //  Therefore, there is no memory overflow in underlying CopyMem.\r
+    //\r
     CopyMem (mPubKeyStore, (UINT8 *) Data, DataSize);\r
     mPubKeyNumber = (UINT32) (DataSize / EFI_CERT_TYPE_RSA2048_SIZE);\r
   }\r
@@ -503,7 +533,7 @@ AddPubKeyInStore (
     //\r
     // Add public key in database.\r
     //\r
-    if (mPubKeyNumber == MAX_KEY_NUM) {\r
+    if (mPubKeyNumber == mMaxKeyNumber) {\r
       //\r
       // Public key dadatase is full, try to reclaim invalid key.\r
       //\r
@@ -542,10 +572,14 @@ AddPubKeyInStore (
       DataSize  = DataSizeOfVariable (Variable.CurrPtr);\r
       Data      = GetVariableDataPtr (Variable.CurrPtr);\r
       ASSERT ((DataSize != 0) && (Data != NULL));\r
+      //\r
+      // "AuthVarKeyDatabase" is an internal used variable. Its DataSize is always ensured not to exceed mPubKeyStore buffer size(See definition before) \r
+      //  Therefore, there is no memory overflow in underlying CopyMem.\r
+      //\r
       CopyMem (mPubKeyStore, (UINT8 *) Data, DataSize);\r
       mPubKeyNumber = (UINT32) (DataSize / EFI_CERT_TYPE_RSA2048_SIZE);\r
 \r
-      if (mPubKeyNumber == MAX_KEY_NUM) {\r
+      if (mPubKeyNumber == mMaxKeyNumber) {\r
         return 0;\r
       }     \r
     }\r
@@ -1388,9 +1422,11 @@ ProcessVariable (
 \r
   if (!IsFirstTime) {\r
     //\r
-    // Check input PubKey.\r
+    // 2 cases need to check here\r
+    //   1. Internal PubKey variable. PubKeyIndex is always 0 \r
+    //   2. Other counter-based AuthVariable. Check input PubKey.\r
     //\r
-    if (CompareMem (PubKey, mPubKeyStore + (KeyIndex - 1) * EFI_CERT_TYPE_RSA2048_SIZE, EFI_CERT_TYPE_RSA2048_SIZE) != 0) {\r
+    if (KeyIndex == 0 || CompareMem (PubKey, mPubKeyStore + (KeyIndex - 1) * EFI_CERT_TYPE_RSA2048_SIZE, EFI_CERT_TYPE_RSA2048_SIZE) != 0) {\r
       return EFI_SECURITY_VIOLATION;\r
     }\r
     //\r
@@ -1996,7 +2032,7 @@ InsertCertsToDb (
   NameSize      = (UINT32) StrLen (VariableName);\r
   CertNodeSize  = sizeof (AUTH_CERT_DB_DATA) + (UINT32) CertDataSize + NameSize * sizeof (CHAR16); \r
   NewCertDbSize = (UINT32) DataSize + CertNodeSize;\r
-  if (NewCertDbSize > MAX_CERTDB_SIZE) {\r
+  if (NewCertDbSize > mMaxCertDbSize) {\r
     return EFI_OUT_OF_RESOURCES;\r
   }\r
   NewCertDb     = (UINT8*) mCertDbStore;\r
@@ -2122,6 +2158,7 @@ VerifyTimeBasedPayload (
   WrapSigData            = NULL;\r
   SignerCerts            = NULL;\r
   RootCert               = NULL;\r
+  CertsInCertDb          = NULL;\r
 \r
   //\r
   // When the attribute EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS is\r