Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO

["Yao, Jiewen" <jiewen.yao@intel.com>]

I don't mean TPM1.2.
I means UEFI secure boot - https://github.com/tianocore/edk2/tree/master/SecurityPkg/Library/AuthVariableLib

For example:

  {EFI_CERT_SHA1_GUID,            0,               20           },
  {EFI_CERT_RSA2048_SHA1_GUID,    0,               256          },

EFI_GUID mSignatureSupport[] = {EFI_CERT_SHA1_GUID, EFI_CERT_SHA256_GUID, EFI_CERT_RSA2048_GUID, EFI_CERT_X509_GUID};

I believe we should give DISABLE_SHA1_DEPRECATED_INTERFACES around them, right?



> -----Original Message-----
> From: Gao, Zhichao <zhichao.gao@intel.com>
> Sent: Monday, September 7, 2020 10:36 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
> Zhang, Qi1 <qi1.zhang@intel.com>
> Subject: RE: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Disable
> SHA1 base on MACRO
> 
> Hi Jiewen,
> 
> There are still some use case in the SecurityPkg. Such as TPM1.2. After the
> security package can build with the disable MACRO, we can remove all the
> content of SHA1.
> For now many platforms keep using the TPM1.2, I am not sure when the TPM1.2
> would be dropped from the SecurityPkg.
> 
> Thanks,
> Zhichao
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Monday, September 7, 2020 10:20 AM
> > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Gao,
> Zhichao
> > <zhichao.gao@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Xu, Min M <min.m.xu@intel.com>;
> > Zhang, Qi1 <qi1.zhang@intel.com>
> > Subject: RE: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib:
> Disable
> > SHA1 base on MACRO
> >
> > Hi Zhichao
> > Thanks for the patch.
> > I gave Reviewed-by because the Bugzilla only mentioned
> > DxeImageVerificationLib.
> >
> > As a full solution to remove SHA1 from SecureBoot, I think we should also
> > remove SHA1 from AuthVariableLib.
> >
> > Any plan on that?
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> > > Jiewen
> > > Sent: Monday, September 7, 2020 10:16 AM
> > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Xu, Min M
> > > <min.m.xu@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib:
> > > Disable
> > > SHA1 base on MACRO
> > >
> > > Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > >
> > > > -----Original Message-----
> > > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > > Sent: Monday, August 31, 2020 1:13 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>;
> > > > Xu, Min M <min.m.xu@intel.com>; Zhang, Qi1 <qi1.zhang@intel.com>
> > > > Subject: [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1
> > > > base on MACRO
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2943
> > > >
> > > > Disable SHA1 base on the MACRO
> DISABLE_SHA1_DEPRECATED_INTERFACES.
> > > > SHA1 is deprecated function and the MACRO is used to remove the
> > > > whole implementation of the SHA1. For the platforms that do not need
> > > > SHA1 for security, the MACRO should works for
> > > > DxeImageVerificationLib as well.
> > > >
> > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Min Xu <min.m.xu@intel.com>
> > > > Cc: Qi Zhang <qi1.zhang@intel.com>
> > > > ---
> > > >  .../DxeImageVerificationLib/DxeImageVerificationLib.c       | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git
> > > > a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi
> > > > b.c
> > > > b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi
> > > > b.c
> > > > index b08fe24e85..7871220140 100644
> > > > ---
> > > > a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLi
> > > > b.c
> > > > +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificati
> > > > +++ onLib.c
> > > > @@ -59,7 +59,11 @@ UINT8 mHashOidValue[] = {
> > > >    };
> > > >
> > > >  HASH_TABLE mHash[] = {
> > > > +#ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
> > > >    { L"SHA1",   20, &mHashOidValue[0],  5, Sha1GetContextSize,   Sha1Init,
> > > > Sha1Update,   Sha1Final  },
> > > > +#else
> > > > +  { L"SHA1",   20, &mHashOidValue[0],  5, NULL,                 NULL,       NULL,
> > > > NULL       },
> > > > +#endif
> > > >    { L"SHA224", 28, &mHashOidValue[5],  9, NULL,                 NULL,       NULL,
> > > > NULL       },
> > > >    { L"SHA256", 32, &mHashOidValue[14], 9, Sha256GetContextSize,
> > > > Sha256Init, Sha256Update, Sha256Final},
> > > >    { L"SHA384", 48, &mHashOidValue[23], 9, Sha384GetContextSize,
> > > > Sha384Init, Sha384Update, Sha384Final}, @@ -315,10 +319,12 @@
> > > > HashPeImage (
> > > >    ZeroMem (mImageDigest, MAX_DIGEST_SIZE);
> > > >
> > > >    switch (HashAlg) {
> > > > +#ifndef DISABLE_SHA1_DEPRECATED_INTERFACES
> > > >    case HASHALG_SHA1:
> > > >      mImageDigestSize = SHA1_DIGEST_SIZE;
> > > >      mCertType        = gEfiCertSha1Guid;
> > > >      break;
> > > > +#endif
> > > >
> > > >    case HASHALG_SHA256:
> > > >      mImageDigestSize = SHA256_DIGEST_SIZE;
> > > > --
> > > > 2.21.0.windows.1
> > >
> > >
> > >