Re: [edk2-devel] [rfc] Remove support for unsupported tool_chain_tags

[Rebecca Cran]

Yes, I've been occasionally running builds on XCODE5. OvmfPkg currently 
fails with an unresolved symbol.


--
Rebecca Cran


On 5/4/22 19:58, Andrew Fish via groups.io wrote:

I can sign up to maintain XCODE.

I think Rebecca has been running test builds with XCODE.

The XCODE5 names comes from the compiler flags needing to change for 
Xcode 5. Yes 2013 called and wants it compiler back.


Thanks,

Andrew Fish


On May 2, 2022, at 4:24 PM, Sean  wrote:

As discussed at the weekly tools meeting, I am proposing that all 
unsupported tool chains get removed from tools_def.template in the 
Basetools/Conf folder (edk2/tools_def.template at master · 
tianocore/edk2 (github.com) 
). 
The fact that VS2008 is still listed there is proof of a problem.  
Many of these old tool chains are not available and if found would 
not reliably work anyway (see dozens of emails on the list, most 
recent by rebecca).


The suggestion from the tools meeting discussions is that we should 
setup maintainers for each tool chain tag and any tool chain tag 
without a maintainer would then be removed.


Today the CI and PR system is testing the most recent versions of VS 
and GCC.  As of this email that tag is VS2019 and GCC5.  There has 
also been a desire to add clang (clangpdb tag) to the list of CI 
builds but it is currently only partially supported and needs some 
community effort.  The GCC5 tag might need more clarity as I know 
this supports many different versions so I am not sure how to 
correctly communicate what is the actual supported version.  Maybe 
the container discussion would help ([CI] Use containers on Linux · 
Discussion #2732 · tianocore/edk2 (github.com) 
) 
and it could be N and N-1 on the latest container images?



Finally, the question is how to define a supported tool chain and 
what are the expectations for the "maintainer".  I would propose two 
things.


1. A category/tag created for the tool chain in the issue tracking 
system and the maintainer will be the default assigned owner for 
incoming bugs and is responsible for triage and resolution.


    a. If all maintainers resign the community would be notified and 
if no one came forward the toolchain would be dropped.


    b. If the tag had a high bug count and low resolution/response 
rate the toolchain maintainer role would be re-evaluated.


2. At defined points in the edk2 development process, each package is 
compiled in debug, release, and noopt for the toolchain for all 
supported architectures.   The report is made available.


    A. Suggested at minimum for each hard freeze stable tag point but 
could be CI, nightly, weekly, etc.


    B. Suggestion is this should somehow be automated for the CI 
system and tool chains supported.


3. Update maintainers.txt to indicate tool chain maintainers


Please use this email and/or tools meeting to discuss the proposal or 
becoming a toolchain maintainer.


RFC will be implemented after the may stable tag if no issues are 
raised.



Thanks

Sean

















-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89527): https://edk2.groups.io/g/devel/message/89527
Mute This Topic: https://groups.io/mt/90848478/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2 1/1] OvmfPkg: Add README for TDVF

[Yao, Jiewen]

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Xu, Min M 
> Sent: Wednesday, May 4, 2022 8:14 AM
> To: devel@edk2.groups.io
> Cc: Xu, Min M ; Brijesh Singh ;
> Aktas, Erdem ; Gerd Hoffmann
> ; James Bottomley ; Yao, Jiewen
> ; Tom Lendacky 
> Subject: [PATCH V2 1/1] OvmfPkg: Add README for TDVF
> 
> RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3249
> 
> Add README for TDVF.
> 
> Cc: Brijesh Singh 
> Cc: Erdem Aktas 
> Cc: Gerd Hoffmann 
> Cc: James Bottomley 
> Cc: Jiewen Yao 
> Cc: Tom Lendacky 
> Signed-off-by: Min Xu 
> ---
>  OvmfPkg/IntelTdx/README | 88
> +
>  1 file changed, 88 insertions(+)
>  create mode 100644 OvmfPkg/IntelTdx/README
> 
> diff --git a/OvmfPkg/IntelTdx/README b/OvmfPkg/IntelTdx/README
> new file mode 100644
> index ..20426a4f0bf8
> --- /dev/null
> +++ b/OvmfPkg/IntelTdx/README
> @@ -0,0 +1,88 @@
> +TDVF Overview
> +-
> +
> +Intel Trust Domain Extension (TDX) is Intel Architecture extension
> +to provide trusted, isolated VM execution by removing CSP software
> +(hypervisor etc) from the TCB. TDX Virtual Firmware (TDVF) is an
> +EDK II based project to enable UEFI support for TDX based Virtual
> +Machines. It provides the capability to launch a TD.
> +
> +The Intel® TDX Virtual Firmware Design Guide is at
> +https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-
> virtual-firmware-design-guide-rev-1.01.pdf.
> +
> +More information can be found at:
> +https://www.intel.com/content/www/us/en/developer/articles/technical/intel
> -trust-domain-extensions.html
> +
> +
> +Configurations and Features
> +
> +
> +There are 2 configurations for TDVF.
> +
> +Config-A:
> + - Merge the *basic* TDVF feature to existing OvmfX64Pkg.dsc. (Align
> +   with existing SEV)
> + - Threat model: VMM is NOT out of TCB. (We don’t make things worse)
> + - The OvmfX64Pkg.dsc includes SEV/TDX/normal OVMF basic boot capability.
> +   The final binary can run on SEV/TDX/normal OVMF.
> + - No changes to existing OvmfPkgX64 image layout.
> + - No need to add additional security features if they do not exist today.
> + - No need to remove features if they exist today.
> + - RTMR is not supported.
> + - PEI phase is NOT skipped in either Td or Non-Td.
> +
> +Config-B:
> + - (*) Add a standalone IntelTdx.dsc to a TDX specific directory for a *full*
> +   feature TDVF.(Align with existing SEV)
> + - (*) Threat model: VMM is out of TCB. (We need necessary change to prevent
> +   attack from VMM)
> + - (*) IntelTdx.dsc includes TDX/normal OVMF basic boot capability. The final
> +   binary can run on TDX/normal OVMF.
> + - It might eventually merge with AmdSev.dsc, but NOT at this point of
> +   time. And we don’t know when it will happen. We need sync with AMD in
> +   the community after both of us think the solutions are mature to merge.
> + - Need to add necessary security feature as mandatory requirement, such
> +   as RTMR based Trusted Boot support.
> + - Need to measure the external input from Host VMM, such as TdHob, CFV.
> + - Need to measure other external input, such as FW_CFG data, os loader,
> +   initrd, etc.
> + - Need to remove unnecessary attack surfaces, such as network stack.
> +
> +In current stage, Config-A has been merged into edk2-master branch.
> +The corresponding pkg file is OvmfPkg/OvmfPkgX64.dsc.
> +
> +Config-B is split into several waves. The corresponding pkg file is
> +OvmfPkg/IntelTdx/IntelTdxX64.dsc. The features with (*) have been
> implemented
> +and merged into edk2-master branch. Others are in upstreaming progress.
> +
> +Build
> +--
> +- Build the TDVF (Config-A) target:
> +`cd /path/to/edk2`
> +`source edksetup.sh`
> +`build.sh -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t GCC5`
> +
> +- Build the TDVF (Config-B) target:
> +`cd /path/to/edk2`
> +`set PACKAGES_PATH=/path/to/edk2/OvmfPkg`
> +`source edksetup.sh`
> +`build.sh -p OvmfPkg/IntelTdx/IntelTdxX64.dsc -a X64 -t GCC5`
> +
> +Usage
> +-
> +
> +Assuming TDX-QEMU/TDX-KVM are already built, one can start a TD virtual
> +machine as [launching-td-guest](https://github.com/intel/qemu-tdx/blob/tdx-
> qemu-upstream-rfc-v3/docs/system/i386/tdx.rst#launching-a-td-tdx-vm):
> +
> +`qemu_system_x86 \`
> +`   -machine ...,confidential-guest-support=tdx0 \`
> +`   -object tdx-guest,id=tdx0,[sept-ve-disable=off] \`
> +`   -drive if=pflash,format=raw,unit=0,file=/path/to/OVMF_CODE.fd \`
> +`   -drive if=pflash,format=raw,unit=1,file=/path/to/OVMF_VARS.fd \`
> +
> +Note:
> +TDX-QEMU/TDX-KVM are still in upstreaming progress. Please refer to:
> + - kvm  : https://github.com/intel/tdx/tree/kvm-upstream
> + - qemu : https://github.com/intel/qemu-tdx/blob/tdx-qemu-upstream-rfc-v3
> +
> +Once above 2 upstreaming are completed a minimum qemu/kvm version will
> be updated here.
> --
> 2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89525): 

Re: [edk2-devel] [rfc] Remove support for unsupported tool_chain_tags

[Andrew Fish via groups.io]

I can sign up to maintain XCODE. 

I think Rebecca has been running test builds with XCODE. 

The XCODE5 names comes from the compiler flags needing to change for Xcode 5. 
Yes 2013 called and wants it compiler back. 

Thanks,

Andrew Fish

> On May 2, 2022, at 4:24 PM, Sean  wrote:
> 
> As discussed at the weekly tools meeting, I am proposing that all unsupported 
> tool chains get removed from tools_def.template in the Basetools/Conf folder 
> (edk2/tools_def.template at master · tianocore/edk2 (github.com) 
> ).
> The fact that VS2008 is still listed there is proof of a problem.  Many 
> of these old tool chains are not available and if found would not reliably 
> work anyway (see dozens of emails on the list, most recent by rebecca).  
> 
> The suggestion from the tools meeting discussions is that we should setup 
> maintainers for each tool chain tag and any tool chain tag without a 
> maintainer would then be removed.  
> 
> Today the CI and PR system is testing the most recent versions of VS and GCC. 
>  As of this email that tag is VS2019 and GCC5.  There has also been a desire 
> to add clang (clangpdb tag) to the list of CI builds but it is currently only 
> partially supported and needs some community effort.  The GCC5 tag might need 
> more clarity as I know this supports many different versions so I am not sure 
> how to correctly communicate what is the actual supported version.  Maybe the 
> container discussion would help ([CI] Use containers on Linux · Discussion 
> #2732 · tianocore/edk2 (github.com) 
> ) and it could be N and 
> N-1 on the latest container images?  
> 
> 
> 
> Finally, the question is how to define a supported tool chain and what are 
> the expectations for the "maintainer".  I would propose two things. 
> 
> 1. A category/tag created for the tool chain in the issue tracking system and 
> the maintainer will be the default assigned owner for incoming bugs and is 
> responsible for triage and resolution.  
> 
> a. If all maintainers resign the community would be notified and if no 
> one came forward the toolchain would be dropped.  
> 
> b. If the tag had a high bug count and low resolution/response rate the 
> toolchain maintainer role would be re-evaluated.  
> 
> 2. At defined points in the edk2 development process, each package is 
> compiled in debug, release, and noopt for the toolchain for all supported 
> architectures.   The report is made available.  
> 
> A. Suggested at minimum for each hard freeze stable tag point but could 
> be CI, nightly, weekly, etc.  
> 
> B. Suggestion is this should somehow be automated for the CI system and 
> tool chains supported.  
> 
> 3. Update maintainers.txt to indicate tool chain maintainers
> 
> 
> 
> Please use this email and/or tools meeting to discuss the proposal or 
> becoming a toolchain maintainer. 
> 
> RFC will be implemented after the may stable tag if no issues are raised.  
> 
> 
> 
> Thanks
> 
> Sean
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89524): https://edk2.groups.io/g/devel/message/89524
Mute This Topic: https://groups.io/mt/90848478/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] 回复: [edk2-rfc] [rfc] Remove support for unsupported tool_chain_tags

[gaoliming]

Sean:
  After remove, the supported tool chain will have its maintainer. This is a 
good idea.

  For CLANG tool chain, CLANGPDB and CLANGDWARF supports IA32 and X64 arch 
only, they can run Windows/Linux/MacOs. CLANGPDB is to directly generate EFI 
image with PDB debug symbol, CLANGDWARF is to generate ELF image with DWARF 
debug symbol. CLANG35 is to support ARM and AARCH64 only. CLANG38 is to support 
IA32, X64, ARM and AARCH64 archs. It runs on Linux OS.

Steven: 
  Have you more information about CLANG tool chain?

Thanks
Liming
> -邮件原件-
> 发件人: r...@edk2.groups.io  代表 Sean
> 发送时间: 2022年5月3日 7:24
> 收件人: r...@edk2.groups.io; devel@edk2.groups.io
> 主题: [edk2-rfc] [rfc] Remove support for unsupported tool_chain_tags
> 
> As discussed at the weekly tools meeting, I am proposing that all
> unsupported tool chains get removed from tools_def.template in the
> Basetools/Conf folder (edk2/tools_def.template at master ·
> tianocore/edk2 (github.com)
>  emplate>).
> The fact that VS2008 is still listed there is proof of a problem.
> Many of these old tool chains are not available and if found would not
> reliably work anyway (see dozens of emails on the list, most recent by
> rebecca).
> 
> The suggestion from the tools meeting discussions is that we should
> setup maintainers for each tool chain tag and any tool chain tag without
> a maintainer would then be removed.
> 
> Today the CI and PR system is testing the most recent versions of VS and
> GCC.  As of this email that tag is VS2019 and GCC5.  There has also been
> a desire to add clang (clangpdb tag) to the list of CI builds but it is
> currently only partially supported and needs some community effort.  The
> GCC5 tag might need more clarity as I know this supports many different
> versions so I am not sure how to correctly communicate what is the
> actual supported version.  Maybe the container discussion would help
> ([CI] Use containers on Linux · Discussion #2732 · tianocore/edk2
> (github.com) ) and
> it could be N and N-1 on the latest container images?
> 
> 
> Finally, the question is how to define a supported tool chain and what
> are the expectations for the "maintainer".  I would propose two things.
> 
> 1. A category/tag created for the tool chain in the issue tracking
> system and the maintainer will be the default assigned owner for
> incoming bugs and is responsible for triage and resolution.
> 
>  a. If all maintainers resign the community would be notified and if
> no one came forward the toolchain would be dropped.
> 
>  b. If the tag had a high bug count and low resolution/response rate
> the toolchain maintainer role would be re-evaluated.
> 
> 2. At defined points in the edk2 development process, each package is
> compiled in debug, release, and noopt for the toolchain for all
> supported architectures.   The report is made available.
> 
>  A. Suggested at minimum for each hard freeze stable tag point but
> could be CI, nightly, weekly, etc.
> 
>  B. Suggestion is this should somehow be automated for the CI system
> and tool chains supported.
> 
> 3. Update maintainers.txt to indicate tool chain maintainers
> 
> 
> Please use this email and/or tools meeting to discuss the proposal or
> becoming a toolchain maintainer.
> 
> RFC will be implemented after the may stable tag if no issues are raised.
> 
> 
> Thanks
> 
> Sean
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89523): https://edk2.groups.io/g/devel/message/89523
Mute This Topic: https://groups.io/mt/90903330/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] UefiCpuPkg: Save PcdConfidentialComputingGuestAttr in mCcGuestAttr

[Ni, Ray]

On Mon, May  2, 2022 at 03:03 PM, Min Xu wrote:

>
> The reason why C global variable cannot be used in PEIM is that in some
> scenario PEIM is executed in FLASH so that the value of C global variable
> cannot be kept in different calls. But I don't think it is a problem in this
> situation.
> 1. This global variable is to keep the PcdConfidentialComputingGuestAttribute
> in mCcGuestAttr. So if this is Tdx guest, then the global variable can be kept
> (CC_GUEST_IS_TDX (mCcGuestAttr) == TRUE).
> 2. If this is Non-Td guest, then even the global variable cannot be kept,
> CC_GUEST_IS_TDX (mCcGuestAttr) is FALSE. So mCcGuestAttr can still work.

Directly writing to flash area might cause some side effects.
Usually we need to avoid that.

> 
> There is another solution that we can use CcProbe (which is in
> MdePkg/CcProbeLib). CcProbe checks the work area to fetch the guest type. It
> calls FixedPcdGet32 (PcdOvmfWorkAreaBase) so there is no SMP safe issue in
> PcdLib.

It seems ok. But it's unclear to me how the work area is built.



> 
> As to a new field in CPU_MP_DATA, Tdx guest doesn't create CPU_MP_DATA object
> in MpInitLibInitialize, instead it return right after it detects the working
> guest is Tdx guest. So this fix will be more complicated than above 2
> solutions.

I agree it'll be more complicated to let the TDX version of MpLib use the 
CPU_MP_DATA.
But, I am very curious why not leave the MpInitLib untouched. The solution is:
1. Platform FDF includes two CpuMpPeim drivers. One links to PeiMpInitLib, the 
other links to MpInitLibUp.
Change platform DSC to let first PEIM depend on "FullMpPpi"
Change platform DSC to let second PEIM depend on "UpMpPpi"
2. SEC code passes "FullMpPpi" to PEI Core when it's not a TDX boot. It passes 
"UpMpPpi" when it's a TDX boot.

I think this solution doesn't even change the MP code. Or saying in another 
way, MP and TDX are decoupled.
I prefer this solution.

> 
> Ray, What's your thought?
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89522): https://edk2.groups.io/g/devel/message/89522
Mute This Topic: https://groups.io/mt/90768905/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] Does BaseTools support VS2022?

[gaoliming]

Ray:

 There is one BZ https://bugzilla.tianocore.org/show_bug.cgi?id=3879 for
VS2022. Now, there is no work plan for it. 

 

Thanks

Liming

发件人: devel@edk2.groups.io  代表 Ni, Ray
发送时间: 2022年4月29日 17:13
收件人: devel@edk2.groups.io
抄送: Feng, Bob C ; Sean Brogan

主题: [edk2-devel] Does BaseTools support VS2022?

 

Hi tool experts,

I noticed that VS2022 17.1 has been released. Does BaseTools support VS2022?
if no, any plan?

 

Thanks,

Ray





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89521): https://edk2.groups.io/g/devel/message/89521
Mute This Topic: https://groups.io/mt/90903134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] [PATCH 10/10] BaseTools: Remove RVCT support

[gaoliming]

Acked-by: Liming Gao 

> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Rebecca Cran
> 发送时间: 2022年5月4日 2:48
> 收件人: devel@edk2.groups.io; Leif Lindholm ;
> Ard Biesheuvel ; Sami Mujawar
> ; Gerd Hoffmann ; Bob Feng
> ; Liming Gao ; Yuwei
> Chen ; Jiewen Yao ; Jian J
> Wang ; Xiaoyu Lu ; Guomin
> Jiang ; Abner Chang ;
> Daniel Schaefer ; Ray Ni ;
> Michael D Kinney ; Zhiguang Liu
> ; Maciej Rabeda ;
> Jiaxin Wu ; Siyuan Fu ; Jordan
> Justen ; Anthony Perard
> ; Julien Grall 
> 抄送: Rebecca Cran 
> 主题: [edk2-devel] [PATCH 10/10] BaseTools: Remove RVCT support
> 
> RVCT is obsolete and no longer used.
> Remove support for it.
> 
> Signed-off-by: Rebecca Cran 
> ---
>  BaseTools/Conf/build_rule.template   |  42 ++
>  BaseTools/Conf/tools_def.template| 157
> 
>  BaseTools/Scripts/Rvct-Align32.sct   |  19 ---
>  BaseTools/Scripts/Rvct-Align4K.sct   |  19 ---
>  BaseTools/Source/C/Include/Common/BaseTypes.h|  10 +-
>  BaseTools/Source/Python/AutoGen/BuildEngine.py   |   2 +-
>  BaseTools/Source/Python/AutoGen/GenMake.py   |   2 +-
>  BaseTools/Source/Python/AutoGen/ModuleAutoGen.py |  17 +--
>  BaseTools/Source/Python/UPT/Library/DataType.py  |   1 -
>  9 files changed, 16 insertions(+), 253 deletions(-)
> 
> diff --git a/BaseTools/Conf/build_rule.template
> b/BaseTools/Conf/build_rule.template
> index 435662351213..5895b48fd88d 100755
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -128,8 +128,7 @@
>  
>  "$(CC)" /Fo${dst} $(DEPS_FLAGS) $(CC_FLAGS) $(INC) ${src}
> 
> -
> -# For RVCTCYGWIN CC_FLAGS must be first to work around pathing
> issues
> +
>  "$(CC)" $(DEPS_FLAGS) $(CC_FLAGS) -c -o ${dst} $(INC) ${src}
> 
>  
> @@ -145,7 +144,7 @@
>  
>  $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
> 
> -
> +
>  "$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -c -o ${dst} $(INC) ${src}
> 
>  [C-Header-File]
> @@ -157,7 +156,7 @@
>  
> 
>  [Assembly-Code-File.COMMON.COMMON]
> -
> +
>  ?.asm, ?.Asm, ?.ASM
> 
>  
> @@ -175,16 +174,15 @@
>  Trim --source-code --convert-hex --trim-long -o
> ${d_path}(+)${s_base}. ${d_path}(+)${s_base}.ii
>  "$(ASM)" /Fo${dst} $(ASM_FLAGS) /I${s_path} $(INC)
> ${d_path}(+)${s_base}.
> 
> -
> +
>  Trim --asm-file -o ${d_path}(+)${s_base}.i -i $(INC_LIST) ${src}
>  "$(PP)" $(DEPS_FLAGS) $(PP_FLAGS) $(INC) ${src} >
> ${d_path}(+)${s_base}.ii
>  Trim --trim-long --source-code -o ${d_path}(+)${s_base}.
> ${d_path}(+)${s_base}.ii
> -# For RVCTCYGWIN ASM_FLAGS must be first to work around
> pathing issues
>  "$(ASM)" $(ASM_FLAGS) -o ${dst} $(INC) ${d_path}(+)${s_base}.
> 
> 
> [Assembly-Code-File.COMMON.ARM,Assembly-Code-File.COMMON.AARCH6
> 4]
>  # Remove --convert-hex for ARM as it breaks MSFT assemblers
> -
> +
>  ?.asm, ?.Asm, ?.ASM
> 
>  
> @@ -208,11 +206,10 @@
>  Trim --source-code --trim-long -o ${d_path}(+)${s_base}.
> ${d_path}(+)${s_base}.ii
>  "$(ASM)" /Fo${dst} $(ASM_FLAGS) /I${s_path} $(INC)
> ${d_path}(+)${s_base}.
> 
> -
> +
>  Trim --asm-file -o ${d_path}(+)${s_base}.i -i $(INC_LIST) ${src}
>  "$(PP)" $(DEPS_FLAGS) $(PP_FLAGS) $(INC) ${src} >
> ${d_path}(+)${s_base}.ii
>  Trim --trim-long --source-code -o ${d_path}(+)${s_base}.
> ${d_path}(+)${s_base}.ii
> -# For RVCTCYGWIN ASM_FLAGS must be first to work around
> pathing issues
>  "$(ASM)" $(ASM_FLAGS) -o ${dst} $(INC) ${d_path}(+)${s_base}.
> 
>  [Nasm-Assembly-Code-File.COMMON.COMMON]
> @@ -276,13 +273,6 @@
>  $(RM) ${dst}
>  "$(SLINK)" cr ${dst} $(SLINK_FLAGS) @$(OBJECT_FILES_LIST)
> 
> -
> -"$(SLINK)" $(SLINK_FLAGS) ${dst} --via $(OBJECT_FILES_LIST)
> -
> -
> -# $(OBJECT_FILES_LIST) has wrong paths for cygwin
> -"$(SLINK)" $(SLINK_FLAGS) ${dst} $(OBJECT_FILES)
> -
>  
>  "$(SLINK)" $(SLINK_FLAGS) ${dst} -filelist $(OBJECT_FILES_LIST)
> 
> @@ -307,13 +297,6 @@
>  "$(DLINK)" -o ${dst} $(DLINK_FLAGS)
> -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS)
> $(DLINK2_FLAGS)
>  "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
> 
> -
> -"$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH) --via
> $(STATIC_LIBRARY_FILES_LIST) $(DLINK2_FLAGS)
> -
> -
> -#$(STATIC_LIBRARY_FILES_LIST) has wrong paths for cygwin
> -"$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH)
> $(STATIC_LIBRARY_FILES) $(DLINK2_FLAGS)
> -
>  
>  "$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH) -filelist
> $(STATIC_LIBRARY_FILES_LIST)  $(DLINK2_FLAGS)
> 
> @@ -349,13 +332,6 @@
>  
>  "$(DLINK)" $(DLINK_FLAGS)
> -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group
> $(DLINK2_FLAGS)
> 
> -
> -"$(DLINK)" $(DLINK_FLAGS) -o ${dst} 

回复: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB

[gaoliming]

Michael:
  I would suggest to reuse MdePkg/MdeLibs.dsc.inc to list the library and PCD 
from the edk2 core packages, such as MdePkg, MdeModulePkg, CryptoPkg, 
SecurirtyPkg and so on. Those packages are required by every platforms. They 
can't be separated. So, I think MdePkg/MdeLibs.dsc.inc is for edk2 core 
packages, not only for MdePkg. 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Michael
> Kubacki
> 发送时间: 2022年4月29日 23:48
> 收件人: Ard Biesheuvel 
> 抄送: edk2-devel-groups-io ; Abner Chang
> ; Andrew Fish ; Anthony Perard
> ; Ard Biesheuvel ;
> Benjamin You ; Brijesh Singh
> ; Erdem Aktas ; Gerd
> Hoffmann ; Guo Dong ; Hao A
> Wu ; James Bottomley ; Jian J
> Wang ; Jiewen Yao ; Jordan
> Justen ; Julien Grall ; Leif
> Lindholm ; Liming Gao
> ; Maurice Ma ; Min Xu
> ; Nickle Wang ; Peter Grehan
> ; Ray Ni ; Rebecca Cran
> ; Sami Mujawar ; Sean
> Rhodes ; Sebastien Boeuf
> ; Tom Lendacky 
> 主题: Re: [edk2-devel] [PATCH v5 0/8] Add Variable Flash Info HOB
> 
> I agree that would be a useful tool and in the case of changes such as
> this that provide backward compatibility with existing functionality,
> particularly helpful.
> 
> Some packages such as MdePkg
> (https://github.com/tianocore/edk2/blob/master/MdePkg/MdeLibs.dsc.inc)
> and NetworkPkg
> (https://github.com/tianocore/edk2/blob/master/NetworkPkg/NetworkCom
> ponents.dsc.inc)
> provide DSC files that a platform can override if necessary.
> 
> However, this does not exist for all edk2 packages. I did not introduce
> such a file in MdeModulePkg because I believe that is an independent
> package design decision outside the scope of this series and, if that
> change was made, it should include libraries other than just this
> instance. That would lead to additional churn and a larger platform
> integration debate, important to that topic, but separate from the
> current state this contribution is based against.
> 
> While includes be helpful, it can encourage platform owners to ignore
> potential creep in functionality they should be aware of.
> 
> For example, the DSC update is mostly being given to platforms to fix
> their immediate build problem. But, a platform owner might also choose
> to update their FVB driver to use this interface to get flash
> information as opposed to directly using PCDs as many do today. That's a
> decision they need to evaluate and make but they should be aware of the
> interface and make that decision. By directly reviewing/integrating the
> change for their platform, they are more explicitly made aware of this
> new interface to form that decision.
> 
> Also, when many include files get involved, platform build complexity
> and developer frustration can increase due to nesting and order of
> include files. Values (library classes, PCDs, etc.) can be overridden
> more than once. Ultimately, this is technically manageable by utilizing
> build reports and understanding the EDK II build output in more detail.
> 
> Again, I think this conversation is useful but requires much more time
> to address questions such as the following:
> 
> 1. Should a different mechanism for default library classes be introduced?
> 
> 2. Should all packages in edk2 provide such an include file? If so, does
> it only provide the DSC file (like MdePkg) or other files (like
> NetworkPkg which includes FDF)?
> 
> 3. Which library classes for a given package should be given default
> instances?
> 
> 4. How would each platform package maintainer like to maintain their
> platform build?
> 
> Example: Would MinPlatformPkg prefer to keep its own "core" include
> files
> (https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/Mi
> nPlatformPkg/Include)
> or reconcile them with include files (possibly introducing an additional
> layer of nesting)? Would others prefer not to use includes at all to
> keep a flat, simpler file?
> 
> How would someone updating the platform code due to an edk2 change be
> aware of this per-platform policy?
> 
> To my understanding, the answers to these today are:
> 
> 1. No
> 2. No / decided on per-change basis
> 3. Unknown
> 4. Unknown
> 
> (2) adds friction to the edk2 contribution process as expectation is
> unclear.
> 
> (3) adds churn into the platform integration process as the integration
> process for existing code is subject to change over time (e.g. realize
> library class is now in DSC and remove from platform DSC to use include
> instance).
> 
> (4) adds friction to the edk2 contribution process as expectation is
> unclear. Also, somewhat error prone. There's also a level of due
> diligence needed to confirm if a platform that already has an include is
> overriding that in another DSC file. If so, is that still the correct
> behavior or should it be modified.
> 
> Regards,
> Michael
> 
> On 4/29/2022 9:45 AM, Ard Biesheuvel wrote:
> > On Tue, 26 Apr 2022 at 03:29,  wrote:
> >>
> >> From: Michael Kubacki 
> >>
> >> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3479
> >>
> >> 

[edk2-devel] [PATCH v1 11/11] EmulatorPkg: Pipeline: Resolve SecureBootVariableLib dependency

[Kun Qin]

The new changes in SecureBootVariableLib brought in a new dependency of
PlatformPKProtectionLib.

This change added the new library instance from SecurityPkg to resolve
pipeline builds.

Cc: Andrew Fish 
Cc: Ray Ni 

Signed-off-by: Kun Qin 
---
 EmulatorPkg/EmulatorPkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index 554c13ddb500..06eb8633608c 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -133,6 +133,7 @@ [LibraryClasses]
   
PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+  
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
 !else
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
-- 
2.34.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89518): https://edk2.groups.io/g/devel/message/89518
Mute This Topic: https://groups.io/mt/90893939/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 10/11] OvmfPkg: Pipeline: Resolve SecureBootVariableLib dependency

[Kun Qin]

The new changes in SecureBootVariableLib brought in a new dependency of
PlatformPKProtectionLib.

This change added the new library instance from SecurityPkg to resolve
pipeline builds.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
Cc: Peter Grehan 
Cc: Sebastien Boeuf 

Signed-off-by: Kun Qin 
---
 OvmfPkg/Bhyve/BhyveX64.dsc   | 1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc   | 1 +
 OvmfPkg/IntelTdx/IntelTdxX64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32.dsc  | 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc   | 1 +
 OvmfPkg/OvmfPkgX64.dsc   | 1 +
 6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 002cef32a355..b3956104bde5 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -199,6 +199,7 @@ [LibraryClasses]
   
PlatformSecureLib|OvmfPkg/Bhyve/Library/PlatformSecureLib/PlatformSecureLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+  
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
 !else
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index d1c85f60c768..aba4711d361a 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -210,6 +210,7 @@ [LibraryClasses]
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+  
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
 !else
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 73a6c30096a8..607d45209361 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -178,6 +178,7 @@ [LibraryClasses]
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+  
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
 !else
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 533bbdb435ad..d11c6bb27f47 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -207,6 +207,7 @@ [LibraryClasses]
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+  
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
 !else
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index cb68e612bd35..13a65bcbe3ec 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -211,6 +211,7 @@ [LibraryClasses]
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+  
PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   
SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
 !else
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 45ffa2dbe35f..6e4fe923cc87 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -219,6 +219,7 @@ [LibraryClasses]
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   

[edk2-devel] [PATCH v1 09/11] SecurityPkg: SecureBootVariableLib: Added unit tests

[Kun Qin]

From: kuqin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3911

This change added unit test and enabled it from pipeline for the updated
SecureBootVariableLib.

The unit test covers all implemented interfaces and certain corner cases.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c
   |   36 +
 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c   
|  201 ++
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.c
   |   13 +
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.c
 | 2037 
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.inf
 |   33 +
 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.inf 
|   45 +
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
 |   25 +
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.inf
   |   36 +
 SecurityPkg/SecurityPkg.ci.yaml
|   11 +
 SecurityPkg/Test/SecurityPkgHostTest.dsc   
|   38 +
 10 files changed, 2475 insertions(+)

diff --git 
a/SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c
 
b/SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c
new file mode 100644
index ..a8644d272df6
--- /dev/null
+++ 
b/SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c
@@ -0,0 +1,36 @@
+/** @file
+  Provides a mocked interface for configuring PK related variable protection.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/**
+  Disable any applicable protection against variable 'PK'. The implementation
+  of this interface is platform specific, depending on the protection 
techniques
+  used per platform.
+
+  Note: It is the platform's responsibility to conduct cautious operation after
+disabling this protection.
+
+  @retval EFI_SUCCESS State has been successfully updated.
+  @retval Others  Error returned from implementation 
specific
+  underying APIs.
+
+**/
+EFI_STATUS
+EFIAPI
+DisablePKProtection (
+  VOID
+  )
+{
+  return (EFI_STATUS)mock ();
+}
diff --git a/SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c 
b/SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c
new file mode 100644
index ..df271c39f26c
--- /dev/null
+++ b/SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c
@@ -0,0 +1,201 @@
+/** @file
+  The UEFI Library provides functions and macros that simplify the development 
of
+  UEFI Drivers and UEFI Applications.  These functions and macros help manage 
EFI
+  events, build simple locks utilizing EFI Task Priority Levels (TPLs), install
+  EFI Driver Model related protocols, manage Unicode string tables for UEFI 
Drivers,
+  and print messages on the console output and standard error devices.
+
+  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+
+/**
+  Returns the status whether get the variable success. The function retrieves
+  variable  through the UEFI Runtime Service GetVariable().  The
+  returned buffer is allocated using AllocatePool().  The caller is responsible
+  for freeing this buffer with FreePool().
+
+  If Name  is NULL, then ASSERT().
+  If Guid  is NULL, then ASSERT().
+  If Value is NULL, then ASSERT().
+
+  @param[in]  Name  The pointer to a Null-terminated Unicode string.
+  @param[in]  Guid  The pointer to an EFI_GUID structure
+  @param[out] Value The buffer point saved the variable info.
+  @param[out] Size  The buffer size of the variable.
+
+  @return EFI_OUT_OF_RESOURCES  Allocate buffer failed.
+  @return EFI_SUCCESS   Find the specified variable.
+  @return Others Errors Return errors from call to 
gRT->GetVariable.
+
+**/
+EFI_STATUS
+EFIAPI
+GetVariable2 (
+  IN CONST CHAR16*Name,
+  IN CONST EFI_GUID  *Guid,
+  OUT VOID   **Value,
+  OUT UINTN  *Size OPTIONAL
+  )
+{
+  EFI_STATUS  Status;
+  UINTN   BufferSize;
+
+  ASSERT (Name != NULL && Guid != NULL && Value != NULL);
+
+  //
+  // Try to get the variable size.
+  //
+  BufferSize = 0;
+  *Value = NULL;
+  if (Size != NULL) {
+*Size = 0;
+  }
+
+  Status = gRT->GetVariable ((CHAR16 *)Name, (EFI_GUID *)Guid, NULL, 
, *Value);
+  if (Status != EFI_BUFFER_TOO_SMALL) {
+return Status;
+  }
+
+  //
+  // Allocate buffer to get the variable.
+  //
+  *Value = 

[edk2-devel] [PATCH v1 08/11] SecurityPkg: SecureBootConfigDxe: Updated invocation pattern

[Kun Qin]

From: Kun Qin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3909

This change is in pair with the previous SecureBootVariableLib change,
which updated the interface of `CreateTimeBasedPayload`.

This change added a helper function to query the current time through
Real Time Clock protocol. This function is used when needing to format
an authenticated variable payload.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c  
| 127 ++--
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf 
|   1 +
 2 files changed, 119 insertions(+), 9 deletions(-)

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index a13c349a0f89..4299a6b5e56d 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -10,6 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "SecureBootConfigImpl.h"
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -136,6 +137,51 @@ CloseEnrolledFile (
   FileContext->FileType = UNKNOWN_FILE_TYPE;
 }
 
+/**
+  Helper function to populate an EFI_TIME instance.
+
+  @param[in] Time   FileContext cached in SecureBootConfig driver
+
+**/
+STATIC
+EFI_STATUS
+GetCurrentTime (
+  IN EFI_TIME  *Time
+  )
+{
+  EFI_STATUS  Status;
+  VOID*TestPointer;
+
+  if (Time == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  Status = gBS->LocateProtocol (, NULL, 
);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  ZeroMem (Time, sizeof (EFI_TIME));
+  Status = gRT->GetTime (Time, NULL);
+  if (EFI_ERROR (Status)) {
+DEBUG ((
+  DEBUG_ERROR,
+  "%a(), GetTime() failed, status = '%r'\n",
+  __FUNCTION__,
+  Status
+  ));
+return Status;
+  }
+
+  Time->Pad1   = 0;
+  Time->Nanosecond = 0;
+  Time->TimeZone   = 0;
+  Time->Daylight   = 0;
+  Time->Pad2   = 0;
+
+  return EFI_SUCCESS;
+}
+
 /**
   This code checks if the FileSuffix is one of the possible DER-encoded 
certificate suffix.
 
@@ -436,6 +482,7 @@ EnrollPlatformKey (
   UINT32  Attr;
   UINTN   DataSize;
   EFI_SIGNATURE_LIST  *PkCert;
+  EFI_TIMETime;
 
   PkCert = NULL;
 
@@ -463,7 +510,13 @@ EnrollPlatformKey (
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
  | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
   DataSize = PkCert->SignatureListSize;
-  Status   = CreateTimeBasedPayload (, (UINT8 **));
+  Status   = GetCurrentTime ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (, (UINT8 **), );
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", 
Status));
 goto ON_EXIT;
@@ -522,6 +575,7 @@ EnrollRsa2048ToKek (
   UINTN   KekSigListSize;
   UINT8   *KeyBuffer;
   UINTN   KeyLenInBytes;
+  EFI_TIMETime;
 
   Attr   = 0;
   DataSize   = 0;
@@ -608,7 +662,13 @@ EnrollRsa2048ToKek (
   //
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
  | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-  Status = CreateTimeBasedPayload (, (UINT8 **));
+  Status = GetCurrentTime ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (, (UINT8 **), 
);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", 
Status));
 goto ON_EXIT;
@@ -689,6 +749,7 @@ EnrollX509ToKek (
   UINTN   DataSize;
   UINTN   KekSigListSize;
   UINT32  Attr;
+  EFI_TIMETime;
 
   X509Data   = NULL;
   X509DataSize   = 0;
@@ -735,7 +796,13 @@ EnrollX509ToKek (
   //
   Attr = EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_RUNTIME_ACCESS
  | EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
-  Status = CreateTimeBasedPayload (, (UINT8 **));
+  Status = GetCurrentTime ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Fail to fetch valid time data: %r", Status));
+goto ON_EXIT;
+  }
+
+  Status = CreateTimeBasedPayload (, (UINT8 **), 
);
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", 
Status));
 goto ON_EXIT;
@@ -861,6 +928,7 @@ EnrollX509toSigDB (
   UINTN   DataSize;
   UINTN   SigDBSize;
   UINT32  Attr;
+  EFI_TIMETime;
 
   X509DataSize  = 0;
   SigDBSize = 0;
@@ -910,7 +978,13 

[edk2-devel] [PATCH v1 05/11] SecurityPkg: SecureBootVariableLib: Added newly supported interfaces

[Kun Qin]

From: kuqin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3911

This change updated the interfaces provided by SecureBootVariableLib.

The new additions provided interfaces to enroll single authenticated
variable from input, a helper function to query secure boot status,
enroll all secure boot variables from UefiSecureBoot.h defined data
structures, a as well as a routine that deletes all secure boot related
variables.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c   | 364 

 SecurityPkg/Include/Library/SecureBootVariableLib.h |  69 
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf |   2 +
 3 files changed, 435 insertions(+)

diff --git a/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c 
b/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
index f56f0322e943..8167e58ca84c 100644
--- a/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
+++ b/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 // This time can be used when deleting variables, as it should be greater than 
any variable time.
 EFI_TIME  mMaxTimestamp = {
@@ -37,6 +38,25 @@ EFI_TIME  mMaxTimestamp = {
   0x00
 };
 
+//
+// MS Default Time-Based Payload Creation Date
+// This is the date that is used when creating SecureBoot default variables.
+// NOTE: This is a placeholder date that doesn't correspond to anything else.
+//
+EFI_TIME  mDefaultPayloadTimestamp = {
+  15,   // Year (2015)
+  8,// Month (Aug)
+  28,   // Day (28)
+  0,// Hour
+  0,// Minute
+  0,// Second
+  0,// Pad1
+  0,// Nanosecond
+  0,// Timezone (Dummy value)
+  0,// Daylight (Dummy value)
+  0 // Pad2
+};
+
 /** Creates EFI Signature List structure.
 
   @param[in]  Data A pointer to signature data.
@@ -413,6 +433,44 @@ GetSetupMode (
   return EFI_SUCCESS;
 }
 
+/**
+  Helper function to quickly determine whether SecureBoot is enabled.
+
+  @retval TRUESecureBoot is verifiably enabled.
+  @retval FALSE   SecureBoot is either disabled or an error prevented 
checking.
+
+**/
+BOOLEAN
+EFIAPI
+IsSecureBootEnabled (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+  UINT8   *SecureBoot;
+
+  SecureBoot = NULL;
+
+  Status = GetEfiGlobalVariable2 (EFI_SECURE_BOOT_MODE_NAME, (VOID 
**), NULL);
+  //
+  // Skip verification if SecureBoot variable doesn't exist.
+  //
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Cannot check SecureBoot variable %r \n ", Status));
+return FALSE;
+  }
+
+  //
+  // Skip verification if SecureBoot is disabled but not AuditMode
+  //
+  if (*SecureBoot == SECURE_BOOT_MODE_DISABLE) {
+FreePool (SecureBoot);
+return FALSE;
+  } else {
+return TRUE;
+  }
+}
+
 /**
   Clears the content of the 'db' variable.
 
@@ -531,3 +589,309 @@ DeletePlatformKey (
  );
   return Status;
 }
+
+/**
+  This function will delete the secure boot keys, thus
+  disabling secure boot.
+
+  @return EFI_SUCCESS or underlying failure code.
+**/
+EFI_STATUS
+EFIAPI
+DeleteSecureBootVariables (
+  VOID
+  )
+{
+  EFI_STATUS  Status, TempStatus;
+
+  DEBUG ((DEBUG_INFO, "%a - Attempting to delete the Secure Boot 
variables.\n", __FUNCTION__));
+
+  //
+  // Step 1: Notify that a PK update is coming shortly...
+  Status = DisablePKProtection ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "%a - Failed to signal PK update start! %r\n", 
__FUNCTION__, Status));
+// Classify this as a PK deletion error.
+Status = EFI_ABORTED;
+  }
+
+  //
+  // Step 2: Attempt to delete the PK.
+  // Let's try to nuke the PK, why not...
+  if (!EFI_ERROR (Status)) {
+Status = DeletePlatformKey ();
+DEBUG ((DEBUG_INFO, "%a - PK Delete = %r\n", __FUNCTION__, Status));
+// If the PK is not found, then our work here is done.
+if (Status == EFI_NOT_FOUND) {
+  Status = EFI_SUCCESS;
+}
+// If any other error occurred, let's inform the caller that the PK delete 
in particular failed.
+else if (EFI_ERROR (Status)) {
+  Status = EFI_ABORTED;
+}
+  }
+
+  //
+  // Step 3: Attempt to delete remaining keys/databases...
+  // Now that the PK is deleted (assuming Status == EFI_SUCCESS) the system is 
in SETUP_MODE.
+  // Arguably we could leave these variables in place and let them be deleted 
by whoever wants to
+  // update all the SecureBoot variables. However, for cleanliness sake, let's 
try to
+  // get rid of them here.
+  if (!EFI_ERROR (Status)) {
+//
+// If any of THESE steps have an error, report the error but attempt to 
delete all keys.
+// Using TempStatus will prevent an error from being trampled by an 
EFI_SUCCESS.
+// Overwrite Status ONLY if TempStatus is an error.
+//
+// If the error is EFI_NOT_FOUND, we can safely ignore it since we were 

[edk2-devel] [PATCH v1 06/11] SecurityPkg: SecureBootVariableProvisionLib: Updated implementation

[Kun Qin]

From: Kun Qin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3910

This change is in pair with the previous SecureBootVariableLib, which
removes the explicit invocation of `CreateTimeBasedPayload` and used new
interface `EnrollFromInput` instead.

The original `SecureBootFetchData` is also moved to this library and
incorporated with the newly defined `SecureBootCreateDataFromInput` to
keep the original code flow.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 
SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
 | 145 
 1 file changed, 115 insertions(+), 30 deletions(-)

diff --git 
a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
 
b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
index 536b0f369907..bed1fe86205d 100644
--- 
a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
+++ 
b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
@@ -8,10 +8,13 @@
   Copyright (c) 2021, Semihalf All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
+#include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -19,6 +22,117 @@
 #include 
 #include 
 #include 
+#include 
+
+/**
+  Create a EFI Signature List with data fetched from section specified as a 
argument.
+  Found keys are verified using RsaGetPublicKeyFromX509().
+
+  @param[in]KeyFileGuidA pointer to to the FFS filename GUID
+  @param[out]   SigListsSize   A pointer to size of signature list
+  @param[out]   SigListOuta pointer to a callee-allocated buffer with 
signature lists
+
+  @retval EFI_SUCCESS  Create time based payload successfully.
+  @retval EFI_NOT_FOUNDSection with key has not been found.
+  @retval EFI_INVALID_PARAMETEREmbedded key has a wrong format.
+  @retval Others   Unexpected error happens.
+
+**/
+STATIC
+EFI_STATUS
+SecureBootFetchData (
+  IN  EFI_GUID*KeyFileGuid,
+  OUT UINTN   *SigListsSize,
+  OUT EFI_SIGNATURE_LIST  **SigListOut
+  )
+{
+  EFI_SIGNATURE_LIST*EfiSig;
+  EFI_STATUSStatus;
+  VOID  *Buffer;
+  VOID  *RsaPubKey;
+  UINTN Size;
+  UINTN KeyIndex;
+  UINTN Index;
+  SECURE_BOOT_CERTIFICATE_INFO  *CertInfo;
+  SECURE_BOOT_CERTIFICATE_INFO  *NewCertInfo;
+
+  KeyIndex  = 0;
+  EfiSig= NULL;
+  *SigListOut   = NULL;
+  *SigListsSize = 0;
+  CertInfo  = AllocatePool (sizeof (SECURE_BOOT_CERTIFICATE_INFO));
+  NewCertInfo   = CertInfo;
+  while (1) {
+if (NewCertInfo == NULL) {
+  Status = EFI_OUT_OF_RESOURCES;
+  break;
+} else {
+  CertInfo = NewCertInfo;
+}
+
+Status = GetSectionFromAnyFv (
+   KeyFileGuid,
+   EFI_SECTION_RAW,
+   KeyIndex,
+   ,
+   
+   );
+
+if (Status == EFI_SUCCESS) {
+  RsaPubKey = NULL;
+  if (RsaGetPublicKeyFromX509 (Buffer, Size, ) == FALSE) {
+DEBUG ((DEBUG_ERROR, "%a: Invalid key format: %d\n", __FUNCTION__, 
KeyIndex));
+if (EfiSig != NULL) {
+  FreePool (EfiSig);
+}
+
+FreePool (Buffer);
+Status = EFI_INVALID_PARAMETER;
+break;
+  }
+
+  CertInfo[KeyIndex].Data = Buffer;
+  CertInfo[KeyIndex].DataSize = Size;
+  KeyIndex++;
+  NewCertInfo = ReallocatePool (
+  sizeof (SECURE_BOOT_CERTIFICATE_INFO) * KeyIndex,
+  sizeof (SECURE_BOOT_CERTIFICATE_INFO) * (KeyIndex + 1),
+  CertInfo
+  );
+}
+
+if (Status == EFI_NOT_FOUND) {
+  Status = EFI_SUCCESS;
+  break;
+}
+  }
+
+  if (EFI_ERROR (Status)) {
+goto Cleanup;
+  }
+
+  if (KeyIndex == 0) {
+Status = EFI_NOT_FOUND;
+goto Cleanup;
+  }
+
+  // Now that we collected all certs from FV, convert it into sig list
+  Status = SecureBootCreateDataFromInput (SigListsSize, SigListOut, KeyIndex, 
CertInfo);
+  if (EFI_ERROR (Status)) {
+goto Cleanup;
+  }
+
+Cleanup:
+  if (CertInfo) {
+for (Index = 0; Index < KeyIndex; Index++) {
+  FreePool ((VOID *)CertInfo[Index].Data);
+}
+
+FreePool (CertInfo);
+  }
+
+  return Status;
+}
 
 /**
   Enroll a key/certificate based on a default variable.
@@ -52,36 +166,7 @@ EnrollFromDefault (
 return Status;
   }
 
-  CreateTimeBasedPayload (, (UINT8 **));
-  if (EFI_ERROR (Status)) {
-DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", 
Status));
-return Status;
-  }
-
-  //
-  // Allocate memory for auth variable
-  //
-  Status = gRT->SetVariable (
-  VariableName,
-  VendorGuid,
-  

[edk2-devel] [PATCH v1 07/11] SecurityPkg: Secure Boot Drivers: Added common header files

[Kun Qin]

From: Kun Qin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3910

This change added common header files to consumer drivers to unblock
pipeline builds.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
   | 1 +
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c   
   | 1 +
 
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c
 | 1 +
 3 files changed, 3 insertions(+)

diff --git a/SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c 
b/SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
index cb7095b269b1..aa4d0c7a993d 100644
--- a/SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
+++ b/SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
@@ -19,6 +19,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include  // AsciiPrint()
 #include  // gRT
 #include 
+#include 
 #include 
 #include 
 
diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 0122e8d55fa0..a13c349a0f89 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
 #include "SecureBootConfigImpl.h"
+#include 
 #include 
 #include 
 #include 
diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c
 
b/SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c
index ef7b01f16119..0abde52a05ae 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c
@@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
-- 
2.34.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89514): https://edk2.groups.io/g/devel/message/89514
Mute This Topic: https://groups.io/mt/90893935/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 03/11] SecurityPkg: SecureBootVariableLib: Updated time based payload creator

[Kun Qin]

From: Kun Qin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3909

This change updated the interface of 'CreateTimeBasedPayload' by
requiring the caller to provide a timestamp, instead of relying on time
protocol to be ready during runtime. It intends to extend the library
availability during boot environment.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c   | 53 

 SecurityPkg/Include/Library/SecureBootVariableLib.h |  9 +++-
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf |  8 +--
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c 
b/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
index e0d137666e0e..3b33a356aba3 100644
--- a/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
+++ b/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
@@ -6,8 +6,10 @@
   (C) Copyright 2018 Hewlett Packard Enterprise Development LP
   Copyright (c) 2021, ARM Ltd. All rights reserved.
   Copyright (c) 2021, Semihalf All rights reserved.
+  Copyright (c) Microsoft Corporation.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
+#include 
 #include 
 #include 
 #include 
@@ -21,6 +23,21 @@
 #include 
 #include "Library/DxeServicesLib.h"
 
+// This time can be used when deleting variables, as it should be greater than 
any variable time.
+EFI_TIME  mMaxTimestamp = {
+  0x, // Year
+  0xFF,   // Month
+  0xFF,   // Day
+  0xFF,   // Hour
+  0xFF,   // Minute
+  0xFF,   // Second
+  0x00,
+  0x, // Nanosecond
+  0,
+  0,
+  0x00
+};
+
 /** Creates EFI Signature List structure.
 
   @param[in]  Data A pointer to signature data.
@@ -118,7 +135,7 @@ ConcatenateSigList (
 
   @param[in]KeyFileGuidA pointer to to the FFS filename GUID
   @param[out]   SigListsSize   A pointer to size of signature list
-  @param[out]   SigListOuta pointer to a callee-allocated buffer with 
signature lists
+  @param[out]   SigListsOuta pointer to a callee-allocated buffer with 
signature lists
 
   @retval EFI_SUCCESS  Create time based payload successfully.
   @retval EFI_NOT_FOUNDSection with key has not been found.
@@ -210,28 +227,30 @@ SecureBootFetchData (
pointer to NULL to wrap an empty payload.
On output, Pointer to the new payload date 
buffer allocated from pool,
it's caller's responsibility to free the 
memory when finish using it.
+  @param[in]Time   Pointer to time information to created time 
based payload.
 
   @retval EFI_SUCCESS  Create time based payload successfully.
   @retval EFI_OUT_OF_RESOURCES There are not enough memory resources to 
create time based payload.
   @retval EFI_INVALID_PARAMETERThe parameter is invalid.
   @retval Others   Unexpected error happens.
 
-**/
+--*/
 EFI_STATUS
+EFIAPI
 CreateTimeBasedPayload (
-  IN OUT UINTN  *DataSize,
-  IN OUT UINT8  **Data
+  IN OUT UINTN *DataSize,
+  IN OUT UINT8 **Data,
+  IN EFI_TIME  *Time
   )
 {
-  EFI_STATUS Status;
   UINT8  *NewData;
   UINT8  *Payload;
   UINTN  PayloadSize;
   EFI_VARIABLE_AUTHENTICATION_2  *DescriptorData;
   UINTN  DescriptorSize;
-  EFI_TIME   Time;
 
-  if ((Data == NULL) || (DataSize == NULL)) {
+  if ((Data == NULL) || (DataSize == NULL) || (Time == NULL)) {
+DEBUG ((DEBUG_ERROR, "%a(), invalid arg\n", __FUNCTION__));
 return EFI_INVALID_PARAMETER;
   }
 
@@ -247,6 +266,7 @@ CreateTimeBasedPayload (
   DescriptorSize = OFFSET_OF (EFI_VARIABLE_AUTHENTICATION_2, AuthInfo) + 
OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData);
   NewData= (UINT8 *)AllocateZeroPool (DescriptorSize + PayloadSize);
   if (NewData == NULL) {
+DEBUG ((DEBUG_ERROR, "%a() Out of resources.\n", __FUNCTION__));
 return EFI_OUT_OF_RESOURCES;
   }
 
@@ -256,19 +276,7 @@ CreateTimeBasedPayload (
 
   DescriptorData = (EFI_VARIABLE_AUTHENTICATION_2 *)(NewData);
 
-  ZeroMem (, sizeof (EFI_TIME));
-  Status = gRT->GetTime (, NULL);
-  if (EFI_ERROR (Status)) {
-FreePool (NewData);
-return Status;
-  }
-
-  Time.Pad1   = 0;
-  Time.Nanosecond = 0;
-  Time.TimeZone   = 0;
-  Time.Daylight   = 0;
-  Time.Pad2   = 0;
-  CopyMem (>TimeStamp, , sizeof (EFI_TIME));
+  CopyMem (>TimeStamp, Time, sizeof (EFI_TIME));
 
   DescriptorData->AuthInfo.Hdr.dwLength = OFFSET_OF 
(WIN_CERTIFICATE_UEFI_GUID, CertData);
   DescriptorData->AuthInfo.Hdr.wRevision= 0x0200;
@@ -277,6 +285,7 @@ CreateTimeBasedPayload (
 
   if (Payload != NULL) {
 FreePool 

[edk2-devel] [PATCH v1 04/11] SecurityPkg: SecureBootVariableLib: Updated signature list creator

[Kun Qin]

From: kuqin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3910

This change removes the interface of SecureBootFetchData, and replaced
it with `SecureBootCreateDataFromInput`, which will require caller to
prepare available certificates in defined structures.

This improvement will eliminate the dependency of reading from FV,
extending the availability of this library instance.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c   | 69 
+++-
 SecurityPkg/Include/Library/SecureBootVariableLib.h | 25 
---
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf |  3 -
 3 files changed, 53 insertions(+), 44 deletions(-)

diff --git a/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c 
b/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
index 3b33a356aba3..f56f0322e943 100644
--- a/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
+++ b/SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c
@@ -10,10 +10,10 @@
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include "Library/DxeServicesLib.h"
 
 // This time can be used when deleting variables, as it should be greater than 
any variable time.
 EFI_TIME  mMaxTimestamp = {
@@ -130,24 +129,29 @@ ConcatenateSigList (
 }
 
 /**
-  Create a EFI Signature List with data fetched from section specified as a 
argument.
-  Found keys are verified using RsaGetPublicKeyFromX509().
+  Create a EFI Signature List with data supplied from input argument.
+  The input certificates from KeyInfo parameter should be DER-encoded
+  format.
 
-  @param[in]KeyFileGuidA pointer to to the FFS filename GUID
   @param[out]   SigListsSize   A pointer to size of signature list
-  @param[out]   SigListsOuta pointer to a callee-allocated buffer with 
signature lists
+  @param[out]   SigListOut A pointer to a callee-allocated buffer with 
signature lists
+  @param[in]KeyInfoCount   The number of certificate pointer and size 
pairs inside KeyInfo.
+  @param[in]KeyInfoA pointer to all certificates, in the 
format of DER-encoded,
+   to be concatenated into signature lists.
 
-  @retval EFI_SUCCESS  Create time based payload successfully.
+  @retval EFI_SUCCESS  Created signature list from payload 
successfully.
   @retval EFI_NOT_FOUNDSection with key has not been found.
-  @retval EFI_INVALID_PARAMETEREmbedded key has a wrong format.
+  @retval EFI_INVALID_PARAMETEREmbedded key has a wrong format or input 
pointers are NULL.
   @retval Others   Unexpected error happens.
 
 **/
 EFI_STATUS
-SecureBootFetchData (
-  IN  EFI_GUID*KeyFileGuid,
-  OUT UINTN   *SigListsSize,
-  OUT EFI_SIGNATURE_LIST  **SigListOut
+EFIAPI
+SecureBootCreateDataFromInput (
+  OUT UINTN   *SigListsSize,
+  OUT EFI_SIGNATURE_LIST  **SigListOut,
+  IN  UINTN   KeyInfoCount,
+  IN  CONST SECURE_BOOT_CERTIFICATE_INFO  *KeyInfo
   )
 {
   EFI_SIGNATURE_LIST  *EfiSig;
@@ -155,36 +159,41 @@ SecureBootFetchData (
   EFI_SIGNATURE_LIST  *TmpEfiSig2;
   EFI_STATUS  Status;
   VOID*Buffer;
-  VOID*RsaPubKey;
   UINTN   Size;
+  UINTN   InputIndex;
   UINTN   KeyIndex;
 
+  if ((SigListOut == NULL) || (SigListsSize == NULL)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if ((KeyInfoCount == 0) || (KeyInfo == NULL)) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  InputIndex= 0;
   KeyIndex  = 0;
   EfiSig= NULL;
   *SigListsSize = 0;
-  while (1) {
-Status = GetSectionFromAnyFv (
-   KeyFileGuid,
-   EFI_SECTION_RAW,
-   KeyIndex,
-   ,
-   
-   );
-
-if (Status == EFI_SUCCESS) {
-  RsaPubKey = NULL;
-  if (RsaGetPublicKeyFromX509 (Buffer, Size, ) == FALSE) {
-DEBUG ((DEBUG_ERROR, "%a: Invalid key format: %d\n", __FUNCTION__, 
KeyIndex));
+  while (InputIndex < KeyInfoCount) {
+if (KeyInfo[InputIndex].Data != NULL) {
+  Size   = KeyInfo[InputIndex].DataSize;
+  Buffer = AllocateCopyPool (Size, KeyInfo[InputIndex].Data);
+  if (Buffer == NULL) {
 if (EfiSig != NULL) {
   FreePool (EfiSig);
 }
 
-FreePool (Buffer);
-return EFI_INVALID_PARAMETER;
+return EFI_OUT_OF_RESOURCES;
   }
 
   Status = CreateSigList (Buffer, Size, );
 
+  if (EFI_ERROR (Status)) {
+FreePool (Buffer);
+break;
+  }
+
   //
   // Concatenate lists if more than one section found
  

[edk2-devel] [PATCH v1 02/11] SecurityPkg: PlatformPKProtectionLib: Added PK protection interface

[Kun Qin]

From: Kun Qin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3911

This patch provides an abstracted interface for platform to implement PK
variable related protection interface, which is designed to be used when
PK variable is about to be changed by UEFI firmware.

This change also provided a variable policy based library implementation
to accomodate platforms that supports variable policy for variable
protections.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 
SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c
   | 51 
 SecurityPkg/Include/Library/PlatformPKProtectionLib.h  
   | 31 
 
SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
 | 36 ++
 SecurityPkg/SecurityPkg.dec
   |  5 ++
 SecurityPkg/SecurityPkg.dsc
   |  2 +
 5 files changed, 125 insertions(+)

diff --git 
a/SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c
 
b/SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c
new file mode 100644
index ..a2649242246f
--- /dev/null
+++ 
b/SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c
@@ -0,0 +1,51 @@
+/** @file
+  Provides an abstracted interface for configuring PK related variable 
protection.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include 
+#include 
+
+#include 
+#include 
+
+/**
+  Disable any applicable protection against variable 'PK'. The implementation
+  of this interface is platform specific, depending on the protection 
techniques
+  used per platform.
+
+  Note: It is the platform's responsibility to conduct cautious operation after
+disabling this protection.
+
+  @retval EFI_SUCCESS State has been successfully updated.
+  @retval Others  Error returned from implementation 
specific
+  underying APIs.
+
+**/
+EFI_STATUS
+EFIAPI
+DisablePKProtection (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+  EDKII_VARIABLE_POLICY_PROTOCOL  *VariablePolicy;
+
+  DEBUG ((DEBUG_INFO, "%a() Entry...\n", __FUNCTION__));
+
+  // IMPORTANT NOTE: This operation is sticky and leaves variable protections 
disabled.
+  //  The system *MUST* be reset after performing this 
operation.
+  Status = gBS->LocateProtocol (, NULL, (VOID 
**));
+  if (!EFI_ERROR (Status)) {
+Status = VariablePolicy->DisableVariablePolicy ();
+// EFI_ALREADY_STARTED means that everything is currently disabled.
+// This should be considered SUCCESS.
+if (Status == EFI_ALREADY_STARTED) {
+  Status = EFI_SUCCESS;
+}
+  }
+
+  return Status;
+}
diff --git a/SecurityPkg/Include/Library/PlatformPKProtectionLib.h 
b/SecurityPkg/Include/Library/PlatformPKProtectionLib.h
new file mode 100644
index ..3586a47b77c2
--- /dev/null
+++ b/SecurityPkg/Include/Library/PlatformPKProtectionLib.h
@@ -0,0 +1,31 @@
+/** @file
+  Provides an abstracted interface for configuring PK related variable 
protection.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef PLATFORM_PK_PROTECTION_LIB_H_
+#define PLATFORM_PK_PROTECTION_LIB_H_
+
+/**
+  Disable any applicable protection against variable 'PK'. The implementation
+  of this interface is platform specific, depending on the protection 
techniques
+  used per platform.
+
+  Note: It is the platform's responsibility to conduct cautious operation after
+disabling this protection.
+
+  @retval EFI_SUCCESS State has been successfully updated.
+  @retval Others  Error returned from implementation 
specific
+  underying APIs.
+
+**/
+EFI_STATUS
+EFIAPI
+DisablePKProtection (
+  VOID
+  );
+
+#endif
diff --git 
a/SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
 
b/SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
new file mode 100644
index ..df42ce06c019
--- /dev/null
+++ 
b/SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
@@ -0,0 +1,36 @@
+## @file
+#  Provides an abstracted interface for configuring PK related variable 
protection.
+#
+#  Copyright (c) Microsoft Corporation.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = PlatformPKProtectionLibVarPolicy
+  FILE_GUID  = AE0C5992-526C-4518-93BA-3C2611B801E0
+  MODULE_TYPE= DXE_DRIVER
+  VERSION_STRING = 1.0
+  

[edk2-devel] [PATCH v1 00/11] Enhance Secure Boot Variable Libraries

[Kun Qin]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3909
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3910
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3911

Current SecureBootVariableLib provide great support for deleting secure
boot related variables, creating time-based payloads.

However, for secure boot enrollment, the SecureBootVariableProvisionLib
interfaces always assume the changes from variable storage, limiting the
usage, requiring existing platforms to change key initialization process
to adapt to the new methods, as well as bringing in extra dependencies
such as FV protocol, time protocols.

This patch series proposes to update the implementation for Secure Boot
Variable libraries and their consumers to better support the related
variables operations.

Patch v1 branch: https://github.com/kuqin12/edk2/tree/secure_boot_enhance_v1

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 
Cc: Sean Brogan 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
Cc: Peter Grehan 
Cc: Sebastien Boeuf 
Cc: Andrew Fish 
Cc: Ray Ni 

Kun Qin (8):
  SecurityPkg: UefiSecureBoot: Definitions of cert and payload
structures
  SecurityPkg: PlatformPKProtectionLib: Added PK protection interface
  SecurityPkg: SecureBootVariableLib: Updated time based payload creator
  SecurityPkg: SecureBootVariableProvisionLib: Updated implementation
  SecurityPkg: Secure Boot Drivers: Added common header files
  SecurityPkg: SecureBootConfigDxe: Updated invocation pattern
  OvmfPkg: Pipeline: Resolve SecureBootVariableLib dependency
  EmulatorPkg: Pipeline: Resolve SecureBootVariableLib dependency

kuqin (3):
  SecurityPkg: SecureBootVariableLib: Updated signature list creator
  SecurityPkg: SecureBootVariableLib: Added newly supported interfaces
  SecurityPkg: SecureBootVariableLib: Added unit tests

 SecurityPkg/EnrollFromDefaultKeysApp/EnrollFromDefaultKeysApp.c
   |1 +
 
SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.c
   |   51 +
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.c  
   |  484 -
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.c
  |   36 +
 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.c   
   |  201 ++
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.c
  |   13 +
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.c
| 2037 
 
SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
   |  145 +-
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c   
   |  128 +-
 
SecurityPkg/VariableAuthenticated/SecureBootDefaultKeysDxe/SecureBootDefaultKeysDxe.c
 |1 +
 EmulatorPkg/EmulatorPkg.dsc
   |1 +
 OvmfPkg/Bhyve/BhyveX64.dsc 
   |1 +
 OvmfPkg/CloudHv/CloudHvX64.dsc 
   |1 +
 OvmfPkg/IntelTdx/IntelTdxX64.dsc   
   |1 +
 OvmfPkg/OvmfPkgIa32.dsc
   |1 +
 OvmfPkg/OvmfPkgIa32X64.dsc 
   |1 +
 OvmfPkg/OvmfPkgX64.dsc 
   |1 +
 SecurityPkg/Include/Library/PlatformPKProtectionLib.h  
   |   31 +
 SecurityPkg/Include/Library/SecureBootVariableLib.h
   |  103 +-
 SecurityPkg/Include/UefiSecureBoot.h   
   |   94 +
 
SecurityPkg/Library/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
 |   36 +
 SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
   |   13 +-
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockPlatformPKProtectionLib.inf
|   33 +
 SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiLib.inf 
   |   45 +
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/MockUefiRuntimeServicesTableLib.inf
|   25 +
 
SecurityPkg/Library/SecureBootVariableLib/UnitTest/SecureBootVariableLibUnitTest.inf
  |   36 +
 SecurityPkg/SecurityPkg.ci.yaml
   |   11 +
 SecurityPkg/SecurityPkg.dec
   |5 +
 SecurityPkg/SecurityPkg.dsc
   |2 +
 SecurityPkg/Test/SecurityPkgHostTest.dsc   
   |   38 +
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf  
  

[edk2-devel] [PATCH v1 01/11] SecurityPkg: UefiSecureBoot: Definitions of cert and payload structures

[Kun Qin]

From: Kun Qin 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3910

This change added certificate and payload structures that can be consumed
by SecureBootVariableLib and other Secure Boot related operations.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Min Xu 

Signed-off-by: Kun Qin 
---
 SecurityPkg/Include/UefiSecureBoot.h | 94 
 1 file changed, 94 insertions(+)

diff --git a/SecurityPkg/Include/UefiSecureBoot.h 
b/SecurityPkg/Include/UefiSecureBoot.h
new file mode 100644
index ..642fef38f3a1
--- /dev/null
+++ b/SecurityPkg/Include/UefiSecureBoot.h
@@ -0,0 +1,94 @@
+/** @file
+  Provides a Secure Boot related data structure definitions.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef UEFI_SECURE_BOOT_H_
+#define UEFI_SECURE_BOOT_H_
+
+#pragma pack (push, 1)
+
+/*
+  Data structure to provide certificates to setup authenticated secure
+  boot variables ('db', 'dbx', 'dbt', 'pk', etc.).
+
+*/
+typedef struct {
+  //
+  // The size, in number of bytes, of supplied certificate in 'Data' field.
+  //
+  UINTN DataSize;
+  //
+  // The pointer to the certificates in DER-encoded format.
+  // Note: This certificate data should not contain the 
EFI_VARIABLE_AUTHENTICATION_2
+  //   for authenticated variables.
+  //
+  CONST VOID*Data;
+} SECURE_BOOT_CERTIFICATE_INFO;
+
+/*
+  Data structure to provide all Secure Boot related certificates.
+
+*/
+typedef struct {
+  //
+  // The human readable name for this set of Secure Boot key sets.
+  //
+  CONST CHAR16*SecureBootKeyName;
+  //
+  // The size, in number of bytes, of supplied certificate in 'DbPtr' field.
+  //
+  UINTN   DbSize;
+  //
+  // The pointer to the DB certificates in signature list format.
+  // Note: This DB certificates should not contain the 
EFI_VARIABLE_AUTHENTICATION_2
+  //   for authenticated variables.
+  //
+  CONST VOID  *DbPtr;
+  //
+  // The size, in number of bytes, of supplied certificate in 'DbxPtr' field.
+  //
+  UINTN   DbxSize;
+  //
+  // The pointer to the DBX certificates in signature list format.
+  // Note: This DBX certificates should not contain the 
EFI_VARIABLE_AUTHENTICATION_2
+  //   for authenticated variables.
+  //
+  CONST VOID  *DbxPtr;
+  //
+  // The size, in number of bytes, of supplied certificate in 'DbtPtr' field.
+  //
+  UINTN   DbtSize;
+  //
+  // The pointer to the DBT certificates in signature list format.
+  // Note: This DBT certificates should not contain the 
EFI_VARIABLE_AUTHENTICATION_2
+  //   for authenticated variables.
+  //
+  CONST VOID  *DbtPtr;
+  //
+  // The size, in number of bytes, of supplied certificate in 'KekPtr' field.
+  //
+  UINTN   KekSize;
+  //
+  // The pointer to the KEK certificates in signature list format.
+  // Note: This KEK certificates should not contain the 
EFI_VARIABLE_AUTHENTICATION_2
+  //   for authenticated variables.
+  //
+  CONST VOID  *KekPtr;
+  //
+  // The size, in number of bytes, of supplied certificate in 'PkPtr' field.
+  //
+  UINTN   PkSize;
+  //
+  // The pointer to the PK certificates in signature list format.
+  // Note: This PK certificates should not contain the 
EFI_VARIABLE_AUTHENTICATION_2
+  //   for authenticated variables.
+  //
+  CONST VOID  *PkPtr;
+} SECURE_BOOT_PAYLOAD_INFO;
+#pragma pack (pop)
+
+#endif // UEFI_SECURE_BOOT_H_
-- 
2.34.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89508): https://edk2.groups.io/g/devel/message/89508
Mute This Topic: https://groups.io/mt/90893929/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS configure macro

[yi1 li]

Hi Jiewen,

Thanks for feedback, I will check it.
For 7), I will submit relevant TLS function code together next patch.

-Original Message-
From: Yao, Jiewen  
Sent: Wednesday, May 4, 2022 6:13 PM
To: devel@edk2.groups.io; Li, Yi1 
Cc: Kinney, Michael D ; Gao, Liming 

Subject: RE: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS configure 
macro

Thanks Yi.

Some feedback:

1) {0x13, *} is defined in TLS1.3 - 
https://datatracker.ietf.org/doc/html/rfc8446#appendix-B.4
The comment ">  /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and 
rfc-5246." should be updated to include 8446 as well.

2) Although it is not absolutely required, I highly recommend to add specific 
value to TLS_HASH_ALGO, to align with definition in RFC.
> +  TlsHashAlgoNone = 0,
> +  TlsHashAlgoMd5 = 1,
> +  TlsHashAlgoSha1 = 2,
> +  TlsHashAlgoSha224 = 3,
> +  TlsHashAlgoSha256 = 4,
> +  TlsHashAlgoSha384 = 5,
> +  TlsHashAlgoSha512 = 6,
> +} TLS_HASH_ALGO;

3) Ditto, for TLS_SIGNATURE_ALGO.

> +  TlsSignatureAlgoAnonymous = 0,
> +  TlsSignatureAlgoRsa = 1,
> +  TlsSignatureAlgoDsa = 2,
> +  TlsSignatureAlgoEcdsa = 3,
> +} TLS_SIGNATURE_ALGO;

The value is assigned in the spec. It cannot be changed.

4) RFC4492 is obsoleted by RFC8422 - 
https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.1

=
   RFC 4492 defined 25 different curves in the NamedCurve registry (now
   renamed the "TLS Supported Groups" registry, although the enumeration
   below is still named NamedCurve) for use in TLS.  Only three have
   seen much use.  This specification is deprecating the rest (with
   numbers 1-22).
=

I don't see a reason to define so many deprecated algorithms.
Would you please align with section 5.1.1 in RFC8422? You may consider to add 
x25519 and x448 as well.


   enum {
   deprecated(1..22),
   secp256r1 (23), secp384r1 (24), secp521r1 (25),
   x25519(29), x448(30),
   reserved (0xFE00..0xFEFF),
   deprecated(0xFF01..0xFF02),
   (0x)
   } NamedCurve;


5) Since you added TLS 1.3 cipher suit, I assume you also want to add 
definition for TLS 1.3.

Please aware that "signature_algorithms" is changed in TLS 1.3 - 
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3.
I am not sure if you need define that as well.


  enum {
  /* RSASSA-PKCS1-v1_5 algorithms */
  rsa_pkcs1_sha256(0x0401),
  rsa_pkcs1_sha384(0x0501),
  rsa_pkcs1_sha512(0x0601),

  /* ECDSA algorithms */
  ecdsa_secp256r1_sha256(0x0403),
  ecdsa_secp384r1_sha384(0x0503),
  ecdsa_secp521r1_sha512(0x0603),

  /* RSASSA-PSS algorithms with public key OID rsaEncryption */
  rsa_pss_rsae_sha256(0x0804),
  rsa_pss_rsae_sha384(0x0805),
  rsa_pss_rsae_sha512(0x0806),

  /* EdDSA algorithms */
  ed25519(0x0807),
  ed448(0x0808),

  /* RSASSA-PSS algorithms with public key OID RSASSA-PSS */
  rsa_pss_pss_sha256(0x0809),
  rsa_pss_pss_sha384(0x080a),
  rsa_pss_pss_sha512(0x080b),
...
  } SignatureScheme;


6) Ditto. Please aware that "NamedCurve" is changed in TLS 1.3 - 
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.7
I am not sure if you need define that as well.


  enum {

  /* Elliptic Curve Groups (ECDHE) */
  secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019),
  x25519(0x001D), x448(0x001E),
...
  } NamedGroup;


7) Last but not least, I hope to see how those new definition is used.
Without consumer, it is hard for me to understand why they are needed, or if we 
miss something else.


Thank you
Yao, Jiewen


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of yi1 li
> Sent: Wednesday, May 4, 2022 5:31 PM
> To: devel@edk2.groups.io
> Cc: Li, Yi1 ; Kinney, Michael D 
> ; Gao, Liming 
> Subject: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS 
> configure macro
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3892
> 
> Which are needed for SUITE-B and SUITE-B-192.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Signed-off-by: yi1 li 
> ---
>  MdePkg/Include/IndustryStandard/Tls1.h | 133 
> ++---
>  1 file changed, 97 insertions(+), 36 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Tls1.h
> b/MdePkg/Include/IndustryStandard/Tls1.h
> index cf67428b1129..6519afe15e78 100644
> --- a/MdePkg/Include/IndustryStandard/Tls1.h
> +++ b/MdePkg/Include/IndustryStandard/Tls1.h
> @@ -15,42 +15,49 @@
>  ///
>  /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and rfc-5246.
>  ///
> -#define TLS_RSA_WITH_NULL_MD5{0x00, 0x01}
> -#define TLS_RSA_WITH_NULL_SHA{0x00, 0x02}
> -#define TLS_RSA_WITH_RC4_128_MD5 {0x00, 0x04}
> -#define TLS_RSA_WITH_RC4_128_SHA 

Re: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS configure macro

[Yao, Jiewen]

Thanks Yi.

Some feedback:

1) {0x13, *} is defined in TLS1.3 - 
https://datatracker.ietf.org/doc/html/rfc8446#appendix-B.4
The comment ">  /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and 
rfc-5246." should be updated to include 8446 as well.

2) Although it is not absolutely required, I highly recommend to add specific 
value to TLS_HASH_ALGO, to align with definition in RFC.
> +  TlsHashAlgoNone = 0,
> +  TlsHashAlgoMd5 = 1,
> +  TlsHashAlgoSha1 = 2,
> +  TlsHashAlgoSha224 = 3,
> +  TlsHashAlgoSha256 = 4,
> +  TlsHashAlgoSha384 = 5,
> +  TlsHashAlgoSha512 = 6,
> +} TLS_HASH_ALGO;

3) Ditto, for TLS_SIGNATURE_ALGO.

> +  TlsSignatureAlgoAnonymous = 0,
> +  TlsSignatureAlgoRsa = 1,
> +  TlsSignatureAlgoDsa = 2,
> +  TlsSignatureAlgoEcdsa = 3,
> +} TLS_SIGNATURE_ALGO;

The value is assigned in the spec. It cannot be changed.

4) RFC4492 is obsoleted by RFC8422 - 
https://datatracker.ietf.org/doc/html/rfc8422#section-5.1.1

=
   RFC 4492 defined 25 different curves in the NamedCurve registry (now
   renamed the "TLS Supported Groups" registry, although the enumeration
   below is still named NamedCurve) for use in TLS.  Only three have
   seen much use.  This specification is deprecating the rest (with
   numbers 1-22).
=

I don't see a reason to define so many deprecated algorithms.
Would you please align with section 5.1.1 in RFC8422? You may consider to add 
x25519 and x448 as well.


   enum {
   deprecated(1..22),
   secp256r1 (23), secp384r1 (24), secp521r1 (25),
   x25519(29), x448(30),
   reserved (0xFE00..0xFEFF),
   deprecated(0xFF01..0xFF02),
   (0x)
   } NamedCurve;


5) Since you added TLS 1.3 cipher suit, I assume you also want to add 
definition for TLS 1.3.

Please aware that "signature_algorithms" is changed in TLS 1.3 - 
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.3.
I am not sure if you need define that as well.


  enum {
  /* RSASSA-PKCS1-v1_5 algorithms */
  rsa_pkcs1_sha256(0x0401),
  rsa_pkcs1_sha384(0x0501),
  rsa_pkcs1_sha512(0x0601),

  /* ECDSA algorithms */
  ecdsa_secp256r1_sha256(0x0403),
  ecdsa_secp384r1_sha384(0x0503),
  ecdsa_secp521r1_sha512(0x0603),

  /* RSASSA-PSS algorithms with public key OID rsaEncryption */
  rsa_pss_rsae_sha256(0x0804),
  rsa_pss_rsae_sha384(0x0805),
  rsa_pss_rsae_sha512(0x0806),

  /* EdDSA algorithms */
  ed25519(0x0807),
  ed448(0x0808),

  /* RSASSA-PSS algorithms with public key OID RSASSA-PSS */
  rsa_pss_pss_sha256(0x0809),
  rsa_pss_pss_sha384(0x080a),
  rsa_pss_pss_sha512(0x080b),
...
  } SignatureScheme;


6) Ditto. Please aware that "NamedCurve" is changed in TLS 1.3 - 
https://datatracker.ietf.org/doc/html/rfc8446#section-4.2.7
I am not sure if you need define that as well.


  enum {

  /* Elliptic Curve Groups (ECDHE) */
  secp256r1(0x0017), secp384r1(0x0018), secp521r1(0x0019),
  x25519(0x001D), x448(0x001E),
...
  } NamedGroup;


7) Last but not least, I hope to see how those new definition is used.
Without consumer, it is hard for me to understand why they are needed, or if we 
miss something else.


Thank you
Yao, Jiewen


> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of yi1 li
> Sent: Wednesday, May 4, 2022 5:31 PM
> To: devel@edk2.groups.io
> Cc: Li, Yi1 ; Kinney, Michael D 
> ;
> Gao, Liming 
> Subject: [edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS configure
> macro
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3892
> 
> Which are needed for SUITE-B and SUITE-B-192.
> 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Signed-off-by: yi1 li 
> ---
>  MdePkg/Include/IndustryStandard/Tls1.h | 133 ++---
>  1 file changed, 97 insertions(+), 36 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Tls1.h
> b/MdePkg/Include/IndustryStandard/Tls1.h
> index cf67428b1129..6519afe15e78 100644
> --- a/MdePkg/Include/IndustryStandard/Tls1.h
> +++ b/MdePkg/Include/IndustryStandard/Tls1.h
> @@ -15,42 +15,49 @@
>  ///
>  /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and rfc-5246.
>  ///
> -#define TLS_RSA_WITH_NULL_MD5{0x00, 0x01}
> -#define TLS_RSA_WITH_NULL_SHA{0x00, 0x02}
> -#define TLS_RSA_WITH_RC4_128_MD5 {0x00, 0x04}
> -#define TLS_RSA_WITH_RC4_128_SHA {0x00, 0x05}
> -#define TLS_RSA_WITH_IDEA_CBC_SHA{0x00, 0x07}
> -#define TLS_RSA_WITH_DES_CBC_SHA {0x00, 0x09}
> -#define TLS_RSA_WITH_3DES_EDE_CBC_SHA{0x00, 0x0A}
> -#define TLS_DH_DSS_WITH_DES_CBC_SHA  {0x00, 0x0C}
> -#define TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA {0x00, 0x0D}
> -#define 

[edk2-devel] [PATCH 1/1] MdePkg: Add WPA3 related TLS configure macro

[yi1 li]

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3892

Which are needed for SUITE-B and SUITE-B-192.

Cc: Michael D Kinney 
Cc: Liming Gao 
Signed-off-by: yi1 li 
---
 MdePkg/Include/IndustryStandard/Tls1.h | 133 ++---
 1 file changed, 97 insertions(+), 36 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Tls1.h 
b/MdePkg/Include/IndustryStandard/Tls1.h
index cf67428b1129..6519afe15e78 100644
--- a/MdePkg/Include/IndustryStandard/Tls1.h
+++ b/MdePkg/Include/IndustryStandard/Tls1.h
@@ -15,42 +15,49 @@
 ///
 /// TLS Cipher Suite, refers to A.5 of rfc-2246, rfc-4346 and rfc-5246.
 ///
-#define TLS_RSA_WITH_NULL_MD5{0x00, 0x01}
-#define TLS_RSA_WITH_NULL_SHA{0x00, 0x02}
-#define TLS_RSA_WITH_RC4_128_MD5 {0x00, 0x04}
-#define TLS_RSA_WITH_RC4_128_SHA {0x00, 0x05}
-#define TLS_RSA_WITH_IDEA_CBC_SHA{0x00, 0x07}
-#define TLS_RSA_WITH_DES_CBC_SHA {0x00, 0x09}
-#define TLS_RSA_WITH_3DES_EDE_CBC_SHA{0x00, 0x0A}
-#define TLS_DH_DSS_WITH_DES_CBC_SHA  {0x00, 0x0C}
-#define TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA {0x00, 0x0D}
-#define TLS_DH_RSA_WITH_DES_CBC_SHA  {0x00, 0x0F}
-#define TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA {0x00, 0x10}
-#define TLS_DHE_DSS_WITH_DES_CBC_SHA {0x00, 0x12}
-#define TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA{0x00, 0x13}
-#define TLS_DHE_RSA_WITH_DES_CBC_SHA {0x00, 0x15}
-#define TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA{0x00, 0x16}
-#define TLS_RSA_WITH_AES_128_CBC_SHA {0x00, 0x2F}
-#define TLS_DH_DSS_WITH_AES_128_CBC_SHA  {0x00, 0x30}
-#define TLS_DH_RSA_WITH_AES_128_CBC_SHA  {0x00, 0x31}
-#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA {0x00, 0x32}
-#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA {0x00, 0x33}
-#define TLS_RSA_WITH_AES_256_CBC_SHA {0x00, 0x35}
-#define TLS_DH_DSS_WITH_AES_256_CBC_SHA  {0x00, 0x36}
-#define TLS_DH_RSA_WITH_AES_256_CBC_SHA  {0x00, 0x37}
-#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA {0x00, 0x38}
-#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA {0x00, 0x39}
-#define TLS_RSA_WITH_NULL_SHA256 {0x00, 0x3B}
-#define TLS_RSA_WITH_AES_128_CBC_SHA256  {0x00, 0x3C}
-#define TLS_RSA_WITH_AES_256_CBC_SHA256  {0x00, 0x3D}
-#define TLS_DH_DSS_WITH_AES_128_CBC_SHA256   {0x00, 0x3E}
-#define TLS_DH_RSA_WITH_AES_128_CBC_SHA256   {0x00, 0x3F}
-#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA256  {0x00, 0x40}
-#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA256  {0x00, 0x67}
-#define TLS_DH_DSS_WITH_AES_256_CBC_SHA256   {0x00, 0x68}
-#define TLS_DH_RSA_WITH_AES_256_CBC_SHA256   {0x00, 0x69}
-#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA256  {0x00, 0x6A}
-#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA256  {0x00, 0x6B}
+#define TLS_RSA_WITH_NULL_MD5  {0x00, 0x01}
+#define TLS_RSA_WITH_NULL_SHA  {0x00, 0x02}
+#define TLS_RSA_WITH_RC4_128_MD5   {0x00, 0x04}
+#define TLS_RSA_WITH_RC4_128_SHA   {0x00, 0x05}
+#define TLS_RSA_WITH_IDEA_CBC_SHA  {0x00, 0x07}
+#define TLS_RSA_WITH_DES_CBC_SHA   {0x00, 0x09}
+#define TLS_RSA_WITH_3DES_EDE_CBC_SHA  {0x00, 0x0A}
+#define TLS_DH_DSS_WITH_DES_CBC_SHA{0x00, 0x0C}
+#define TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA   {0x00, 0x0D}
+#define TLS_DH_RSA_WITH_DES_CBC_SHA{0x00, 0x0F}
+#define TLS_DH_RSA_WITH_3DES_EDE_CBC_SHA   {0x00, 0x10}
+#define TLS_DHE_DSS_WITH_DES_CBC_SHA   {0x00, 0x12}
+#define TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA  {0x00, 0x13}
+#define TLS_DHE_RSA_WITH_DES_CBC_SHA   {0x00, 0x15}
+#define TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA  {0x00, 0x16}
+#define TLS_RSA_WITH_AES_128_CBC_SHA   {0x00, 0x2F}
+#define TLS_DH_DSS_WITH_AES_128_CBC_SHA{0x00, 0x30}
+#define TLS_DH_RSA_WITH_AES_128_CBC_SHA{0x00, 0x31}
+#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA   {0x00, 0x32}
+#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA   {0x00, 0x33}
+#define TLS_RSA_WITH_AES_256_CBC_SHA   {0x00, 0x35}
+#define TLS_DH_DSS_WITH_AES_256_CBC_SHA{0x00, 0x36}
+#define TLS_DH_RSA_WITH_AES_256_CBC_SHA{0x00, 0x37}
+#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA   {0x00, 0x38}
+#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA   {0x00, 0x39}
+#define TLS_RSA_WITH_NULL_SHA256   {0x00, 0x3B}
+#define TLS_RSA_WITH_AES_128_CBC_SHA256{0x00, 0x3C}
+#define TLS_RSA_WITH_AES_256_CBC_SHA256{0x00, 0x3D}
+#define TLS_DH_DSS_WITH_AES_128_CBC_SHA256 {0x00, 0x3E}
+#define TLS_DH_RSA_WITH_AES_128_CBC_SHA256 {0x00, 0x3F}
+#define TLS_DHE_DSS_WITH_AES_128_CBC_SHA256{0x00, 0x40}
+#define TLS_DHE_RSA_WITH_AES_128_CBC_SHA256{0x00, 0x67}
+#define TLS_DH_DSS_WITH_AES_256_CBC_SHA256 {0x00, 0x68}
+#define TLS_DH_RSA_WITH_AES_256_CBC_SHA256 {0x00, 0x69}
+#define TLS_DHE_DSS_WITH_AES_256_CBC_SHA256{0x00, 0x6A}
+#define TLS_DHE_RSA_WITH_AES_256_CBC_SHA256{0x00, 0x6B}
+#define TLS_DHE_RSA_WITH_AES_256_GCM_SHA384{0x00, 

Re: [edk2-devel] [PATCH 00/10] Multiple packages: Remove RVCT support

[Leif Lindholm]

On Tue, May 03, 2022 at 21:13:18 +0200, Ard Biesheuvel wrote:
> On Tue, 3 May 2022 at 20:48, Rebecca Cran  wrote:
> >
> > RVCT is obsolete, no longer supported and only supports 32-bit systems.
> > Since it's no longer used, remove it.
> >
> > Personal PR: https://github.com/tianocore/edk2/pull/2833
> >
> > Rebecca Cran (10):
> >   ArmPkg: Remove RVCT support
> >   ArmPlatformPkg: Remove RVCT support
> >   CryptoPkg: Remove RVCT support
> >   MdePkg: Remove RVCT support
> >   FatPkg: Remove RVCT support
> >   NetworkPkg: Remove RVCT support
> >   ArmVirtPkg: Remove RVCT support
> >   EmbeddedPkg: Remove RVCT support
> >   OvmfPkg: Remove RVCT support
> >   BaseTools: Remove RVCT support
> >
> 
> Thanks a lot for doing this.
> 
> Reviewed-by: Ard Biesheuvel 
> 
> Unless anyone objects, let's get this merged asap.

Acked-by: Leif Lindholm 
> 
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89503): https://edk2.groups.io/g/devel/message/89503
Mute This Topic: https://groups.io/mt/90866282/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-