From nobody Mon Feb 9 23:38:50 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+53322+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+53322+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201648; cv=none; d=zohomail.com; s=zohoarc; b=Q8ZkH0l/5r0b0a7oJw5y13/RHNWp2YI6R5KdRVc11We61eUyJjBtZJV82ycdwnzXWASTljx6donidByqBi0xxPi5EemyXn7gYEHnBNVh3kdw/MTiRFdJnfpd8bzaIBis1xcIK1fBwW/KmGP0UU6ReFMeJj2RCK8nFasJj0q9Y/w= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201648; 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=E6VS03seMuXw8Luw8mRhJxto1QvCPuBrETmPG2u1+KM=; b=derYw+lMdJn51RxSAF7FiDwlku4pSWrpegGcDplYuqE0Toqj3fCSymHieqDe4fNOkBa8QL0z50rP1aKlnBdFUc3fi70OywVSriKBJk1aBV1r0XOjaAhiL92o/bTXY4xj7VA1SfF+PiizOFUON3fvbmcrTUE4PdhBjIf0118piEA= 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+53322+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 15792016481841017.372927846869; Thu, 16 Jan 2020 11:07:28 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id ARWQYY1788612xONgF3zxnGe; Thu, 16 Jan 2020 11:07:27 -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.1471.1579201646852750425 for ; Thu, 16 Jan 2020 11:07:27 -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-205-gpRzJHPYMK-NsQKh8WEQBA-1; Thu, 16 Jan 2020 14:07:20 -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 333808017CC; Thu, 16 Jan 2020 19:07:19 +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 14E1A80890; Thu, 16 Jan 2020 19:07:17 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 07/11] SecurityPkg/DxeImageVerificationHandler: unnest AddImageExeInfo() call Date: Thu, 16 Jan 2020 20:07:01 +0100 Message-Id: <20200116190705.18816-8-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: gpRzJHPYMK-NsQKh8WEQBA-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: NBY0EcHbtFT0BLbgJwywQ6YKx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201647; bh=E6VS03seMuXw8Luw8mRhJxto1QvCPuBrETmPG2u1+KM=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=PtKVirOm8e04TfWno6Zz92WW/VMD1fEE+kJWBWceIH3jFvoxHM2z8eNVxywoFNO7Y0n ToOZ7gy7gy3Zvvahm9hBjgjkCwMQtTNazv4hLcQ9gQOvCilkh9EEgZ+gcIQECIz/Z93nl 9+6jPhiytDjUt3aubiAHS+Blzene7Coin2g= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" Before the "Done" label at the end of DxeImageVerificationHandler(), we now have a single access to "Status": we set "Status" to EFI_ACCESS_DENIED at the top of the function. Therefore, the (Status !=3D EFI_SUCCESS) condition is always true under the "Done" label. Accordingly, unnest the AddImageExeInfo() call dependent on that condition, remove the condition, and also rename the "Done" label to "Failed". Functionally, this patch is a no-op. It's easier to review with: git show -b -W 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 | 34= +++++++++----------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 6ccce1f35843..b98404ab465b 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1555,348 +1555,346 @@ 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; RETURN_STATUS PeCoffStatus; EFI_STATUS HashStatus; =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; 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; } 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 // PeCoffStatus =3D 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 Done; + goto Failed; } =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; + goto Failed; } =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; + goto Failed; } =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; + goto Failed; } =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; + goto Failed; } =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 HashStatus =3D HashPeImageByType (AuthData, AuthDataSize); if (EFI_ERROR (HashStatus)) { 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; IsVerified =3D FALSE; break; } =20 // // Check the digital signature against the valid certificate in allowe= d database (db). // if (!IsVerified) { if (IsAllowedByDb (AuthData, AuthDataSize)) { 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)); IsVerified =3D FALSE; break; } if (!IsVerified) { if (IsSignatureFoundInDatabase (EFI_IMAGE_SECURITY_DATABASE, mImageD= igest, &mCertType, mImageDigestSize)) { 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. // IsVerified =3D FALSE; } =20 if (IsVerified) { return EFI_SUCCESS; } if (Action =3D=3D EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED || Action =3D=3D E= FI_IMAGE_EXECUTION_AUTH_SIG_FOUND) { // // Get image hash value as signature of executable. // SignatureListSize =3D sizeof (EFI_SIGNATURE_LIST) + sizeof (EFI_SIGNAT= URE_DATA) - 1 + mImageDigestSize; SignatureList =3D (EFI_SIGNATURE_LIST *) AllocateZeroPool (Signatu= reListSize); if (SignatureList =3D=3D NULL) { - goto Done; + goto Failed; } SignatureList->SignatureHeaderSize =3D 0; SignatureList->SignatureListSize =3D (UINT32) SignatureListSize; SignatureList->SignatureSize =3D (UINT32) (sizeof (EFI_SIGNATUR= E_DATA) - 1 + mImageDigestSize); CopyMem (&SignatureList->SignatureType, &mCertType, sizeof (EFI_GUID)); Signature =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) SignatureList + sizeof= (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; +Failed: + // + // Policy decides to defer or reject the image; add its information in i= mage executable information table. + // + NameStr =3D ConvertDevicePathToText (File, FALSE, TRUE); + AddImageExeInfo (Action, NameStr, File, SignatureList, SignatureListSize= ); + if (NameStr !=3D NULL) { + DEBUG((EFI_D_INFO, "The image doesn't pass verification: %s\n", NameSt= r)); + 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 (#53322): https://edk2.groups.io/g/devel/message/53322 Mute This Topic: https://groups.io/mt/69752226/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-