X-Git-Url: https://git.proxmox.com/?p=mirror_edk2.git;a=blobdiff_plain;f=SecurityPkg%2FVariableAuthenticated%2FSecureBootConfigDxe%2FSecureBootConfigImpl.c;h=2eaf24633db15d880c95c23ae493b12ed551de49;hp=3f80441602226c6616af88d0a0bbe31427a0c524;hb=9d9b8b77bc54718a4debfea3a7a3dd59bba16840;hpb=070827be5a5727c736acf5d645722be073640952

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 3f80441602..2eaf24633d 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -1,7 +1,7 @@
 /** @file
   HII Config Access protocol implementation of SecureBoot configuration module.
 
-Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2011 - 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
@@ -63,7 +63,6 @@ UINT8 mHashOidValue[] = {
   };
 
 HASH_TABLE mHash[] = {
-  { L"SHA1",   20, &mHashOidValue[8],  5, Sha1GetContextSize,   Sha1Init,   Sha1Update,   Sha1Final  },
   { L"SHA224", 28, &mHashOidValue[13], 9, NULL,                 NULL,       NULL,         NULL       },
   { L"SHA256", 32, &mHashOidValue[22], 9, Sha256GetContextSize, Sha256Init, Sha256Update, Sha256Final},
   { L"SHA384", 48, &mHashOidValue[31], 9, Sha384GetContextSize, Sha384Init, Sha384Update, Sha384Final},
@@ -97,6 +96,31 @@ CHAR16* mSupportX509Suffix = L"*.cer/der/crt";
 
 SECUREBOOT_CONFIG_PRIVATE_DATA  *gSecureBootPrivateData = NULL;
 
+/**
+  This code cleans up enrolled file by closing file & free related resources attached to
+  enrolled file.
+
+  @param[in] FileContext            FileContext cached in SecureBootConfig driver
+
+**/
+VOID
+CloseEnrolledFile(
+  IN SECUREBOOT_FILE_CONTEXT *FileContext
+)
+{
+  if (FileContext->FHandle != NULL) {
+    CloseFile (FileContext->FHandle);
+    FileContext->FHandle = NULL;
+  }
+
+  if (FileContext->FileName != NULL){
+    FreePool(FileContext->FileName);
+    FileContext->FileName = NULL;
+  }
+  FileContext->FileType = UNKNOWN_FILE_TYPE;
+
+}
+
 /**
   This code checks if the FileSuffix is one of the possible DER-encoded certificate suffix.
 
@@ -120,6 +144,61 @@ IsDerEncodeCertificate (
   return FALSE;
 }
 
+/**
+  This code checks if the file content complies with EFI_VARIABLE_AUTHENTICATION_2 format
+The function reads file content but won't open/close given FileHandle.
+
+  @param[in] FileHandle            The FileHandle to be checked
+
+  @retval    TRUE            The content is EFI_VARIABLE_AUTHENTICATION_2 format.
+  @retval    FALSE          The content is NOT a EFI_VARIABLE_AUTHENTICATION_2 format.
+
+**/
+BOOLEAN
+IsAuthentication2Format (
+  IN   EFI_FILE_HANDLE    FileHandle
+)
+{
+  EFI_STATUS                     Status;
+  EFI_VARIABLE_AUTHENTICATION_2  *Auth2;
+  BOOLEAN                        IsAuth2Format;
+
+  IsAuth2Format = FALSE;
+
+  //
+  // Read the whole file content
+  //
+  Status = ReadFileContent(
+             FileHandle,
+             (VOID **) &mImageBase,
+             &mImageSize,
+             0
+             );
+  if (EFI_ERROR (Status)) {
+    goto ON_EXIT;
+  }
+
+  Auth2 = (EFI_VARIABLE_AUTHENTICATION_2 *)mImageBase;
+  if (Auth2->AuthInfo.Hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID) {
+    goto ON_EXIT;
+  }
+
+  if (CompareGuid(&gEfiCertPkcs7Guid, &Auth2->AuthInfo.CertType)) {
+    IsAuth2Format = TRUE;
+  }
+
+ON_EXIT:
+  //
+  // Do not close File. simply check file content
+  //
+  if (mImageBase != NULL) {
+    FreePool (mImageBase);
+    mImageBase = NULL;
+  }
+
+  return IsAuth2Format;
+}
+
 /**
   Set Secure Boot option into variable space.
 
@@ -475,10 +554,7 @@ ON_EXIT:
     FreePool(PkCert);
   }
 
-  if (Private->FileContext->FHandle != NULL) {
-    CloseFile (Private->FileContext->FHandle);
-    Private->FileContext->FHandle = NULL;
-  }
+  CloseEnrolledFile(Private->FileContext);
 
   return Status;
 }
@@ -655,13 +731,7 @@ EnrollRsa2048ToKek (
 
 ON_EXIT:
 
-  CloseFile (Private->FileContext->FHandle);
-  Private->FileContext->FHandle = NULL;
-
-  if (Private->FileContext->FileName != NULL){
-    FreePool(Private->FileContext->FileName);
-    Private->FileContext->FileName = NULL;
-  }
+  CloseEnrolledFile(Private->FileContext);
 
   if (Private->SignatureGUID != NULL) {
     FreePool (Private->SignatureGUID);
@@ -782,13 +852,7 @@ EnrollX509ToKek (
 
 ON_EXIT:
 
-  CloseFile (Private->FileContext->FHandle);
-  if (Private->FileContext->FileName != NULL){
-    FreePool(Private->FileContext->FileName);
-    Private->FileContext->FileName = NULL;
-  }
-
-  Private->FileContext->FHandle = NULL;
+  CloseEnrolledFile(Private->FileContext);
 
   if (Private->SignatureGUID != NULL) {
     FreePool (Private->SignatureGUID);
@@ -822,7 +886,7 @@ EnrollKeyExchangeKey (
   EFI_STATUS  Status;
   UINTN       NameLength;
 
-  if ((Private->FileContext->FileName == NULL) || (Private->SignatureGUID == NULL)) {
+  if ((Private->FileContext->FHandle == NULL) || (Private->FileContext->FileName == NULL) || (Private->SignatureGUID == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
@@ -845,6 +909,11 @@ EnrollKeyExchangeKey (
   } else if (CompareMem (FilePostFix, L".pbk",4) == 0) {
     return EnrollRsa2048ToKek (Private);
   } else {
+    //
+    // File type is wrong, simply close it
+    //
+    CloseEnrolledFile(Private->FileContext);
+
     return EFI_INVALID_PARAMETER;
   }
 }
@@ -956,13 +1025,7 @@ EnrollX509toSigDB (
 
 ON_EXIT:
 
-  CloseFile (Private->FileContext->FHandle);
-  if (Private->FileContext->FileName != NULL){
-    FreePool(Private->FileContext->FileName);
-    Private->FileContext->FileName = NULL;
-  }
-
-  Private->FileContext->FHandle = NULL;
+  CloseEnrolledFile(Private->FileContext);
 
   if (Private->SignatureGUID != NULL) {
     FreePool (Private->SignatureGUID);
@@ -1520,13 +1583,8 @@ EnrollX509HashtoSigDB (
   }
 
 ON_EXIT:
-  CloseFile (Private->FileContext->FHandle);
-  if (Private->FileContext->FileName != NULL){
-    FreePool(Private->FileContext->FileName);
-    Private->FileContext->FileName = NULL;
-  }
 
-  Private->FileContext->FHandle = NULL;
+  CloseEnrolledFile(Private->FileContext);
 
   if (Private->SignatureGUID != NULL) {
     FreePool (Private->SignatureGUID);
@@ -1609,6 +1667,54 @@ ON_EXIT:
   return IsFound;
 }
 
+/**
+  Reads contents of a PE/COFF image in memory buffer.
+
+  Caution: This function may receive untrusted input.
+  PE/COFF image is external input, so this function will make sure the PE/COFF image content
+  read is within the image buffer.
+
+  @param  FileHandle      Pointer to the file handle to read the PE/COFF image.
+  @param  FileOffset      Offset into the PE/COFF image to begin the read operation.
+  @param  ReadSize        On input, the size in bytes of the requested read operation.
+                          On output, the number of bytes actually read.
+  @param  Buffer          Output buffer that contains the data read from the PE/COFF image.
+
+  @retval EFI_SUCCESS     The specified portion of the PE/COFF image was read and the size
+**/
+EFI_STATUS
+EFIAPI
+SecureBootConfigImageRead (
+  IN     VOID    *FileHandle,
+  IN     UINTN   FileOffset,
+  IN OUT UINTN   *ReadSize,
+  OUT    VOID    *Buffer
+  )
+{
+  UINTN               EndPosition;
+
+  if (FileHandle == NULL || ReadSize == NULL || Buffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (MAX_ADDRESS - FileOffset < *ReadSize) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  EndPosition = FileOffset + *ReadSize;
+  if (EndPosition > mImageSize) {
+    *ReadSize = (UINT32)(mImageSize - FileOffset);
+  }
+
+  if (FileOffset >= mImageSize) {
+    *ReadSize = 0;
+  }
+
+  CopyMem (Buffer, (UINT8 *)((UINTN) FileHandle + FileOffset), *ReadSize);
+
+  return EFI_SUCCESS;
+}
+
 /**
   Load PE/COFF image information into internal buffer and check its validity.
 
@@ -1625,9 +1731,28 @@ LoadPeImage (
   EFI_IMAGE_DOS_HEADER                  *DosHdr;
   EFI_IMAGE_NT_HEADERS32                *NtHeader32;
   EFI_IMAGE_NT_HEADERS64                *NtHeader64;
+  PE_COFF_LOADER_IMAGE_CONTEXT          ImageContext;
+  EFI_STATUS                            Status;
 
   NtHeader32 = NULL;
   NtHeader64 = NULL;
+
+  ZeroMem (&ImageContext, sizeof (ImageContext));
+  ImageContext.Handle    = (VOID *) mImageBase;
+  ImageContext.ImageRead = (PE_COFF_LOADER_READ_FILE) SecureBootConfigImageRead;
+
+  //
+  // Get information about the image being loaded
+  //
+  Status = PeCoffLoaderGetImageInfo (&ImageContext);
+  if (EFI_ERROR (Status)) {
+    //
+    // The information can't be got from the invalid PeImage
+    //
+    DEBUG ((DEBUG_INFO, "SecureBootConfigDxe: PeImage invalid. \n"));
+    return Status;
+  }
+
   //
   // Read the Dos header
   //
@@ -1689,6 +1814,9 @@ LoadPeImage (
   Calculate hash of Pe/Coff image based on the authenticode image hashing in
   PE/COFF Specification 8.0 Appendix A
 
+  Notes: PE/COFF image has been checked by BasePeCoffLib PeCoffLoaderGetImageInfo() in 
+  the function LoadPeImage ().
+
   @param[in]    HashAlg   Hash algorithm type.
 
   @retval TRUE            Successfully hash image.
@@ -1716,7 +1844,7 @@ HashPeImage (
   SectionHeader = NULL;
   Status        = FALSE;
 
-  if ((HashAlg != HASHALG_SHA1) && (HashAlg != HASHALG_SHA256)) {
+  if (HashAlg != HASHALG_SHA256) {
     return FALSE;
   }
 
@@ -1725,13 +1853,8 @@ HashPeImage (
   //
   ZeroMem (mImageDigest, MAX_DIGEST_SIZE);
 
-  if (HashAlg == HASHALG_SHA1) {
-    mImageDigestSize  = SHA1_DIGEST_SIZE;
-    mCertType         = gEfiCertSha1Guid;
-  } else if (HashAlg == HASHALG_SHA256) {
-    mImageDigestSize  = SHA256_DIGEST_SIZE;
-    mCertType         = gEfiCertSha256Guid;
-  }
+  mImageDigestSize  = SHA256_DIGEST_SIZE;
+  mCertType         = gEfiCertSha256Guid;
 
   CtxSize   = mHash[HashAlg].GetContextSize();
 
@@ -1773,12 +1896,12 @@ HashPeImage (
     //
     // Use PE32 offset.
     //
-    HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - HashBase);
+    HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) HashBase;
   } else {
     //
     // Use PE32+ offset.
     //
-    HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32Plus->OptionalHeader.CheckSum) - HashBase);
+    HashSize = (UINTN) (&mNtHeader.Pe32Plus->OptionalHeader.CheckSum) - (UINTN) HashBase;
   }
 
   Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
@@ -1795,13 +1918,13 @@ HashPeImage (
     // Use PE32 offset.
     //
     HashBase = (UINT8 *) &mNtHeader.Pe32->OptionalHeader.CheckSum + sizeof (UINT32);
-    HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]) - HashBase);
+    HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]) - (UINTN) HashBase;
   } else {
     //
     // Use PE32+ offset.
     //
     HashBase = (UINT8 *) &mNtHeader.Pe32Plus->OptionalHeader.CheckSum + sizeof (UINT32);
-    HashSize = (UINTN) ((UINT8 *) (&mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]) - HashBase);
+    HashSize = (UINTN) (&mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]) - (UINTN) HashBase;
   }
 
   Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
@@ -1817,13 +1940,13 @@ HashPeImage (
     // Use PE32 offset
     //
     HashBase = (UINT8 *) &mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1];
-    HashSize = mNtHeader.Pe32->OptionalHeader.SizeOfHeaders - (UINTN) ((UINT8 *) (&mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - mImageBase);
+    HashSize = mNtHeader.Pe32->OptionalHeader.SizeOfHeaders - ((UINTN) (&mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - (UINTN) mImageBase);
   } else {
     //
     // Use PE32+ offset.
     //
     HashBase = (UINT8 *) &mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1];
-    HashSize = mNtHeader.Pe32Plus->OptionalHeader.SizeOfHeaders - (UINTN) ((UINT8 *) (&mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - mImageBase);
+    HashSize = mNtHeader.Pe32Plus->OptionalHeader.SizeOfHeaders - ((UINTN) (&mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY + 1]) - (UINTN) mImageBase);
   }
 
   Status  = mHash[HashAlg].HashUpdate(HashCtx, HashBase, HashSize);
@@ -2002,6 +2125,107 @@ HashPeImageByType (
   return EFI_SUCCESS;
 }
 
+/**
+  Enroll a new executable's signature into Signature Database.
+
+  @param[in] PrivateData     The module's private data.
+  @param[in] VariableName    Variable name of signature database, must be
+                             EFI_IMAGE_SECURITY_DATABASE, EFI_IMAGE_SECURITY_DATABASE1
+                             or EFI_IMAGE_SECURITY_DATABASE2.
+
+  @retval   EFI_SUCCESS            New signature is enrolled successfully.
+  @retval   EFI_INVALID_PARAMETER  The parameter is invalid.
+  @retval   EFI_UNSUPPORTED        Unsupported command.
+  @retval   EFI_OUT_OF_RESOURCES   Could not allocate needed resources.
+
+**/
+EFI_STATUS
+EnrollAuthentication2Descriptor (
+  IN SECUREBOOT_CONFIG_PRIVATE_DATA *Private,
+  IN CHAR16                         *VariableName
+  )
+{
+  EFI_STATUS                        Status;
+  VOID                              *Data;
+  UINTN                             DataSize;
+  UINT32                            Attr;
+
+  Data = NULL;
+
+  //
+  // DBT only support DER-X509 Cert Enrollment
+  //
+  if (StrCmp (VariableName, EFI_IMAGE_SECURITY_DATABASE2) == 0) {
+    return EFI_UNSUPPORTED;
+  }
+
+  //
+  // Read the whole file content
+  //
+  Status = ReadFileContent(
+             Private->FileContext->FHandle,
+             (VOID **) &mImageBase,
+             &mImageSize,
+             0
+             );
+  if (EFI_ERROR (Status)) {
+    goto ON_EXIT;
+  }
+  ASSERT (mImageBase != NULL);
+
+  Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
+         | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
+
+  //
+  // Check if SigDB variable has been already existed.
+  // If true, use EFI_VARIABLE_APPEND_WRITE attribute to append the
+  // new signature data to original variable
+  //
+  DataSize = 0;
+  Status = gRT->GetVariable(
+                  VariableName,
+                  &gEfiImageSecurityDatabaseGuid,
+                  NULL,
+                  &DataSize,
+                  NULL
+                  );
+  if (Status == EFI_BUFFER_TOO_SMALL) {
+    Attr |= EFI_VARIABLE_APPEND_WRITE;
+  } else if (Status != EFI_NOT_FOUND) {
+    goto ON_EXIT;
+  }
+
+  //
+  // Diretly set AUTHENTICATION_2 data to SetVariable
+  //
+  Status = gRT->SetVariable(
+                  VariableName,
+                  &gEfiImageSecurityDatabaseGuid,
+                  Attr,
+                  mImageSize,
+                  mImageBase
+                  );
+
+  DEBUG((DEBUG_INFO, "Enroll AUTH_2 data to Var:%s Status: %x\n", VariableName, Status));
+
+ON_EXIT:
+
+  CloseEnrolledFile(Private->FileContext);
+
+  if (Data != NULL) {
+    FreePool (Data);
+  }
+
+  if (mImageBase != NULL) {
+    FreePool (mImageBase);
+    mImageBase = NULL;
+  }
+
+  return Status;
+
+}
+
+
 /**
   Enroll a new executable's signature into Signature Database.
 
@@ -2171,13 +2395,7 @@ EnrollImageSignatureToSigDB (
 
 ON_EXIT:
 
-  CloseFile (Private->FileContext->FHandle);
-  Private->FileContext->FHandle = NULL;
-
-  if (Private->FileContext->FileName != NULL){
-    FreePool(Private->FileContext->FileName);
-    Private->FileContext->FileName = NULL;
-  }
+  CloseEnrolledFile(Private->FileContext);
 
   if (Private->SignatureGUID != NULL) {
     FreePool (Private->SignatureGUID);
@@ -2241,9 +2459,11 @@ EnrollSignatureDatabase (
     // Supports DER-encoded X509 certificate.
     //
     return EnrollX509toSigDB (Private, VariableName);
+  } else if (IsAuthentication2Format(Private->FileContext->FHandle)){
+    return EnrollAuthentication2Descriptor(Private, VariableName);
+  } else {
+    return EnrollImageSignatureToSigDB (Private, VariableName);
   }
-
-  return EnrollImageSignatureToSigDB (Private, VariableName);
 }
 
 /**
@@ -2872,11 +3092,13 @@ UpdateSecureBootString(
 /**
   This function extracts configuration from variable.
 
+  @param[in]       Private      Point to SecureBoot configuration driver private data.
   @param[in, out]  ConfigData   Point to SecureBoot configuration private data.
 
 **/
 VOID
 SecureBootExtractConfigFromVariable (
+  IN SECUREBOOT_CONFIG_PRIVATE_DATA  *Private,
   IN OUT SECUREBOOT_CONFIGURATION    *ConfigData
   )
 {
@@ -2901,7 +3123,11 @@ SecureBootExtractConfigFromVariable (
   ConfigData->RevocationTime.Hour   = CurrTime.Hour;
   ConfigData->RevocationTime.Minute = CurrTime.Minute;
   ConfigData->RevocationTime.Second = 0;
-
+  if (Private->FileContext->FHandle != NULL) {
+    ConfigData->FileEnrollType = Private->FileContext->FileType;
+  } else {
+    ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;
+  }
 
   //
   // If it is Physical Presence User, set the PhysicalPresent to true.
@@ -3024,10 +3250,12 @@ SecureBootExtractConfig (
     return EFI_NOT_FOUND;
   }
 
+  ZeroMem(&Configuration, sizeof(SECUREBOOT_CONFIGURATION));
+
   //
   // Get Configuration from Variable.
   //
-  SecureBootExtractConfigFromVariable (&Configuration);
+  SecureBootExtractConfigFromVariable (PrivateData, &Configuration);
 
   BufferSize = sizeof (SECUREBOOT_CONFIGURATION);
   ConfigRequest = Request;
@@ -3102,9 +3330,10 @@ SecureBootRouteConfig (
        OUT EFI_STRING                          *Progress
   )
 {
-  SECUREBOOT_CONFIGURATION   IfrNvData;
-  UINTN                      BufferSize;
-  EFI_STATUS                 Status;
+  SECUREBOOT_CONFIGURATION          IfrNvData;
+  UINTN                             BufferSize;
+  SECUREBOOT_CONFIG_PRIVATE_DATA    *PrivateData;
+  EFI_STATUS                        Status;
 
   if (Configuration == NULL || Progress == NULL) {
     return EFI_INVALID_PARAMETER;
@@ -3115,10 +3344,12 @@ SecureBootRouteConfig (
     return EFI_NOT_FOUND;
   }
 
+  PrivateData = SECUREBOOT_CONFIG_PRIVATE_FROM_THIS (This);
+
   //
   // Get Configuration from Variable.
   //
-  SecureBootExtractConfigFromVariable (&IfrNvData);
+  SecureBootExtractConfigFromVariable (PrivateData, &IfrNvData);
 
   //
   // Map the Configuration to the configuration block.
@@ -3184,6 +3415,7 @@ SecureBootCallback (
 {
   EFI_INPUT_KEY                   Key;
   EFI_STATUS                      Status;
+  RETURN_STATUS                   RStatus;
   SECUREBOOT_CONFIG_PRIVATE_DATA  *Private;
   UINTN                           BufferSize;
   SECUREBOOT_CONFIGURATION        *IfrNvData;
@@ -3194,6 +3426,9 @@ SecureBootCallback (
   UINT8                           *SetupMode;
   CHAR16                          PromptString[100];
   EFI_DEVICE_PATH_PROTOCOL        *File;
+  UINTN                           NameLength;
+  UINT16                          *FilePostFix;
+  SECUREBOOT_CONFIG_PRIVATE_DATA  *PrivateData;
 
   Status           = EFI_SUCCESS;
   SecureBootEnable = NULL;
@@ -3226,8 +3461,20 @@ SecureBootCallback (
       // Update secure boot strings when opening this form
       //
       Status = UpdateSecureBootString(Private);
-      SecureBootExtractConfigFromVariable (IfrNvData);
+      SecureBootExtractConfigFromVariable (Private, IfrNvData);
       mIsEnterSecureBootForm = TRUE;
+    } else {
+      //
+      // When entering SecureBoot OPTION Form
+      // always close opened file & free resource
+      //
+      if ((QuestionId == KEY_SECURE_BOOT_PK_OPTION) ||
+          (QuestionId == KEY_SECURE_BOOT_KEK_OPTION) ||
+          (QuestionId == KEY_SECURE_BOOT_DB_OPTION) ||
+          (QuestionId == KEY_SECURE_BOOT_DBX_OPTION) ||
+          (QuestionId == KEY_SECURE_BOOT_DBT_OPTION)) {
+        CloseEnrolledFile(Private->FileContext);
+      }
     }
     goto EXIT;
   }
@@ -3281,6 +3528,7 @@ SecureBootCallback (
     case KEY_SECURE_BOOT_DB_OPTION:
     case KEY_SECURE_BOOT_DBX_OPTION:
     case KEY_SECURE_BOOT_DBT_OPTION:
+      PrivateData = SECUREBOOT_CONFIG_PRIVATE_FROM_THIS (This);
       //
       // Clear Signature GUID.
       //
@@ -3292,6 +3540,11 @@ SecureBootCallback (
         }
       }
 
+      //
+      // Cleanup VFRData once leaving PK/KEK/DB/DBX/DBT enroll/delete page
+      //
+      SecureBootExtractConfigFromVariable (PrivateData, IfrNvData);
+
       if (QuestionId == KEY_SECURE_BOOT_DB_OPTION) {
         LabelId = SECUREBOOT_ENROLL_SIGNATURE_TO_DB;
       } else if (QuestionId == KEY_SECURE_BOOT_DBX_OPTION) {
@@ -3329,6 +3582,38 @@ SecureBootCallback (
 
     case SECUREBOOT_ENROLL_SIGNATURE_TO_DBX:
       ChooseFile (NULL, NULL, UpdateDBXFromFile, &File);
+
+      if (Private->FileContext->FHandle != NULL) {
+        //
+        // Parse the file's postfix.
+        //
+        NameLength = StrLen (Private->FileContext->FileName);
+        if (NameLength <= 4) {
+          return FALSE;
+        }
+        FilePostFix = Private->FileContext->FileName + NameLength - 4;
+
+        if (IsDerEncodeCertificate (FilePostFix)) {
+          //
+          // Supports DER-encoded X509 certificate.
+          //
+          IfrNvData->FileEnrollType = X509_CERT_FILE_TYPE;
+        } else if (IsAuthentication2Format(Private->FileContext->FHandle)){
+          IfrNvData->FileEnrollType = AUTHENTICATION_2_FILE_TYPE;
+        } else {
+          IfrNvData->FileEnrollType = PE_IMAGE_FILE_TYPE;
+        }
+        Private->FileContext->FileType = IfrNvData->FileEnrollType;
+
+        //
+        // Clean up Certificate Format if File type is not X509 DER
+        //
+        if (IfrNvData->FileEnrollType != X509_CERT_FILE_TYPE) {
+          IfrNvData->CertificateFormat = HASHALG_RAW;
+        }
+        DEBUG((DEBUG_ERROR, "IfrNvData->FileEnrollType %d\n", Private->FileContext->FileType));
+      }
+
       break;
 
     case SECUREBOOT_ENROLL_SIGNATURE_TO_DBT:
@@ -3438,7 +3723,12 @@ SecureBootCallback (
           L"Enrollment failed! Same certificate had already been in the dbx!",
           NULL
           );
-          break;
+
+        //
+        // Cert already exists in DBX. Close opened file before exit.
+        //
+        CloseEnrolledFile(Private->FileContext);
+        break;
       }
 
       if ((IfrNvData != NULL) && (IfrNvData->CertificateFormat < HASHALG_MAX)) {
@@ -3449,6 +3739,7 @@ SecureBootCallback (
                    &IfrNvData->RevocationTime,
                    IfrNvData->AlwaysRevocation
                    );
+        IfrNvData->CertificateFormat = HASHALG_RAW;
       } else {
         Status = EnrollSignatureDatabase (Private, EFI_IMAGE_SECURITY_DATABASE1);
       }
@@ -3457,7 +3748,7 @@ SecureBootCallback (
           EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
           &Key,
           L"ERROR: Unsupported file type!",
-          L"Only supports DER-encoded X509 certificate and executable EFI image",
+          L"Only supports DER-encoded X509 certificate, AUTH_2 format data & executable EFI image",
           NULL
           );
       }
@@ -3538,14 +3829,7 @@ SecureBootCallback (
     case KEY_VALUE_NO_SAVE_AND_EXIT_DB:
     case KEY_VALUE_NO_SAVE_AND_EXIT_DBX:
     case KEY_VALUE_NO_SAVE_AND_EXIT_DBT:
-      if (Private->FileContext->FHandle != NULL) {
-        CloseFile (Private->FileContext->FHandle);
-        Private->FileContext->FHandle = NULL;
-        if (Private->FileContext->FileName!= NULL){
-          FreePool(Private->FileContext->FileName);
-          Private->FileContext->FileName = NULL;
-        }
-      }
+      CloseEnrolledFile(Private->FileContext);
 
       if (Private->SignatureGUID != NULL) {
         FreePool (Private->SignatureGUID);
@@ -3566,18 +3850,14 @@ SecureBootCallback (
     case KEY_SECURE_BOOT_SIGNATURE_GUID_DBX:
     case KEY_SECURE_BOOT_SIGNATURE_GUID_DBT:
       ASSERT (Private->SignatureGUID != NULL);
-      Status = StringToGuid (
-                 IfrNvData->SignatureGuid,
-                 StrLen (IfrNvData->SignatureGuid),
-                 Private->SignatureGUID
-                 );
-      if (EFI_ERROR (Status)) {
+      RStatus = StrToGuid (IfrNvData->SignatureGuid, Private->SignatureGUID);
+      if (RETURN_ERROR (RStatus) || (IfrNvData->SignatureGuid[GUID_STRING_LENGTH] != L'\0')) {
+        Status = EFI_INVALID_PARAMETER;
         break;
       }
 
       *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_APPLY;
       break;
-
     case KEY_SECURE_BOOT_DELETE_PK:
       GetVariable2 (EFI_SETUP_MODE_NAME, &gEfiGlobalVariableGuid, (VOID**)&SetupMode, NULL);
       if (SetupMode == NULL || (*SetupMode) == SETUP_MODE) {