[PATCH 11/11] SecurityPkg/DxeImageVerificationHandler: fix "defer" vs. "deny" policies

["Laszlo Ersek" <lersek@redhat.com>]

In DxeImageVerificationHandler(), we should return EFI_SECURITY_VIOLATION
for a rejected image only if the platform sets
DEFER_EXECUTE_ON_SECURITY_VIOLATION as the policy for the image's source.
Otherwise, EFI_ACCESS_DENIED must be returned.

Right now, EFI_SECURITY_VIOLATION is returned for all rejected images,
which is wrong -- it causes LoadImage() to hold on to rejected images (in
untrusted state), for further platform actions. However, if a platform
already set DENY_EXECUTE_ON_SECURITY_VIOLATION, the platform will not
expect the rejected image to stick around in memory (regardless of its
untrusted state).

Therefore, adhere to the platform policy in the return value of the
DxeImageVerificationHandler() function.

Furthermore, according to "32.4.2 Image Execution Information Table" in
the UEFI v2.8 spec, and considering that edk2 only supports (AuditMode==0)
at the moment:

> When AuditMode==0, if the image's signature is not found in the
> authorized database, or is found in the forbidden database, the image
> will not be started and instead, information about it will be placed in
> this table.

we have to store an EFI_IMAGE_EXECUTION_INFO record in both the "defer"
case and the "deny" case. Thus, the AddImageExeInfo() call is not being
made conditional on (Policy == DEFER_EXECUTE_ON_SECURITY_VIOLATION); the
documentation is updated instead.

Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129
Fixes: 5db28a6753d307cdfb1cfdeb2f63739a9f959837
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 53ef340c08ad..ff79e30ef83e 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1385,170 +1385,171 @@ BOOLEAN
 IsAllowedByDb (
   IN UINT8              *AuthData,
   IN UINTN              AuthDataSize
   )
 {
   EFI_STATUS                Status;
   BOOLEAN                   VerifyStatus;
   EFI_SIGNATURE_LIST        *CertList;
   EFI_SIGNATURE_DATA        *CertData;
   UINTN                     DataSize;
   UINT8                     *Data;
   UINT8                     *RootCert;
   UINTN                     RootCertSize;
   UINTN                     Index;
   UINTN                     CertCount;
   UINTN                     DbxDataSize;
   UINT8                     *DbxData;
   EFI_TIME                  RevocationTime;
 
   Data              = NULL;
   CertList          = NULL;
   CertData          = NULL;
   RootCert          = NULL;
   DbxData           = NULL;
   RootCertSize      = 0;
   VerifyStatus      = FALSE;
 
   DataSize = 0;
   Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL);
   if (Status == EFI_BUFFER_TOO_SMALL) {
     Data = (UINT8 *) AllocateZeroPool (DataSize);
     if (Data == NULL) {
       return VerifyStatus;
     }
 
     Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data);
     if (EFI_ERROR (Status)) {
       goto Done;
     }
 
     //
     // Find X509 certificate in Signature List to verify the signature in pkcs7 signed data.
     //
     CertList = (EFI_SIGNATURE_LIST *) Data;
     while ((DataSize > 0) && (DataSize >= CertList->SignatureListSize)) {
       if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) {
         CertData  = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);
         CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize;
 
         for (Index = 0; Index < CertCount; Index++) {
           //
           // Iterate each Signature Data Node within this CertList for verify.
           //
           RootCert     = CertData->SignatureData;
           RootCertSize = CertList->SignatureSize - sizeof (EFI_GUID);
 
           //
           // Call AuthenticodeVerify library to Verify Authenticode struct.
           //
           VerifyStatus = AuthenticodeVerify (
                            AuthData,
                            AuthDataSize,
                            RootCert,
                            RootCertSize,
                            mImageDigest,
                            mImageDigestSize
                            );
           if (VerifyStatus) {
             //
             // Here We still need to check if this RootCert's Hash is revoked
             //
             Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL);
             if (Status == EFI_BUFFER_TOO_SMALL) {
               goto Done;
             }
             DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize);
             if (DbxData == NULL) {
               goto Done;
             }
 
             Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData);
             if (EFI_ERROR (Status)) {
               goto Done;
             }
 
             if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) {
               //
               // Check the timestamp signature and signing time to determine if the RootCert can be trusted.
               //
               VerifyStatus = PassTimestampCheck (AuthData, AuthDataSize, &RevocationTime);
               if (!VerifyStatus) {
                 DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed and signature is accepted by DB, but its root cert failed the timestamp check.\n"));
               }
             }
 
             goto Done;
           }
 
           CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList->SignatureSize);
         }
       }
 
       DataSize -= CertList->SignatureListSize;
       CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize);
     }
   }
 
 Done:
 
   if (VerifyStatus) {
     SecureBootHook (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, CertList->SignatureSize, CertData);
   }
 
   if (Data != NULL) {
     FreePool (Data);
   }
   if (DbxData != NULL) {
     FreePool (DbxData);
   }
 
   return VerifyStatus;
 }
 
 /**
   Provide verification service for signed images, which include both signature validation
   and platform policy control. For signature types, both UEFI WIN_CERTIFICATE_UEFI_GUID and
   MSFT Authenticode type signatures are supported.
 
   In this implementation, only verify external executables when in USER MODE.
   Executables from FV is bypass, so pass in AuthenticationStatus is ignored.
 
   The image verification policy is:
     If the image is signed,
       At least one valid signature or at least one hash value of the image must match a record
       in the security database "db", and no valid signature nor any hash value of the image may
       be reflected in the security database "dbx".
     Otherwise, the image is not signed,
       The SHA256 hash value of the image must match a record in the security database "db", and
       not be reflected in the security data base "dbx".
 
   Caution: This function may receive untrusted input.
   PE/COFF image is external input, so this function will validate its data structure
   within this image buffer before use.
 
   @param[in]    AuthenticationStatus
                            This is the authentication status returned from the security
                            measurement services for the input file.
   @param[in]    File       This is a pointer to the device path of the file that is
                            being dispatched. This will optionally be used for logging.
   @param[in]    FileBuffer File buffer matches the input file device path.
   @param[in]    FileSize   Size of File buffer matches the input file device path.
   @param[in]    BootPolicy A boot policy that was used to call LoadImage() UEFI service.
 
   @retval EFI_SUCCESS            The file specified by DevicePath and non-NULL
                                  FileBuffer did authenticate, and the platform policy dictates
                                  that the DXE Foundation may use the file.
   @retval EFI_SUCCESS            The device path specified by NULL device path DevicePath
                                  and non-NULL FileBuffer did authenticate, and the platform
                                  policy dictates that the DXE Foundation may execute the image in
                                  FileBuffer.
   @retval EFI_SECURITY_VIOLATION The file specified by File did not authenticate, and
                                  the platform policy dictates that File should be placed
                                  in the untrusted state. The image has been added to the file
                                  execution table.
   @retval EFI_ACCESS_DENIED      The file specified by File and FileBuffer did not
                                  authenticate, and the platform policy dictates that the DXE
-                                 Foundation many not use File.
+                                 Foundation may not use File. The image has
+                                 been added to the file execution table.
 
 **/
 EFI_STATUS
@@ -1556,344 +1557,348 @@ EFIAPI
 DxeImageVerificationHandler (
   IN  UINT32                           AuthenticationStatus,
   IN  CONST EFI_DEVICE_PATH_PROTOCOL   *File,
   IN  VOID                             *FileBuffer,
   IN  UINTN                            FileSize,
   IN  BOOLEAN                          BootPolicy
   )
 {
   EFI_IMAGE_DOS_HEADER                 *DosHdr;
   BOOLEAN                              IsVerified;
   EFI_SIGNATURE_LIST                   *SignatureList;
   UINTN                                SignatureListSize;
   EFI_SIGNATURE_DATA                   *Signature;
   EFI_IMAGE_EXECUTION_ACTION           Action;
   WIN_CERTIFICATE                      *WinCertificate;
   UINT32                               Policy;
   UINT8                                *SecureBoot;
   PE_COFF_LOADER_IMAGE_CONTEXT         ImageContext;
   UINT32                               NumberOfRvaAndSizes;
   WIN_CERTIFICATE_EFI_PKCS             *PkcsCertData;
   WIN_CERTIFICATE_UEFI_GUID            *WinCertUefiGuid;
   UINT8                                *AuthData;
   UINTN                                AuthDataSize;
   EFI_IMAGE_DATA_DIRECTORY             *SecDataDir;
   UINT32                               OffSet;
   CHAR16                               *NameStr;
   RETURN_STATUS                        PeCoffStatus;
   EFI_STATUS                           HashStatus;
 
   SignatureList     = NULL;
   SignatureListSize = 0;
   WinCertificate    = NULL;
   SecDataDir        = NULL;
   PkcsCertData      = NULL;
   Action            = EFI_IMAGE_EXECUTION_AUTH_UNTESTED;
   IsVerified        = FALSE;
 
 
   //
   // Check the image type and get policy setting.
   //
   switch (GetImageType (File)) {
 
   case IMAGE_FROM_FV:
     Policy = ALWAYS_EXECUTE;
     break;
 
   case IMAGE_FROM_OPTION_ROM:
     Policy = PcdGet32 (PcdOptionRomImageVerificationPolicy);
     break;
 
   case IMAGE_FROM_REMOVABLE_MEDIA:
     Policy = PcdGet32 (PcdRemovableMediaImageVerificationPolicy);
     break;
 
   case IMAGE_FROM_FIXED_MEDIA:
     Policy = PcdGet32 (PcdFixedMediaImageVerificationPolicy);
     break;
 
   default:
     Policy = DENY_EXECUTE_ON_SECURITY_VIOLATION;
     break;
   }
   //
   // If policy is always/never execute, return directly.
   //
   if (Policy == ALWAYS_EXECUTE) {
     return EFI_SUCCESS;
   }
   if (Policy == NEVER_EXECUTE) {
     return EFI_ACCESS_DENIED;
   }
 
   //
   // The policy QUERY_USER_ON_SECURITY_VIOLATION and ALLOW_EXECUTE_ON_SECURITY_VIOLATION
   // violates the UEFI spec and has been removed.
   //
   ASSERT (Policy != QUERY_USER_ON_SECURITY_VIOLATION && Policy != ALLOW_EXECUTE_ON_SECURITY_VIOLATION);
   if (Policy == QUERY_USER_ON_SECURITY_VIOLATION || Policy == ALLOW_EXECUTE_ON_SECURITY_VIOLATION) {
     CpuDeadLoop ();
   }
 
   GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID**)&SecureBoot, NULL);
   //
   // Skip verification if SecureBoot variable doesn't exist.
   //
   if (SecureBoot == NULL) {
     return EFI_SUCCESS;
   }
 
   //
   // Skip verification if SecureBoot is disabled but not AuditMode
   //
   if (*SecureBoot == SECURE_BOOT_MODE_DISABLE) {
     FreePool (SecureBoot);
     return EFI_SUCCESS;
   }
   FreePool (SecureBoot);
 
   //
   // Read the Dos header.
   //
   if (FileBuffer == NULL) {
     return EFI_ACCESS_DENIED;
   }
 
   mImageBase  = (UINT8 *) FileBuffer;
   mImageSize  = FileSize;
 
   ZeroMem (&ImageContext, sizeof (ImageContext));
   ImageContext.Handle    = (VOID *) FileBuffer;
   ImageContext.ImageRead = (PE_COFF_LOADER_READ_FILE) DxeImageVerificationLibImageRead;
 
   //
   // Get information about the image being loaded
   //
   PeCoffStatus = PeCoffLoaderGetImageInfo (&ImageContext);
   if (RETURN_ERROR (PeCoffStatus)) {
     //
     // The information can't be got from the invalid PeImage
     //
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: PeImage invalid. Cannot retrieve image information.\n"));
     goto Failed;
   }
 
   DosHdr = (EFI_IMAGE_DOS_HEADER *) mImageBase;
   if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
     //
     // DOS image header is present,
     // so read the PE header after the DOS image header.
     //
     mPeCoffHeaderOffset = DosHdr->e_lfanew;
   } else {
     mPeCoffHeaderOffset = 0;
   }
   //
   // Check PE/COFF image.
   //
   mNtHeader.Pe32 = (EFI_IMAGE_NT_HEADERS32 *) (mImageBase + mPeCoffHeaderOffset);
   if (mNtHeader.Pe32->Signature != EFI_IMAGE_NT_SIGNATURE) {
     //
     // It is not a valid Pe/Coff file.
     //
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Not a valid PE/COFF image.\n"));
     goto Failed;
   }
 
   if (mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
     //
     // Use PE32 offset.
     //
     NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
     if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
       SecDataDir = (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY];
     }
   } else {
     //
     // Use PE32+ offset.
     //
     NumberOfRvaAndSizes = mNtHeader.Pe32Plus->OptionalHeader.NumberOfRvaAndSizes;
     if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
       SecDataDir = (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32Plus->OptionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY];
     }
   }
 
   //
   // Start Image Validation.
   //
   if (SecDataDir == NULL || SecDataDir->Size == 0) {
     //
     // This image is not signed. The SHA256 hash value of the image must match a record in the security database "db",
     // and not be reflected in the security data base "dbx".
     //
     if (!HashPeImage (HASHALG_SHA256)) {
       DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Failed to hash this image using %s.\n", mHashTypeStr));
       goto Failed;
     }
 
     if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) {
       //
       // Image Hash is in forbidden database (DBX).
       //
       DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is forbidden by DBX.\n", mHashTypeStr));
       goto Failed;
     }
 
     if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) {
       //
       // Image Hash is in allowed database (DB).
       //
       return EFI_SUCCESS;
     }
 
     //
     // Image Hash is not found in both forbidden and allowed database.
     //
     DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed and %s hash of image is not found in DB/DBX.\n", mHashTypeStr));
     goto Failed;
   }
 
   //
   // Verify the signature of the image, multiple signatures are allowed as per PE/COFF Section 4.7
   // "Attribute Certificate Table".
   // The first certificate starts at offset (SecDataDir->VirtualAddress) from the start of the file.
   //
   for (OffSet = SecDataDir->VirtualAddress;
        OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size);
        OffSet += (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate->dwLength))) {
     WinCertificate = (WIN_CERTIFICATE *) (mImageBase + OffSet);
     if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <= sizeof (WIN_CERTIFICATE) ||
         (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCertificate->dwLength) {
       break;
     }
 
     //
     // Verify the image's Authenticode signature, only DER-encoded PKCS#7 signed data is supported.
     //
     if (WinCertificate->wCertificateType == WIN_CERT_TYPE_PKCS_SIGNED_DATA) {
       //
       // The certificate is formatted as WIN_CERTIFICATE_EFI_PKCS which is described in the
       // Authenticode specification.
       //
       PkcsCertData = (WIN_CERTIFICATE_EFI_PKCS *) WinCertificate;
       if (PkcsCertData->Hdr.dwLength <= sizeof (PkcsCertData->Hdr)) {
         break;
       }
       AuthData   = PkcsCertData->CertData;
       AuthDataSize = PkcsCertData->Hdr.dwLength - sizeof(PkcsCertData->Hdr);
     } else if (WinCertificate->wCertificateType == WIN_CERT_TYPE_EFI_GUID) {
       //
       // The certificate is formatted as WIN_CERTIFICATE_UEFI_GUID which is described in UEFI Spec.
       //
       WinCertUefiGuid = (WIN_CERTIFICATE_UEFI_GUID *) WinCertificate;
       if (WinCertUefiGuid->Hdr.dwLength <= OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData)) {
         break;
       }
       if (!CompareGuid (&WinCertUefiGuid->CertType, &gEfiCertPkcs7Guid)) {
         continue;
       }
       AuthData = WinCertUefiGuid->CertData;
       AuthDataSize = WinCertUefiGuid->Hdr.dwLength - OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData);
     } else {
       if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) {
         break;
       }
       continue;
     }
 
     HashStatus = HashPeImageByType (AuthData, AuthDataSize);
     if (EFI_ERROR (HashStatus)) {
       continue;
     }
 
     //
     // Check the digital signature against the revoked certificate in forbidden database (dbx).
     //
     if (IsForbiddenByDbx (AuthData, AuthDataSize)) {
       Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED;
       IsVerified = FALSE;
       break;
     }
 
     //
     // Check the digital signature against the valid certificate in allowed database (db).
     //
     if (!IsVerified) {
       if (IsAllowedByDb (AuthData, AuthDataSize)) {
         IsVerified = TRUE;
       }
     }
 
     //
     // Check the image's hash value.
     //
     if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDigest, &mCertType, mImageDigestSize)) {
       Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND;
       DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but %s hash of image is found in DBX.\n", mHashTypeStr));
       IsVerified = FALSE;
       break;
     }
     if (!IsVerified) {
       if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) {
         IsVerified = TRUE;
       } else {
         DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed but signature is not allowed by DB and %s hash of image is not found in DB/DBX.\n", mHashTypeStr));
       }
     }
   }
 
   if (OffSet != (SecDataDir->VirtualAddress + SecDataDir->Size)) {
     //
     // The Size in Certificate Table or the attribute certificate table is corrupted.
     //
     IsVerified = FALSE;
   }
 
   if (IsVerified) {
     return EFI_SUCCESS;
   }
   if (Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action == EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND) {
     //
     // Get image hash value as signature of executable.
     //
     SignatureListSize = sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize;
     SignatureList     = (EFI_SIGNATURE_LIST *) AllocateZeroPool (SignatureListSize);
     if (SignatureList == NULL) {
       SignatureListSize = 0;
       goto Failed;
     }
     SignatureList->SignatureHeaderSize  = 0;
     SignatureList->SignatureListSize    = (UINT32) SignatureListSize;
     SignatureList->SignatureSize        = (UINT32) (sizeof (EFI_SIGNATURE_DATA) - 1 + mImageDigestSize);
     CopyMem (&SignatureList->SignatureType, &mCertType, sizeof (EFI_GUID));
     Signature = (EFI_SIGNATURE_DATA *) ((UINT8 *) SignatureList + sizeof (EFI_SIGNATURE_LIST));
     CopyMem (Signature->SignatureData, mImageDigest, mImageDigestSize);
   }
 
 Failed:
   //
-  // Policy decides to defer or reject the image; add its information in image executable information table.
+  // Policy decides to defer or reject the image; add its information in image
+  // executable information table in either case.
   //
   NameStr = ConvertDevicePathToText (File, FALSE, TRUE);
   AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize);
   if (NameStr != NULL) {
     DEBUG((EFI_D_INFO, "The image doesn't pass verification: %s\n", NameStr));
     FreePool(NameStr);
   }
 
   if (SignatureList != NULL) {
     FreePool (SignatureList);
   }
 
-  return EFI_SECURITY_VIOLATION;
+  if (Policy == DEFER_EXECUTE_ON_SECURITY_VIOLATION) {
+    return EFI_SECURITY_VIOLATION;
+  }
+  return EFI_ACCESS_DENIED;
 }
 
 /**
   On Ready To Boot Services Event notification handler.
 
   Add the image execution information table if it is not in system configuration table.
 
   @param[in]  Event     Event whose notification function is being invoked
   @param[in]  Context   Pointer to the notification function's context
 
 **/
-- 
2.19.1.3.g30247aa5d201