From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E4EFB2095B07C for ; Thu, 5 Oct 2017 13:25:46 -0700 (PDT) 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 mx1.redhat.com (Postfix) with ESMTPS id 37C7EC059B7F; Thu, 5 Oct 2017 20:29:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 37C7EC059B7F Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-16.rdu2.redhat.com [10.10.120.16]) by smtp.corp.redhat.com (Postfix) with ESMTP id B8C0D5D765; Thu, 5 Oct 2017 20:29:07 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Chao Zhang , Jordan Justen , Tom Lendacky References: <20171005201642.122619-1-brijesh.singh@amd.com> <20171005201642.122619-2-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: Date: Thu, 5 Oct 2017 22:29:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <20171005201642.122619-2-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 05 Oct 2017 20:29:09 +0000 (UTC) Subject: Re: [PATCH v2 2/2] OvmfPkg/PlatformPei: DENY_EXECUTE_ON_SECURITY_VIOLATION when SEV is active X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 05 Oct 2017 20:25:47 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/05/17 22:16, Brijesh Singh wrote: > The following commit: > > 1fea9ddb4e3f OvmfPkg: execute option ROM images regardless of Secure Boot > > sets the OptionRomImageVerificationPolicy to ALWAYS_EXECUTE the expansion > ROMs attached to the emulated PCI devices. A expansion ROM constitute > another channel through which a cloud provider (i.e hypervisor) can > inject a code in guest boot flow to compromise it. > > When SEV is enabled, the bios code has been verified by the guest owner > via the SEV guest launch sequence before its executed. When secure boot, > is enabled, lets make sure that we do not allow guest bios to execute a > code which is not signed by the guest owner. > > Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=728 > Cc: Chao Zhang > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Tom Lendacky > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh > --- > Changes since v1: > * Add Contributed-under tag > * Fix OvmfPkgIa32.dsc build > > OvmfPkg/OvmfPkgIa32.dsc | 9 +++++---- > OvmfPkg/OvmfPkgIa32X64.dsc | 9 +++++---- > OvmfPkg/OvmfPkgX64.dsc | 9 +++++---- > OvmfPkg/PlatformPei/PlatformPei.inf | 2 ++ > OvmfPkg/PlatformPei/AmdSev.c | 7 +++++++ > 5 files changed, 24 insertions(+), 12 deletions(-) > > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc > index 92e943d4a0d0..7fb557b7c9cd 100644 > --- a/OvmfPkg/OvmfPkgIa32.dsc > +++ b/OvmfPkg/OvmfPkgIa32.dsc > @@ -483,10 +483,6 @@ [PcdsFixedAtBuild] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000 > !endif > > -!if $(SECURE_BOOT_ENABLE) == TRUE > - gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00 > -!endif > - > # IRQs 5, 9, 10, 11 are level-triggered > gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20 > > @@ -544,6 +540,11 @@ [PcdsDynamicDefault] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000 > !endif > > +!if $(SECURE_BOOT_ENABLE) == TRUE > + gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00 > +!endif > + > + > ################################################################################ > # > # Components Section - list of all EDK II Modules needed by this Platform. > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc > index 7f9220ccb90a..4bcbddb95768 100644 > --- a/OvmfPkg/OvmfPkgIa32X64.dsc > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > @@ -489,10 +489,6 @@ [PcdsFixedAtBuild.X64] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000 > !endif > > -!if $(SECURE_BOOT_ENABLE) == TRUE > - gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00 > -!endif > - > # IRQs 5, 9, 10, 11 are level-triggered > gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20 > > @@ -552,6 +548,11 @@ [PcdsDynamicDefault] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000 > !endif > > +!if $(SECURE_BOOT_ENABLE) == TRUE > + gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00 > +!endif > + > + > ################################################################################ > # > # Components Section - list of all EDK II Modules needed by this Platform. > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 36c60fc19c40..e52a3bd4db9b 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -488,10 +488,6 @@ [PcdsFixedAtBuild] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStackSize|0x4000 > !endif > > -!if $(SECURE_BOOT_ENABLE) == TRUE > - gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00 > -!endif > - > # IRQs 5, 9, 10, 11 are level-triggered > gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20 > > @@ -551,6 +547,11 @@ [PcdsDynamicDefault] > gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000 > !endif > > +!if $(SECURE_BOOT_ENABLE) == TRUE > + gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00 > +!endif > + > + > ################################################################################ > # > # Components Section - list of all EDK II Modules needed by this Platform. > diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf > index 16a8db7b0bd2..de7434d93dc0 100644 > --- a/OvmfPkg/PlatformPei/PlatformPei.inf > +++ b/OvmfPkg/PlatformPei/PlatformPei.inf > @@ -41,6 +41,7 @@ [Packages] > IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > + SecurityPkg/SecurityPkg.dec > UefiCpuPkg/UefiCpuPkg.dec > OvmfPkg/OvmfPkg.dec > > @@ -96,6 +97,7 @@ [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable > gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask > + gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy > gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress > gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds > diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c > index 26f7c3fdbb13..1539e5b5cdce 100644 > --- a/OvmfPkg/PlatformPei/AmdSev.c > +++ b/OvmfPkg/PlatformPei/AmdSev.c > @@ -59,4 +59,11 @@ AmdSevInitialize ( > ASSERT_RETURN_ERROR (PcdStatus); > > DEBUG ((DEBUG_INFO, "SEV is enabled (mask 0x%lx)\n", EncryptionMask)); > + > + // > + // Set Pcd to Deny the execution of option ROM when security > + // violation. > + // > + PcdStatus = PcdSet32S (PcdOptionRomImageVerificationPolicy, 0x4); > + ASSERT_RETURN_ERROR (PcdStatus); > } > Reviewed-by: Laszlo Ersek Thanks! Laszlo