In the DxeImageVerificationHandler() function, the "VerifyStatus" variable can only contain one of two values: EFI_SUCCESS and EFI_ACCESS_DENIED. Furthermore, the variable is only consumed with EFI_ERROR().
Therefore, using the EFI_STATUS type for the variable is unnecessary. Worse, given the complex meanings of the function's return values, using EFI_STATUS for "VerifyStatus" is actively confusing. Rename the variable to "IsVerified", and make it a simple BOOLEAN. This patch is a no-op, regarding behavior. Cc: Chao Zhang <chao.b.zh...@intel.com> Cc: Jian J Wang <jian.j.w...@intel.com> Cc: Jiewen Yao <jiewen....@intel.com> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2129 Signed-off-by: Laszlo Ersek <ler...@redhat.com> --- SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c index a0a12b50ddd1..5afd723adae8 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1556,349 +1556,349 @@ EFIAPI DxeImageVerificationHandler ( IN UINT32 AuthenticationStatus, IN CONST EFI_DEVICE_PATH_PROTOCOL *File, IN VOID *FileBuffer, IN UINTN FileSize, IN BOOLEAN BootPolicy ) { EFI_STATUS Status; EFI_IMAGE_DOS_HEADER *DosHdr; - EFI_STATUS VerifyStatus; + 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; SignatureList = NULL; SignatureListSize = 0; WinCertificate = NULL; SecDataDir = NULL; PkcsCertData = NULL; Action = EFI_IMAGE_EXECUTION_AUTH_UNTESTED; Status = EFI_ACCESS_DENIED; - VerifyStatus = EFI_ACCESS_DENIED; + 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; } else 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_INVALID_PARAMETER; } 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 // Status = PeCoffLoaderGetImageInfo (&ImageContext); if (EFI_ERROR (Status)) { // // The information can't be got from the invalid PeImage // DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: PeImage invalid. Cannot retrieve image information.\n")); goto Done; } Status = EFI_ACCESS_DENIED; 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 Done; } 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 Done; } 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 Done; } 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 Done; } // // 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; } Status = HashPeImageByType (AuthData, AuthDataSize); if (EFI_ERROR (Status)) { continue; } // // Check the digital signature against the revoked certificate in forbidden database (dbx). // if (IsForbiddenByDbx (AuthData, AuthDataSize)) { Action = EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED; - VerifyStatus = EFI_ACCESS_DENIED; + IsVerified = FALSE; break; } // // Check the digital signature against the valid certificate in allowed database (db). // - if (EFI_ERROR (VerifyStatus)) { + if (!IsVerified) { if (IsAllowedByDb (AuthData, AuthDataSize)) { - VerifyStatus = EFI_SUCCESS; + 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)); - VerifyStatus = EFI_ACCESS_DENIED; + IsVerified = FALSE; break; - } else if (EFI_ERROR (VerifyStatus)) { + } else if (!IsVerified) { if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDigest, &mCertType, mImageDigestSize)) { - VerifyStatus = EFI_SUCCESS; + 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. // - VerifyStatus = EFI_ACCESS_DENIED; + IsVerified = FALSE; } - if (!EFI_ERROR (VerifyStatus)) { + if (IsVerified) { return EFI_SUCCESS; } else { Status = EFI_ACCESS_DENIED; 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) { Status = EFI_OUT_OF_RESOURCES; goto Done; } 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); } } Done: if (Status != EFI_SUCCESS) { // // Policy decides to defer or reject the image; add its information in image executable information table. // 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); } Status = EFI_SECURITY_VIOLATION; } if (SignatureList != NULL) { FreePool (SignatureList); } return Status; } /** 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#53317): https://edk2.groups.io/g/devel/message/53317 Mute This Topic: https://groups.io/mt/69752221/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
