From nobody Sun May 19 23:41:33 2024 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- From nobody Sun May 19 23:41:33 2024 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+53315+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+53315+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201639; cv=none; d=zohomail.com; s=zohoarc; b=Pj3yzuFUO0wiW05KDskVte4iIRGfwsftLCp1mK6fkw0TF/T3sRptQ/pCIi3WRUXTJlfjkXx8Coftv7ah0oQPNA1nS18z4bQs6yrLgJ8mGrXBAF0gEdplmrVJeiROzQNI0M5kOC5/xWC06ptHe49ZIjz3Q9DetgeE4J4aE/GfNYU= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201639; 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=Yn1LtQArQYNqJp0a8AeBEMEdBd7wuRehVnTAas4+2r0=; b=TRkakTz4Zpp9CsZOAdXWiUNjDRrGkKBILRUDCHPeqzQYrLho8Zm/lwQWCVHf6fUG33B/44svhBFioWu5eF+UaiBu9b6UpjPFPTwgu3lTv73Xfmn9c5EfJ1HOelYJkbc+SIz0UobXjO2IPW+FHqgXZ+V+L9WIqd26o+0vYyJo8fY= 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+53315+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 1579201639712536.7854849229888; Thu, 16 Jan 2020 11:07:19 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id CAUrYY1788612xvfaeNh9UMq; Thu, 16 Jan 2020 11:07:19 -0800 X-Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.1415.1579201638616189528 for ; Thu, 16 Jan 2020 11:07:18 -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-145-oxviG3u1O6CKk_g6rWDlBw-1; Thu, 16 Jan 2020 14:07:13 -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 D692A800D5E; Thu, 16 Jan 2020 19:07:11 +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 8C6DB80890; Thu, 16 Jan 2020 19:07:10 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 02/11] SecurityPkg/DxeImageVerificationHandler: remove "else" after return/break Date: Thu, 16 Jan 2020 20:06:56 +0100 Message-Id: <20200116190705.18816-3-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: oxviG3u1O6CKk_g6rWDlBw-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: mJPuKaLQvcxLuXDhHLReBZTHx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201639; bh=Yn1LtQArQYNqJp0a8AeBEMEdBd7wuRehVnTAas4+2r0=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=RDzednt/sMy1ezV6Tw12vqK9r5R/Wk6fdb3pBf3c6xpObllCw/bc2PLhuMRHc2LloJ9 J6Qe11EOko9w17bT2TfSNAcR9ExJWXUpg//5wJYhKhISt6tj6InhnOJRce4trqKbja/SR gcMl6516DanOLYndDiC46c+ygAObjgDUSI0= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" 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 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 | 41= ++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.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; =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; - } else if (Policy =3D=3D NEVER_EXECUTE) { + } + 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; 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; - } else if (!IsVerified) { + } + 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; - } 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); + } + Status =3D EFI_ACCESS_DENIED; + 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) { + Status =3D EFI_OUT_OF_RESOURCES; + goto Done; } + 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; } =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 (#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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- From nobody Sun May 19 23:41:33 2024 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+53316+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+53316+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=Cw5Kutt2BAVxKqNQs5zSE24/vJjXJiiP7DTqmoXP6HBG2ICN27RzmzO9afF/cIo2Ul1yqtH82XnZGxwRLZ6lxG9eFsXC4XIq4T4psiWgHiHLQgQaj+NtF4khzm4jGO03j3WJtg42Mf5J6nFhI+XjXtFc2yDOa0wc5Tx2HnWED3U= 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=jTCpwEaE5CebjctdeSlhT8OfLEWZi7JL9+gSWE7Zrp0=; b=UUF4MaVR2QbXDWoR69K3kOQZhWJ9mIVmST2qQYJE1Ntla6rt5OTCsVtRpUvQfpM6vpqywF1wk6Rq4feOtQ34gk8N1Mv6IiO6TrPAaVecEU0OYl4RSOfUskmEIyDB9vUYDwpz5bhWcdmaWo0cl6KbcSexrZReOPCMMMKLLlY9EYU= 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+53316+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 1579201640017980.6205533977197; Thu, 16 Jan 2020 11:07:20 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id vkYBYY1788612x9Of5te3SNs; Thu, 16 Jan 2020 11:07:19 -0800 X-Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web12.1435.1579201638889166706 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-400-jG9zNqjRMkiwfDmBculgrg-1; Thu, 16 Jan 2020 14:07:14 -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 536648017CC; Thu, 16 Jan 2020 19:07:13 +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 3497680890; Thu, 16 Jan 2020 19:07:12 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 03/11] SecurityPkg/DxeImageVerificationHandler: keep PE/COFF info status internal Date: Thu, 16 Jan 2020 20:06:57 +0100 Message-Id: <20200116190705.18816-4-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: jG9zNqjRMkiwfDmBculgrg-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: 5edoU0xt89AMssPgWJhPnqrAx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201639; bh=jTCpwEaE5CebjctdeSlhT8OfLEWZi7JL9+gSWE7Zrp0=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=MasYQW27tFZWv7HMR+gCBjI0bnWa92qcM0Hma6ohM5e+wWdi5JTL6ftCjnx+kjqBmIO 3FZ5SNOIsbhZhfRvkLJh62xiSaIu2xSRG7onohK/q//9R+kmeqru6xBhaA7zvpmPWCUTL R8ML8gqf+CO/buwbej3ta/FtPkI4pgZTC8I= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" The PeCoffLoaderGetImageInfo() function may return various error codes, such as RETURN_INVALID_PARAMETER and RETURN_UNSUPPORTED. Such error values should not be assigned to our "Status" variable in the DxeImageVerificationHandler() function, because "Status" generally stands for the main exit value of the function. And SECURITY2_FILE_AUTHENTICATION_HANDLER functions are expected to return one of EFI_SUCCESS, EFI_SECURITY_VIOLATION, and EFI_ACCESS_DENIED only. Introduce the "PeCoffStatus" helper variable for keeping the return value of PeCoffLoaderGetImageInfo() internal to the function. If PeCoffLoaderGetImageInfo() fails, we'll jump to the "Done" label with "Status" being EFI_ACCESS_DENIED, inherited from the top of the function. Note that this is consistent with the subsequent PE/COFF Signature check, where we jump to the "Done" label with "Status" having been re-set to EFI_ACCESS_DENIED. As a consequence, we can at once remove the Status =3D EFI_ACCESS_DENIED; assignment right after the "PeCoffStatus" check. This patch does not change the control flow in the function, it only changes the "Status" outcome from API-incompatible error codes to EFI_ACCESS_DENIED, under some circumstances. 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 | 7 = +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 8204c9c0f105..e6c8a5408752 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1556,350 +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; 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; =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 // - Status =3D PeCoffLoaderGetImageInfo (&ImageContext); - if (EFI_ERROR (Status)) { + 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; } =20 - Status =3D EFI_ACCESS_DENIED; - 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; 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; } Status =3D EFI_ACCESS_DENIED; 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) { Status =3D EFI_OUT_OF_RESOURCES; goto Done; } 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; } =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 (#53316): https://edk2.groups.io/g/devel/message/53316 Mute This Topic: https://groups.io/mt/69752220/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- From nobody Sun May 19 23:41:33 2024 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+53318+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+53318+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201641; cv=none; d=zohomail.com; s=zohoarc; b=YK91pX/VUltAzEJh0b6XoGaDeAAsWKTSY1cuaJW1ZXfuKjC9eJbPQD1rmd7u2X2iayQvGK0hTQoJm+cLRQ0raS+71A3J2sU5NnjANnPDtcqs7Kss53BOxvus0sPv34CMdQrWZlWPU6sK5lRfpexjbv/nCKS9uXrPKTP4s2a9pF0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201641; 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=/GK6h6p/sKUciybrcBP3vSi2aGFHIyG8KrpB0Vb9mlw=; b=Oc74cfXGd+0OYLuuqv0UvWQ0oafLeOanK1R+IWp+kgfq1HOEOsDBpW2mgbfabgIFgo+c6RCo6dgKykiWLOYrw39bR/EhJbUMwk/sP7JTNtMowRB4I22pRiLcZFwReRjOxJ/pIpN32OYHSjlyKkgenCaiXiqWUvzYH2byUZL5OFg= 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+53318+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 1579201641057237.8405569230838; Thu, 16 Jan 2020 11:07:21 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id xrenYY1788612xJ158rB2LPT; Thu, 16 Jan 2020 11:07:20 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.1436.1579201639959809261 for ; Thu, 16 Jan 2020 11:07:20 -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-174-LKCc0JB0OECKx4SrCa4gfg-1; Thu, 16 Jan 2020 14:07:15 -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 C39141005502; Thu, 16 Jan 2020 19:07:14 +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 A7E0184333; Thu, 16 Jan 2020 19:07:13 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 04/11] SecurityPkg/DxeImageVerificationHandler: narrow down PE/COFF hash status Date: Thu, 16 Jan 2020 20:06:58 +0100 Message-Id: <20200116190705.18816-5-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: LKCc0JB0OECKx4SrCa4gfg-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: dr9t7IN7supVa81yrV5P7mdjx1787277AA= 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=/GK6h6p/sKUciybrcBP3vSi2aGFHIyG8KrpB0Vb9mlw=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=hzA1IG4AO9rwsyXPDjx9idV67EaUMmp0ZdCCblgnOsjMX7dMCmDol1ARyNKUdFvcY33 LtdFvZeTPF0OsYTvEGl9k0Js0yja49KwMeQpwbywCP59VZrdvINhj+KfoOY3NTsq+rbLZ uXfCwld74/yc/qHAO0uKZdTpL5V9D9wyF5Q= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" Inside the "for" loop that scans the signatures of the image, we call HashPeImageByType(), and assign its return value to "Status". Beyond the immediate retval check, this assignment is useless (never consumed). That's because a subsequent access to "Status" may only be one of the following: - the "Status" assignment when we call HashPeImageByType() in the next iteration of the loop, - the "Status =3D EFI_ACCESS_DENIED" assignment right after the final "IsVerified" check. To make it clear that the assignment is only useful for the immediate HashPeImageByType() retval check, introduce a specific helper variable, called "HashStatus". This patch is a no-op, functionally. 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 | 5 = +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index e6c8a5408752..5cc82c1b3b22 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; 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; } =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)) { + 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; } Status =3D EFI_ACCESS_DENIED; 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) { Status =3D EFI_OUT_OF_RESOURCES; goto Done; } 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; } =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 (#53318): https://edk2.groups.io/g/devel/message/53318 Mute This Topic: https://groups.io/mt/69752222/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- From nobody Sun May 19 23:41:33 2024 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+53319+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+53319+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201642; cv=none; d=zohomail.com; s=zohoarc; b=dfDCI9O1neAFQGQZS1EuLalvrq/OMFtDOB5SS7VtSEuWSmeetRtr42R65m5bMfd5xtfI4BoMRQaFWKa/NECegSvDxweYLndLEY57KBtVk5xVft9xTm8mcpBEoEUbNlpBtBOEgJghm70sPjOHGCRsYmQbZv32+27sjn74ar09SXs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201642; 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=sVFUZ+vnvmXEwSLr5IXSYzLphU6bvuKRAkHU4bf8xPw=; b=jBvNBIu9pNUQL3tbxfqfjY7v2vLfiyw8RSU//kGg3/YWqooORDDFVSB0DPdEGfj6ttfRR32tO2vmPsMSiGhGpJIFU6APbAtJTiKKs2BF2XZuTcoCqexQYBkiGtdyGMEDs57y9pg2t6TGDCaS/g0qKho5i1IkYWbJIrp7tLQvFoM= 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+53319+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 1579201642791250.3727605990092; Thu, 16 Jan 2020 11:07:22 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id 5I8BYY1788612xTEt3goUczR; Thu, 16 Jan 2020 11:07:22 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.1437.1579201641616157700 for ; Thu, 16 Jan 2020 11:07:21 -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-266-hc1oEyhEMhySDEOkgCZjhg-1; Thu, 16 Jan 2020 14:07:17 -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 41E7B183B520; Thu, 16 Jan 2020 19:07:16 +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 24D5F80890; Thu, 16 Jan 2020 19:07:14 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 05/11] SecurityPkg/DxeImageVerificationHandler: fix retval on memalloc failure Date: Thu, 16 Jan 2020 20:06:59 +0100 Message-Id: <20200116190705.18816-6-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: hc1oEyhEMhySDEOkgCZjhg-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: 0aZyCj4VhDodD9hJ7y5wdW6Zx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201642; bh=sVFUZ+vnvmXEwSLr5IXSYzLphU6bvuKRAkHU4bf8xPw=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=twXGHfBme2Z0gQ876oPCWdIp+cxXSUlis6BJnrjSL8FBQ/qEndREYjw47QrGOoteHrI 0oEtzssTGl67aQXhQK/xM9II9yRkTep1RQyyrducUDtH/C17h9cJegua0v1FzZjosqLvw efdMU6jQxi/7c28dBq4+1RQbJ99TQOCyX0A= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" A SECURITY2_FILE_AUTHENTICATION_HANDLER function is not expected to return EFI_OUT_OF_RESOURCES. We should only return EFI_SUCCESS, EFI_SECURITY_VIOLATION, or EFI_ACCESS_DENIED. In case we run out of memory while preparing "SignatureList" for AddImageExeInfo(), we should simply stick with the EFI_ACCESS_DENIED value that is already in "Status" -- from just before the "Action" condition --, and not suppress it with EFI_OUT_OF_RESOURCES. This patch does not change the control flow in the function, it only changes the "Status" outcome from API-incompatible error codes to EFI_ACCESS_DENIED, under some circumstances. Cc: Chao Zhang Cc: Jian J Wang Cc: Jiewen Yao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2129 Fixes: 570b3d1a7278df29878da87990e8366bd42d0ec5 Signed-off-by: Laszlo Ersek --- SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 2 = -- 1 file changed, 2 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 5cc82c1b3b22..5f09a66bc9ce 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1384,170 +1384,169 @@ 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; =20 Data =3D NULL; CertList =3D NULL; CertData =3D NULL; RootCert =3D NULL; DbxData =3D NULL; RootCertSize =3D 0; VerifyStatus =3D FALSE; =20 DataSize =3D 0; Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSe= curityDatabaseGuid, NULL, &DataSize, NULL); if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { Data =3D (UINT8 *) AllocateZeroPool (DataSize); if (Data =3D=3D NULL) { return VerifyStatus; } =20 Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSe= curityDatabaseGuid, NULL, &DataSize, (VOID *) Data); if (EFI_ERROR (Status)) { goto Done; } =20 // // Find X509 certificate in Signature List to verify the signature in = pkcs7 signed data. // CertList =3D (EFI_SIGNATURE_LIST *) Data; while ((DataSize > 0) && (DataSize >=3D CertList->SignatureListSize)) { if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) { CertData =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof = (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize); CertCount =3D (CertList->SignatureListSize - sizeof (EFI_SIGNATURE= _LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize; =20 for (Index =3D 0; Index < CertCount; Index++) { // // Iterate each Signature Data Node within this CertList for ver= ify. // RootCert =3D CertData->SignatureData; RootCertSize =3D CertList->SignatureSize - sizeof (EFI_GUID); =20 // // Call AuthenticodeVerify library to Verify Authenticode struct. // VerifyStatus =3D AuthenticodeVerify ( AuthData, AuthDataSize, RootCert, RootCertSize, mImageDigest, mImageDigestSize ); if (VerifyStatus) { // // Here We still need to check if this RootCert's Hash is revo= ked // Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &= gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { goto Done; } DbxData =3D (UINT8 *) AllocateZeroPool (DbxDataSize); if (DbxData =3D=3D NULL) { goto Done; } =20 Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gE= fiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData); if (EFI_ERROR (Status)) { goto Done; } =20 if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SI= GNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) { // // Check the timestamp signature and signing time to determi= ne if the RootCert can be trusted. // VerifyStatus =3D PassTimestampCheck (AuthData, AuthDataSize,= &RevocationTime); if (!VerifyStatus) { DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is sig= ned and signature is accepted by DB, but its root cert failed the timestamp= check.\n")); } } =20 goto Done; } =20 CertData =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertLi= st->SignatureSize); } } =20 DataSize -=3D CertList->SignatureListSize; CertList =3D (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->= SignatureListSize); } } =20 Done: =20 if (VerifyStatus) { SecureBootHook (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabas= eGuid, CertList->SignatureSize, CertData); } =20 if (Data !=3D NULL) { FreePool (Data); } if (DbxData !=3D NULL) { FreePool (DbxData); } =20 return VerifyStatus; } =20 /** Provide verification service for signed images, which include both signa= ture validation and platform policy control. For signature types, both UEFI WIN_CERTIFIC= ATE_UEFI_GUID and MSFT Authenticode type signatures are supported. =20 In this implementation, only verify external executables when in USER MO= DE. Executables from FV is bypass, so pass in AuthenticationStatus is ignore= d. =20 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 v= alue 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 securi= ty database "db", and not be reflected in the security data base "dbx". =20 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. =20 @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 fil= e 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 devi= ce path. @param[in] BootPolicy A boot policy that was used to call LoadImage()= UEFI service. =20 @retval EFI_SUCCESS The file specified by DevicePath and non-= NULL FileBuffer did authenticate, and the plat= form 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 m= ay execute the image in FileBuffer. - @retval EFI_OUT_RESOURCE Fail to allocate memory. @retval EFI_SECURITY_VIOLATION The file specified by File did not authen= ticate, and the platform policy dictates that File sh= ould be placed in the untrusted state. The image has bee= n added to the file execution table. @retval EFI_ACCESS_DENIED The file specified by File and FileBuffer= did not authenticate, and the platform policy dic= tates that the DXE Foundation many not use File. =20 **/ @@ -1556,350 +1555,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; 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; } =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 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; } Status =3D EFI_ACCESS_DENIED; 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) { - Status =3D EFI_OUT_OF_RESOURCES; goto Done; } 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; } =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 (#53319): https://edk2.groups.io/g/devel/message/53319 Mute This Topic: https://groups.io/mt/69752223/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- From nobody Sun May 19 23:41:33 2024 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+53320+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+53320+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201644; cv=none; d=zohomail.com; s=zohoarc; b=iWbOPZnRzFkUP7946hdi58+fDL9OKEE2failLQG0ai7IZTh9ZPmfDYfeO7ZrbmoGL0HcwmusRHSQtA/rRgrnVTOpgMwar/C5/SHDb2HtKUQPDy9e4OMBwXkYFrxGWbzSlv3NPscU+67UNz7uCTCoVu49CAu36sou0v/etl+vQkY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201644; 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=Jl8MJemLfy/029MJlQUC33x4cDR7IqhjmovOOeLWkA0=; b=aiAKHl4PIKLFpWQYst2T80YX5J9NfDTCHWVshofo2DTefWqGNYDEPDUoVNwcldLdFmkEU1cpmrrRH6wsfYK179g+Wi8KAs0PoH650XF6V5OAVAoqvaDlJR1bzlELmToh9FwbtJeUmOAeP/b6s1N90WRB6v7i+Hag53myq8IAiGs= 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+53320+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 1579201644373240.29538520367544; Thu, 16 Jan 2020 11:07:24 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id IyhjYY1788612xQwzplCM7u3; Thu, 16 Jan 2020 11:07:23 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web09.1482.1579201643182993957 for ; Thu, 16 Jan 2020 11:07:23 -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-52-X6nAwjf8P5Kyj9T6T04nGw-1; Thu, 16 Jan 2020 14:07:18 -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 B6583183B532; Thu, 16 Jan 2020 19:07:17 +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 96FDC80890; Thu, 16 Jan 2020 19:07:16 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 06/11] SecurityPkg/DxeImageVerificationHandler: remove superfluous Status setting Date: Thu, 16 Jan 2020 20:07:00 +0100 Message-Id: <20200116190705.18816-7-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: X6nAwjf8P5Kyj9T6T04nGw-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: unkt4iVQNhBs6HZzMO6byY3gx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201643; bh=Jl8MJemLfy/029MJlQUC33x4cDR7IqhjmovOOeLWkA0=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=Mc76ZAScE/tIbH+hdtu8kfQkGj8rLVaxVlAYgIOmyu9mn7/oiu3nU3ncnp1fUM5pEx2 +Szr5wxcXF1v58IVCFsouEjxD6jUPko8dJGVXq79QAL2lZ6NtDdOD3/IVAf+CFwgFEQTt pbUHn1OS1IVdpt7H4Oxtd+QU9ing7Amq5DU= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" After the final "IsVerified" check, we set "Status" to EFI_ACCESS_DENIED. This is superfluous, as "Status" already carries EFI_ACCESS_DENIED value there, from the top of the function. Remove the assignment. Functionally, this change is a no-op. 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 | 1 - 1 file changed, 1 deletion(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 5f09a66bc9ce..6ccce1f35843 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1555,349 +1555,348 @@ 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; } =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 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; } - Status =3D EFI_ACCESS_DENIED; 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; } 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; } =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 (#53320): https://edk2.groups.io/g/devel/message/53320 Mute This Topic: https://groups.io/mt/69752224/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- From nobody Sun May 19 23:41:33 2024 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- From nobody Sun May 19 23:41:33 2024 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+53323+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+53323+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=hsE2zf5IPSLbPJRM2RFHAT9xkRKmMoJiLPS3yhJ3pxLeLS43G1CqYWxhvWr4qzBd3Aa50WQB6IoQeGzOIPnCybklXWDooeWviJZUKonnDSktE7UpUBxEZ6Gjngevz7Q56KaCsdp/Gk1Sq49lmSe7XRWFrbAfBFeQdGPQOM8pkSU= 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=L2xa7Ke5755cXCBY9S40UTrwjvFLPygaV4IXU5P128g=; b=VxwYLiXK30cW7zNlaND8rGTTsjGPR7bgfGyLK82+vm0pAF3xOL5uireFe3lwvXK1SM6sY+lceLkRJCEsN7TLqXdarVnDUxs4VVQxGeyp640deANJkUIW1ywseI8w0U6I0WGOY9KCpH5zbVYUfOFpOJ67XYU1wn4ygSRach7UN/s= 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+53323+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 1579201648236977.2496495721116; Thu, 16 Jan 2020 11:07:28 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id CRVKYY1788612xg0VIz7Y0PX; 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.web12.1440.1579201646854129787 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-31-W_luB2xzPB6nLOBmxVChNQ-1; Thu, 16 Jan 2020 14:07:22 -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 A5E55800EBF; Thu, 16 Jan 2020 19:07:20 +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 8785C80890; Thu, 16 Jan 2020 19:07:19 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 08/11] SecurityPkg/DxeImageVerificationHandler: eliminate "Status" variable Date: Thu, 16 Jan 2020 20:07:02 +0100 Message-Id: <20200116190705.18816-9-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: W_luB2xzPB6nLOBmxVChNQ-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: rT08EXfEXk3YDMR85SdIKlE8x1787277AA= 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=L2xa7Ke5755cXCBY9S40UTrwjvFLPygaV4IXU5P128g=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=SDZQOFr4ORNc9ZiKugn0vWCDeyLcXAMjxTks/W14qq40lDnmqML2nwDLOImadiM3SHp xJV7fgZGhuD7Q/51a6MWUaE80NgO5v3zPrL67EovVuTBC9fs3TW3IHVc7A932obyjC6rD BpCQRmsSuDkv8Kn0nX6wLlu07XUUetscDXY= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" The "Status" variable is set to EFI_ACCESS_DENIED at the top of the function. Then it is overwritten with EFI_SECURITY_VIOLATION under the "Failed" (earlier: "Done") label. We finally return "Status". The above covers the complete usage of "Status" in DxeImageVerificationHandler(). Remove the variable, and simply return EFI_SECURITY_VIOLATION in the end. 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 | 5 = +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index b98404ab465b..3041c8718beb 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1555,346 +1555,343 @@ 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 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 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 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 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 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 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 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; + return EFI_SECURITY_VIOLATION; } =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 (#53323): https://edk2.groups.io/g/devel/message/53323 Mute This Topic: https://groups.io/mt/69752227/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- From nobody Sun May 19 23:41:33 2024 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+53321+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+53321+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201647; cv=none; d=zohomail.com; s=zohoarc; b=KCtlAILJMvWHcIjpLhW0KJNvDOLDhMXdZ6wnOdu7omLtbzYN1pI9Mkwz5ZJeidAfXkqQYxChKXstXjWAC9mDydQ0KWdGa47sv54kFNGdfYUhrThrc7iUEMkWaVcV24GTrCLGBTWXlHQ4yrwLZ+V7fzrL6zPF+B+L5NxcI3U/XhI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201647; 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=bvyr6m65t8cSDc+Ofx7lDTT9hg/y98BYBP1vVSMt5AE=; b=ZqY3D0Yech1GLv23cGcYiaqWhJQJOQqgbtR0LaalYXFLAaBTHfRb2Zmv7QC015WG4x6GQL2SEzmVjxsA7nivC+kSaUXtDSmfWAs7lZ/5swG2dh6TJkRY03lXLCvT8Emn617bSjPU6bjXCW0gC/NtwrpdSlNtxorJdT5xZyn/uJY= 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+53321+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 1579201647183119.71267166939833; Thu, 16 Jan 2020 11:07:27 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id iwQzYY1788612xs8zq0GtXmC; Thu, 16 Jan 2020 11:07:26 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web09.1484.1579201646001811597 for ; Thu, 16 Jan 2020 11:07:26 -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-342-dskqzkfXN5Ko-_8bbFv60Q-1; Thu, 16 Jan 2020 14:07:23 -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 251FD107ACCA; Thu, 16 Jan 2020 19:07:22 +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 0595580890; Thu, 16 Jan 2020 19:07:20 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 09/11] SecurityPkg/DxeImageVerificationHandler: fix retval for (FileBuffer==NULL) Date: Thu, 16 Jan 2020 20:07:03 +0100 Message-Id: <20200116190705.18816-10-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: dskqzkfXN5Ko-_8bbFv60Q-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: zjbZaweg27enU7JIOhj8bXO2x1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201646; bh=bvyr6m65t8cSDc+Ofx7lDTT9hg/y98BYBP1vVSMt5AE=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=DJZNNSzfCWtIrxWkF0sFpgB4H22Gpbx0PEqXG9kymaiRmxpVLWkws3kJX2iCYF12fCx JVTsXdDnkmrDI3SYz6aytq23Pzl01qgnxElrtMV62W1ejXdmHUriFpaikZGDfUnUQpNXg 0g2sQ0Q69x0BJLsxwR900ifq9CVlJIGm3Bs= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" "FileBuffer" is a non-optional input (pointer) parameter to DxeImageVerificationHandler(). Normally, when an edk2 function receives a NULL argument for such a parameter, we return EFI_INVALID_PARAMETER or RETURN_INVALID_PARAMETER. However, those don't conform to the SECURITY2_FILE_AUTHENTICATION_HANDLER prototype. Return EFI_ACCESS_DENIED when "FileBuffer" is NULL; it means that no image has been loaded. This patch does not change the control flow in the function, it only changes the "Status" outcome from API-incompatible error codes to EFI_ACCESS_DENIED, under some circumstances. Cc: Chao Zhang Cc: Jian J Wang Cc: Jiewen Yao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2129 Fixes: 570b3d1a7278df29878da87990e8366bd42d0ec5 Signed-off-by: Laszlo Ersek --- SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 2 = +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index 3041c8718beb..f6ad5931be07 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -1555,343 +1555,343 @@ 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; =20 SignatureList =3D NULL; SignatureListSize =3D 0; WinCertificate =3D NULL; SecDataDir =3D NULL; PkcsCertData =3D NULL; Action =3D EFI_IMAGE_EXECUTION_AUTH_UNTESTED; 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; + return EFI_ACCESS_DENIED; } =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 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 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 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 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 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 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 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); } =20 if (SignatureList !=3D NULL) { FreePool (SignatureList); } =20 return EFI_SECURITY_VIOLATION; } =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 (#53321): https://edk2.groups.io/g/devel/message/53321 Mute This Topic: https://groups.io/mt/69752225/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- From nobody Sun May 19 23:41:33 2024 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+53324+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+53324+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201650; cv=none; d=zohomail.com; s=zohoarc; b=BXLYvqZM0Jcr03yBtYaNMz/NZc9lab1OaqlCcwV0HUnHWiIwOCP78rNVYTFTOb8bTwS3VPjOHmpzbMJjSoMrW5I+wQiSHphTeWNom3QczFX/ipGddn1QguvRQqao0wy4CD0RINcuj8xZtmJ/w64Hn1ESSZwi76DLKqVbI7JoDXM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201650; 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=4JcamRuq6xgIz05FpBU9lS3WYDiv1JTIWklFIFUz0Ec=; b=A+lOvzmmTr8HEQxnb0zO7raYezfajU6Tmlgw9ucZdFAa5il2UtZOEwAe7eh3/KXnxWdZu+EgIfh3nAYnq9P7a74Ohg+yiJhQQWBoTxeynh1m5dunvydynxz1XLPioHoEE2YzMmucFQZWJJUtYIj/tX7bjN67wMcNEQgHCFkuep4= 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+53324+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 1579201650547772.2879036967175; Thu, 16 Jan 2020 11:07:30 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id NX5fYY1788612xd7vn0m627J; Thu, 16 Jan 2020 11:07:30 -0800 X-Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.1419.1579201649298982079 for ; Thu, 16 Jan 2020 11:07:29 -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-404-s1yaIvVeOn-vRNndUxckmg-1; Thu, 16 Jan 2020 14:07:25 -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 98E8310054E3; Thu, 16 Jan 2020 19:07:23 +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 7720680890; Thu, 16 Jan 2020 19:07:22 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 10/11] SecurityPkg/DxeImageVerificationHandler: fix imgexec info on memalloc fail Date: Thu, 16 Jan 2020 20:07:04 +0100 Message-Id: <20200116190705.18816-11-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: s1yaIvVeOn-vRNndUxckmg-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: h6U1mg9CiB29YSoBfRTXFalFx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201650; bh=4JcamRuq6xgIz05FpBU9lS3WYDiv1JTIWklFIFUz0Ec=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=lfwB/FF2TrvRLVJIfbCYfiiw8khrxxwqX9LPhRMdQxI/KP17k7EzfRq3/+RRpKZjX7b 3y0G2WhblcJxO3OC0rljnOd+xzOQ0LrUwGpS62wCigx7Zu9FDI5bpyMS7Kc9iP/fUrnxu uUUMX5ZGo9VricGufzIVfrXf9cgt7Ra180Q= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" It makes no sense to call AddImageExeInfo() with (Signature =3D=3D NULL) and (SignatureSize > 0). AddImageExeInfo() does not crash in such a case -- it avoids the CopyMem() call --, but it creates an invalid EFI_IMAGE_EXECUTION_INFO record. Namely, the "EFI_IMAGE_EXECUTION_INFO.InfoSize" field includes "SignatureSize", but the actual signature bytes are not filled in. Document and ASSERT() this condition in AddImageExeInfo(). In DxeImageVerificationHandler(), zero out "SignatureListSize" if we set "SignatureList" to NULL due to AllocateZeroPool() failure. (Another approach could be to avoid calling AddImageExeInfo() completely, in case AllocateZeroPool() fails. Unfortunately, the UEFI v2.8 spec does not seem to state clearly whether a signature is mandatory in EFI_IMAGE_EXECUTION_INFO, if the "Action" field is EFI_IMAGE_EXECUTION_AUTH_SIG_FAILED or EFI_IMAGE_EXECUTION_AUTH_SIG_FOUND. For now, the EFI_IMAGE_EXECUTION_INFO addition logic is not changed; we only make sure that the record we add is not malformed.) 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 | 4 = +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.c index f6ad5931be07..53ef340c08ad 100644 --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c @@ -678,151 +678,152 @@ UINTN GetImageExeInfoTableSize ( EFI_IMAGE_EXECUTION_INFO_TABLE *ImageExeInfoTable ) { UINTN Index; EFI_IMAGE_EXECUTION_INFO *ImageExeInfoItem; UINTN TotalSize; =20 if (ImageExeInfoTable =3D=3D NULL) { return 0; } =20 ImageExeInfoItem =3D (EFI_IMAGE_EXECUTION_INFO *) ((UINT8 *) ImageExeIn= foTable + sizeof (EFI_IMAGE_EXECUTION_INFO_TABLE)); TotalSize =3D sizeof (EFI_IMAGE_EXECUTION_INFO_TABLE); for (Index =3D 0; Index < ImageExeInfoTable->NumberOfImages; Index++) { TotalSize +=3D ReadUnaligned32 ((UINT32 *) &ImageExeInfoItem->InfoSize= ); ImageExeInfoItem =3D (EFI_IMAGE_EXECUTION_INFO *) ((UINT8 *) ImageExeI= nfoItem + ReadUnaligned32 ((UINT32 *) &ImageExeInfoItem->InfoSize)); } =20 return TotalSize; } =20 /** Create an Image Execution Information Table entry and add it to system c= onfiguration table. =20 @param[in] Action Describes the action taken by the firmware r= egarding this image. @param[in] Name Input a null-terminated, user-friendly name. @param[in] DevicePath Input device path pointer. @param[in] Signature Input signature info in EFI_SIGNATURE_LIST d= ata structure. - @param[in] SignatureSize Size of signature. + @param[in] SignatureSize Size of signature. Must be zero if Signature= is NULL. =20 **/ VOID AddImageExeInfo ( IN EFI_IMAGE_EXECUTION_ACTION Action, IN CHAR16 *Name OPTIONAL, IN CONST EFI_DEVICE_PATH_PROTOCOL *DevicePath, IN EFI_SIGNATURE_LIST *Signature OPTIONAL, IN UINTN SignatureSize ) { EFI_IMAGE_EXECUTION_INFO_TABLE *ImageExeInfoTable; EFI_IMAGE_EXECUTION_INFO_TABLE *NewImageExeInfoTable; EFI_IMAGE_EXECUTION_INFO *ImageExeInfoEntry; UINTN ImageExeInfoTableSize; UINTN NewImageExeInfoEntrySize; UINTN NameStringLen; UINTN DevicePathSize; CHAR16 *NameStr; =20 ImageExeInfoTable =3D NULL; NewImageExeInfoTable =3D NULL; ImageExeInfoEntry =3D NULL; NameStringLen =3D 0; NameStr =3D NULL; =20 if (DevicePath =3D=3D NULL) { return ; } =20 if (Name !=3D NULL) { NameStringLen =3D StrSize (Name); } else { NameStringLen =3D sizeof (CHAR16); } =20 EfiGetSystemConfigurationTable (&gEfiImageSecurityDatabaseGuid, (VOID **= ) &ImageExeInfoTable); if (ImageExeInfoTable !=3D NULL) { // // The table has been found! // We must enlarge the table to accommodate the new exe info entry. // ImageExeInfoTableSize =3D GetImageExeInfoTableSize (ImageExeInfoTable); } else { // // Not Found! // We should create a new table to append to the configuration table. // ImageExeInfoTableSize =3D sizeof (EFI_IMAGE_EXECUTION_INFO_TABLE); } =20 DevicePathSize =3D GetDevicePathSize (DevicePath); =20 // // Signature size can be odd. Pad after signature to ensure next EXECUTI= ON_INFO entry align // + ASSERT (Signature !=3D NULL || SignatureSize =3D=3D 0); NewImageExeInfoEntrySize =3D sizeof (EFI_IMAGE_EXECUTION_INFO) + NameStr= ingLen + DevicePathSize + SignatureSize; =20 NewImageExeInfoTable =3D (EFI_IMAGE_EXECUTION_INFO_TABLE *) Allocat= eRuntimePool (ImageExeInfoTableSize + NewImageExeInfoEntrySize); if (NewImageExeInfoTable =3D=3D NULL) { return ; } =20 if (ImageExeInfoTable !=3D NULL) { CopyMem (NewImageExeInfoTable, ImageExeInfoTable, ImageExeInfoTableSiz= e); } else { NewImageExeInfoTable->NumberOfImages =3D 0; } NewImageExeInfoTable->NumberOfImages++; ImageExeInfoEntry =3D (EFI_IMAGE_EXECUTION_INFO *) ((UINT8 *) NewImageEx= eInfoTable + ImageExeInfoTableSize); // // Update new item's information. // WriteUnaligned32 ((UINT32 *) ImageExeInfoEntry, Action); WriteUnaligned32 ((UINT32 *) ((UINT8 *) ImageExeInfoEntry + sizeof (EFI_= IMAGE_EXECUTION_ACTION)), (UINT32) NewImageExeInfoEntrySize); =20 NameStr =3D (CHAR16 *)(ImageExeInfoEntry + 1); if (Name !=3D NULL) { CopyMem ((UINT8 *) NameStr, Name, NameStringLen); } else { ZeroMem ((UINT8 *) NameStr, sizeof (CHAR16)); } =20 CopyMem ( (UINT8 *) NameStr + NameStringLen, DevicePath, DevicePathSize ); if (Signature !=3D NULL) { CopyMem ( (UINT8 *) NameStr + NameStringLen + DevicePathSize, Signature, SignatureSize ); } // // Update/replace the image execution table. // gBS->InstallConfigurationTable (&gEfiImageSecurityDatabaseGuid, (VOID *)= NewImageExeInfoTable); =20 // // Free Old table data! // if (ImageExeInfoTable !=3D NULL) { FreePool (ImageExeInfoTable); } } =20 /** Check whether the hash of an given X.509 certificate is in forbidden dat= abase (DBX). =20 @param[in] Certificate Pointer to X.509 Certificate that is searc= hed for. @param[in] CertSize Size of X.509 Certificate. @param[in] SignatureList Pointer to the Signature List in forbidden= database. @param[in] SignatureListSize Size of Signature List. @param[out] RevocationTime Return the time that the certificate was r= evoked. =20 @return TRUE The certificate hash is found in the forbidden database. @return FALSE The certificate hash is not found in the forbidden databa= se. =20 **/ @@ -1555,343 +1556,344 @@ 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; =20 SignatureList =3D NULL; SignatureListSize =3D 0; WinCertificate =3D NULL; SecDataDir =3D NULL; PkcsCertData =3D NULL; Action =3D EFI_IMAGE_EXECUTION_AUTH_UNTESTED; 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_ACCESS_DENIED; } =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 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 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 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 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 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) { + SignatureListSize =3D 0; 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 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); } =20 if (SignatureList !=3D NULL) { FreePool (SignatureList); } =20 return EFI_SECURITY_VIOLATION; } =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 (#53324): https://edk2.groups.io/g/devel/message/53324 Mute This Topic: https://groups.io/mt/69752228/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- From nobody Sun May 19 23:41:33 2024 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+53325+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+53325+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1579201653; cv=none; d=zohomail.com; s=zohoarc; b=Sx1hxiVzpFJXYapB/Qh6Cdevkv48YNbcwRJjRx8K/BFwP3czY00245iW6qo/uDZxsPQQCTulNWDxo3U2voe8Zf4AXkTU+hZaeOprO3+o30DQkFgKE43bzICv2Q6WjXY1GzycYJTFsq0a70YPjg4W/vDEmZiLdRVJI1/Wm9WBgM8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1579201653; 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=v0ujXxF/q+7AwMgjo4petehlQgpjsfL8djZrsg5LxGI=; b=PmxS01ZyGSyj2/cFoL9d/grx7oJGEKH00ju/k2GJ96fEBylf+QmJkLJ0KakGyQQ8dLMlj3Lh24G8dPH1l5vGd3nOhTnRplkkYmWbVODpaXQz9oPN3viKB7YY2EWnQYA0cEO2lOt5Ytz3lmSQSp2rNidcfdRDEDgQB2ZfdDquv4k= 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+53325+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 1579201653926653.1525785642365; Thu, 16 Jan 2020 11:07:33 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id miEkYY1788612x2twStpw1pY; Thu, 16 Jan 2020 11:07:33 -0800 X-Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web11.1420.1579201650331384007 for ; Thu, 16 Jan 2020 11:07:30 -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-366-cQYBxeI7M-u1WgQoqkX72w-1; Thu, 16 Jan 2020 14:07:26 -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 140D8DB23; Thu, 16 Jan 2020 19:07:25 +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 EBD9180890; Thu, 16 Jan 2020 19:07:23 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Chao Zhang , Jian J Wang , Jiewen Yao Subject: [edk2-devel] [PATCH 11/11] SecurityPkg/DxeImageVerificationHandler: fix "defer" vs. "deny" policies Date: Thu, 16 Jan 2020 20:07:05 +0100 Message-Id: <20200116190705.18816-12-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: cQYBxeI7M-u1WgQoqkX72w-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: 4Yngy0gmZiuqejam5PYKfDp8x1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1579201653; bh=v0ujXxF/q+7AwMgjo4petehlQgpjsfL8djZrsg5LxGI=; h=Cc:Content-Type:Date:From:Reply-To:Subject:To; b=FVrIQpUJ3trwm0O+zvI53pPnUVezYwYaoIBIR9UdDOC0cpfKpKRCDEqCLN8onBzY7PD lHL695IAtpPCJ8qOgPAKdNs019pJhTG01VWy5cUv9fhS2IXTkmeN+twyZDDoielNeZOQg AfaGfo3iOBmGI7GmBoa/5sXpnT2yNrXQVN8= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" 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=3D= =3D0) at the moment: > When AuditMode=3D=3D0, 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 =3D=3D DEFER_EXECUTE_ON_SECURITY_VIOLATION); the documentation is updated instead. Cc: Chao Zhang Cc: Jian J Wang Cc: Jiewen Yao Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2129 Fixes: 5db28a6753d307cdfb1cfdeb2f63739a9f959837 Signed-off-by: Laszlo Ersek --- SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 11= ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati= onLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationL= ib.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; =20 Data =3D NULL; CertList =3D NULL; CertData =3D NULL; RootCert =3D NULL; DbxData =3D NULL; RootCertSize =3D 0; VerifyStatus =3D FALSE; =20 DataSize =3D 0; Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSe= curityDatabaseGuid, NULL, &DataSize, NULL); if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { Data =3D (UINT8 *) AllocateZeroPool (DataSize); if (Data =3D=3D NULL) { return VerifyStatus; } =20 Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSe= curityDatabaseGuid, NULL, &DataSize, (VOID *) Data); if (EFI_ERROR (Status)) { goto Done; } =20 // // Find X509 certificate in Signature List to verify the signature in = pkcs7 signed data. // CertList =3D (EFI_SIGNATURE_LIST *) Data; while ((DataSize > 0) && (DataSize >=3D CertList->SignatureListSize)) { if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) { CertData =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof = (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize); CertCount =3D (CertList->SignatureListSize - sizeof (EFI_SIGNATURE= _LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize; =20 for (Index =3D 0; Index < CertCount; Index++) { // // Iterate each Signature Data Node within this CertList for ver= ify. // RootCert =3D CertData->SignatureData; RootCertSize =3D CertList->SignatureSize - sizeof (EFI_GUID); =20 // // Call AuthenticodeVerify library to Verify Authenticode struct. // VerifyStatus =3D AuthenticodeVerify ( AuthData, AuthDataSize, RootCert, RootCertSize, mImageDigest, mImageDigestSize ); if (VerifyStatus) { // // Here We still need to check if this RootCert's Hash is revo= ked // Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &= gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL); if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { goto Done; } DbxData =3D (UINT8 *) AllocateZeroPool (DbxDataSize); if (DbxData =3D=3D NULL) { goto Done; } =20 Status =3D gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gE= fiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData); if (EFI_ERROR (Status)) { goto Done; } =20 if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SI= GNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) { // // Check the timestamp signature and signing time to determi= ne if the RootCert can be trusted. // VerifyStatus =3D PassTimestampCheck (AuthData, AuthDataSize,= &RevocationTime); if (!VerifyStatus) { DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is sig= ned and signature is accepted by DB, but its root cert failed the timestamp= check.\n")); } } =20 goto Done; } =20 CertData =3D (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertLi= st->SignatureSize); } } =20 DataSize -=3D CertList->SignatureListSize; CertList =3D (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->= SignatureListSize); } } =20 Done: =20 if (VerifyStatus) { SecureBootHook (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabas= eGuid, CertList->SignatureSize, CertData); } =20 if (Data !=3D NULL) { FreePool (Data); } if (DbxData !=3D NULL) { FreePool (DbxData); } =20 return VerifyStatus; } =20 /** Provide verification service for signed images, which include both signa= ture validation and platform policy control. For signature types, both UEFI WIN_CERTIFIC= ATE_UEFI_GUID and MSFT Authenticode type signatures are supported. =20 In this implementation, only verify external executables when in USER MO= DE. Executables from FV is bypass, so pass in AuthenticationStatus is ignore= d. =20 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 v= alue 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 securi= ty database "db", and not be reflected in the security data base "dbx". =20 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. =20 @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 fil= e 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 devi= ce path. @param[in] BootPolicy A boot policy that was used to call LoadImage()= UEFI service. =20 @retval EFI_SUCCESS The file specified by DevicePath and non-= NULL FileBuffer did authenticate, and the plat= form 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 m= ay execute the image in FileBuffer. @retval EFI_SECURITY_VIOLATION The file specified by File did not authen= ticate, and the platform policy dictates that File sh= ould be placed in the untrusted state. The image has bee= n added to the file execution table. @retval EFI_ACCESS_DENIED The file specified by File and FileBuffer= did not authenticate, and the platform policy dic= tates that the DXE - Foundation many not use File. + Foundation may not use File. The image has + been added to the file execution table. =20 **/ 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; =20 SignatureList =3D NULL; SignatureListSize =3D 0; WinCertificate =3D NULL; SecDataDir =3D NULL; PkcsCertData =3D NULL; Action =3D EFI_IMAGE_EXECUTION_AUTH_UNTESTED; 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_ACCESS_DENIED; } =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 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 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 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 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 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) { SignatureListSize =3D 0; 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 Failed: // - // Policy decides to defer or reject the image; add its information in i= mage executable information table. + // Policy decides to defer or reject the image; add its information in i= mage + // executable information table in either case. // 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); } =20 if (SignatureList !=3D NULL) { FreePool (SignatureList); } =20 - return EFI_SECURITY_VIOLATION; + if (Policy =3D=3D DEFER_EXECUTE_ON_SECURITY_VIOLATION) { + return EFI_SECURITY_VIOLATION; + } + return EFI_ACCESS_DENIED; } =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 (#53325): https://edk2.groups.io/g/devel/message/53325 Mute This Topic: https://groups.io/mt/69752229/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-