public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Long, Qin" <qin.long@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>, Laszlo Ersek <lersek@redhat.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Justen, Jordan L" <jordan.l.justen@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>,
	"Zhang, Chao B" <chao.b.zhang@intel.com>
Subject: Re: [PATCH v2 1/2] SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic
Date: Tue, 10 Oct 2017 16:53:30 +0000	[thread overview]
Message-ID: <BF2CCE9263284D428840004653A28B6E540141B8@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A9D7CF9@shsmsx102.ccr.corp.intel.com>

The patch looks good to me.

Reviewed-by: Long Qin <qin.long@intel.com>


Best Regards & Thanks,
LONG, Qin

From: Yao, Jiewen
Sent: Tuesday, October 10, 2017 9:47 PM
To: Laszlo Ersek <lersek@redhat.com>; Long, Qin <qin.long@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; edk2-devel@lists.01.org; Justen, Jordan L <jordan.l.justen@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Zhang, Chao B <chao.b.zhang@intel.com>
Subject: RE: [edk2] [PATCH v2 1/2] SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic

I am OK on this patch.

Reviewed-by: Jiewen.yao@intel.com<mailto:Jiewen.yao@intel.com>

BTW: Do you also need update PcdRemovableMediaImageVerificationPolicy and PcdFixedMediaImageVerificationPolicy?


Thank you
Yao Jiewen


From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Tuesday, October 10, 2017 7:28 PM
To: Long, Qin <qin.long@intel.com<mailto:qin.long@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>; Tom Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
Subject: Re: [edk2] [PATCH v2 1/2] SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic

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<mailto:chao.b.zhang@intel.com>>
> Cc: Jordan Justen <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Cc: Tom Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com<mailto: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>
>

  reply	other threads:[~2017-10-10 16:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-05 20:16 [PATCH v2 1/2] SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic Brijesh Singh
2017-10-05 20:16 ` [PATCH v2 2/2] OvmfPkg/PlatformPei: DENY_EXECUTE_ON_SECURITY_VIOLATION when SEV is active Brijesh Singh
2017-10-05 20:29   ` Laszlo Ersek
2017-10-10 11:28 ` [PATCH v2 1/2] SecurityPkg: make PcdOptionRomImageVerificationPolicy dynamic Laszlo Ersek
2017-10-10 13:46   ` Yao, Jiewen
2017-10-10 16:53     ` Long, Qin [this message]
2017-10-10 17:29     ` Laszlo Ersek
2017-10-17 19:30 ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BF2CCE9263284D428840004653A28B6E540141B8@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox