Re: [PATCH v2 1/2] SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic

[Laszlo Ersek <lersek@redhat.com>]

Jiewen, Qin,

can you guys perhaps help with reviewing this patch? (The second patch
in the series is for OvmfPkg, and it depends on this one.)

Thanks!
Laszlo

On 10/05/17 22:16, Brijesh Singh wrote:
> By default the image verification policy for option ROM images is 0x4
> (DENY_EXECUTE_ON_SECURITY_VIOLATION) but the following OvmfPkg commit:
> 
> 1fea9ddb4e3f OvmfPkg: execute option ROM images regardless of Secure Boot
> 
> set it to 0x0 (ALWAYS_EXECUTE). This is fine because typically option
> ROMs comes from host-side and most of the time cloud provider (i.e
> hypervisor) have full access over a guest anyway. But when secure boot
> is enabled, we would like to deny the execution of option ROM when
> SEV is active. Having dynamic Pcd will give us flexibility to set the
> security policy at the runtime.
> 
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=728
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> 
> Changes since v1:
>  * Add Contributed-under tag
> 
>  SecurityPkg/SecurityPkg.dec | 24 ++++++++++----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index 01bff01ed50a..4e32d172d7d9 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -230,18 +230,6 @@ [Ppis]
>  #
>  
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
> -  ## Image verification policy for OptionRom. Only following values are valid:<BR><BR>
> -  #  NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has been removed.<BR>
> -  #  0x00000000      Always trust the image.<BR>
> -  #  0x00000001      Never trust the image.<BR>
> -  #  0x00000002      Allow execution when there is security violation.<BR>
> -  #  0x00000003      Defer execution when there is security violation.<BR>
> -  #  0x00000004      Deny execution when there is security violation.<BR>
> -  #  0x00000005      Query user when there is security violation.<BR>
> -  # @Prompt Set policy for the image from OptionRom.
> -  # @ValidRange 0x80000001 | 0x00000000 - 0x00000005
> -  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04|UINT32|0x00000001
> -
>    ## Image verification policy for removable media which includes CD-ROM, Floppy, USB and network.
>    #  Only following values are valid:<BR><BR>
>    #  NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has been removed.<BR>
> @@ -304,6 +292,18 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    gEfiSecurityPkgTokenSpaceGuid.PcdStatusCodeSubClassTpmDevice|0x010D0000|UINT32|0x00000007
>  
>  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
> +  ## Image verification policy for OptionRom. Only following values are valid:<BR><BR>
> +  #  NOTE: Do NOT use 0x5 and 0x2 since it violates the UEFI specification and has been removed.<BR>
> +  #  0x00000000      Always trust the image.<BR>
> +  #  0x00000001      Never trust the image.<BR>
> +  #  0x00000002      Allow execution when there is security violation.<BR>
> +  #  0x00000003      Defer execution when there is security violation.<BR>
> +  #  0x00000004      Deny execution when there is security violation.<BR>
> +  #  0x00000005      Query user when there is security violation.<BR>
> +  # @Prompt Set policy for the image from OptionRom.
> +  # @ValidRange 0x80000001 | 0x00000000 - 0x00000005
> +  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04|UINT32|0x00000001
> +
>    ## Indicates the presence or absence of the platform operator during firmware booting.
>    #  If platform operator is not physical presence during boot. TPM will be locked and the TPM commands 
>    #  that required operator physical presence can not run.<BR><BR>
>