[edk2-devel] [PATCH V4 00/13] Disable the deprecated MD5 and SHA1 support

Gao, Zhichao posted 13 patches 3 years, 5 months ago
Failed in applying to current master (apply log)
ArmVirtPkg/ArmVirtQemu.dsc                    |  6 ++++-
ArmVirtPkg/ArmVirtQemuKernel.dsc              |  5 ++++-
CryptoPkg/CryptoPkg.dsc                       |  6 +++++
CryptoPkg/Driver/Crypto.c                     |  4 ++--
CryptoPkg/Include/Library/BaseCryptLib.h      |  2 +-
.../Library/BaseCryptLib/Hash/CryptMd5.c      |  2 +-
.../BaseCryptLibOnProtocolPpi/CryptLib.c      |  2 +-
NetworkPkg/Network.dsc.inc                    |  5 ++++-
NetworkPkg/NetworkBuildOptions.dsc.inc        | 22 +++++++++++++++++++
NetworkPkg/NetworkDefines.dsc.inc             |  4 ++--
NetworkPkg/NetworkPkg.dsc                     |  4 +++-
OvmfPkg/Bhyve/BhyveX64.dsc                    |  5 ++++-
OvmfPkg/OvmfPkgIa32.dsc                       |  3 +++
OvmfPkg/OvmfPkgIa32X64.dsc                    |  3 +++
OvmfPkg/OvmfPkgX64.dsc                        |  3 +++
OvmfPkg/OvmfXen.dsc                           |  3 +++
SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.c   |  2 --
SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf |  4 +---
18 files changed, 68 insertions(+), 17 deletions(-)
create mode 100644 NetworkPkg/NetworkBuildOptions.dsc.inc
[edk2-devel] [PATCH V4 00/13] Disable the deprecated MD5 and SHA1 support
Posted by Gao, Zhichao 3 years, 5 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3021
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3027

MD5 is deprecated, make it disable as default for security.
It required to set MD5 enable explicitly if the module is still using 
MD5. List the modules that are still using it:
iSCSI, Hash2DxeCrypto, CryptoDxe(Pei, Smm) (with PACKAGE or ALL config).

This patch set would affact the platforms that are using iSCSI 
function.

V2:
Remove MD5 and SHA1 support of Hash2DxeCrypto.
Remove the MD5 GUID defination in MdePkg.dec. SHA1 related GUIDs
are still using in TPM2, so keep them.
No requirement to add MD5 enable MACRO in SecurityPkg.

V3:
Explicitly enable iSCSI for ArmVirtQemu, ArmVirtQemuKernel,
OvmfPkgIa32, OvmfPkgIa32X64, OvmfPkgX64 and BhyveX64.
And set the MD5 enable base on the new MD5 MACRO.
Rejust the patch order.

V14:
Fix some typos.
Change the commit message.
Add NetworkBuildOptions.dsc.inc and add the MACRO for
different toolchain.
Using inc file in the related package dsc file:
ArmVirtQemu, ArmVirtQemuKernel, OvmfPkgIa32, OvmfPkgIa32X64,
OvmfPkgX64, OvmfXen and BhyveX64.
Enable iSCSI in NetworkPkg.dsc for build test.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
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: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Cc: Zailiang Sun <zailiang.sun@intel.com>
Cc: Yi Qian <yi.qian@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
Cc: Roger Feng <roger.feng@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>

Zhichao Gao (13):
  SecurityPkg/Hash2DxeCrypto: Remove MD5 support
  SecurityPkg/Hash2DxeCrypto: Remove SHA1 support
  CryptoPkg/dsc: Enable MD5 when CRYPTO_SERVICES enable MD5
  NetworkPkg: Enable MD5 while enable iSCSI
  ArmVirtPkg/ArmVirtQemu.dsc: Enable MD5 while enable iSCSI
  ArmVirtPkg/ArmVirtQemuKernel.dsc: Enable MD5 while enable iSCSI
  OvmfPkg/OvmfPkgIa32.dsc: Enable MD5 while enable iSCSI
  OvmfPkg/OvmfPkgIa32X64.dsc: Enable MD5 while enable iSCSI
  OvmfPkg/OvmfPkgX64.dsc: Enable MD5 while enable iSCSI
  OvmfPkg/OvmfXen.dsc: Enable MD5 while enable iSCSI
  OvmfPkg/BhyveX64.dsc: Enable MD5 while enable iSCSI
  NetworkPkg/Defines: Make iSCSI disable as default
  CryptoPkg: Make the MD5 disable as default for security

 ArmVirtPkg/ArmVirtQemu.dsc                    |  6 ++++-
 ArmVirtPkg/ArmVirtQemuKernel.dsc              |  5 ++++-
 CryptoPkg/CryptoPkg.dsc                       |  6 +++++
 CryptoPkg/Driver/Crypto.c                     |  4 ++--
 CryptoPkg/Include/Library/BaseCryptLib.h      |  2 +-
 .../Library/BaseCryptLib/Hash/CryptMd5.c      |  2 +-
 .../BaseCryptLibOnProtocolPpi/CryptLib.c      |  2 +-
 NetworkPkg/Network.dsc.inc                    |  5 ++++-
 NetworkPkg/NetworkBuildOptions.dsc.inc        | 22 +++++++++++++++++++
 NetworkPkg/NetworkDefines.dsc.inc             |  4 ++--
 NetworkPkg/NetworkPkg.dsc                     |  4 +++-
 OvmfPkg/Bhyve/BhyveX64.dsc                    |  5 ++++-
 OvmfPkg/OvmfPkgIa32.dsc                       |  3 +++
 OvmfPkg/OvmfPkgIa32X64.dsc                    |  3 +++
 OvmfPkg/OvmfPkgX64.dsc                        |  3 +++
 OvmfPkg/OvmfXen.dsc                           |  3 +++
 SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.c   |  2 --
 SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf |  4 +---
 18 files changed, 68 insertions(+), 17 deletions(-)
 create mode 100644 NetworkPkg/NetworkBuildOptions.dsc.inc

-- 
2.21.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67359): https://edk2.groups.io/g/devel/message/67359
Mute This Topic: https://groups.io/mt/78201057/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH V4 00/13] Disable the deprecated MD5 and SHA1 support
Posted by Laszlo Ersek 3 years, 5 months ago
On 11/12/20 06:55, Gao, Zhichao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3021
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3027
>
> MD5 is deprecated, make it disable as default for security.
> It required to set MD5 enable explicitly if the module is still using
> MD5. List the modules that are still using it:
> iSCSI, Hash2DxeCrypto, CryptoDxe(Pei, Smm) (with PACKAGE or ALL config).
>
> This patch set would affact the platforms that are using iSCSI
> function.
>
> V2:
> Remove MD5 and SHA1 support of Hash2DxeCrypto.
> Remove the MD5 GUID defination in MdePkg.dec. SHA1 related GUIDs
> are still using in TPM2, so keep them.
> No requirement to add MD5 enable MACRO in SecurityPkg.
>
> V3:
> Explicitly enable iSCSI for ArmVirtQemu, ArmVirtQemuKernel,
> OvmfPkgIa32, OvmfPkgIa32X64, OvmfPkgX64 and BhyveX64.
> And set the MD5 enable base on the new MD5 MACRO.
> Rejust the patch order.
>
> V14:
> Fix some typos.
> Change the commit message.
> Add NetworkBuildOptions.dsc.inc and add the MACRO for
> different toolchain.
> Using inc file in the related package dsc file:
> ArmVirtQemu, ArmVirtQemuKernel, OvmfPkgIa32, OvmfPkgIa32X64,
> OvmfPkgX64, OvmfXen and BhyveX64.
> Enable iSCSI in NetworkPkg.dsc for build test.

I'm in the process of merging this series.

* For composing the branch / merge request:

(a) I have applied the v4 patches locally.

(b) I have added the feedback tags from the v4 thread.

(c) I have implemented changes (1) through (3) suggested here:

    [edk2-devel] [PATCH V4 04/13] NetworkPkg: Enable MD5 while enable iSCSI
    https://edk2.groups.io/g/devel/message/67558
    https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00718.html

(d) I have also lifted the [BuildOptions] section above [Components] in:

    [edk2-devel] [PATCH V4 04/13] NetworkPkg: Enable MD5 while enable iSCSI

    according to the following discussion:

    https://edk2.groups.io/g/devel/message/67525
    https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00684.html

    and

    https://edk2.groups.io/g/devel/message/67591
    https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00751.html

    My (c) and (d) changes match the updates in Zhichao's commit
    41ac3e7fa126, on his "MD5_disable" branch.

* The above actions of mine result in the following range-diff, relative
to the v4 posting on the list:

>  1:  c476b165a734 =  1:  5ee36bcf2a98 SecurityPkg/Hash2DxeCrypto: Remove MD5 support
>  2:  ee71ac319d1b =  2:  0c45603e7208 SecurityPkg/Hash2DxeCrypto: Remove SHA1 support
>  3:  eebfdc4d1c34 !  3:  3d97de8735b8 CryptoPkg/dsc: Enable MD5 when CRYPTO_SERVICES enable MD5
>     @@ -15,6 +15,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>          Message-Id: <20201112055558.2348-4-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
>      --- a/CryptoPkg/CryptoPkg.dsc
>  4:  a9de45be7e52 !  4:  c17e682d01ce NetworkPkg: Enable MD5 while enable iSCSI
>     @@ -19,6 +19,11 @@
>          Cc: Siyuan Fu <siyuan.fu@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>          Message-Id: <20201112055558.2348-5-zhichao.gao@intel.com>
>     +    [lersek@redhat.com: clean up comments in "NetworkBuildOptions.dsc.inc"]
>     +    [lersek@redhat.com: hoist "BuildOptions" above "Components" in
>     +     "Network.dsc.inc" for bug compat with edk2-platforms]
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
>
>      diff --git a/NetworkPkg/Network.dsc.inc b/NetworkPkg/Network.dsc.inc
>      --- a/NetworkPkg/Network.dsc.inc
>     @@ -33,12 +38,15 @@
>       #
>       #    SPDX-License-Identifier: BSD-2-Clause-Patent
>      @@
>     - !include NetworkPkg/NetworkComponents.dsc.inc
>     + [LibraryClasses]
>     + !include NetworkPkg/NetworkLibs.dsc.inc
>
>     - !endif
>     -+
>      +[BuildOptions]
>      +!include NetworkPkg/NetworkBuildOptions.dsc.inc
>     ++
>     + !if $(PLATFORMX64_ENABLE) == TRUE
>     + [Components.X64]
>     + !include NetworkPkg/NetworkComponents.dsc.inc
>
>      diff --git a/NetworkPkg/NetworkBuildOptions.dsc.inc b/NetworkPkg/NetworkBuildOptions.dsc.inc
>      new file mode 100644
>     @@ -48,12 +56,12 @@
>      +## @file
>      +# Network DSC include file for [BuildOptions] sections of all Architectures.
>      +#
>     -+# This file can be included in the [BuildOptions*] section(s) of a platform # DSC file
>     ++# This file can be included in the [BuildOptions*] section(s) of a platform DSC file
>      +# by using "!include NetworkPkg/NetworkBuildOptions.dsc.inc", to specify the C language
>      +# feature test macros (eg., API deprecation macros) according to the flags described
>      +# in "NetworkDefines.dsc.inc".
>      +#
>     -+# Supported tool chain: "GCC:", "INTEL:", "MSFT:", "RVCT".
>     ++# Supported tool chain families: "GCC", "INTEL", "MSFT", "RVCT".
>      +#
>      +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
>      +#
>  5:  69e9477abff3 !  5:  408c15466aa6 ArmVirtPkg/ArmVirtQemu.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,8 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-6-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>      --- a/ArmVirtPkg/ArmVirtQemu.dsc
>  6:  2a4e37a3eb1b !  6:  6f699db9003e ArmVirtPkg/ArmVirtQemuKernel.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,7 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-7-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>      --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
>  7:  46a433fb23d9 !  7:  f94376fa1c42 OvmfPkg/OvmfPkgIa32.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,8 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-8-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>      --- a/OvmfPkg/OvmfPkgIa32.dsc
>  8:  943230f06d14 !  8:  d886a5ee0c1c OvmfPkg/OvmfPkgIa32X64.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,8 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-9-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>      --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>  9:  73a2128a6cca !  9:  5b942298b4c1 OvmfPkg/OvmfPkgX64.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,8 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-10-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>      --- a/OvmfPkg/OvmfPkgX64.dsc
> 10:  31677228ba91 ! 10:  d30e7861db19 OvmfPkg/OvmfXen.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,8 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-11-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
>      --- a/OvmfPkg/OvmfXen.dsc
> 11:  c6c2704963e1 ! 11:  97d95fc17245 OvmfPkg/BhyveX64.dsc: Enable MD5 while enable iSCSI
>     @@ -17,6 +17,8 @@
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>          Message-Id: <20201112055558.2348-12-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
>      --- a/OvmfPkg/Bhyve/BhyveX64.dsc
> 12:  7a5e013b925c ! 12:  900809889d32 NetworkPkg/Defines: Make iSCSI disable as default
>     @@ -31,6 +31,9 @@
>          Cc: Siyuan Fu <siyuan.fu@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>          Message-Id: <20201112055558.2348-13-zhichao.gao@intel.com>
>     +    Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
>     +    Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>
>      diff --git a/NetworkPkg/NetworkDefines.dsc.inc b/NetworkPkg/NetworkDefines.dsc.inc
>      --- a/NetworkPkg/NetworkDefines.dsc.inc
> 13:  7ed3343c20ba = 13:  31e5b14fc7ce CryptoPkg: Make the MD5 disable as default for security

* At this point, I have also compared my local branch (to be merged)
with Zhichao's MD5_disable branch, currently at commit e9f94f099f83.
Please see the range-diff below, with my comments (the changes are
expressed as working from Zhichao's branch to mine):

>  1:  3be76c44753f !  1:  5ee36bcf2a98 SecurityPkg/Hash2DxeCrypto: Remove MD5 support
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          SecurityPkg/Hash2DxeCrypto: Remove MD5 support
>
>     @@ -11,6 +11,7 @@
>          Cc: Jian J Wang <jian.j.wang@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>          Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>     +    Message-Id: <20201112055558.2348-2-zhichao.gao@intel.com>
>
>      diff --git a/SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf b/SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf
>      --- a/SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf
>  2:  6ad0fd30037e !  2:  0c45603e7208 SecurityPkg/Hash2DxeCrypto: Remove SHA1 support
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          SecurityPkg/Hash2DxeCrypto: Remove SHA1 support
>
>     @@ -11,6 +11,7 @@
>          Cc: Jian J Wang <jian.j.wang@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>          Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>     +    Message-Id: <20201112055558.2348-3-zhichao.gao@intel.com>
>
>      diff --git a/SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf b/SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf
>      --- a/SecurityPkg/Hash2DxeCrypto/Hash2DxeCrypto.inf
>  3:  2439f393cb9b !  3:  3d97de8735b8 CryptoPkg/dsc: Enable MD5 when CRYPTO_SERVICES enable MD5
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          CryptoPkg/dsc: Enable MD5 when CRYPTO_SERVICES enable MD5
>
>     @@ -14,7 +14,7 @@
>          Cc: Guomin Jiang <guomin.jiang@intel.com>
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>     -    Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>     +    Message-Id: <20201112055558.2348-4-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc

This is worth highlighting.

Jiewen had given his R-b originally on patch #3 "CryptoPkg/dsc: Enable
MD5 when CRYPTO_SERVICES enable MD5", namely in the v2 thread:

  https://edk2.groups.io/g/devel/message/66629
  https://www.redhat.com/archives/edk2-devel-archive/2020-October/msg00848.html

However, under v3 I pointed out that the patch was not right:

  https://edk2.groups.io/g/devel/message/67319
  https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00476.html

Accordingly, Zhichao updated the patch in v4, and *correctly* dropped
Jiewen's R-b from v2 (as the patch has changed non-trivially):

  https://edk2.groups.io/g/devel/message/67362
  https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00521.html

Note that Jiewen did not re-review patch#3 in v4!

Therefore, Zhichao, it was *incorrect* from you to just go back to v2
and reapply Jiewen's R-b from there. You were right to *drop* it in v4,
and you were very wrong to *silently* re-add it on your "MD5_disble"
branch! (Commit 2439f393cb9b.)

Therefore, this CryptoPkg patch is now going in with *only* my review,
even though I'm not even a designated reviewer for CryptoPkg, let alone
a maintainer.

Merging patch#3 like this is not "ideal", to say the least, but the v4
patch has been on the list for 5 days now, and Jiewen was correctly CC'd
on it. It would have been a really simple incremental review. I'm quite
fed up with reviewers ignoring their responsibilities, so it's either
this, or we can delay (or revert) the series until after the stable tag
is released.

>  4:  41ac3e7fa126 !  4:  c17e682d01ce NetworkPkg: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          NetworkPkg: Enable MD5 while enable iSCSI
>
>     @@ -18,9 +18,12 @@
>          Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>          Cc: Siyuan Fu <siyuan.fu@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>     +    Message-Id: <20201112055558.2348-5-zhichao.gao@intel.com>
>     +    [lersek@redhat.com: clean up comments in "NetworkBuildOptions.dsc.inc"]
>     +    [lersek@redhat.com: hoist "BuildOptions" above "Components" in
>     +     "Network.dsc.inc" for bug compat with edk2-platforms]
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
>     -    Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
>
>      diff --git a/NetworkPkg/Network.dsc.inc b/NetworkPkg/Network.dsc.inc
>      --- a/NetworkPkg/Network.dsc.inc

Here Zhichao mistakenly applied an R-b from Maciej. Maciej did not
provide an R-b for this patch, as far as I can see.

Siyuan did review the patch, so the patch is OK to merge:

  https://edk2.groups.io/g/devel/message/67602
  https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00762.html

>  5:  7ffc00523613 !  5:  408c15466aa6 ArmVirtPkg/ArmVirtQemu.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          ArmVirtPkg/ArmVirtQemu.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,7 +16,9 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-6-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>     +    Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>      --- a/ArmVirtPkg/ArmVirtQemu.dsc

Here Zhichao missed my Build-tested-by:

  https://edk2.groups.io/g/devel/message/67559
  https://www.redhat.com/archives/edk2-devel-archive/2020-November/msg00719.html

>  6:  f832f46e1330 !  6:  6f699db9003e ArmVirtPkg/ArmVirtQemuKernel.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          ArmVirtPkg/ArmVirtQemuKernel.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,6 +16,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-7-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>      diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
>  7:  eede1be8df57 !  7:  f94376fa1c42 OvmfPkg/OvmfPkgIa32.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          OvmfPkg/OvmfPkgIa32.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,6 +16,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-8-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>  8:  6318da723b53 !  8:  d886a5ee0c1c OvmfPkg/OvmfPkgIa32X64.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          OvmfPkg/OvmfPkgIa32X64.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,6 +16,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-9-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
>  9:  e7c35bb40136 !  9:  5b942298b4c1 OvmfPkg/OvmfPkgX64.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          OvmfPkg/OvmfPkgX64.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,6 +16,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-10-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
> 10:  901324b3b33f ! 10:  d30e7861db19 OvmfPkg/OvmfXen.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          OvmfPkg/OvmfXen.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,6 +16,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-11-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
> 11:  64691d5671ba ! 11:  97d95fc17245 OvmfPkg/BhyveX64.dsc: Enable MD5 while enable iSCSI
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          OvmfPkg/BhyveX64.dsc: Enable MD5 while enable iSCSI
>
>     @@ -16,6 +16,7 @@
>          Cc: Laszlo Ersek <lersek@redhat.com>
>          Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>          Cc: Leif Lindholm <leif@nuviainc.com>
>     +    Message-Id: <20201112055558.2348-12-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Build-tested-by: Laszlo Ersek <lersek@redhat.com>
>
> 12:  40cb8b3e522a ! 12:  900809889d32 NetworkPkg/Defines: Make iSCSI disable as default
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          NetworkPkg/Defines: Make iSCSI disable as default
>
>     @@ -30,6 +30,7 @@
>          Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>          Cc: Siyuan Fu <siyuan.fu@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>     +    Message-Id: <20201112055558.2348-13-zhichao.gao@intel.com>
>          Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>          Reviewed-by: Siyuan Fu <siyuan.fu@intel.com>
>          Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
> 13:  e9f94f099f83 ! 13:  31e5b14fc7ce CryptoPkg: Make the MD5 disable as default for security
>     @@ -1,4 +1,4 @@
>     -Author: Zhichao Gao <zhichao.gao@intel.com>
>     +Author: Gao, Zhichao <zhichao.gao@intel.com>
>
>          CryptoPkg: Make the MD5 disable as default for security
>
>     @@ -13,6 +13,7 @@
>          Cc: Guomin Jiang <guomin.jiang@intel.com>
>          Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>          Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
>     +    Message-Id: <20201112055558.2348-14-zhichao.gao@intel.com>
>
>      diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h b/CryptoPkg/Include/Library/BaseCryptLib.h
>      --- a/CryptoPkg/Include/Library/BaseCryptLib.h

The above process mistakes exemplify why I'm very reluctant to merge
branches from personal repositories "after review", as opposed to
picking up patches, feedback, and updates from the list, and composing
the branch pull request manually.

I'm going to submit a pull request now with my branch, and report back
with the commit range (hopefully).

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67652): https://edk2.groups.io/g/devel/message/67652
Mute This Topic: https://groups.io/mt/78201057/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH V4 00/13] Disable the deprecated MD5 and SHA1 support
Posted by Laszlo Ersek 3 years, 5 months ago
On 11/17/20 20:16, Laszlo Ersek wrote:

> I'm going to submit a pull request now with my branch, and report back
> with the commit range (hopefully).

Series merged as commit range 29d59baa3907..e6a12a0fc817, via
<https://github.com/tianocore/edk2/pull/1130>.

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67653): https://edk2.groups.io/g/devel/message/67653
Mute This Topic: https://groups.io/mt/78201057/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-