public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO
@ 2020-08-31  5:13 Gao, Zhichao
  2020-09-07  2:15 ` Yao, Jiewen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gao, Zhichao @ 2020-08-31  5:13 UTC (permalink / raw)
  To: devel; +Cc: Jiewen Yao, Jian J Wang, Min Xu, Qi Zhang

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/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index b08fe24e85..7871220140 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.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


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

* Re: [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO
  2020-08-31  5:13 [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO Gao, Zhichao
@ 2020-09-07  2:15 ` Yao, Jiewen
  2020-09-07  2:17 ` Wang, Jian J
       [not found] ` <16325EB1DAFF59F3.20857@groups.io>
  2 siblings, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2020-09-07  2:15 UTC (permalink / raw)
  To: Gao, Zhichao, devel@edk2.groups.io; +Cc: Wang, Jian J, Xu, Min M, Zhang, Qi1

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/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index b08fe24e85..7871220140 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.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


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

* Re: [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO
  2020-08-31  5:13 [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO Gao, Zhichao
  2020-09-07  2:15 ` Yao, Jiewen
@ 2020-09-07  2:17 ` Wang, Jian J
       [not found] ` <16325EB1DAFF59F3.20857@groups.io>
  2 siblings, 0 replies; 6+ messages in thread
From: Wang, Jian J @ 2020-09-07  2:17 UTC (permalink / raw)
  To: Gao, Zhichao, devel@edk2.groups.io; +Cc: Yao, Jiewen, Xu, Min M, Zhang, Qi1


Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

> -----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/DxeImageVerificationLib.c
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index b08fe24e85..7871220140 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.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


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

* Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO
       [not found] ` <16325EB1DAFF59F3.20857@groups.io>
@ 2020-09-07  2:20   ` Yao, Jiewen
  2020-09-07  2:36     ` Gao, Zhichao
  0 siblings, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2020-09-07  2:20 UTC (permalink / raw)
  To: devel@edk2.groups.io, Yao, Jiewen, Gao, Zhichao
  Cc: Wang, Jian J, Xu, Min M, Zhang, Qi1

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/DxeImageVerificationLib.c
> > b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> > index b08fe24e85..7871220140 100644
> > --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> > +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.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
> 
> 
> 


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

* Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO
  2020-09-07  2:20   ` [edk2-devel] " Yao, Jiewen
@ 2020-09-07  2:36     ` Gao, Zhichao
  2020-09-07  2:55       ` Yao, Jiewen
  0 siblings, 1 reply; 6+ messages in thread
From: Gao, Zhichao @ 2020-09-07  2:36 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io; +Cc: Wang, Jian J, Xu, Min M, Zhang, Qi1

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
> >
> >
> > 


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

* Re: [edk2-devel] [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO
  2020-09-07  2:36     ` Gao, Zhichao
@ 2020-09-07  2:55       ` Yao, Jiewen
  0 siblings, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2020-09-07  2:55 UTC (permalink / raw)
  To: Gao, Zhichao, devel@edk2.groups.io; +Cc: Wang, Jian J, Xu, Min M, Zhang, Qi1

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
> > >
> > >
> > > 


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

end of thread, other threads:[~2020-09-07  2:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-31  5:13 [PATCH] SecurityPkg/DxeImageVerificationLib: Disable SHA1 base on MACRO Gao, Zhichao
2020-09-07  2:15 ` Yao, Jiewen
2020-09-07  2:17 ` Wang, Jian J
     [not found] ` <16325EB1DAFF59F3.20857@groups.io>
2020-09-07  2:20   ` [edk2-devel] " Yao, Jiewen
2020-09-07  2:36     ` Gao, Zhichao
2020-09-07  2:55       ` Yao, Jiewen

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