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


From: Li, Zhihao <zhihao...@intel.com>
Sent: Friday, September 3, 2021 2:58 PM
To: Yao, Jiewen <jiewen....@intel.com>; Andrew Fish <af...@apple.com>; 
edk2-devel-groups-io <devel@edk2.groups.io>; Kinney, Michael D 
<michael.d.kin...@intel.com>
Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A <hao.a...@intel.com>; Lu, 
XiaoyuX <xiaoyux...@intel.com>; Jiang, Guomin <guomin.ji...@intel.com>; 
gaolim...@byosoft.com.cn; Fu, Siyuan <siyuan...@intel.com>; Wu, Yidong 
<yidong...@intel.com>; Li, Aaron <aaron...@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....@intel.com<mailto:jiewen....@intel.com>>
Sent: Friday, September 3, 2021 9:02 AM
To: Andrew Fish <af...@apple.com<mailto:af...@apple.com>>; edk2-devel-groups-io 
<devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Kinney, Michael D 
<michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Cc: Li, Zhihao <zhihao...@intel.com<mailto:zhihao...@intel.com>>; Wang, Jian J 
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Wu, Hao A 
<hao.a...@intel.com<mailto:hao.a...@intel.com>>; Lu, XiaoyuX 
<xiaoyux...@intel.com<mailto:xiaoyux...@intel.com>>; Jiang, Guomin 
<guomin.ji...@intel.com<mailto:guomin.ji...@intel.com>>; 
gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>; Fu, Siyuan 
<siyuan...@intel.com<mailto:siyuan...@intel.com>>; Wu, Yidong 
<yidong...@intel.com<mailto:yidong...@intel.com>>; Li, Aaron 
<aaron...@intel.com<mailto:aaron...@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 <af...@apple.com<mailto:af...@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.kin...@intel.com<mailto:michael.d.kin...@intel.com>>
Cc: Li, Zhihao <zhihao...@intel.com<mailto:zhihao...@intel.com>>; Yao, Jiewen 
<jiewen....@intel.com<mailto:jiewen....@intel.com>>; Wang, Jian J 
<jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Wu, Hao A 
<hao.a...@intel.com<mailto:hao.a...@intel.com>>; Lu, XiaoyuX 
<xiaoyux...@intel.com<mailto:xiaoyux...@intel.com>>; Jiang, Guomin 
<guomin.ji...@intel.com<mailto:guomin.ji...@intel.com>>; 
gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>; Fu, Siyuan 
<siyuan...@intel.com<mailto:siyuan...@intel.com>>; Wu, Yidong 
<yidong...@intel.com<mailto:yidong...@intel.com>>; Li, Aaron 
<aaron...@intel.com<mailto:aaron...@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.kin...@intel.com<mailto:michael.d.kin...@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....@intel.com<mailto:jiewen....@intel.com>>; Wang, Jian 
J <jian.j.w...@intel.com<mailto:jian.j.w...@intel.com>>; Wu, Hao A 
<hao.a...@intel.com<mailto:hao.a...@intel.com>>; Lu, XiaoyuX 
<xiaoyux...@intel.com<mailto:xiaoyux...@intel.com>>; Jiang, Guomin 
<guomin.ji...@intel.com<mailto:guomin.ji...@intel.com>>; 
gaolim...@byosoft.com.cn<mailto:gaolim...@byosoft.com.cn>; Fu, Siyuan 
<siyuan...@intel.com<mailto:siyuan...@intel.com>>; Wu, Yidong 
<yidong...@intel.com<mailto:yidong...@intel.com>>; Li, Aaron 
<aaron...@intel.com<mailto:aaron...@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





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


Reply via email to