Hi, Laszlo

Thanks for the detail explanation of the whole story, the change is good to me.

Reviewed-by: Fu Siyuan <siyuan...@intel.com>

-----Original Message-----
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, January 7, 2016 5:19 AM
To: edk2-de...@ml01.01.org
Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Fu, Siyuan 
<siyuan...@intel.com>; Zhang, Chao B <chao.b.zh...@intel.com>
Subject: [PATCH 1/2] OvmfPkg: inherit Image Verification Policy defaults from 
SecurityPkg

Secure Boot support was originally addded to OvmfPkg on 2012-Mar-09, in SVN 
r13093 (git 8cee3de7e9f4), titled

  OvmfPkg: Enable secure-boot support when SECURE_BOOT_ENABLE==TRUE

At that time the image verification policies in SecurityPkg/SecurityPkg.dec 
were:

- option ROM image:      0x00 (ALWAYS_EXECUTE)
- removable media image: 0x05 (QUERY_USER_ON_SECURITY_VIOLATION)
- fixed media image:     0x05 (QUERY_USER_ON_SECURITY_VIOLATION)

The author of SVN r13093 apparently didn't want to depend on the SecurityPkg 
defaults for the latter two image origins, plus the ALWAYS_EXECUTE policy for 
option ROM images must have been deemed too lax.
For this reason SVN r13093 immediately spelled out 0x05
(QUERY_USER_ON_SECURITY_VIOLATION) within OvmfPkg for all three image origins.

Fast forward to 2013-Aug-28: policy 0x05
(QUERY_USER_ON_SECURITY_VIOLATION) had been forbidden in the UEFI spec, and SVN 
r14607 (git db44ea6c4e09) reflected this in the source code:

- The policies for the latter two image origins were switched from 0x05 to
  0x04 (DENY_EXECUTE_ON_SECURITY_VIOLATION) in SecurityPkg,

- the patch changed the default policy for option ROM images too, from
  0x00 (ALWAYS_EXECUTE) to 0x04 (DENY_EXECUTE_ON_SECURITY_VIOLATION),

- any other client DSC files, including OvmfPkg's, underwent a whole-sale
  0x05 (QUERY_USER_ON_SECURITY_VIOLATION) -> 0x04
  (DENY_EXECUTE_ON_SECURITY_VIOLATION) replacement too.

The practical result of that patch for OvmfPkg was that the explicit 0x04 
settings would equal the strict SecurityPkg defaults exactly.

And that's what we have today: the "override the default values from 
SecurityPkg" comments in OvmfPkg's DSC files are stale, in practice.

It is extremely unlikely that SecurityPkg would change the defaults from
0x04 (DENY_EXECUTE_ON_SECURITY_VIOLATION) any time in the future, so let's just 
inherit those in OvmfPkg.

Cc: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Fu Siyuan <siyuan...@intel.com>
Cc: Chao Zhang <chao.b.zh...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 7 -------
 OvmfPkg/OvmfPkgIa32X64.dsc | 7 -------
 OvmfPkg/OvmfPkgX64.dsc     | 7 -------
 3 files changed, 21 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 
1c6c852..afde345 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -404,13 +404,6 @@ [PcdsFixedAtBuild]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
 !endif
 
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  # override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
-  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
-  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
-  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
-!endif
-
   # IRQs 5, 9, 10, 11 are level-triggered
   gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index 
1a2f2a4..2cbd417 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -410,13 +410,6 @@ [PcdsFixedAtBuild.X64]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
 !endif
 
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  # override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
-  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
-  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
-  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
-!endif
-
   # IRQs 5, 9, 10, 11 are level-triggered
   gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
 
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index 
0bbbfa8..309b5e2 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -409,13 +409,6 @@ [PcdsFixedAtBuild]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000
 !endif
 
-!if $(SECURE_BOOT_ENABLE) == TRUE
-  # override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
-  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
-  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
-  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
-!endif
-
   # IRQs 5, 9, 10, 11 are level-triggered
   gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
 
--
1.8.3.1


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to