In the code structure
if (condition) {
//
// block1
//
return;
} else {
//
// block2
//
}
nesting "block2" in an "else" branch is superfluous, and harms
readability. It can be transformed to:
if (condition) {
//
// block1
//
return;
}
//
// block2
//
with identical behavior, and improved readability (less nesting).
The same applies to "break" (instead of "return") in a loop body.
Perform these transformations on DxeImageVerificationHandler().
This patch is a no-op for behavior. Use
git show -b -W
for reviewing it more easily.
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
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 41 ++++++++++----------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 5afd723adae8..8204c9c0f105 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1556,349 +1556,350 @@ 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;
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;
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) {
+ }
+ 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;
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;
- } else if (!IsVerified) {
+ }
+ 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;
- } 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);
+ }
+ 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 (#53315): https://edk2.groups.io/g/devel/message/53315
Mute This Topic: https://groups.io/mt/69752219/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.