[edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib
@ 2021-09-02  1:37 zhihao.li
  2021-09-02 15:50 ` Michael D Kinney
  0 siblings, 1 reply; 13+ messages in thread
From: zhihao.li @ 2021-09-02  1:37 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: Yao, Jiewen, Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]

Hi, everyone.
We want to add new hash algorithm-cSHAKE256/ParallelHash256 defined by NIST SP 800-185-into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 6955 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib
  2021-09-02  1:37 [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib zhihao.li
@ 2021-09-02 15:50 ` Michael D Kinney
  2021-09-02 23:16   ` Andrew Fish
  0 siblings, 1 reply; 13+ messages in thread
From: Michael D Kinney @ 2021-09-02 15:50 UTC (permalink / raw)
  To: devel@edk2.groups.io, Li, Zhihao, Kinney, Michael D
  Cc: Yao, Jiewen, Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 3936 bytes --]

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

How do you decide how many CPU threads to use?

Thanks,

Mike

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 47228 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-02 15:50 ` Michael D Kinney
@ 2021-09-02 23:16   ` Andrew Fish
  2021-09-03  1:01     ` Yao, Jiewen
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Fish @ 2021-09-02 23:16 UTC (permalink / raw)
  To: edk2-devel-groups-io, Mike Kinney
  Cc: Li, Zhihao, Yao, Jiewen, Wang, Jian J, Wu, Hao A, Lu, XiaoyuX,
	Jiang, Guomin, gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong,
	Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 6283 bytes --]



> On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com> wrote:
> 
> Hi Zhihao,
>  
> Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?
>  
> I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.
>  


Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?


> How do you decide how many CPU threads to use? 
>  

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that. 


[1] We don’t have a Base implementation of BaseCryptLib. 
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
 LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

> Thanks,
>  
> Mike
>  
> From: devel@edk2.groups.io <mailto:devel@edk2.groups.io> <devel@edk2.groups.io <mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
> Sent: Wednesday, September 1, 2021 6:38 PM
> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: Yao, Jiewen <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com <mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com <mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com <mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com <mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com <mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com <mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com <mailto:aaron.li@intel.com>>
> Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib
>  
> Hi, everyone.
> We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.
>  
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596 <https://bugzilla.tianocore.org/show_bug.cgi?id=3596>
>  
> [Background]
> The intention of this change is to improve the capsule authentication performance.
> Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.
>  
> [Proposal]
> Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
> Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.
>  
> [Required Changes]
> Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
> 1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
> 2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
> 3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
>  
> Please let me know if you have any comment or concern on this proposed change.
>  
> Thanks for your time and feedback!
> 
> Best regards,
> Zhihao
>  
> 


[-- Attachment #2: Type: text/html, Size: 21173 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-02 23:16   ` Andrew Fish
@ 2021-09-03  1:01     ` Yao, Jiewen
  2021-09-03  6:57       ` Li, Zhihao
  2021-09-03  7:47       ` Ethin Probst
  0 siblings, 2 replies; 13+ messages in thread
From: Yao, Jiewen @ 2021-09-03  1:01 UTC (permalink / raw)
  To: Andrew Fish, edk2-devel-groups-io, Kinney, Michael D
  Cc: Li, Zhihao, Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 7150 bytes --]

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Li, Zhihao <zhihao.li@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 19491 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-03  1:01     ` Yao, Jiewen
@ 2021-09-03  6:57       ` Li, Zhihao
  2021-09-03  7:06         ` Yao, Jiewen
  2021-09-03  7:47       ` Ethin Probst
  1 sibling, 1 reply; 13+ messages in thread
From: Li, Zhihao @ 2021-09-03  6:57 UTC (permalink / raw)
  To: Yao, Jiewen, Andrew Fish, edk2-devel-groups-io, Kinney, Michael D
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 11192 bytes --]

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Li, Zhihao <zhihao.li@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 36746 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-03  6:57       ` Li, Zhihao
@ 2021-09-03  7:06         ` Yao, Jiewen
  2021-09-03  8:43           ` Li, Zhihao
  0 siblings, 1 reply; 13+ messages in thread
From: Yao, Jiewen @ 2021-09-03  7:06 UTC (permalink / raw)
  To: Li, Zhihao, Andrew Fish, edk2-devel-groups-io, Kinney, Michael D
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 12267 bytes --]

Sorry, I hardly understand the explanation.
Do you have a URL for the POC code?

From: Li, Zhihao <zhihao.li@intel.com>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 38924 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-03  7:06         ` Yao, Jiewen
@ 2021-09-03  8:43           ` Li, Zhihao
  2021-09-09  9:48             ` Li, Zhihao
  0 siblings, 1 reply; 13+ messages in thread
From: Li, Zhihao @ 2021-09-03  8:43 UTC (permalink / raw)
  To: Yao, Jiewen, Andrew Fish, edk2-devel-groups-io, Kinney, Michael D
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 14572 bytes --]

Hi, Jiewen
I try to explant what means “more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist)”.
When we confirm the image is effective, we have to confirm not only that image certificate is on the whitelist, but also that it is not on the blacklist.
So it have two steps in verification process(only talk about FmpAuthentication)----- VerifyImageWithDenylist and VerifyImageWithAllowlist.
VerifyImageWithDenylist confirms it not in blacklist while VerifyImageWithAllowlist confirms it in whitelist.
==>VerifyImageWithDenylist should do FmpAuthentication and failed. VerifyImageWithAllowlist Should do FmpAuthentication and success.
In our design:
Result=parallelhash256(image);------①
Status1= VerifyImageWithDenylist(image,result);---------②
Status2= VerifyImageWithAllowlist(image,result);---------③
Status1 is failed, status2 is success==>image is effective.
If do it inside of AuthenticateFmpImage
In step ②，it need do parallelhash256(image) .
And in step ③，it also need do parallelhash256(image) .
Because AuthenticateFmpImage Function is inside of VerifyImageWithDenylist And VerifyImageWithAllowlist.
Poc code link of edk2:
https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c

From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, September 3, 2021 3:07 PM
To: Li, Zhihao <zhihao.li@intel.com>; Andrew Fish <afish@apple.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Sorry, I hardly understand the explanation.
Do you have a URL for the POC code?

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 42942 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-03  8:43           ` Li, Zhihao
@ 2021-09-09  9:48             ` Li, Zhihao
  2021-09-09 10:04               ` Yao, Jiewen
  0 siblings, 1 reply; 13+ messages in thread
From: Li, Zhihao @ 2021-09-09  9:48 UTC (permalink / raw)
  To: Yao, Jiewen, Andrew Fish, Ethin Probst, Kinney, Michael D,
	edk2-devel-groups-io
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 16070 bytes --]

I send this mail for asking that if there are any comments about parallel hash feature.

Mainly modification:
CryptoPkg: https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
MdeMoudulePkg: https://github.com/zhihaoli1064/edk2/blob/master/MdeModulePkg/Include/Library/FmpAuthenticationLib.h              line59-67
SecurityPkg: https://github.com/zhihaoli1064/edk2/blob/master/SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.c line119-188,287-350

From: Li, Zhihao
Sent: Friday, September 3, 2021 4:44 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <XiaoyuX.Lu@intel.com>; Jiang, Guomin <Guomin.Jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi, Jiewen
I try to explant what means “more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist)”.
When we confirm the image is effective, we have to confirm not only that image certificate is on the whitelist, but also that it is not on the blacklist.
So it have two steps in verification process(only talk about FmpAuthentication)----- VerifyImageWithDenylist and VerifyImageWithAllowlist.
VerifyImageWithDenylist confirms it not in blacklist while VerifyImageWithAllowlist confirms it in whitelist.
==>VerifyImageWithDenylist should do FmpAuthentication and failed. VerifyImageWithAllowlist Should do FmpAuthentication and success.
In our design:
Result=parallelhash256(image);------①
Status1= VerifyImageWithDenylist(image,result);---------②
Status2= VerifyImageWithAllowlist(image,result);---------③
Status1 is failed, status2 is success==>image is effective.
If do it inside of AuthenticateFmpImage
In step ②，it need do parallelhash256(image) .
And in step ③，it also need do parallelhash256(image) .
Because AuthenticateFmpImage Function is inside of VerifyImageWithDenylist And VerifyImageWithAllowlist.
Poc code link of edk2:
https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 3:07 PM
To: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Sorry, I hardly understand the explanation.
Do you have a URL for the POC code?

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 45488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-09  9:48             ` Li, Zhihao
@ 2021-09-09 10:04               ` Yao, Jiewen
  2021-09-14  4:02                 ` Li, Zhihao
  0 siblings, 1 reply; 13+ messages in thread
From: Yao, Jiewen @ 2021-09-09 10:04 UTC (permalink / raw)
  To: Li, Zhihao, Andrew Fish, Ethin Probst, Kinney, Michael D,
	edk2-devel-groups-io
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 17684 bytes --]

Hi
AllowList and DenyList are *secure boot* concept.
FMP auth lib is for *signed capsule* and it does not consider secure boot – allow list and deny list.

In my mind, neither secure boot and FSP auth shall know the existence of parallel hash. The verification logic shall be isolated.

Sorry, I don’t understand the design.

I am worried that you put too many concept together.

Adding parallel hash is one thing.
Adding allow list and deny list to FMP is another thing.
Please don’t mix them together.

I would like to request a design review for this feature.

Thank you
Yao Jiewen

From: Li, Zhihao <zhihao.li@intel.com>
Sent: Thursday, September 9, 2021 5:49 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Ethin Probst <harlydavidsen@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

I send this mail for asking that if there are any comments about parallel hash feature.

Mainly modification:
CryptoPkg: https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
MdeMoudulePkg: https://github.com/zhihaoli1064/edk2/blob/master/MdeModulePkg/Include/Library/FmpAuthenticationLib.h              line59-67
SecurityPkg: https://github.com/zhihaoli1064/edk2/blob/master/SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.c line119-188,287-350

From: Li, Zhihao
Sent: Friday, September 3, 2021 4:44 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <XiaoyuX.Lu@intel.com<mailto:XiaoyuX.Lu@intel.com>>; Jiang, Guomin <Guomin.Jiang@intel.com<mailto:Guomin.Jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi, Jiewen
I try to explant what means “more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist)”.
When we confirm the image is effective, we have to confirm not only that image certificate is on the whitelist, but also that it is not on the blacklist.
So it have two steps in verification process(only talk about FmpAuthentication)----- VerifyImageWithDenylist and VerifyImageWithAllowlist.
VerifyImageWithDenylist confirms it not in blacklist while VerifyImageWithAllowlist confirms it in whitelist.
==>VerifyImageWithDenylist should do FmpAuthentication and failed. VerifyImageWithAllowlist Should do FmpAuthentication and success.
In our design:
Result=parallelhash256(image);------①
Status1= VerifyImageWithDenylist(image,result);---------②
Status2= VerifyImageWithAllowlist(image,result);---------③
Status1 is failed, status2 is success==>image is effective.
If do it inside of AuthenticateFmpImage
In step ②，it need do parallelhash256(image) .
And in step ③，it also need do parallelhash256(image) .
Because AuthenticateFmpImage Function is inside of VerifyImageWithDenylist And VerifyImageWithAllowlist.
Poc code link of edk2:
https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 3:07 PM
To: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Sorry, I hardly understand the explanation.
Do you have a URL for the POC code?

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 48410 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-09 10:04               ` Yao, Jiewen
@ 2021-09-14  4:02                 ` Li, Zhihao
  2021-09-15  0:55                   ` Yao, Jiewen
  0 siblings, 1 reply; 13+ messages in thread
From: Li, Zhihao @ 2021-09-14  4:02 UTC (permalink / raw)
  To: Yao, Jiewen, Andrew Fish, Ethin Probst, Kinney, Michael D,
	edk2-devel-groups-io
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 19037 bytes --]

Hi, Jiewen
Thanks for your suggestions.
In view of your advice, this RFC only talk about the new feature----Parallel hash.
Mainly modification in https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
Others will be designed anew.

From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Thursday, September 9, 2021 6:04 PM
To: Li, Zhihao <zhihao.li@intel.com>; Andrew Fish <afish@apple.com>; Ethin Probst <harlydavidsen@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
AllowList and DenyList are *secure boot* concept.
FMP auth lib is for *signed capsule* and it does not consider secure boot – allow list and deny list.

In my mind, neither secure boot and FSP auth shall know the existence of parallel hash. The verification logic shall be isolated.

Sorry, I don’t understand the design.

I am worried that you put too many concept together.

Adding parallel hash is one thing.
Adding allow list and deny list to FMP is another thing.
Please don’t mix them together.

I would like to request a design review for this feature.

Thank you
Yao Jiewen

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Thursday, September 9, 2021 5:49 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Ethin Probst <harlydavidsen@gmail.com<mailto:harlydavidsen@gmail.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

I send this mail for asking that if there are any comments about parallel hash feature.

Mainly modification:
CryptoPkg: https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
MdeMoudulePkg: https://github.com/zhihaoli1064/edk2/blob/master/MdeModulePkg/Include/Library/FmpAuthenticationLib.h              line59-67
SecurityPkg: https://github.com/zhihaoli1064/edk2/blob/master/SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.c line119-188,287-350

From: Li, Zhihao
Sent: Friday, September 3, 2021 4:44 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <XiaoyuX.Lu@intel.com<mailto:XiaoyuX.Lu@intel.com>>; Jiang, Guomin <Guomin.Jiang@intel.com<mailto:Guomin.Jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi, Jiewen
I try to explant what means “more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist)”.
When we confirm the image is effective, we have to confirm not only that image certificate is on the whitelist, but also that it is not on the blacklist.
So it have two steps in verification process(only talk about FmpAuthentication)----- VerifyImageWithDenylist and VerifyImageWithAllowlist.
VerifyImageWithDenylist confirms it not in blacklist while VerifyImageWithAllowlist confirms it in whitelist.
==>VerifyImageWithDenylist should do FmpAuthentication and failed. VerifyImageWithAllowlist Should do FmpAuthentication and success.
In our design:
Result=parallelhash256(image);------①
Status1= VerifyImageWithDenylist(image,result);---------②
Status2= VerifyImageWithAllowlist(image,result);---------③
Status1 is failed, status2 is success==>image is effective.
If do it inside of AuthenticateFmpImage
In step ②，it need do parallelhash256(image) .
And in step ③，it also need do parallelhash256(image) .
Because AuthenticateFmpImage Function is inside of VerifyImageWithDenylist And VerifyImageWithAllowlist.
Poc code link of edk2:
https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 3:07 PM
To: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Sorry, I hardly understand the explanation.
Do you have a URL for the POC code?

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 50747 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-14  4:02                 ` Li, Zhihao
@ 2021-09-15  0:55                   ` Yao, Jiewen
  0 siblings, 0 replies; 13+ messages in thread
From: Yao, Jiewen @ 2021-09-15  0:55 UTC (permalink / raw)
  To: Li, Zhihao, Andrew Fish, Ethin Probst, Kinney, Michael D,
	edk2-devel-groups-io
  Cc: Wang, Jian J, Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	gaoliming@byosoft.com.cn, Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 20858 bytes --]

OK. The interface ParallelHash256HashAll() seems OK for me.

Some comment:
1) Please add EFIAPI for ParallelHash256HashAll().
2) Please use BOOLEAN as return value.
3) Please remove LeftEncode() and RightEncode() from https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Include/Library/BaseCryptLib.h. They shall be internal functions.
4) Do we really need expose cSHAKE* API? If the request is just to support ParallelHash256, then cSHAKE API could be internal one.
5) I suggest to make this API work in non-MP case as well. In current version, it will fail for non-MP system, right? I think we can let BSP do all the work in that case.

  if (StartedApNum == 0) {
    ReturnValue = 1;
    goto Exit;
  }

Thank you
Yao Jiewen

From: Li, Zhihao <zhihao.li@intel.com>
Sent: Tuesday, September 14, 2021 12:03 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Ethin Probst <harlydavidsen@gmail.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel-groups-io <devel@edk2.groups.io>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi, Jiewen
Thanks for your suggestions.
In view of your advice, this RFC only talk about the new feature----Parallel hash.
Mainly modification in https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
Others will be designed anew.

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Thursday, September 9, 2021 6:04 PM
To: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Ethin Probst <harlydavidsen@gmail.com<mailto:harlydavidsen@gmail.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
AllowList and DenyList are *secure boot* concept.
FMP auth lib is for *signed capsule* and it does not consider secure boot – allow list and deny list.

In my mind, neither secure boot and FSP auth shall know the existence of parallel hash. The verification logic shall be isolated.

Sorry, I don’t understand the design.

I am worried that you put too many concept together.

Adding parallel hash is one thing.
Adding allow list and deny list to FMP is another thing.
Please don’t mix them together.

I would like to request a design review for this feature.

Thank you
Yao Jiewen

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Thursday, September 9, 2021 5:49 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Ethin Probst <harlydavidsen@gmail.com<mailto:harlydavidsen@gmail.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

I send this mail for asking that if there are any comments about parallel hash feature.

Mainly modification:
CryptoPkg: https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
MdeMoudulePkg: https://github.com/zhihaoli1064/edk2/blob/master/MdeModulePkg/Include/Library/FmpAuthenticationLib.h              line59-67
SecurityPkg: https://github.com/zhihaoli1064/edk2/blob/master/SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.c line119-188,287-350

From: Li, Zhihao
Sent: Friday, September 3, 2021 4:44 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <XiaoyuX.Lu@intel.com<mailto:XiaoyuX.Lu@intel.com>>; Jiang, Guomin <Guomin.Jiang@intel.com<mailto:Guomin.Jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi, Jiewen
I try to explant what means “more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist)”.
When we confirm the image is effective, we have to confirm not only that image certificate is on the whitelist, but also that it is not on the blacklist.
So it have two steps in verification process(only talk about FmpAuthentication)----- VerifyImageWithDenylist and VerifyImageWithAllowlist.
VerifyImageWithDenylist confirms it not in blacklist while VerifyImageWithAllowlist confirms it in whitelist.
==>VerifyImageWithDenylist should do FmpAuthentication and failed. VerifyImageWithAllowlist Should do FmpAuthentication and success.
In our design:
Result=parallelhash256(image);------①
Status1= VerifyImageWithDenylist(image,result);---------②
Status2= VerifyImageWithAllowlist(image,result);---------③
Status1 is failed, status2 is success==>image is effective.
If do it inside of AuthenticateFmpImage
In step ②，it need do parallelhash256(image) .
And in step ③，it also need do parallelhash256(image) .
Because AuthenticateFmpImage Function is inside of VerifyImageWithDenylist And VerifyImageWithAllowlist.
Poc code link of edk2:
https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 3:07 PM
To: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Sorry, I hardly understand the explanation.
Do you have a URL for the POC code?

From: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Some explanation for confusion.

  1.  Is the result of the parallel hash identical to the current hash?

The result of parallelhash256 do not identical to the current hash. And we are not intention to let parallelhash256 replace the current hash(SHA-256). But doing the parallel hash before the current hash to reduce the size of current hash input. Otherwise, the parallel hash effect is compressing the size of FmpAuthentication input and the use of MP Services is the inseparable part of this algorithm. It’s a new hash algorithm. So it should not move to FmpAuthenticationLib.

  1.  Why we cannot do it inside of AuthenticateFmpImage?

Because of more than once authentication(e.g. VerifyImageWithDenylist and VerifyImageWithAllowlist), if we do the parallel hash inside of AuthenticateFmpImage(Denylist auth), we have to do another parallel hash for Allowlist’s AuthenticateFmpImage. It’s repeat operation.

Poc code in branch named dev/sfu5/parallel_hash_ossl
The verify flow is:
  ImageParaHash = ParallelHash-256 (Image)
PKCS7_Verify (PublicKey, ImageParaHash)

In FmpAuthenticationLibPkcs7 ,the parameter Output of FmpAuthenticatedHandlerPkcs7WithParallelhash is image digest. It replace the original image.

FmpAuthenticatedHandlerPkcs7WithParallelhash (
  IN EFI_FIRMWARE_IMAGE_AUTHENTICATION  *Image,
  IN UINTN                              ImageSize,
  IN CONST UINT8                        *PublicKeyData,
  IN UINTN                              PublicKeyDataLength,
  IN UINT8                              *Output
  )
{
  RETURN_STATUS                             Status;
  BOOLEAN                                   CryptoStatus;
  VOID                                      *P7Data;
  UINTN                                     P7Length;
  VOID                                      *TempBuffer;
  UINTN                                     PayloadHeaderSize = 69;
  UINTN                                     ParallelhashSize = 64;

  P7Length = Image->AuthInfo.Hdr.dwLength - (OFFSET_OF(WIN_CERTIFICATE_UEFI_GUID, CertData));
  P7Data = Image->AuthInfo.CertData;

  // It is a signature across the variable data and the Monotonic Count value.
  TempBuffer = AllocatePool(sizeof(Image->MonotonicCount) + ParallelhashSize + PayloadHeaderSize);
  CopyMem(
    (UINT8 *)TempBuffer,
    (UINT8 *)Image + sizeof(Image->MonotonicCount) + Image->AuthInfo.Hdr.dwLength,
    PayloadHeaderSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize,
    Output,
    ParallelhashSize
    );
  CopyMem(
    (UINT8 *)TempBuffer + PayloadHeaderSize + ParallelhashSize,
    &Image->MonotonicCount,
    sizeof(Image->MonotonicCount)
    );
  CryptoStatus = Pkcs7Verify(
                   P7Data,
                   P7Length,
                   PublicKeyData,
                   PublicKeyDataLength,
                   (UINT8 *)TempBuffer,
                   PayloadHeaderSize + ParallelhashSize + sizeof(Image->MonotonicCount)
                   );
  FreePool(TempBuffer);

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: RE: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

Hi
Comment on 2/3.

I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
Why we cannot do it inside of AuthenticateFmpImage?

Ideally, we hope to hide *algorithm* from *business logic*.
Do you have any POC link?

Thank you
Yao Jiewen

From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Sent: Friday, September 3, 2021 7:16 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.

On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:

Hi Zhihao,

Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?

I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.

Mike,

Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?

How do you decide how many CPU threads to use?

If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.

[1] We don’t have a Base implementation of BaseCryptLib.
CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER

CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE

 CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER

CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
  LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE

[2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77

Thanks,

Andrew Fish

Thanks,

Mike

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
Sent: Wednesday, September 1, 2021 6:38 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib

Hi, everyone.
We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596

[Background]
The intention of this change is to improve the capsule authentication performance.
Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.

[Proposal]
Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.

[Required Changes]
Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.

Please let me know if you have any comment or concern on this proposed change.

Thanks for your time and feedback!

Best regards,
Zhihao

[-- Attachment #2: Type: text/html, Size: 58948 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-03  1:01     ` Yao, Jiewen
  2021-09-03  6:57       ` Li, Zhihao
@ 2021-09-03  7:47       ` Ethin Probst
  2021-09-03 11:29         ` Li, Zhihao
  1 sibling, 1 reply; 13+ messages in thread
From: Ethin Probst @ 2021-09-03  7:47 UTC (permalink / raw)
  To: edk2-devel-groups-io, Yao, Jiewen
  Cc: Andrew Fish, Kinney, Michael D, Li, Zhihao, Wang, Jian J,
	Wu, Hao A, Lu, XiaoyuX, Jiang, Guomin,
	Liming Gao (Byosoft address), Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 8267 bytes --]

I think another problem that we need to consider is that to my knowledge,
the MP services do not allow for thread scheduling at all. You can run a
call back on multiple processors, but that won't increase the performance
of the function you're calling because the function will be executed
independently of all other processors doing the work, so you would need to
intelligently write the function to determine what processor it's on and
that in turn would determine what work the function does. This would also
bring in the requirement for synchronization primitives like mutexes and
locks. I'm not sure how exactly that could be accomplished without changing
the API, or at least adding new functionality to it. But I may be missing
something and this may be possible. But last time I checked, UEFI did not
contain a thread-based scheduler.

On Thu, Sep 2, 2021, 20:02 Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Hi
>
> Comment on 2/3.
>
>
>
> I am not sure if the a new function
AuthenticateFmpImageWithParallelhash() is absolutely necessary.
>
> Why you do the parallel hash before authentication and transfer the
result to AuthenticateFmpImage?
>
> Why we cannot do it inside of AuthenticateFmpImage?
>
>
>
> Ideally, we hope to hide *algorithm* from *business logic*.
>
> Do you have any POC link?
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
> From: Andrew Fish <afish@apple.com>
> Sent: Friday, September 3, 2021 7:16 AM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D <
michael.d.kinney@intel.com>
> Cc: Li, Zhihao <zhihao.li@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <
yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
> Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into
CryptoPkg.BaseCryptLib.
>
>
>
>
>
>
>
>> On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com>
wrote:
>>
>>
>>
>> Hi Zhihao,
>>
>>
>>
>> Is the result of the parallel hash identical to the current hash?  If
so, then can we simply have a new instance of the FmpAuthenticationLib and
hide the ParallelHash256 digest inside this implementation of this new
instance?
>>
>>
>>
>> I do not think BaseCryptLib should depend on CPU MP Services
Protocol.  Can the use of MP Services be moved up into the implementation
of the new FmpAuthenticationLib?  If new BASE compatible primitives need to
be added to BaseCryptLib to support parallel hash, then those likely make
sense.
>>
>>
>
>
>
>
>
> Mike,
>
>
>
> Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]?
So are you worried about adding the dependency to this DXE Lib? It depends
on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses
gRT->GetTime(). It looks like if the time services are not available it
returns 0 from time(), so there is only a quality of service implication to
when it it is used but no Depex?
>
>
>
>
>>
>> How do you decide how many CPU threads to use?
>>
>>
>
>
>
> If we end up splitting this up for “others” to handle the MP in DXE, PEI,
or MM then I think we probably need a more robust API set that abstracts
breaking up the work, and combining it back tougher. Well you would need
the worker functions to processes the broken up data on the APs. So I would
imagine and API that splits the work and you pass in the number of APs (or
APs + BSP) and you get N buffers out to process? Those buffers should
describe the chunk to the worker function, and when the worker function is
done the get the answer function can calculate the results from that.
>
>
>
>
>
> [1] We don’t have a Base implementation of BaseCryptLib.
>
> CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>
> LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE
UEFI_APPLICATION UEFI_DRIVER
>
>
>
> CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
>
> LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE
>
>
>
>  CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>
> LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER
>
>
>
> CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
>
>   LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE
MM_STANDALONE
>
>
>
> [2]
https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77
>
>
>
> Thanks,
>
>
>
> Andrew Fish
>
>
>
>> Thanks,
>>
>>
>>
>> Mike
>>
>>
>>
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Li, Zhihao
>> Sent: Wednesday, September 1, 2021 6:38 PM
>> To: devel@edk2.groups.io
>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <
jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <
xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
gaoliming@byosoft.com.cn; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <
yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
>> Subject: [edk2-devel] [RFC] Add parallel hash feature into
CryptoPkg.BaseCryptLib
>>
>>
>>
>> Hi, everyone.
>>
>> We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by
NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied
for digital authentication functions like Capsule Update. It utilizes
multi-processor to calculate the image digest in parallel for update
capsule authentication so that lessen the time of capsule authentication.
>>
>>
>>
>> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596
>>
>>
>>
>> [Background]
>>
>> The intention of this change is to improve the capsule authentication
performance.
>>
>> Currently, the image is calculated to a hash value (usually by SHA-256),
then the hash value be signed by a certificate. The header, certificate,
and image binary be sealed to the capsule. In authentication phase, the
program should calculate the hash using image binary in capsule and then
perform authentication procedures.
>>
>>
>>
>> [Proposal]
>>
>> Now, we propose a new authentication flow, which firstly pre-calculates
the ParallelHash256 digest of the image binary in parallel with
multi-processors, then use the ParallelHash256 digest (instead of original
image binary) in subsequent SHA-256 hash for sign/authentication.
>>
>> Since the big size image be compressed to the ParallelHash256 digest
that only have 256 bytes, the time of SHA-256 running would be less.
>>
>>
>>
>> [Required Changes]
>>
>> Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
>>
>> 1. CryptoPkg: need to add the new hash algorithm named
cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will
consume CPU MP Service Protocol, not sure if this is allowed in
BaseCryptLib?
>>
>> 2. MdeMoudulePkg: Add new authenticate function
AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is
because original AuthenticateFmpImage() interface only have 4 parameters
while the new have 5 parameters. The 5th parameter is ParallelHash256
digest raised above. We try to do the parallel hash before authentication
and transfer the result to AuthenticateFmpImage function as parameter. So
that we can do only once parallel hash externally in the case of multiple
authentication which saves more time.
>>
>> 3. SecurityPkg: Add new function named
FmpAuthenticatedHandlerPkcs7WithParallelhash() and
AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This
is because original interfaces not have the formal parameter
(ParallelHash256 digest) we need. We try to do the parallel hash before
authentication and transfer the result to AuthenticateFmpImage and
FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only
once parallel hash externally in the case of multiple authentication which
saves more time.
>>
>>
>>
>> Please let me know if you have any comment or concern on this proposed
change.
>>
>>
>>
>> Thanks for your time and feedback!
>>
>> Best regards,
>> Zhihao
>>
>>
>
>
>
> 

[-- Attachment #2: Type: text/html, Size: 11460 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
  2021-09-03  7:47       ` Ethin Probst
@ 2021-09-03 11:29         ` Li, Zhihao
  0 siblings, 0 replies; 13+ messages in thread
From: Li, Zhihao @ 2021-09-03 11:29 UTC (permalink / raw)
  To: Ethin Probst, edk2-devel-groups-io, Yao, Jiewen
  Cc: Andrew Fish, Kinney, Michael D, Wang, Jian J, Wu, Hao A,
	Lu, XiaoyuX, Jiang, Guomin, Liming Gao (Byosoft address),
	Fu, Siyuan, Wu, Yidong, Li, Aaron

[-- Attachment #1: Type: text/plain, Size: 11078 bytes --]

Hi, Ethin.

Thanks for your reminder. We have considered what you said.
In poc code https://github.com/zhihaoli1064/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/Hash/Smm/ParallelHashSmm.c
for (Index = 0; Index < gSmst->NumberOfCpus; Index++) {
    if (Index != gSmst->CurrentlyExecutingCpu) {
      Status = gSmst->SmmStartupThisAp (ParallelHashApExecute, Index, NULL);
      if (!EFI_ERROR(Status)) {
        StartedApNum++;
      }
    }
  }
VOID
EFIAPI
ParallelHashApExecute (
  IN VOID          *ProcedureArgument
  )
{
  UINTN         Index;

  for (Index = 0; Index < mBlockNum; Index++) {
    //
    // Acquire lock for a target block.
    //
    if (!AcquireSpinLockOrFail (&mSpinLockList[Index])) {
      //
      // Failed, try next one.
      //
      continue;
    }

    //
    // Calculate cSHAKE256 for this block
    //
    CShake256HashAll (
      mInput + Index * mBlockSize,
      (Index == (mBlockNum - 1)) ? mLastBlockSize : mBlockSize,
      mBlockResultSize,
      NULL,
      0,
      NULL,
      0,
      mBlockHashResult + Index * mBlockResultSize
      );

    mBlockIsCompleted[Index] = TRUE;
  }
}
The code start ap to run the function ParallelHashApExecute, the parallelHashApExecute running result will fill in the corresponding block.
And each ap running ParallelHashApExecute will not block.
The performance test result is ideal.

From: Ethin Probst <harlydavidsen@gmail.com>
Sent: Friday, September 3, 2021 3:48 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Yao, Jiewen <jiewen.yao@intel.com>
Cc: Andrew Fish <afish@apple.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Li, Zhihao <zhihao.li@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Liming Gao (Byosoft address) <gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com>; Wu, Yidong <yidong.wu@intel.com>; Li, Aaron <aaron.li@intel.com>
Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.


I think another problem that we need to consider is that to my knowledge, the MP services do not allow for thread scheduling at all. You can run a call back on multiple processors, but that won't increase the performance of the function you're calling because the function will be executed independently of all other processors doing the work, so you would need to intelligently write the function to determine what processor it's on and that in turn would determine what work the function does. This would also bring in the requirement for synchronization primitives like mutexes and locks. I'm not sure how exactly that could be accomplished without changing the API, or at least adding new functionality to it. But I may be missing something and this may be possible. But last time I checked, UEFI did not contain a thread-based scheduler.

On Thu, Sep 2, 2021, 20:02 Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
>
> Hi
>
> Comment on 2/3.
>
>
>
> I am not sure if the a new function AuthenticateFmpImageWithParallelhash() is absolutely necessary.
>
> Why you do the parallel hash before authentication and transfer the result to AuthenticateFmpImage?
>
> Why we cannot do it inside of AuthenticateFmpImage?
>
>
>
> Ideally, we hope to hide *algorithm* from *business logic*.
>
> Do you have any POC link?
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
> From: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
> Sent: Friday, September 3, 2021 7:16 AM
> To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Cc: Li, Zhihao <zhihao.li@intel.com<mailto:zhihao.li@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
> Subject: Re: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib.
>
>
>
>
>
>
>
>> On Sep 2, 2021, at 8:50 AM, Michael D Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> wrote:
>>
>>
>>
>> Hi Zhihao,
>>
>>
>>
>> Is the result of the parallel hash identical to the current hash?  If so, then can we simply have a new instance of the FmpAuthenticationLib and hide the ParallelHash256 digest inside this implementation of this new instance?
>>
>>
>>
>> I do not think BaseCryptLib should depend on CPU MP Services Protocol.  Can the use of MP Services be moved up into the implementation of the new FmpAuthenticationLib?  If new BASE compatible primitives need to be added to BaseCryptLib to support parallel hash, then those likely make sense.
>>
>>
>
>
>
>
>
> Mike,
>
>
>
> Stupid question but the BaseCryptLib seems to really be DxeCryptLib[1]? So are you worried about adding the dependency to this DXE Lib? It depends on UefiRuntimeServicesTableLib. Looks like SysCall/TimerWrapper.c[2] uses gRT->GetTime(). It looks like if the time services are not available it returns 0 from time(), so there is only a quality of service implication to when it it is used but no Depex?
>
>
>
>
>>
>> How do you decide how many CPU threads to use?
>>
>>
>
>
>
> If we end up splitting this up for “others” to handle the MP in DXE, PEI, or MM then I think we probably need a more robust API set that abstracts breaking up the work, and combining it back tougher. Well you would need the worker functions to processes the broken up data on the APs. So I would imagine and API that splits the work and you pass in the number of APs (or APs + BSP) and you get N buffers out to process? Those buffers should describe the chunk to the worker function, and when the worker function is done the get the answer function can calculate the results from that.
>
>
>
>
>
> [1] We don’t have a Base implementation of BaseCryptLib.
>
> CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>
> LIBRARY_CLASS                  = BaseCryptLib|DXE_DRIVER DXE_CORE UEFI_APPLICATION UEFI_DRIVER
>
>
>
> CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
>
> LIBRARY_CLASS                  = BaseCryptLib|PEIM PEI_CORE
>
>
>
>  CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>
> LIBRARY_CLASS                  = BaseCryptLib|DXE_RUNTIME_DRIVER
>
>
>
> CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
>
>   LIBRARY_CLASS                  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE MM_STANDALONE
>
>
>
> [2] https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/BaseCryptLib/SysCall/TimerWrapper.c#L77
>
>
>
> Thanks,
>
>
>
> Andrew Fish
>
>
>
>> Thanks,
>>
>>
>>
>> Mike
>>
>>
>>
>> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Li, Zhihao
>> Sent: Wednesday, September 1, 2021 6:38 PM
>> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
>> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>; Lu, XiaoyuX <xiaoyux.lu@intel.com<mailto:xiaoyux.lu@intel.com>>; Jiang, Guomin <guomin.jiang@intel.com<mailto:guomin.jiang@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Fu, Siyuan <siyuan.fu@intel.com<mailto:siyuan.fu@intel.com>>; Wu, Yidong <yidong.wu@intel.com<mailto:yidong.wu@intel.com>>; Li, Aaron <aaron.li@intel.com<mailto:aaron.li@intel.com>>
>> Subject: [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib
>>
>>
>>
>> Hi, everyone.
>>
>> We want to add new hash algorithm—cSHAKE256/ParallelHash256 defined by NIST SP 800-185—into BaseCryptLib of CryptoPkg. This feature can be applied for digital authentication functions like Capsule Update. It utilizes multi-processor to calculate the image digest in parallel for update capsule authentication so that lessen the time of capsule authentication.
>>
>>
>>
>> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3596
>>
>>
>>
>> [Background]
>>
>> The intention of this change is to improve the capsule authentication performance.
>>
>> Currently, the image is calculated to a hash value (usually by SHA-256), then the hash value be signed by a certificate. The header, certificate, and image binary be sealed to the capsule. In authentication phase, the program should calculate the hash using image binary in capsule and then perform authentication procedures.
>>
>>
>>
>> [Proposal]
>>
>> Now, we propose a new authentication flow, which firstly pre-calculates the ParallelHash256 digest of the image binary in parallel with multi-processors, then use the ParallelHash256 digest (instead of original image binary) in subsequent SHA-256 hash for sign/authentication.
>>
>> Since the big size image be compressed to the ParallelHash256 digest that only have 256 bytes, the time of SHA-256 running would be less.
>>
>>
>>
>> [Required Changes]
>>
>> Mainly in CryptoPkg, MdeModulePkg, SecurityPkg:
>>
>> 1. CryptoPkg: need to add the new hash algorithm named cSHAKE256/ParallelHash256 in BaseCrypLib. The ParallelHash function will consume CPU MP Service Protocol, not sure if this is allowed in BaseCryptLib?
>>
>> 2. MdeMoudulePkg: Add new authenticate function AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLib. This is because original AuthenticateFmpImage() interface only have 4 parameters  while the new have 5 parameters. The 5th parameter is ParallelHash256 digest raised above. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
>>
>> 3. SecurityPkg: Add new function named FmpAuthenticatedHandlerPkcs7WithParallelhash() and AuthenticateFmpImageWithParallelhash() to FmpAuthenticationLibPkcs7. This is because original interfaces not have the formal parameter (ParallelHash256 digest) we need. We try to do the parallel hash before authentication and transfer the result to AuthenticateFmpImage and FmpAuthenticatedHandlerPkcs7 function as parameter. So that we can do only once parallel hash externally in the case of multiple authentication which saves more time.
>>
>>
>>
>> Please let me know if you have any comment or concern on this proposed change.
>>
>>
>>
>> Thanks for your time and feedback!
>>
>> Best regards,
>> Zhihao
>>
>>
>
>
>
> 

[-- Attachment #2: Type: text/html, Size: 21115 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-09-15  0:55 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-02  1:37 [edk2-devel] [RFC] Add parallel hash feature into CryptoPkg.BaseCryptLib zhihao.li
2021-09-02 15:50 ` Michael D Kinney
2021-09-02 23:16   ` Andrew Fish
2021-09-03  1:01     ` Yao, Jiewen
2021-09-03  6:57       ` Li, Zhihao
2021-09-03  7:06         ` Yao, Jiewen
2021-09-03  8:43           ` Li, Zhihao
2021-09-09  9:48             ` Li, Zhihao
2021-09-09 10:04               ` Yao, Jiewen
2021-09-14  4:02                 ` Li, Zhihao
2021-09-15  0:55                   ` Yao, Jiewen
2021-09-03  7:47       ` Ethin Probst
2021-09-03 11:29         ` Li, Zhihao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox