[edk2-devel] [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default

Gao, Zhichao posted 7 patches 5 years, 3 months ago
There is a newer version of this series
[edk2-devel] [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default
Posted by Gao, Zhichao 5 years, 3 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003

iSCSI is using the undeprecated function MD5. It is
better to make the default setting secure. If the platforms
want to use the iSCSI, they should enable it in the platforms'
dsc file and be aware they are using an unsafe function.

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>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 NetworkPkg/NetworkDefines.dsc.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/NetworkDefines.dsc.inc b/NetworkPkg/NetworkDefines.dsc.inc
index a442d1b157..18921d81f6 100644
--- a/NetworkPkg/NetworkDefines.dsc.inc
+++ b/NetworkPkg/NetworkDefines.dsc.inc
@@ -17,7 +17,7 @@
 #   DEFINE NETWORK_TLS_ENABLE             = TRUE
 #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
 #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
-#   DEFINE NETWORK_ISCSI_ENABLE           = TRUE
+#   DEFINE NETWORK_ISCSI_ENABLE           = FALSE
 #   DEFINE NETWORK_VLAN_ENABLE            = TRUE
 #
 # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
@@ -101,7 +101,7 @@
   #       Both OpensslLib.inf and OpensslLibCrypto.inf library instance can be used
   #       since libssl is not required for iSCSI.
   #
-  DEFINE NETWORK_ISCSI_ENABLE = TRUE
+  DEFINE NETWORK_ISCSI_ENABLE = FALSE
 !endif
 
 !if $(NETWORK_ENABLE) == TRUE
-- 
2.21.0.windows.1



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


Re: [edk2-devel] [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default
Posted by Laszlo Ersek 5 years, 3 months ago
Hi Zhichao,

thanks for the CC, I appreciate it. Please see my comments below.

On 10/27/20 03:42, Zhichao Gao wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003
> 
> iSCSI is using the undeprecated function MD5. It is
> better to make the default setting secure. If the platforms
> want to use the iSCSI, they should enable it in the platforms'
> dsc file and be aware they are using an unsafe function.
> 
> 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>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  NetworkPkg/NetworkDefines.dsc.inc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/NetworkDefines.dsc.inc b/NetworkPkg/NetworkDefines.dsc.inc
> index a442d1b157..18921d81f6 100644
> --- a/NetworkPkg/NetworkDefines.dsc.inc
> +++ b/NetworkPkg/NetworkDefines.dsc.inc
> @@ -17,7 +17,7 @@
>  #   DEFINE NETWORK_TLS_ENABLE             = TRUE
>  #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
>  #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> -#   DEFINE NETWORK_ISCSI_ENABLE           = TRUE
> +#   DEFINE NETWORK_ISCSI_ENABLE           = FALSE
>  #   DEFINE NETWORK_VLAN_ENABLE            = TRUE
>  #
>  # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> @@ -101,7 +101,7 @@
>    #       Both OpensslLib.inf and OpensslLibCrypto.inf library instance can be used
>    #       since libssl is not required for iSCSI.
>    #
> -  DEFINE NETWORK_ISCSI_ENABLE = TRUE
> +  DEFINE NETWORK_ISCSI_ENABLE = FALSE
>  !endif
>  
>  !if $(NETWORK_ENABLE) == TRUE
> 

I know of people that use iSCSI with the ArmVirtQemu and OVMF platforms.

Please prepend two patches to this series (that is, the v3 series should
begin with these two patches below):

(1) locate "NETWORK_ALLOW_HTTP_CONNECTIONS" in the files:

- ArmVirtPkg/ArmVirtQemu.dsc
- ArmVirtPkg/ArmVirtQemuKernel.dsc

and explicitly enable NETWORK_ISCSI_ENABLE in the same place.

(2) Please do the same for the following files, in a separate patch:

- OvmfPkg/Bhyve/BhyveX64.dsc
- OvmfPkg/OvmfPkgIa32.dsc
- OvmfPkg/OvmfPkgIa32X64.dsc
- OvmfPkg/OvmfPkgX64.dsc
- OvmfPkg/OvmfXen.dsc

Thanks!
Laszlo



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


Re: [edk2-devel] [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default
Posted by Gao, Zhichao 5 years, 3 months ago
Sure. I would do it. I am thinking using Network.dsc.inc instead of others inc's combination. But there may be a question: the default Network.dsc.inc would only cover below build:
Components.IA32, Components.X64, Components.ARM, Components.AARCH64, Components.RISCV64
I am not sure if the above would match ArmVirt and Ovmf's requirements.

Thanks,
Zhichao

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, October 27, 2020 6:48 PM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>; Sami Mujawar <sami.mujawar@arm.com>; Leif
> Lindholm <leif@nuviainc.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian
> J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Steele, Kelly <kelly.steele@intel.com>; Sun, Zailiang <zailiang.sun@intel.com>;
> Qian, Yi <yi.qian@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Maciej
> Rabeda <maciej.rabeda@linux.intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: Re: [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default
> 
> Hi Zhichao,
> 
> thanks for the CC, I appreciate it. Please see my comments below.
> 
> On 10/27/20 03:42, Zhichao Gao wrote:
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003
> >
> > iSCSI is using the undeprecated function MD5. It is better to make the
> > default setting secure. If the platforms want to use the iSCSI, they
> > should enable it in the platforms'
> > dsc file and be aware they are using an unsafe function.
> >
> > 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>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  NetworkPkg/NetworkDefines.dsc.inc | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/NetworkPkg/NetworkDefines.dsc.inc
> > b/NetworkPkg/NetworkDefines.dsc.inc
> > index a442d1b157..18921d81f6 100644
> > --- a/NetworkPkg/NetworkDefines.dsc.inc
> > +++ b/NetworkPkg/NetworkDefines.dsc.inc
> > @@ -17,7 +17,7 @@
> >  #   DEFINE NETWORK_TLS_ENABLE             = TRUE
> >  #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
> >  #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
> > -#   DEFINE NETWORK_ISCSI_ENABLE           = TRUE
> > +#   DEFINE NETWORK_ISCSI_ENABLE           = FALSE
> >  #   DEFINE NETWORK_VLAN_ENABLE            = TRUE
> >  #
> >  # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> @@
> > -101,7 +101,7 @@
> >    #       Both OpensslLib.inf and OpensslLibCrypto.inf library instance can be used
> >    #       since libssl is not required for iSCSI.
> >    #
> > -  DEFINE NETWORK_ISCSI_ENABLE = TRUE
> > +  DEFINE NETWORK_ISCSI_ENABLE = FALSE
> >  !endif
> >
> >  !if $(NETWORK_ENABLE) == TRUE
> >
> 
> I know of people that use iSCSI with the ArmVirtQemu and OVMF platforms.
> 
> Please prepend two patches to this series (that is, the v3 series should begin with
> these two patches below):
> 
> (1) locate "NETWORK_ALLOW_HTTP_CONNECTIONS" in the files:
> 
> - ArmVirtPkg/ArmVirtQemu.dsc
> - ArmVirtPkg/ArmVirtQemuKernel.dsc
> 
> and explicitly enable NETWORK_ISCSI_ENABLE in the same place.
> 
> (2) Please do the same for the following files, in a separate patch:
> 
> - OvmfPkg/Bhyve/BhyveX64.dsc
> - OvmfPkg/OvmfPkgIa32.dsc
> - OvmfPkg/OvmfPkgIa32X64.dsc
> - OvmfPkg/OvmfPkgX64.dsc
> - OvmfPkg/OvmfXen.dsc
> 
> Thanks!
> Laszlo



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


Re: [edk2-devel] [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default
Posted by Laszlo Ersek 5 years, 3 months ago
On 10/29/20 03:34, Gao, Zhichao wrote:
> Sure. I would do it. I am thinking using Network.dsc.inc instead of others inc's combination. But there may be a question: the default Network.dsc.inc would only cover below build:
> Components.IA32, Components.X64, Components.ARM, Components.AARCH64, Components.RISCV64
> I am not sure if the above would match ArmVirt and Ovmf's requirements.

Indeed, modifying just "Network.dsc.inc" is insufficient.

"Network.dsc.inc" is convenient when it is applicable, but for some
platforms, it is not flexible enough. That's why we have the separate
DSC include files under NetworkPkg that do not contain the section
headers themselves (such as [LibraryClasses], [Components] etc).

This lets platforms decide *where* they include those snippets.

"Network.dsc.inc" is not used by either ArmVirtPkg or OvmfPkg platforms.
The platform DSC files in those package directories reference
"NetworkDefines.dsc.inc" and "NetworkComponents.dsc.inc" instead.

Thanks,
Laszlo


>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, October 27, 2020 6:48 PM
>> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
>> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@arm.com>; Sami Mujawar <sami.mujawar@arm.com>; Leif
>> Lindholm <leif@nuviainc.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian
>> J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Jiang, Guomin
>> <guomin.jiang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
>> Steele, Kelly <kelly.steele@intel.com>; Sun, Zailiang <zailiang.sun@intel.com>;
>> Qian, Yi <yi.qian@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Maciej
>> Rabeda <maciej.rabeda@linux.intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Fu,
>> Siyuan <siyuan.fu@intel.com>
>> Subject: Re: [PATCH V2 1/7] NetworkPkg/Defines: Make iSCSI disable as default
>>
>> Hi Zhichao,
>>
>> thanks for the CC, I appreciate it. Please see my comments below.
>>
>> On 10/27/20 03:42, Zhichao Gao wrote:
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3003
>>>
>>> iSCSI is using the undeprecated function MD5. It is better to make the
>>> default setting secure. If the platforms want to use the iSCSI, they
>>> should enable it in the platforms'
>>> dsc file and be aware they are using an unsafe function.
>>>
>>> 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>
>>> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>>> ---
>>>  NetworkPkg/NetworkDefines.dsc.inc | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/NetworkPkg/NetworkDefines.dsc.inc
>>> b/NetworkPkg/NetworkDefines.dsc.inc
>>> index a442d1b157..18921d81f6 100644
>>> --- a/NetworkPkg/NetworkDefines.dsc.inc
>>> +++ b/NetworkPkg/NetworkDefines.dsc.inc
>>> @@ -17,7 +17,7 @@
>>>  #   DEFINE NETWORK_TLS_ENABLE             = TRUE
>>>  #   DEFINE NETWORK_HTTP_BOOT_ENABLE       = TRUE
>>>  #   DEFINE NETWORK_ALLOW_HTTP_CONNECTIONS = FALSE
>>> -#   DEFINE NETWORK_ISCSI_ENABLE           = TRUE
>>> +#   DEFINE NETWORK_ISCSI_ENABLE           = FALSE
>>>  #   DEFINE NETWORK_VLAN_ENABLE            = TRUE
>>>  #
>>>  # Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> @@
>>> -101,7 +101,7 @@
>>>    #       Both OpensslLib.inf and OpensslLibCrypto.inf library instance can be used
>>>    #       since libssl is not required for iSCSI.
>>>    #
>>> -  DEFINE NETWORK_ISCSI_ENABLE = TRUE
>>> +  DEFINE NETWORK_ISCSI_ENABLE = FALSE
>>>  !endif
>>>
>>>  !if $(NETWORK_ENABLE) == TRUE
>>>
>>
>> I know of people that use iSCSI with the ArmVirtQemu and OVMF platforms.
>>
>> Please prepend two patches to this series (that is, the v3 series should begin with
>> these two patches below):
>>
>> (1) locate "NETWORK_ALLOW_HTTP_CONNECTIONS" in the files:
>>
>> - ArmVirtPkg/ArmVirtQemu.dsc
>> - ArmVirtPkg/ArmVirtQemuKernel.dsc
>>
>> and explicitly enable NETWORK_ISCSI_ENABLE in the same place.
>>
>> (2) Please do the same for the following files, in a separate patch:
>>
>> - OvmfPkg/Bhyve/BhyveX64.dsc
>> - OvmfPkg/OvmfPkgIa32.dsc
>> - OvmfPkg/OvmfPkgIa32X64.dsc
>> - OvmfPkg/OvmfPkgX64.dsc
>> - OvmfPkg/OvmfXen.dsc
>>
>> Thanks!
>> Laszlo
> 



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