]> git.proxmox.com Git - mirror_edk2.git/blobdiff - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
SecurityPkg: Add constraints on PK strength
[mirror_edk2.git] / SecurityPkg / VariableAuthenticated / SecureBootConfigDxe / SecureBootConfigImpl.c
index d035763106852ef8b4dde3e363a272dc81a6f9f6..e82bfe77570de0960daa8fb2e0a226ac44cbf55a 100644 (file)
@@ -1,14 +1,9 @@
 /** @file\r
   HII Config Access protocol implementation of SecureBoot configuration module.\r
 \r
-Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.<BR>\r
-This program and the accompanying materials\r
-are licensed and made available under the terms and conditions of the BSD License\r
-which accompanies this distribution.  The full text of the license may be found at\r
-http://opensource.org/licenses/bsd-license.php\r
-\r
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,\r
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.\r
+Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>\r
+(C) Copyright 2018 Hewlett Packard Enterprise Development LP<BR>\r
+SPDX-License-Identifier: BSD-2-Clause-Patent\r
 \r
 **/\r
 \r
@@ -95,6 +90,22 @@ CHAR16* mDerEncodedSuffix[] = {
 };\r
 CHAR16* mSupportX509Suffix = L"*.cer/der/crt";\r
 \r
+//\r
+// Prompt strings during certificate enrollment.\r
+//\r
+CHAR16* mX509EnrollPromptTitle[] = {\r
+  L"",\r
+  L"ERROR: Unsupported file type!",\r
+  L"ERROR: Unsupported certificate!",\r
+  NULL\r
+};\r
+CHAR16* mX509EnrollPromptString[] = {\r
+  L"",\r
+  L"Only DER encoded certificate file (*.cer/der/crt) is supported.",\r
+  L"Public key length should be equal to or greater than 2048 bits.",\r
+  NULL\r
+};\r
+\r
 SECUREBOOT_CONFIG_PRIVATE_DATA  *gSecureBootPrivateData = NULL;\r
 \r
 /**\r
@@ -239,7 +250,7 @@ SaveSecureBootVariable (
                                    it's caller's responsibility to free the memory when finish using it.\r
 \r
   @retval EFI_SUCCESS              Create time based payload successfully.\r
-  @retval EFI_OUT_OF_RESOURCES     There are not enough memory resourses to create time based payload.\r
+  @retval EFI_OUT_OF_RESOURCES     There are not enough memory resources to create time based payload.\r
   @retval EFI_INVALID_PARAMETER    The parameter is invalid.\r
   @retval Others                   Unexpected error happens.\r
 \r
@@ -388,6 +399,102 @@ SetSecureBootMode (
                 );\r
 }\r
 \r
+/**\r
+  This code checks if the encode type and key strength of X.509\r
+  certificate is qualified.\r
+\r
+  @param[in]  X509FileContext     FileContext of X.509 certificate storing\r
+                                  file.\r
+  @param[out] Error               Error type checked in the certificate.\r
+\r
+  @return EFI_SUCCESS             The certificate checked successfully.\r
+  @return EFI_INVALID_PARAMETER   The parameter is invalid.\r
+  @return EFI_OUT_OF_RESOURCES    Memory allocation failed.\r
+\r
+**/\r
+EFI_STATUS\r
+CheckX509Certificate (\r
+  IN    SECUREBOOT_FILE_CONTEXT*    X509FileContext,\r
+  OUT   ENROLL_KEY_ERROR*           Error\r
+)\r
+{\r
+  EFI_STATUS     Status;\r
+  UINT16*        FilePostFix;\r
+  UINTN          NameLength;\r
+  UINT8*         X509Data;\r
+  UINTN          X509DataSize;\r
+  void*          X509PubKey;\r
+  UINTN          PubKeyModSize;\r
+\r
+  if (X509FileContext->FileName == NULL) {\r
+    *Error = Unsupported_Type;\r
+    return EFI_INVALID_PARAMETER;\r
+  }\r
+\r
+  X509Data       = NULL;\r
+  X509DataSize   = 0;\r
+  X509PubKey     = NULL;\r
+  PubKeyModSize  = 0;\r
+\r
+  //\r
+  // Parse the file's postfix. Only support DER encoded X.509 certificate files.\r
+  //\r
+  NameLength = StrLen (X509FileContext->FileName);\r
+  if (NameLength <= 4) {\r
+    DEBUG ((DEBUG_ERROR, "Wrong X509 NameLength\n"));\r
+    *Error = Unsupported_Type;\r
+    return EFI_INVALID_PARAMETER;\r
+  }\r
+  FilePostFix = X509FileContext->FileName + NameLength - 4;\r
+  if (!IsDerEncodeCertificate (FilePostFix)) {\r
+    DEBUG ((DEBUG_ERROR, "Unsupported file type, only DER encoded certificate (%s) is supported.\n", mSupportX509Suffix));\r
+    *Error = Unsupported_Type;\r
+    return EFI_INVALID_PARAMETER;\r
+  }\r
+  DEBUG ((DEBUG_INFO, "FileName= %s\n", X509FileContext->FileName));\r
+  DEBUG ((DEBUG_INFO, "FilePostFix = %s\n", FilePostFix));\r
+\r
+  //\r
+  // Read the certificate file content\r
+  //\r
+  Status = ReadFileContent (X509FileContext->FHandle, (VOID**) &X509Data, &X509DataSize, 0);\r
+  if (EFI_ERROR (Status)) {\r
+    DEBUG ((DEBUG_ERROR, "Error occured while reading the file.\n"));\r
+    goto ON_EXIT;\r
+  }\r
+\r
+  //\r
+  // Parse the public key context.\r
+  //\r
+  if (RsaGetPublicKeyFromX509 (X509Data, X509DataSize, &X509PubKey) == FALSE) {\r
+    DEBUG ((DEBUG_ERROR, "Error occured while parsing the pubkey from certificate.\n"));\r
+    Status = EFI_INVALID_PARAMETER;\r
+    *Error = Unsupported_Type;\r
+    goto ON_EXIT;\r
+  }\r
+\r
+  //\r
+  // Parse Module size of public key using interface provided by CryptoPkg, which is\r
+  // actually the size of public key.\r
+  //\r
+  if (X509PubKey != NULL) {\r
+    RsaGetKey (X509PubKey, RsaKeyN, NULL, &PubKeyModSize);\r
+    if (PubKeyModSize < CER_PUBKEY_MIN_SIZE) {\r
+      DEBUG ((DEBUG_ERROR, "Unqualified PK size, key size should be equal to or greater than 2048 bits.\n"));\r
+      Status = EFI_INVALID_PARAMETER;\r
+      *Error = Unqualified_Key;\r
+    }\r
+    RsaFree (X509PubKey);\r
+  }\r
+\r
+ ON_EXIT:\r
+  if (X509Data != NULL) {\r
+    FreePool (X509Data);\r
+  }\r
+\r
+  return Status;\r
+}\r
+\r
 /**\r
   Generate the PK signature list from the X509 Certificate storing file (.cer)\r
 \r
@@ -395,7 +502,7 @@ SetSecureBootMode (
   @param[out]  PkCert                Point to the data buffer to store the signature list.\r
 \r
   @return EFI_UNSUPPORTED            Unsupported Key Length.\r
-  @return EFI_OUT_OF_RESOURCES       There are not enough memory resourses to form the signature list.\r
+  @return EFI_OUT_OF_RESOURCES       There are not enough memory resources to form the signature list.\r
 \r
 **/\r
 EFI_STATUS\r
@@ -482,12 +589,6 @@ EnrollPlatformKey (
   UINT32                          Attr;\r
   UINTN                           DataSize;\r
   EFI_SIGNATURE_LIST              *PkCert;\r
-  UINT16*                         FilePostFix;\r
-  UINTN                           NameLength;\r
-\r
-  if (Private->FileContext->FileName == NULL) {\r
-    return EFI_INVALID_PARAMETER;\r
-  }\r
 \r
   PkCert = NULL;\r
 \r
@@ -497,22 +598,7 @@ EnrollPlatformKey (
   }\r
 \r
   //\r
-  // Parse the file's postfix. Only support DER encoded X.509 certificate files.\r
-  //\r
-  NameLength = StrLen (Private->FileContext->FileName);\r
-  if (NameLength <= 4) {\r
-    return EFI_INVALID_PARAMETER;\r
-  }\r
-  FilePostFix = Private->FileContext->FileName + NameLength - 4;\r
-  if (!IsDerEncodeCertificate(FilePostFix)) {\r
-    DEBUG ((EFI_D_ERROR, "Unsupported file type, only DER encoded certificate (%s) is supported.", mSupportX509Suffix));\r
-    return EFI_INVALID_PARAMETER;\r
-  }\r
-  DEBUG ((EFI_D_INFO, "FileName= %s\n", Private->FileContext->FileName));\r
-  DEBUG ((EFI_D_INFO, "FilePostFix = %s\n", FilePostFix));\r
-\r
-  //\r
-  // Prase the selected PK file and generature PK certificate list.\r
+  // Prase the selected PK file and generate PK certificate list.\r
   //\r
   Status = CreatePkX509SignatureList (\r
             Private->FileContext->FHandle,\r
@@ -1093,7 +1179,7 @@ IsSignatureFoundInDatabase (
   }\r
 \r
   //\r
-  // Enumerate all signature data in SigDB to check if executable's signature exists.\r
+  // Enumerate all signature data in SigDB to check if signature exists for executable.\r
   //\r
   CertList = (EFI_SIGNATURE_LIST *) Data;\r
   while ((DataSize > 0) && (DataSize >= CertList->SignatureListSize)) {\r
@@ -1317,7 +1403,7 @@ Done:
 /**\r
   Check whether the signature list exists in given variable data.\r
 \r
-  It searches the signature list for the ceritificate hash by CertType.\r
+  It searches the signature list for the certificate hash by CertType.\r
   If the signature list is found, get the offset of Database for the\r
   next hash of a certificate.\r
 \r
@@ -1815,7 +1901,7 @@ LoadPeImage (
   Calculate hash of Pe/Coff image based on the authenticode image hashing in\r
   PE/COFF Specification 8.0 Appendix A\r
 \r
-  Notes: PE/COFF image has been checked by BasePeCoffLib PeCoffLoaderGetImageInfo() in \r
+  Notes: PE/COFF image has been checked by BasePeCoffLib PeCoffLoaderGetImageInfo() in\r
   the function LoadPeImage ().\r
 \r
   @param[in]    HashAlg   Hash algorithm type.\r
@@ -1830,7 +1916,6 @@ HashPeImage (
   )\r
 {\r
   BOOLEAN                   Status;\r
-  UINT16                    Magic;\r
   EFI_IMAGE_SECTION_HEADER  *Section;\r
   VOID                      *HashCtx;\r
   UINTN                     CtxSize;\r
@@ -1873,27 +1958,13 @@ HashPeImage (
   // Measuring PE/COFF Image Header;\r
   // But CheckSum field and SECURITY data directory (certificate) are excluded\r
   //\r
-  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
-    //\r
-    // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic value\r
-    //       in the PE/COFF Header. If the MachineType is Itanium(IA64) and the\r
-    //       Magic value in the OptionalHeader is EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC\r
-    //       then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC\r
-    //\r
-    Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;\r
-  } else {\r
-    //\r
-    // Get the magic value from the PE/COFF Optional Header\r
-    //\r
-    Magic = mNtHeader.Pe32->OptionalHeader.Magic;\r
-  }\r
 \r
   //\r
   // 3.  Calculate the distance from the base of the image header to the image checksum address.\r
   // 4.  Hash the image header from its base to beginning of the image checksum.\r
   //\r
   HashBase = mImageBase;\r
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
+  if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
     //\r
     // Use PE32 offset.\r
     //\r
@@ -1914,7 +1985,7 @@ HashPeImage (
   // 6.  Get the address of the beginning of the Cert Directory.\r
   // 7.  Hash everything from the end of the checksum to the start of the Cert Directory.\r
   //\r
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
+  if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
     //\r
     // Use PE32 offset.\r
     //\r
@@ -1936,7 +2007,7 @@ HashPeImage (
   // 8.  Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) bytes.)\r
   // 9.  Hash everything from the end of the Cert Directory to the end of image header.\r
   //\r
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
+  if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
     //\r
     // Use PE32 offset\r
     //\r
@@ -1957,7 +2028,7 @@ HashPeImage (
   //\r
   // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.\r
   //\r
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
+  if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
     //\r
     // Use PE32 offset.\r
     //\r
@@ -2031,7 +2102,7 @@ HashPeImage (
   //\r
   if (mImageSize > SumOfBytesHashed) {\r
     HashBase = mImageBase + SumOfBytesHashed;\r
-    if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
+    if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {\r
       //\r
       // Use PE32 offset.\r
       //\r
@@ -2127,7 +2198,7 @@ HashPeImageByType (
 }\r
 \r
 /**\r
-  Enroll a new executable's signature into Signature Database.\r
+  Enroll a new signature of executable into Signature Database.\r
 \r
   @param[in] PrivateData     The module's private data.\r
   @param[in] VariableName    Variable name of signature database, must be\r
@@ -2197,7 +2268,7 @@ EnrollAuthentication2Descriptor (
   }\r
 \r
   //\r
-  // Diretly set AUTHENTICATION_2 data to SetVariable\r
+  // Directly set AUTHENTICATION_2 data to SetVariable\r
   //\r
   Status = gRT->SetVariable(\r
                   VariableName,\r
@@ -2228,7 +2299,7 @@ ON_EXIT:
 \r
 \r
 /**\r
-  Enroll a new executable's signature into Signature Database.\r
+  Enroll a new signature of executable into Signature Database.\r
 \r
   @param[in] PrivateData     The module's private data.\r
   @param[in] VariableName    Variable name of signature database, must be\r
@@ -2267,7 +2338,7 @@ EnrollImageSignatureToSigDB (
   // Form the SigDB certificate list.\r
   // Format the data item into EFI_SIGNATURE_LIST type.\r
   //\r
-  // We need to parse executable's signature data from specified signed executable file.\r
+  // We need to parse signature data of executable from specified signed executable file.\r
   // In current implementation, we simply trust the pass-in signed executable file.\r
   // In reality, it's OS's responsibility to verify the signed executable file.\r
   //\r
@@ -3145,6 +3216,9 @@ DeleteSignatureEx (
   if (DelType == Delete_Signature_List_All) {\r
     VariableDataSize = 0;\r
   } else {\r
+    //\r
+    //  Traverse to target EFI_SIGNATURE_LIST but others will be skipped.\r
+    //\r
     while ((RemainingSize > 0) && (RemainingSize >= ListWalker->SignatureListSize) && ListIndex < PrivateData->ListIndex) {\r
       CopyMem ((UINT8 *)NewVariableData + Offset, ListWalker, ListWalker->SignatureListSize);\r
       Offset += ListWalker->SignatureListSize;\r
@@ -3154,15 +3228,17 @@ DeleteSignatureEx (
       ListIndex++;\r
     }\r
 \r
-    if (CheckedCount == SIGNATURE_DATA_COUNTS (ListWalker) || DelType == Delete_Signature_List_One) {\r
-      RemainingSize -= ListWalker->SignatureListSize;\r
-      ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);\r
-    } else {\r
+    //\r
+    //  Handle the target EFI_SIGNATURE_LIST.\r
+    //  If CheckedCount == SIGNATURE_DATA_COUNTS (ListWalker) or DelType == Delete_Signature_List_One\r
+    //  it means delete the whole EFI_SIGNATURE_LIST, So we just skip this EFI_SIGNATURE_LIST.\r
+    //\r
+    if (CheckedCount < SIGNATURE_DATA_COUNTS (ListWalker) && DelType == Delete_Signature_Data) {\r
       NewCertList = (EFI_SIGNATURE_LIST *)(NewVariableData + Offset);\r
       //\r
       // Copy header.\r
       //\r
-      CopyMem ((UINT8 *)NewVariableData, ListWalker, sizeof (EFI_SIGNATURE_LIST) + ListWalker->SignatureHeaderSize);\r
+      CopyMem ((UINT8 *)NewVariableData + Offset, ListWalker, sizeof (EFI_SIGNATURE_LIST) + ListWalker->SignatureHeaderSize);\r
       Offset += sizeof (EFI_SIGNATURE_LIST) + ListWalker->SignatureHeaderSize;\r
 \r
       DataWalker = (EFI_SIGNATURE_DATA *)((UINT8 *)ListWalker + sizeof(EFI_SIGNATURE_LIST) + ListWalker->SignatureHeaderSize);\r
@@ -3181,10 +3257,11 @@ DeleteSignatureEx (
         }\r
         DataWalker = (EFI_SIGNATURE_DATA *)((UINT8 *)DataWalker + ListWalker->SignatureSize);\r
       }\r
-\r
-      RemainingSize -= ListWalker->SignatureListSize;\r
     }\r
 \r
+    RemainingSize -= ListWalker->SignatureListSize;\r
+    ListWalker = (EFI_SIGNATURE_LIST *)((UINT8 *)ListWalker + ListWalker->SignatureListSize);\r
+\r
     //\r
     // Copy remaining data, maybe 0.\r
     //\r
@@ -3283,7 +3360,7 @@ SecureBootExtractConfigFromVariable (
   SecureBootMode   = NULL;\r
 \r
   //\r
-  // Initilize the Date and Time using system time.\r
+  // Initialize the Date and Time using system time.\r
   //\r
   ConfigData->CertificateFormat = HASHALG_RAW;\r
   ConfigData->AlwaysRevocation = TRUE;\r
@@ -3320,15 +3397,15 @@ SecureBootExtractConfigFromVariable (
   }\r
 \r
   //\r
-  // Check SecureBootEnable & Pk status, fix the inconsistence. \r
+  // Check SecureBootEnable & Pk status, fix the inconsistency.\r
   // If the SecureBootEnable Variable doesn't exist, hide the SecureBoot Enable/Disable\r
   // Checkbox.\r
   //\r
   ConfigData->AttemptSecureBoot = FALSE;\r
-  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);  \r
+  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);\r
 \r
   //\r
-  // Fix Pk, SecureBootEnable inconsistence\r
+  // Fix Pk and SecureBootEnable inconsistency\r
   //\r
   if ((SetupMode != NULL) && (*SetupMode) == USER_MODE) {\r
     ConfigData->HideSecureBoot = FALSE;\r
@@ -4085,7 +4162,7 @@ ON_EXIT:
 }\r
 \r
 /**\r
-  This functino to load signature data under the signature list.\r
+  This function to load signature data under the signature list.\r
 \r
   @param[in]  PrivateData         Module's private data.\r
   @param[in]  LabelId             Label number to insert opcodes.\r
@@ -4313,12 +4390,15 @@ SecureBootCallback (
   UINTN                           NameLength;\r
   UINT16                          *FilePostFix;\r
   SECUREBOOT_CONFIG_PRIVATE_DATA  *PrivateData;\r
+  BOOLEAN                         GetBrowserDataResult;\r
+  ENROLL_KEY_ERROR                EnrollKeyErrorCode;\r
 \r
-  Status           = EFI_SUCCESS;\r
-  SecureBootEnable = NULL;\r
-  SecureBootMode   = NULL;\r
-  SetupMode        = NULL;\r
-  File             = NULL;\r
+  Status             = EFI_SUCCESS;\r
+  SecureBootEnable   = NULL;\r
+  SecureBootMode     = NULL;\r
+  SetupMode          = NULL;\r
+  File               = NULL;\r
+  EnrollKeyErrorCode = None_Error;\r
 \r
   if ((This == NULL) || (Value == NULL) || (ActionRequest == NULL)) {\r
     return EFI_INVALID_PARAMETER;\r
@@ -4337,7 +4417,7 @@ SecureBootCallback (
     return EFI_OUT_OF_RESOURCES;\r
   }\r
 \r
-  HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *) IfrNvData);\r
+  GetBrowserDataResult = HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *) IfrNvData);\r
 \r
   if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {\r
     if (QuestionId == KEY_SECURE_BOOT_MODE) {\r
@@ -4377,7 +4457,7 @@ SecureBootCallback (
         Value->u8 = SECURE_BOOT_MODE_STANDARD;\r
         Status = EFI_SUCCESS;\r
       }\r
-    } \r
+    }\r
     goto EXIT;\r
   }\r
 \r
@@ -4731,18 +4811,35 @@ SecureBootCallback (
       }\r
       break;\r
     case KEY_VALUE_SAVE_AND_EXIT_PK:\r
-      Status = EnrollPlatformKey (Private);\r
+      //\r
+      // Check the suffix, encode type and the key strength of PK certificate.\r
+      //\r
+      Status = CheckX509Certificate (Private->FileContext, &EnrollKeyErrorCode);\r
+      if (EFI_ERROR (Status)) {\r
+        if (EnrollKeyErrorCode != None_Error && EnrollKeyErrorCode < Enroll_Error_Max) {\r
+          CreatePopUp (\r
+            EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,\r
+            &Key,\r
+            mX509EnrollPromptTitle[EnrollKeyErrorCode],\r
+            mX509EnrollPromptString[EnrollKeyErrorCode],\r
+            NULL\r
+            );\r
+          break;\r
+        }\r
+      } else {\r
+        Status = EnrollPlatformKey (Private);\r
+      }\r
       if (EFI_ERROR (Status)) {\r
         UnicodeSPrint (\r
           PromptString,\r
           sizeof (PromptString),\r
-          L"Only DER encoded certificate file (%s) is supported.",\r
-          mSupportX509Suffix\r
+          L"Error status: %x.",\r
+          Status\r
           );\r
         CreatePopUp (\r
           EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,\r
           &Key,\r
-          L"ERROR: Unsupported file type!",\r
+          L"ERROR: Enrollment failed!",\r
           PromptString,\r
           NULL\r
           );\r
@@ -4883,7 +4980,7 @@ SecureBootCallback (
 \r
 EXIT:\r
 \r
-  if (!EFI_ERROR (Status)) {\r
+  if (!EFI_ERROR (Status) && GetBrowserDataResult) {\r
     BufferSize = sizeof (SECUREBOOT_CONFIGURATION);\r
     HiiSetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8*) IfrNvData, NULL);\r
   }\r