From nobody Mon Feb 9 11:46:47 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+53317+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+53317+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201640; cv=none; d=zohomail.com; s=zohoarc; b=YkruZ4LU24tKZpVp/irGQ3AkAKYTJmTLLx/CowH9yWhyP9dTEh+YM6mDJEZ7b/BJ/voZGqkrFfKiAlikW6NYQdBaHz7aSat0SObeNjO6/2YOSSfwcaovO2wtelfTSs9uj7bkObT9BAVvZEusvAoKu2NKoInmJhH23tZWEL5mj2c= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201640; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=qCnTQ+gglnI4SHJsOA6R6y9d//EfUE25eYYa9TAhPM4=; b=Zk55AiCKBU7GNuvw1rsDMA+5qDIQN2vhmD5aiZqoDrIfaMV/yucveZyBe4J9HYBHhRUGwFvtzxiJY4EQKqnD9pAgNED1/20+NOzz2SlIM0ALkvZNSaJfDCyFKdwo7CuZg+p0H5Tpi0xsox+f2dSHpMRrlbociq0OuYS1neSZ94Q= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+53317+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1579201640782596.4676421929266; Thu, 16 Jan 2020 11:07:20 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id 5JCwYY1788612xfBlovIatx4; Thu, 16 Jan 2020 11:07:20 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web10.1468.1579201639097359964 for ; Thu, 16 Jan 2020 11:07:19 -0800 X-Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-25-VDGdJ2CNMwS-gC0getIQgg-1; Thu, 16 Jan 2020 14:07:11 -0500 X-Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 389DD800D50; Thu, 16 Jan 2020 19:07:10 +0000 (UTC) X-Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-120.ams2.redhat.com [10.36.116.120]) by smtp.corp.redhat.com (Postfix) with ESMTP id 192F484333; Thu, 16 Jan 2020 19:07:08 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 01/11] SecurityPkg/DxeImageVerificationHandler: simplify "VerifyStatus" Date: Thu, 16 Jan 2020 20:06:55 +0100 Message-Id: <20200116190705.18816-2-lersek@redhat.com> In-Reply-To: <20200116190705.18816-1-lersek@redhat.com> References: <20200116190705.18816-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-MC-Unique: VDGdJ2CNMwS-gC0getIQgg-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com X-Gm-Message-State: eCjNhXlgQUMgxyxOPmROI1JZx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201640; bh=qCnTQ+gglnI4SHJsOA6R6y9d//EfUE25eYYa9TAhPM4=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=mrrtT5cstfQxw4/n6M71LG8hgnUMHdNOyH+cdVaUOTiiSf7r/hbdy1Qqr0/AsffT/XO FhaplbaQrw+K9L82ucdw4WprY07U89RwN1yvuTwbzQFJTeTF4YRyDxaBUvbHueXD292Z+ kgjDGE60WIX9dFDoT0hLIKjzn4Z6iiJtXzQ= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" 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 Cc: Jian J Wang Cc: Jiewen Yao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2129 Signed-off-by: Laszlo Ersek --- SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 20= ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.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; =20 SignatureList =3D NULL; SignatureListSize =3D 0; WinCertificate =3D NULL; SecDataDir =3D NULL; PkcsCertData =3D NULL; Action =3D EFI_IMAGE_EXECUTION_AUTH_UNTESTED; Status =3D EFI_ACCESS_DENIED; - VerifyStatus =3D EFI_ACCESS_DENIED; + IsVerified =3D FALSE; =20 =20 // // Check the image type and get policy setting. // switch (GetImageType (File)) { =20 case IMAGE_FROM_FV: Policy =3D ALWAYS_EXECUTE; break; =20 case IMAGE_FROM_OPTION_ROM: Policy =3D PcdGet32 (PcdOptionRomImageVerificationPolicy); break; =20 case IMAGE_FROM_REMOVABLE_MEDIA: Policy =3D PcdGet32 (PcdRemovableMediaImageVerificationPolicy); break; =20 case IMAGE_FROM_FIXED_MEDIA: Policy =3D PcdGet32 (PcdFixedMediaImageVerificationPolicy); break; =20 default: Policy =3D DENY_EXECUTE_ON_SECURITY_VIOLATION; break; } // // If policy is always/never execute, return directly. // if (Policy =3D=3D ALWAYS_EXECUTE) { return EFI_SUCCESS; } else if (Policy =3D=3D NEVER_EXECUTE) { return EFI_ACCESS_DENIED; } =20 // // The policy QUERY_USER_ON_SECURITY_VIOLATION and ALLOW_EXECUTE_ON_SECU= RITY_VIOLATION // violates the UEFI spec and has been removed. // ASSERT (Policy !=3D QUERY_USER_ON_SECURITY_VIOLATION && Policy !=3D ALLO= W_EXECUTE_ON_SECURITY_VIOLATION); if (Policy =3D=3D QUERY_USER_ON_SECURITY_VIOLATION || Policy =3D=3D ALLO= W_EXECUTE_ON_SECURITY_VIOLATION) { CpuDeadLoop (); } =20 GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID**)&SecureBoot, N= ULL); // // Skip verification if SecureBoot variable doesn't exist. // if (SecureBoot =3D=3D NULL) { return EFI_SUCCESS; } =20 // // Skip verification if SecureBoot is disabled but not AuditMode // if (*SecureBoot =3D=3D SECURE_BOOT_MODE_DISABLE) { FreePool (SecureBoot); return EFI_SUCCESS; } FreePool (SecureBoot); =20 // // Read the Dos header. // if (FileBuffer =3D=3D NULL) { return EFI_INVALID_PARAMETER; } =20 mImageBase =3D (UINT8 *) FileBuffer; mImageSize =3D FileSize; =20 ZeroMem (&ImageContext, sizeof (ImageContext)); ImageContext.Handle =3D (VOID *) FileBuffer; ImageContext.ImageRead =3D (PE_COFF_LOADER_READ_FILE) DxeImageVerificati= onLibImageRead; =20 // // Get information about the image being loaded // Status =3D 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; } =20 Status =3D EFI_ACCESS_DENIED; =20 DosHdr =3D (EFI_IMAGE_DOS_HEADER *) mImageBase; if (DosHdr->e_magic =3D=3D EFI_IMAGE_DOS_SIGNATURE) { // // DOS image header is present, // so read the PE header after the DOS image header. // mPeCoffHeaderOffset =3D DosHdr->e_lfanew; } else { mPeCoffHeaderOffset =3D 0; } // // Check PE/COFF image. // mNtHeader.Pe32 =3D (EFI_IMAGE_NT_HEADERS32 *) (mImageBase + mPeCoffHeade= rOffset); if (mNtHeader.Pe32->Signature !=3D EFI_IMAGE_NT_SIGNATURE) { // // It is not a valid Pe/Coff file. // DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Not a valid PE/COFF imag= e.\n")); goto Done; } =20 if (mNtHeader.Pe32->OptionalHeader.Magic =3D=3D EFI_IMAGE_NT_OPTIONAL_HD= R32_MAGIC) { // // Use PE32 offset. // NumberOfRvaAndSizes =3D mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndS= izes; if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) { SecDataDir =3D (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32->Optiona= lHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; } } else { // // Use PE32+ offset. // NumberOfRvaAndSizes =3D mNtHeader.Pe32Plus->OptionalHeader.NumberOfRva= AndSizes; if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) { SecDataDir =3D (EFI_IMAGE_DATA_DIRECTORY *) &mNtHeader.Pe32Plus->Opt= ionalHeader.DataDirectory[EFI_IMAGE_DIRECTORY_ENTRY_SECURITY]; } } =20 // // Start Image Validation. // if (SecDataDir =3D=3D NULL || SecDataDir->Size =3D=3D 0) { // // This image is not signed. The SHA256 hash value of the image must m= atch 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 im= age using %s.\n", mHashTypeStr)); goto Done; } =20 if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDi= gest, &mCertType, mImageDigestSize)) { // // Image Hash is in forbidden database (DBX). // DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is not signed an= d %s hash of image is forbidden by DBX.\n", mHashTypeStr)); goto Done; } =20 if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageDig= est, &mCertType, mImageDigestSize)) { // // Image Hash is in allowed database (DB). // return EFI_SUCCESS; } =20 // // 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; } =20 // // 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) f= rom the start of the file. // for (OffSet =3D SecDataDir->VirtualAddress; OffSet < (SecDataDir->VirtualAddress + SecDataDir->Size); OffSet +=3D (WinCertificate->dwLength + ALIGN_SIZE (WinCertificate-= >dwLength))) { WinCertificate =3D (WIN_CERTIFICATE *) (mImageBase + OffSet); if ((SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) <=3D size= of (WIN_CERTIFICATE) || (SecDataDir->VirtualAddress + SecDataDir->Size - OffSet) < WinCert= ificate->dwLength) { break; } =20 // // Verify the image's Authenticode signature, only DER-encoded PKCS#7 = signed data is supported. // if (WinCertificate->wCertificateType =3D=3D WIN_CERT_TYPE_PKCS_SIGNED_= DATA) { // // The certificate is formatted as WIN_CERTIFICATE_EFI_PKCS which is= described in the // Authenticode specification. // PkcsCertData =3D (WIN_CERTIFICATE_EFI_PKCS *) WinCertificate; if (PkcsCertData->Hdr.dwLength <=3D sizeof (PkcsCertData->Hdr)) { break; } AuthData =3D PkcsCertData->CertData; AuthDataSize =3D PkcsCertData->Hdr.dwLength - sizeof(PkcsCertData->H= dr); } else if (WinCertificate->wCertificateType =3D=3D WIN_CERT_TYPE_EFI_G= UID) { // // The certificate is formatted as WIN_CERTIFICATE_UEFI_GUID which i= s described in UEFI Spec. // WinCertUefiGuid =3D (WIN_CERTIFICATE_UEFI_GUID *) WinCertificate; if (WinCertUefiGuid->Hdr.dwLength <=3D OFFSET_OF(WIN_CERTIFICATE_UEF= I_GUID, CertData)) { break; } if (!CompareGuid (&WinCertUefiGuid->CertType, &gEfiCertPkcs7Guid)) { continue; } AuthData =3D WinCertUefiGuid->CertData; AuthDataSize =3D WinCertUefiGuid->Hdr.dwLength - OFFSET_OF(WIN_CERTI= FICATE_UEFI_GUID, CertData); } else { if (WinCertificate->dwLength < sizeof (WIN_CERTIFICATE)) { break; } continue; } =20 Status =3D HashPeImageByType (AuthData, AuthDataSize); if (EFI_ERROR (Status)) { continue; } =20 // // Check the digital signature against the revoked certificate in forb= idden database (dbx). // if (IsForbiddenByDbx (AuthData, AuthDataSize)) { Action =3D EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED; - VerifyStatus =3D EFI_ACCESS_DENIED; + IsVerified =3D FALSE; break; } =20 // // Check the digital signature against the valid certificate in allowe= d database (db). // - if (EFI_ERROR (VerifyStatus)) { + if (!IsVerified) { if (IsAllowedByDb (AuthData, AuthDataSize)) { - VerifyStatus =3D EFI_SUCCESS; + IsVerified =3D TRUE; } } =20 // // Check the image's hash value. // if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE1, mImageDi= gest, &mCertType, mImageDigestSize)) { Action =3D 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 =3D EFI_ACCESS_DENIED; + IsVerified =3D FALSE; break; - } else if (EFI_ERROR (VerifyStatus)) { + } else if (!IsVerified) { if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageD= igest, &mCertType, mImageDigestSize)) { - VerifyStatus =3D EFI_SUCCESS; + IsVerified =3D 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)); } } } =20 if (OffSet !=3D (SecDataDir->VirtualAddress + SecDataDir->Size)) { // // The Size in Certificate Table or the attribute certificate table is= corrupted. // - VerifyStatus =3D EFI_ACCESS_DENIED; + IsVerified =3D FALSE; } =20 - if (!EFI_ERROR (VerifyStatus)) { + if (IsVerified) { return EFI_SUCCESS; } else { Status =3D EFI_ACCESS_DENIED; if (Action =3D=3D EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action =3D=3D= EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND) { // // Get image hash value as signature of executable. // SignatureListSize =3D sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGN= ATURE_DATA) - 1 + mImageDigestSize; SignatureList =3D (EFI_SIGNATURE_LIST *) AllocateZeroPool (Signa= tureListSize); if (SignatureList =3D=3D NULL) { Status =3D EFI_OUT_OF_RESOURCES; goto Done; } SignatureList->SignatureHeaderSize =3D 0; SignatureList->SignatureListSize =3D (UINT32) SignatureListSize; SignatureList->SignatureSize =3D (UINT32) (sizeof (EFI_SIGNAT= URE_DATA) - 1 + mImageDigestSize); CopyMem (&SignatureList->SignatureType, &mCertType, sizeof (EFI_GUID= )); Signature =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) SignatureList + size= of (EFI_SIGNATURE_LIST)); CopyMem (Signature->SignatureData, mImageDigest, mImageDigestSize); } } =20 Done: if (Status !=3D EFI_SUCCESS) { // // Policy decides to defer or reject the image; add its information in= image executable information table. // NameStr =3D ConvertDevicePathToText (File, FALSE, TRUE); AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSi= ze); if (NameStr !=3D NULL) { DEBUG((EFI_D_INFO, "The image doesn't pass verification: %s\n", Name= Str)); FreePool(NameStr); } Status =3D EFI_SECURITY_VIOLATION; } =20 if (SignatureList !=3D NULL) { FreePool (SignatureList); } =20 return Status; } =20 /** On Ready To Boot Services Event notification handler. =20 Add the image execution information table if it is not in system configu= ration table. =20 @param[in] Event Event whose notification function is being invoked @param[in] Context Pointer to the notification function's context =20 **/ --=20 2.19.1.3.g30247aa5d201 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-