[PATCH V1 0/2] CryptoPkg bug fixes

[PATCH V1 0/2] CryptoPkg bug fixes

[Judah Vang]

https://bugzilla.tianocore.org/show_bug.cgi?id=3991
https://bugzilla.tianocore.org/show_bug.cgi?id=3992

There is a #define to deprecate Sha1 functions but not
all the Sha1 function are wrapped around this #define causing
a build error. The fix is to wrap all Sha1 functions with
the #define.

Need crypto AES to be supported for PEI phase and need
crypto KDF to be supported for SMM phase.

Judah Vang (2):
  CryptoPkg: Sha1 functions causing build errors
  CryptoPkg: Need to enable crypto functions

 CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
 3 files changed, 15 insertions(+), 3 deletions(-)

-- 
2.35.1.windows.2

[PATCH V1 1/2] CryptoPkg: Sha1 functions causing build errors

[Judah Vang]

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

Fix build issue when DiSABLE_SHA1_DEPRECATED_INTERFACES
is defined. Percolate the #ifndef DiSABLE_SHA1_DEPRECATED_INTERFACES
to all the Sha1 functions.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Judah Vang <judah.vang@intel.com>
---
 CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
index f9796b215865..ede9fa8c09ec 100644
--- a/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
+++ b/CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c
@@ -6,7 +6,7 @@
   This API, when called, will calculate the Hash using the
   hashing algorithm specified by PcdHashApiLibPolicy.
 
-  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2020-2022, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -33,9 +33,11 @@ HashApiGetContextSize (
   )
 {
   switch (PcdGet32 (PcdHashApiLibPolicy)) {
+ #ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
     case HASH_ALG_SHA1:
       return Sha1GetContextSize ();
       break;
+ #endif
 
     case HASH_ALG_SHA256:
       return Sha256GetContextSize ();
@@ -75,9 +77,11 @@ HashApiInit (
   )
 {
   switch (PcdGet32 (PcdHashApiLibPolicy)) {
+ #ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
     case HASH_ALG_SHA1:
       return Sha1Init (HashContext);
       break;
+ #endif
 
     case HASH_ALG_SHA256:
       return Sha256Init (HashContext);
@@ -119,9 +123,11 @@ HashApiDuplicate (
   )
 {
   switch (PcdGet32 (PcdHashApiLibPolicy)) {
+ #ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
     case HASH_ALG_SHA1:
       return Sha1Duplicate (HashContext, NewHashContext);
       break;
+ #endif
 
     case HASH_ALG_SHA256:
       return Sha256Duplicate (HashContext, NewHashContext);
@@ -165,9 +171,11 @@ HashApiUpdate (
   )
 {
   switch (PcdGet32 (PcdHashApiLibPolicy)) {
+ #ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
     case HASH_ALG_SHA1:
       return Sha1Update (HashContext, DataToHash, DataToHashLen);
       break;
+ #endif
 
     case HASH_ALG_SHA256:
       return Sha256Update (HashContext, DataToHash, DataToHashLen);
@@ -209,9 +217,11 @@ HashApiFinal (
   )
 {
   switch (PcdGet32 (PcdHashApiLibPolicy)) {
+ #ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
     case HASH_ALG_SHA1:
       return Sha1Final (HashContext, Digest);
       break;
+ #endif
 
     case HASH_ALG_SHA256:
       return Sha256Final (HashContext, Digest);
@@ -255,9 +265,11 @@ HashApiHashAll (
   )
 {
   switch (PcdGet32 (PcdHashApiLibPolicy)) {
+ #ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
     case HASH_ALG_SHA1:
       return Sha1HashAll (DataToHash, DataToHashLen, Digest);
       break;
+ #endif
 
     case HASH_ALG_SHA256:
       return Sha256HashAll (DataToHash, DataToHashLen, Digest);
-- 
2.35.1.windows.2

[PATCH V1 2/2] CryptoPkg: Need to enable crypto functions

[Judah Vang]

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

Enable CryptAes for PEI phase.
Enable CryptHkdf for SMM phase.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Cc: Guomin Jiang <guomin.jiang@intel.com>
Cc: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Nishant C Mistry <nishant.c.mistry@intel.com>
Signed-off-by: Judah Vang <judah.vang@intel.com>
---
 CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf | 2 +-
 CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
index b1629647f9c6..ee5f3cd5d4b6 100644
--- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
@@ -43,7 +43,7 @@ [Sources]
   Hash/CryptParallelHashNull.c
   Hmac/CryptHmac.c
   Kdf/CryptHkdf.c
-  Cipher/CryptAesNull.c
+  Cipher/CryptAes.c
   Cipher/CryptAeadAesGcmNull.c
   Pk/CryptRsaBasic.c
   Pk/CryptRsaExtNull.c
diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index 0af7a3f96e8f..cc5a53ca92cd 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -43,7 +43,7 @@ [Sources]
   Hash/CryptCShake256.c
   Hash/CryptParallelHash.c
   Hmac/CryptHmac.c
-  Kdf/CryptHkdfNull.c
+  Kdf/CryptHkdf.c
   Cipher/CryptAes.c
   Cipher/CryptAeadAesGcmNull.c
   Pk/CryptRsaBasic.c
-- 
2.35.1.windows.2

Re: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

[Michael D Kinney]

Hi Judah,

There was an update to CryptoPkg pushed yesterday.

1) There is a CryptoPkg/Readme.md with tables and DSC content for services that are
   enabled in each phase.  I think that needs updates too for the AES and KDF features.
2) The CryptoPkg.dsc file has recommended settings for PEI, DXE, SMM.  I think
   they need to be updated for the AES and KDF features.
3) It looks like the SHA1 disable caused a build break.  I would like to see the
   standard package builds for EDK II CI be updated to cover the failure case so
   we know that this case is covered in the future.  It looks like the default is
   for SHA1 enabled and the build break is when define for SHA1 disabled is 
   asserted.
4) There is an overlap between the defines to deprecate MD5 and SH1 and the
   structured PCD that allows those services to be disabled in the Crypto 
   Protocol/PPI.  The defines to deprecate MD5 and SH1 extend into the BaseCryptLib
   instance implementations such that a call to those services when static linking
   will generate a build error instead of a runtime ASSERT().  Which behavior do
   you prefer?

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah Vang
> Sent: Monday, October 24, 2022 9:42 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3991
> https://bugzilla.tianocore.org/show_bug.cgi?id=3992
> 
> There is a #define to deprecate Sha1 functions but not
> all the Sha1 function are wrapped around this #define causing
> a build error. The fix is to wrap all Sha1 functions with
> the #define.
> 
> Need crypto AES to be supported for PEI phase and need
> crypto KDF to be supported for SMM phase.
> 
> Judah Vang (2):
>   CryptoPkg: Sha1 functions causing build errors
>   CryptoPkg: Need to enable crypto functions
> 
>  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
>  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
>  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> --
> 2.35.1.windows.2
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

[Judah Vang]

Mike,

Can you explain #3?  I have no idea how to update/modify the EDK2 CI.
I know for MTL, we have this define there by default, that is why when I enabled crypto for RPMC feature
for MTL we ran into the issue.

#4,  I prefer a build error.

Judah

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Monday, October 24, 2022 10:22 AM
To: devel@edk2.groups.io; Vang, Judah <judah.vang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

Hi Judah,

There was an update to CryptoPkg pushed yesterday.

1) There is a CryptoPkg/Readme.md with tables and DSC content for services that are
   enabled in each phase.  I think that needs updates too for the AES and KDF features.
2) The CryptoPkg.dsc file has recommended settings for PEI, DXE, SMM.  I think
   they need to be updated for the AES and KDF features.
3) It looks like the SHA1 disable caused a build break.  I would like to see the
   standard package builds for EDK II CI be updated to cover the failure case so
   we know that this case is covered in the future.  It looks like the default is
   for SHA1 enabled and the build break is when define for SHA1 disabled is 
   asserted.
4) There is an overlap between the defines to deprecate MD5 and SH1 and the
   structured PCD that allows those services to be disabled in the Crypto 
   Protocol/PPI.  The defines to deprecate MD5 and SH1 extend into the BaseCryptLib
   instance implementations such that a call to those services when static linking
   will generate a build error instead of a runtime ASSERT().  Which behavior do
   you prefer?

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah 
> Vang
> Sent: Monday, October 24, 2022 9:42 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3991
> https://bugzilla.tianocore.org/show_bug.cgi?id=3992
> 
> There is a #define to deprecate Sha1 functions but not all the Sha1 
> function are wrapped around this #define causing a build error. The 
> fix is to wrap all Sha1 functions with the #define.
> 
> Need crypto AES to be supported for PEI phase and need crypto KDF to 
> be supported for SMM phase.
> 
> Judah Vang (2):
>   CryptoPkg: Sha1 functions causing build errors
>   CryptoPkg: Need to enable crypto functions
> 
>  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
>  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
>  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> --
> 2.35.1.windows.2
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

[Michael D Kinney]

Hi Judah,

Does CryptoPkg.dsc break if DISABLE_SHA1_DEPRECATED_INTERFACE is defined in
that DSC file [BuildOptions] section?

All packages in edk2 repo have a .ci.yaml file that provides the set of CI checks
that are performed when a PR contains source changes to that package.  Here is
link to CryptoPkg.ci.yaml file:

    https://github.com/tianocore/edk2/blob/master/CryptoPkg/CryptoPkg.ci.yaml

The section of this file that identifies the package build step is "CompilerPlugin".
It specifies the relative path to the DSC file to build to perform a package 
scoped build verification.

    "CompilerPlugin": {
        "DscPath": "CryptoPkg.dsc"
    },

The easiest way to make sure there is build coverage for SHA1 disabled is to
make sure this DSC file is updated to include builds with and without SHA1
disabled.  SHA1 is enabled by default, so DSC file needs to be amended to
perform additional build(s) of components that disable SHA1.  This is a
challenge because this define is used in both libraries and modules so the
define needs to be global to cover library instances.


The define DISABLE_SHA1_DEPRECATED_INTERFACES is also used in the 
SecurityPkg, so that package may also need updates to get CI coverage
with and without this define.

https://github.com/tianocore/edk2/search?q=DISABLE_SHA1_DEPRECATED_INTERFACES&type=code

I just did a search for similar defines in edk2 repo:
* ENABLE_MD5_DEPRECATED_INTERFACES
* DISABLE_SHA1_DEPRECATED_INTERFACES
* DISABLE_NEW_DEPRECATED_INTERFACES

Perhaps Sean can provide advice on how to get full CI coverage for these
types of defines.

Best regards,

Mike


> -----Original Message-----
> From: Vang, Judah <judah.vang@intel.com>
> Sent: Wednesday, October 26, 2022 11:42 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Mistry, Nishant C <nishant.c.mistry@intel.com>
> Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> Mike,
> 
> Can you explain #3?  I have no idea how to update/modify the EDK2 CI.
> I know for MTL, we have this define there by default, that is why when I enabled crypto for RPMC feature
> for MTL we ran into the issue.
> 
> #4,  I prefer a build error.
> 
> Judah
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, October 24, 2022 10:22 AM
> To: devel@edk2.groups.io; Vang, Judah <judah.vang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> Hi Judah,
> 
> There was an update to CryptoPkg pushed yesterday.
> 
> 1) There is a CryptoPkg/Readme.md with tables and DSC content for services that are
>    enabled in each phase.  I think that needs updates too for the AES and KDF features.
> 2) The CryptoPkg.dsc file has recommended settings for PEI, DXE, SMM.  I think
>    they need to be updated for the AES and KDF features.
> 3) It looks like the SHA1 disable caused a build break.  I would like to see the
>    standard package builds for EDK II CI be updated to cover the failure case so
>    we know that this case is covered in the future.  It looks like the default is
>    for SHA1 enabled and the build break is when define for SHA1 disabled is
>    asserted.
> 4) There is an overlap between the defines to deprecate MD5 and SH1 and the
>    structured PCD that allows those services to be disabled in the Crypto
>    Protocol/PPI.  The defines to deprecate MD5 and SH1 extend into the BaseCryptLib
>    instance implementations such that a call to those services when static linking
>    will generate a build error instead of a runtime ASSERT().  Which behavior do
>    you prefer?
> 
> Best regards,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah
> > Vang
> > Sent: Monday, October 24, 2022 9:42 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3991
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3992
> >
> > There is a #define to deprecate Sha1 functions but not all the Sha1
> > function are wrapped around this #define causing a build error. The
> > fix is to wrap all Sha1 functions with the #define.
> >
> > Need crypto AES to be supported for PEI phase and need crypto KDF to
> > be supported for SMM phase.
> >
> > Judah Vang (2):
> >   CryptoPkg: Sha1 functions causing build errors
> >   CryptoPkg: Need to enable crypto functions
> >
> >  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
> >  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
> >  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
> >  3 files changed, 15 insertions(+), 3 deletions(-)
> >
> > --
> > 2.35.1.windows.2
> >
> >
> >
> > 
> >

Re: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

[Judah Vang]

Hi Mike,

This is not my realm of expertise.   I'm just trying to fix an issue that I ran into when enabling crypto.
Can I suggest that someone like the maintainers or someone who knows how the build works update the build?

Judah

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Wednesday, October 26, 2022 2:17 PM
To: Vang, Judah <judah.vang@intel.com>; devel@edk2.groups.io; Sean Brogan <sean.brogan@microsoft.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Mistry, Nishant C <nishant.c.mistry@intel.com>
Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

Hi Judah,

Does CryptoPkg.dsc break if DISABLE_SHA1_DEPRECATED_INTERFACE is defined in that DSC file [BuildOptions] section?

All packages in edk2 repo have a .ci.yaml file that provides the set of CI checks that are performed when a PR contains source changes to that package.  Here is link to CryptoPkg.ci.yaml file:

    https://github.com/tianocore/edk2/blob/master/CryptoPkg/CryptoPkg.ci.yaml

The section of this file that identifies the package build step is "CompilerPlugin".
It specifies the relative path to the DSC file to build to perform a package scoped build verification.

    "CompilerPlugin": {
        "DscPath": "CryptoPkg.dsc"
    },

The easiest way to make sure there is build coverage for SHA1 disabled is to make sure this DSC file is updated to include builds with and without SHA1 disabled.  SHA1 is enabled by default, so DSC file needs to be amended to perform additional build(s) of components that disable SHA1.  This is a challenge because this define is used in both libraries and modules so the define needs to be global to cover library instances.


The define DISABLE_SHA1_DEPRECATED_INTERFACES is also used in the SecurityPkg, so that package may also need updates to get CI coverage with and without this define.

https://github.com/tianocore/edk2/search?q=DISABLE_SHA1_DEPRECATED_INTERFACES&type=code

I just did a search for similar defines in edk2 repo:
* ENABLE_MD5_DEPRECATED_INTERFACES
* DISABLE_SHA1_DEPRECATED_INTERFACES
* DISABLE_NEW_DEPRECATED_INTERFACES

Perhaps Sean can provide advice on how to get full CI coverage for these types of defines.

Best regards,

Mike


> -----Original Message-----
> From: Vang, Judah <judah.vang@intel.com>
> Sent: Wednesday, October 26, 2022 11:42 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; 
> devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Mistry, Nishant C 
> <nishant.c.mistry@intel.com>
> Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> Mike,
> 
> Can you explain #3?  I have no idea how to update/modify the EDK2 CI.
> I know for MTL, we have this define there by default, that is why when 
> I enabled crypto for RPMC feature for MTL we ran into the issue.
> 
> #4,  I prefer a build error.
> 
> Judah
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, October 24, 2022 10:22 AM
> To: devel@edk2.groups.io; Vang, Judah <judah.vang@intel.com>; Kinney, 
> Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> Hi Judah,
> 
> There was an update to CryptoPkg pushed yesterday.
> 
> 1) There is a CryptoPkg/Readme.md with tables and DSC content for services that are
>    enabled in each phase.  I think that needs updates too for the AES and KDF features.
> 2) The CryptoPkg.dsc file has recommended settings for PEI, DXE, SMM.  I think
>    they need to be updated for the AES and KDF features.
> 3) It looks like the SHA1 disable caused a build break.  I would like to see the
>    standard package builds for EDK II CI be updated to cover the failure case so
>    we know that this case is covered in the future.  It looks like the default is
>    for SHA1 enabled and the build break is when define for SHA1 disabled is
>    asserted.
> 4) There is an overlap between the defines to deprecate MD5 and SH1 and the
>    structured PCD that allows those services to be disabled in the Crypto
>    Protocol/PPI.  The defines to deprecate MD5 and SH1 extend into the BaseCryptLib
>    instance implementations such that a call to those services when static linking
>    will generate a build error instead of a runtime ASSERT().  Which behavior do
>    you prefer?
> 
> Best regards,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah 
> > Vang
> > Sent: Monday, October 24, 2022 9:42 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3991
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3992
> >
> > There is a #define to deprecate Sha1 functions but not all the Sha1 
> > function are wrapped around this #define causing a build error. The 
> > fix is to wrap all Sha1 functions with the #define.
> >
> > Need crypto AES to be supported for PEI phase and need crypto KDF to 
> > be supported for SMM phase.
> >
> > Judah Vang (2):
> >   CryptoPkg: Sha1 functions causing build errors
> >   CryptoPkg: Need to enable crypto functions
> >
> >  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
> >  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
> >  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 
> > +++++++++++++-
> >  3 files changed, 15 insertions(+), 3 deletions(-)
> >
> > --
> > 2.35.1.windows.2
> >
> >
> >
> > 
> >

Re: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

[Judah Vang]

Hi all,

I resubmitted the patches with an update to the CryptoPkg/Readme.
The CryptoPkg.dsc has already been updated with the AES and KDF feature changes.

Thanks!

Judah

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Monday, October 24, 2022 10:22 AM
To: devel@edk2.groups.io; Vang, Judah <judah.vang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

Hi Judah,

There was an update to CryptoPkg pushed yesterday.

1) There is a CryptoPkg/Readme.md with tables and DSC content for services that are
   enabled in each phase.  I think that needs updates too for the AES and KDF features.
2) The CryptoPkg.dsc file has recommended settings for PEI, DXE, SMM.  I think
   they need to be updated for the AES and KDF features.
3) It looks like the SHA1 disable caused a build break.  I would like to see the
   standard package builds for EDK II CI be updated to cover the failure case so
   we know that this case is covered in the future.  It looks like the default is
   for SHA1 enabled and the build break is when define for SHA1 disabled is 
   asserted.
4) There is an overlap between the defines to deprecate MD5 and SH1 and the
   structured PCD that allows those services to be disabled in the Crypto 
   Protocol/PPI.  The defines to deprecate MD5 and SH1 extend into the BaseCryptLib
   instance implementations such that a call to those services when static linking
   will generate a build error instead of a runtime ASSERT().  Which behavior do
   you prefer?

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah 
> Vang
> Sent: Monday, October 24, 2022 9:42 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=3991
> https://bugzilla.tianocore.org/show_bug.cgi?id=3992
> 
> There is a #define to deprecate Sha1 functions but not all the Sha1 
> function are wrapped around this #define causing a build error. The 
> fix is to wrap all Sha1 functions with the #define.
> 
> Need crypto AES to be supported for PEI phase and need crypto KDF to 
> be supported for SMM phase.
> 
> Judah Vang (2):
>   CryptoPkg: Sha1 functions causing build errors
>   CryptoPkg: Need to enable crypto functions
> 
>  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
>  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
>  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> --
> 2.35.1.windows.2
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes

[Michael D Kinney]

Judah,

This was the feedback that I have back on 10/24/22 that was not incorporated into
the CryptoPkg patches before they were merged today by Jiewen.

Please generate an additional patch series to address (1) and (2).

Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, October 24, 2022 10:22 AM
> To: devel@edk2.groups.io; Vang, Judah <judah.vang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> 
> Hi Judah,
> 
> There was an update to CryptoPkg pushed yesterday.
> 
> 1) There is a CryptoPkg/Readme.md with tables and DSC content for services that are
>    enabled in each phase.  I think that needs updates too for the AES and KDF features.
> 2) The CryptoPkg.dsc file has recommended settings for PEI, DXE, SMM.  I think
>    they need to be updated for the AES and KDF features.
> 3) It looks like the SHA1 disable caused a build break.  I would like to see the
>    standard package builds for EDK II CI be updated to cover the failure case so
>    we know that this case is covered in the future.  It looks like the default is
>    for SHA1 enabled and the build break is when define for SHA1 disabled is
>    asserted.
> 4) There is an overlap between the defines to deprecate MD5 and SH1 and the
>    structured PCD that allows those services to be disabled in the Crypto
>    Protocol/PPI.  The defines to deprecate MD5 and SH1 extend into the BaseCryptLib
>    instance implementations such that a call to those services when static linking
>    will generate a build error instead of a runtime ASSERT().  Which behavior do
>    you prefer?
> 
> Best regards,
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Judah Vang
> > Sent: Monday, October 24, 2022 9:42 AM
> > To: devel@edk2.groups.io
> > Subject: [edk2-devel] [PATCH V1 0/2] CryptoPkg bug fixes
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3991
> > https://bugzilla.tianocore.org/show_bug.cgi?id=3992
> >
> > There is a #define to deprecate Sha1 functions but not
> > all the Sha1 function are wrapped around this #define causing
> > a build error. The fix is to wrap all Sha1 functions with
> > the #define.
> >
> > Need crypto AES to be supported for PEI phase and need
> > crypto KDF to be supported for SMM phase.
> >
> > Judah Vang (2):
> >   CryptoPkg: Sha1 functions causing build errors
> >   CryptoPkg: Need to enable crypto functions
> >
> >  CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf    |  2 +-
> >  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf    |  2 +-
> >  CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.c | 14 +++++++++++++-
> >  3 files changed, 15 insertions(+), 3 deletions(-)
> >
> > --
> > 2.35.1.windows.2
> >
> >
> >
> > 
> >