* [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
@ 2018-02-09 3:59 Jiaxin Wu
2018-02-09 3:59 ` [Patch 1/2] NetworkPkg: Define one private variable for TLS CipherList configuration Jiaxin Wu
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Jiaxin Wu @ 2018-02-09 3:59 UTC (permalink / raw)
To: edk2-devel
Cc: Laszlo Ersek, Kinney Michael D, Zimmer Vincent, Yao Jiewen,
Ye Ting, Fu Siyuan, Wu Jiaxin
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kinney Michael D <michael.d.kinney@intel.com>
Cc: Zimmer Vincent <vincent.zimmer@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
Jiaxin Wu (2):
NetworkPkg: Define one private variable for TLS CipherList
configuration.
NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
session.
NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
NetworkPkg/HttpDxe/HttpsSupport.c | 92 ++++++++++++++++++++++++++++++++-
NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
NetworkPkg/NetworkPkg.dec | 3 ++
5 files changed, 136 insertions(+), 3 deletions(-)
create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
--
1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Patch 1/2] NetworkPkg: Define one private variable for TLS CipherList configuration.
2018-02-09 3:59 [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Jiaxin Wu
@ 2018-02-09 3:59 ` Jiaxin Wu
2018-02-09 3:59 ` [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session Jiaxin Wu
2018-02-09 5:22 ` [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Fu, Siyuan
2 siblings, 0 replies; 12+ messages in thread
From: Jiaxin Wu @ 2018-02-09 3:59 UTC (permalink / raw)
To: edk2-devel
Cc: Laszlo Ersek, Kinney Michael D, Zimmer Vincent, Yao Jiewen,
Ye Ting, Fu Siyuan, Wu Jiaxin
This variable can be set by any platform that want to control its own preferred
TlsCipherList for the later HTTPS session.
The valid contents of variable must follow the TLS CipherList format defined
in RFC 5246. The valid length of variable must be an integral multiple of 2.
For example, if below cipher suites are preferred:
CipherSuite TLS_RSA_WITH_AES_128_CBC_SHA256 = {0x00,0x3C}
CipherSuite TLS_RSA_WITH_AES_256_CBC_SHA256 = {0x00,0x3D}
Then, the contents of variable should be:
{0x00,0x3C,0x00,0x3D}
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kinney Michael D <michael.d.kinney@intel.com>
Cc: Zimmer Vincent <vincent.zimmer@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
NetworkPkg/Include/Guid/TlsCipherList.h | 38 +++++++++++++++++++++++++++++++++
NetworkPkg/NetworkPkg.dec | 3 +++
2 files changed, 41 insertions(+)
create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
diff --git a/NetworkPkg/Include/Guid/TlsCipherList.h b/NetworkPkg/Include/Guid/TlsCipherList.h
new file mode 100644
index 0000000..e31b7bf
--- /dev/null
+++ b/NetworkPkg/Include/Guid/TlsCipherList.h
@@ -0,0 +1,38 @@
+/** @file
+ This file defines the TlsCipherList variable for HTTPS to configure Tls Cipher List.
+
+Copyright (c) 2018, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials are licensed and made available under
+the terms and conditions of the BSD License that accompanies this distribution.
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __TLS_CIPHER_LIST_H__
+#define __TLS_CIPHER_LIST_H__
+
+//
+// Private Variable for HTTPS to configure Tls Cipher List.
+// The valid contents of variable must follow the TLS CipherList format defined in RFC 5246.
+// The valid length of variable must be an integral multiple of 2.
+// For example, if below cipher suites are preferred:
+// CipherSuite TLS_RSA_WITH_AES_128_CBC_SHA256 = {0x00,0x3C}
+// CipherSuite TLS_RSA_WITH_AES_256_CBC_SHA256 = {0x00,0x3D}
+// Then, the contents of variable should be:
+// {0x00,0x3C,0x00,0x3D}
+//
+#define EDKII_TLS_CIPHER_LIST_GUID \
+ { \
+ 0x46ddb415, 0x5244, 0x49c7, { 0x93, 0x74, 0xf0, 0xe2, 0x98, 0xe7, 0xd3, 0x86 } \
+ }
+
+#define EDKII_TLS_CIPHER_LIST_VARIABLE L"TlsCipherList"
+
+extern EFI_GUID gTlsCipherListGuid;
+
+#endif
+
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index 902df37..bdf8361 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -44,10 +44,13 @@
gTlsAuthConfigGuid = { 0xb0eae4f8, 0x9a04, 0x4c6d, { 0xa7, 0x48, 0x79, 0x3d, 0xaa, 0xf, 0x65, 0xdf }}
# Include/Guid/TlsAuthentication.h
gEfiTlsCaCertificateGuid = { 0xfd2340D0, 0x3dab, 0x4349, { 0xa6, 0xc7, 0x3b, 0x4f, 0x12, 0xb4, 0x8e, 0xae }}
+ # Include/Guid/TlsCipherList.h
+ gTlsCipherListGuid = { 0x46ddb415, 0x5244, 0x49c7, { 0x93, 0x74, 0xf0, 0xe2, 0x98, 0xe7, 0xd3, 0x86 }}
+
[PcdsFixedAtBuild]
## The max attempt number will be created by iSCSI driver.
# @Prompt Max attempt number.
gEfiNetworkPkgTokenSpaceGuid.PcdMaxIScsiAttemptNumber|0x08|UINT8|0x0000000D
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session.
2018-02-09 3:59 [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Jiaxin Wu
2018-02-09 3:59 ` [Patch 1/2] NetworkPkg: Define one private variable for TLS CipherList configuration Jiaxin Wu
@ 2018-02-09 3:59 ` Jiaxin Wu
2018-02-09 10:16 ` Laszlo Ersek
2018-02-09 5:22 ` [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Fu, Siyuan
2 siblings, 1 reply; 12+ messages in thread
From: Jiaxin Wu @ 2018-02-09 3:59 UTC (permalink / raw)
To: edk2-devel
Cc: Laszlo Ersek, Kinney Michael D, Zimmer Vincent, Yao Jiewen,
Ye Ting, Fu Siyuan, Wu Jiaxin
This patch is to read the TlsCipherList variable and configure it for the
later HTTPS session.
If the variable is not set by any platform, EFI_NOT_FOUND will be returned
from GetVariable service. In such a case, the default CipherList created in
TlsDxe driver will be used.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kinney Michael D <michael.d.kinney@intel.com>
Cc: Zimmer Vincent <vincent.zimmer@intel.com>
Cc: Yao Jiewen <jiewen.yao@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
NetworkPkg/HttpDxe/HttpsSupport.c | 92 ++++++++++++++++++++++++++++++++++++++-
3 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 93a412a..eba7d32 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -1,9 +1,9 @@
/** @file
The header files of the driver binding and service binding protocol for HttpDxe driver.
- Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -59,10 +59,11 @@
// Produced Protocols
//
#include <Protocol/Http.h>
#include <Guid/TlsAuthentication.h>
+#include <Guid/TlsCipherList.h>
#include <IndustryStandard/Tls1.h>
//
// Driver Version
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 20075f5..b1d7bd2 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,9 +1,9 @@
## @file
# Implementation of EFI HTTP protocol interfaces.
#
-# Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
# http://opensource.org/licenses/bsd-license.php.
@@ -72,10 +72,11 @@
gEfiTlsProtocolGuid ## SOMETIMES_CONSUMES
gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
[Guids]
gEfiTlsCaCertificateGuid ## SOMETIMES_CONSUMES ## Variable:L"TlsCaCertificate"
+ gTlsCipherListGuid ## SOMETIMES_CONSUMES ## Variable:L"TlsCipherList"
[Pcd]
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
gEfiNetworkPkgTokenSpaceGuid.PcdHttpsAuthenticationMode ## SOMETIMES_CONSUMES
gEfiNetworkPkgTokenSpaceGuid.PcdHttpsHostPublicCert ## SOMETIMES_CONSUMES
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index 288082a..62cb867 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -1,9 +1,9 @@
/** @file
Miscellaneous routines specific to Https for HttpDxe driver.
-Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
http://opensource.org/licenses/bsd-license.php
@@ -492,10 +492,91 @@ TlsConfigCertificate (
return Status;
}
/**
+ Read the TlsCipherList variable and configure it for HTTPS session.
+
+ @param[in, out] HttpInstance The HTTP instance private data.
+
+ @retval EFI_SUCCESS The prefered TLS CipherList is configured.
+ @retval EFI_NOT_FOUND Fail to get 'TlsCipherList' variable.
+ @retval EFI_INVALID_PARAMETER The contents of variable are invalid.
+ @retval EFI_OUT_OF_RESOURCES Can't allocate memory resources.
+
+ @retval Others Other error as indicated.
+
+**/
+EFI_STATUS
+TlsConfigCipherList (
+ IN OUT HTTP_PROTOCOL *HttpInstance
+ )
+{
+ EFI_STATUS Status;
+ UINT8 *CipherList;
+ UINTN CipherListSize;
+
+ CipherList = NULL;
+ CipherListSize = 0;
+
+ //
+ // Try to read the TlsCipherList variable.
+ //
+ Status = gRT->GetVariable (
+ EDKII_TLS_CIPHER_LIST_VARIABLE,
+ &gTlsCipherListGuid,
+ NULL,
+ &CipherListSize,
+ NULL
+ );
+
+ if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
+ return Status;
+ }
+
+ if (CipherListSize % sizeof (EFI_TLS_CIPHER) != 0) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Allocate buffer and read the config variable.
+ //
+ CipherList = AllocatePool (CipherListSize);
+ if (CipherList == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = gRT->GetVariable (
+ EDKII_TLS_CIPHER_LIST_VARIABLE,
+ &gTlsCipherListGuid,
+ NULL,
+ &CipherListSize,
+ CipherList
+ );
+ if (EFI_ERROR (Status)) {
+ //
+ // GetVariable still error or the variable is corrupted.
+ //
+ goto ON_EXIT;
+ }
+
+ ASSERT (CipherList != NULL);
+
+ Status = HttpInstance->Tls->SetSessionData (
+ HttpInstance->Tls,
+ EfiTlsCipherList,
+ CipherList,
+ CipherListSize
+ );
+
+ON_EXIT:
+ FreePool (CipherList);
+
+ return Status;
+}
+
+/**
Configure TLS session data.
@param[in, out] HttpInstance The HTTP instance private data.
@retval EFI_SUCCESS TLS session data is configured.
@@ -551,10 +632,19 @@ TlsConfigureSession (
if (EFI_ERROR (Status)) {
return Status;
}
//
+ // Tls Cipher List
+ //
+ Status = TlsConfigCipherList (HttpInstance);
+ if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
+ DEBUG ((EFI_D_ERROR, "TlsConfigCipherList: return %r error.\n", Status));
+ return Status;
+ }
+
+ //
// Tls Config Certificate
//
Status = TlsConfigCertificate (HttpInstance);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "TlsConfigCertificate: return %r error.\n", Status));
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-09 3:59 [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Jiaxin Wu
2018-02-09 3:59 ` [Patch 1/2] NetworkPkg: Define one private variable for TLS CipherList configuration Jiaxin Wu
2018-02-09 3:59 ` [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session Jiaxin Wu
@ 2018-02-09 5:22 ` Fu, Siyuan
2018-02-09 5:25 ` Wu, Jiaxin
` (2 more replies)
2 siblings, 3 replies; 12+ messages in thread
From: Fu, Siyuan @ 2018-02-09 5:22 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org
Cc: Laszlo Ersek, Kinney, Michael D, Zimmer, Vincent, Yao, Jiewen,
Ye, Ting
Hi, Jiaxin
I think we can remove the "TlsCipherList.h" to another name like "HttpTlsCipherListVariable.h" to highlight that the variable is only used for HTTP configuration. And also the variable name and GUID name.
Siyuan
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, February 9, 2018 12:00 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> CipherList.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Kinney Michael D <michael.d.kinney@intel.com>
> Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>
> Jiaxin Wu (2):
> NetworkPkg: Define one private variable for TLS CipherList
> configuration.
> NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
> session.
>
> NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> NetworkPkg/HttpDxe/HttpsSupport.c | 92
> ++++++++++++++++++++++++++++++++-
> NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
> NetworkPkg/NetworkPkg.dec | 3 ++
> 5 files changed, 136 insertions(+), 3 deletions(-)
> create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
>
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-09 5:22 ` [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Fu, Siyuan
@ 2018-02-09 5:25 ` Wu, Jiaxin
2018-02-09 7:08 ` Li, Ruth
2018-02-09 10:11 ` Laszlo Ersek
2 siblings, 0 replies; 12+ messages in thread
From: Wu, Jiaxin @ 2018-02-09 5:25 UTC (permalink / raw)
To: Fu, Siyuan, edk2-devel@lists.01.org
Cc: Laszlo Ersek, Kinney, Michael D, Zimmer, Vincent, Yao, Jiewen,
Ye, Ting
Thanks the comment, I will refine the series patch.
> -----Original Message-----
> From: Fu, Siyuan
> Sent: Friday, February 9, 2018 1:23 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ye,
> Ting <ting.ye@intel.com>
> Subject: RE: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> CipherList.
>
> Hi, Jiaxin
>
> I think we can remove the "TlsCipherList.h" to another name like
> "HttpTlsCipherListVariable.h" to highlight that the variable is only used for
> HTTP configuration. And also the variable name and GUID name.
>
> Siyuan
>
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Friday, February 9, 2018 12:00 PM
> > To: edk2-devel@lists.01.org
> > Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> > Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> > CipherList.
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Kinney Michael D <michael.d.kinney@intel.com>
> > Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> > Cc: Yao Jiewen <jiewen.yao@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >
> > Jiaxin Wu (2):
> > NetworkPkg: Define one private variable for TLS CipherList
> > configuration.
> > NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
> > session.
> >
> > NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> > NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> > NetworkPkg/HttpDxe/HttpsSupport.c | 92
> > ++++++++++++++++++++++++++++++++-
> > NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
> > NetworkPkg/NetworkPkg.dec | 3 ++
> > 5 files changed, 136 insertions(+), 3 deletions(-)
> > create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
> >
> > --
> > 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-09 5:22 ` [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Fu, Siyuan
2018-02-09 5:25 ` Wu, Jiaxin
@ 2018-02-09 7:08 ` Li, Ruth
2018-02-09 7:10 ` Wu, Jiaxin
2018-02-09 10:11 ` Laszlo Ersek
2 siblings, 1 reply; 12+ messages in thread
From: Li, Ruth @ 2018-02-09 7:08 UTC (permalink / raw)
To: Fu, Siyuan, Wu, Jiaxin, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Zimmer, Vincent, Ye, Ting, Yao, Jiewen
Jiaxin
With this capability introduced, could you update Wiki page to notify platform to configure that if needed? https://github.com/tianocore/tianocore.github.io/wiki/HTTPS-Boot
Thanks,
Ruth
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu, Siyuan
Sent: Friday, February 9, 2018 1:23 PM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>; Ye, Ting <ting.ye@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: Re: [edk2] [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
Hi, Jiaxin
I think we can remove the "TlsCipherList.h" to another name like "HttpTlsCipherListVariable.h" to highlight that the variable is only used for HTTP configuration. And also the variable name and GUID name.
Siyuan
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Friday, February 9, 2018 12:00 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> CipherList.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Kinney Michael D <michael.d.kinney@intel.com>
> Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>
> Jiaxin Wu (2):
> NetworkPkg: Define one private variable for TLS CipherList
> configuration.
> NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
> session.
>
> NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> NetworkPkg/HttpDxe/HttpsSupport.c | 92
> ++++++++++++++++++++++++++++++++-
> NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
> NetworkPkg/NetworkPkg.dec | 3 ++
> 5 files changed, 136 insertions(+), 3 deletions(-)
> create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
>
> --
> 1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-09 7:08 ` Li, Ruth
@ 2018-02-09 7:10 ` Wu, Jiaxin
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Jiaxin @ 2018-02-09 7:10 UTC (permalink / raw)
To: Li, Ruth, Fu, Siyuan, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Zimmer, Vincent, Ye, Ting, Yao, Jiewen
Sure, I will update the wiki once the patch is committed.
Thanks
Jiaxin
> -----Original Message-----
> From: Li, Ruth
> Sent: Friday, February 9, 2018 3:08 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Ye, Ting <ting.ye@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: RE: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> CipherList.
>
> Jiaxin
>
> With this capability introduced, could you update Wiki page to notify platform
> to configure that if needed?
> https://github.com/tianocore/tianocore.github.io/wiki/HTTPS-Boot
>
> Thanks,
> Ruth
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu,
> Siyuan
> Sent: Friday, February 9, 2018 1:23 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Ye, Ting <ting.ye@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] [Patch 0/2] NetworkPkg: Support the platform to
> configure TLS CipherList.
>
> Hi, Jiaxin
>
> I think we can remove the "TlsCipherList.h" to another name like
> "HttpTlsCipherListVariable.h" to highlight that the variable is only used for
> HTTP configuration. And also the variable name and GUID name.
>
> Siyuan
>
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Friday, February 9, 2018 12:00 PM
> > To: edk2-devel@lists.01.org
> > Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> > Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> > CipherList.
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Kinney Michael D <michael.d.kinney@intel.com>
> > Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> > Cc: Yao Jiewen <jiewen.yao@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >
> > Jiaxin Wu (2):
> > NetworkPkg: Define one private variable for TLS CipherList
> > configuration.
> > NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
> > session.
> >
> > NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> > NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> > NetworkPkg/HttpDxe/HttpsSupport.c | 92
> > ++++++++++++++++++++++++++++++++-
> > NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
> > NetworkPkg/NetworkPkg.dec | 3 ++
> > 5 files changed, 136 insertions(+), 3 deletions(-)
> > create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
> >
> > --
> > 1.9.5.msysgit.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-09 5:22 ` [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Fu, Siyuan
2018-02-09 5:25 ` Wu, Jiaxin
2018-02-09 7:08 ` Li, Ruth
@ 2018-02-09 10:11 ` Laszlo Ersek
2018-02-11 2:33 ` Wu, Jiaxin
2 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2018-02-09 10:11 UTC (permalink / raw)
To: Fu, Siyuan, Wu, Jiaxin, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Zimmer, Vincent, Yao, Jiewen, Ye, Ting
On 02/09/18 06:22, Fu, Siyuan wrote:
> Hi, Jiaxin
>
> I think we can remove the "TlsCipherList.h" to another name like
> "HttpTlsCipherListVariable.h" to highlight that the variable is only
> used for HTTP configuration. And also the variable name and GUID
> name.
If we are renaming gEfiTlsCaCertificateGuid, can we pick a generic term
as new name, something like "gHttpTlsVariableGuid"? And then put both
variables, the CA List and the Cipher List, in that (same) namespace GUID?
It's not that we'll run out of GUIDs any time soon :) , but I think
these variables belong closely together.
Thanks,
Laszlo
>> -----Original Message-----
>> From: Wu, Jiaxin
>> Sent: Friday, February 9, 2018 12:00 PM
>> To: edk2-devel@lists.01.org
>> Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>;
>> Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
>> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
>> Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
>> CipherList.
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Kinney Michael D <michael.d.kinney@intel.com>
>> Cc: Zimmer Vincent <vincent.zimmer@intel.com>
>> Cc: Yao Jiewen <jiewen.yao@intel.com>
>> Cc: Ye Ting <ting.ye@intel.com>
>> Cc: Fu Siyuan <siyuan.fu@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>
>> Jiaxin Wu (2):
>> NetworkPkg: Define one private variable for TLS CipherList
>> configuration.
>> NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
>> session.
>>
>> NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
>> NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
>> NetworkPkg/HttpDxe/HttpsSupport.c | 92
>> ++++++++++++++++++++++++++++++++-
>> NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
>> NetworkPkg/NetworkPkg.dec | 3 ++
>> 5 files changed, 136 insertions(+), 3 deletions(-)
>> create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
>>
>> --
>> 1.9.5.msysgit.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session.
2018-02-09 3:59 ` [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session Jiaxin Wu
@ 2018-02-09 10:16 ` Laszlo Ersek
2018-02-11 2:45 ` Wu, Jiaxin
0 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2018-02-09 10:16 UTC (permalink / raw)
To: Jiaxin Wu, edk2-devel
Cc: Kinney Michael D, Zimmer Vincent, Yao Jiewen, Ye Ting, Fu Siyuan
On 02/09/18 04:59, Jiaxin Wu wrote:
> This patch is to read the TlsCipherList variable and configure it for the
> later HTTPS session.
>
> If the variable is not set by any platform, EFI_NOT_FOUND will be returned
> from GetVariable service. In such a case, the default CipherList created in
> TlsDxe driver will be used.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Kinney Michael D <michael.d.kinney@intel.com>
> Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> Cc: Yao Jiewen <jiewen.yao@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
> NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> NetworkPkg/HttpDxe/HttpsSupport.c | 92 ++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 95 insertions(+), 3 deletions(-)
>
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
> index 93a412a..eba7d32 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.h
> +++ b/NetworkPkg/HttpDxe/HttpDriver.h
> @@ -1,9 +1,9 @@
> /** @file
> The header files of the driver binding and service binding protocol for HttpDxe driver.
>
> - Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -59,10 +59,11 @@
> // Produced Protocols
> //
> #include <Protocol/Http.h>
>
> #include <Guid/TlsAuthentication.h>
> +#include <Guid/TlsCipherList.h>
>
> #include <IndustryStandard/Tls1.h>
>
> //
> // Driver Version
> diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
> index 20075f5..b1d7bd2 100644
> --- a/NetworkPkg/HttpDxe/HttpDxe.inf
> +++ b/NetworkPkg/HttpDxe/HttpDxe.inf
> @@ -1,9 +1,9 @@
> ## @file
> # Implementation of EFI HTTP protocol interfaces.
> #
> -# Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> # which accompanies this distribution. The full text of the license may be found at
> # http://opensource.org/licenses/bsd-license.php.
> @@ -72,10 +72,11 @@
> gEfiTlsProtocolGuid ## SOMETIMES_CONSUMES
> gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
>
> [Guids]
> gEfiTlsCaCertificateGuid ## SOMETIMES_CONSUMES ## Variable:L"TlsCaCertificate"
> + gTlsCipherListGuid ## SOMETIMES_CONSUMES ## Variable:L"TlsCipherList"
>
> [Pcd]
> gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
> gEfiNetworkPkgTokenSpaceGuid.PcdHttpsAuthenticationMode ## SOMETIMES_CONSUMES
> gEfiNetworkPkgTokenSpaceGuid.PcdHttpsHostPublicCert ## SOMETIMES_CONSUMES
> diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
> index 288082a..62cb867 100644
> --- a/NetworkPkg/HttpDxe/HttpsSupport.c
> +++ b/NetworkPkg/HttpDxe/HttpsSupport.c
> @@ -1,9 +1,9 @@
> /** @file
> Miscellaneous routines specific to Https for HttpDxe driver.
>
> -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> http://opensource.org/licenses/bsd-license.php
> @@ -492,10 +492,91 @@ TlsConfigCertificate (
>
> return Status;
> }
>
> /**
> + Read the TlsCipherList variable and configure it for HTTPS session.
> +
> + @param[in, out] HttpInstance The HTTP instance private data.
> +
> + @retval EFI_SUCCESS The prefered TLS CipherList is configured.
> + @retval EFI_NOT_FOUND Fail to get 'TlsCipherList' variable.
> + @retval EFI_INVALID_PARAMETER The contents of variable are invalid.
> + @retval EFI_OUT_OF_RESOURCES Can't allocate memory resources.
> +
> + @retval Others Other error as indicated.
> +
> +**/
> +EFI_STATUS
> +TlsConfigCipherList (
> + IN OUT HTTP_PROTOCOL *HttpInstance
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 *CipherList;
> + UINTN CipherListSize;
> +
> + CipherList = NULL;
> + CipherListSize = 0;
> +
> + //
> + // Try to read the TlsCipherList variable.
> + //
> + Status = gRT->GetVariable (
> + EDKII_TLS_CIPHER_LIST_VARIABLE,
> + &gTlsCipherListGuid,
> + NULL,
> + &CipherListSize,
> + NULL
> + );
> +
> + if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
> + return Status;
> + }
I think the above GetVariable service call can never succeed. What about:
ASSERT (EFI_ERROR (Status));
if (Status != EFI_BUFFER_TOO_SMALL) {
return Status;
}
Not very important (the behavior won't change), but I think this is
better for documentation purposes.
The patch looks good to me otherwise.
Thank you!
Laszlo
> +
> + if (CipherListSize % sizeof (EFI_TLS_CIPHER) != 0) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + //
> + // Allocate buffer and read the config variable.
> + //
> + CipherList = AllocatePool (CipherListSize);
> + if (CipherList == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + Status = gRT->GetVariable (
> + EDKII_TLS_CIPHER_LIST_VARIABLE,
> + &gTlsCipherListGuid,
> + NULL,
> + &CipherListSize,
> + CipherList
> + );
> + if (EFI_ERROR (Status)) {
> + //
> + // GetVariable still error or the variable is corrupted.
> + //
> + goto ON_EXIT;
> + }
> +
> + ASSERT (CipherList != NULL);
> +
> + Status = HttpInstance->Tls->SetSessionData (
> + HttpInstance->Tls,
> + EfiTlsCipherList,
> + CipherList,
> + CipherListSize
> + );
> +
> +ON_EXIT:
> + FreePool (CipherList);
> +
> + return Status;
> +}
> +
> +/**
> Configure TLS session data.
>
> @param[in, out] HttpInstance The HTTP instance private data.
>
> @retval EFI_SUCCESS TLS session data is configured.
> @@ -551,10 +632,19 @@ TlsConfigureSession (
> if (EFI_ERROR (Status)) {
> return Status;
> }
>
> //
> + // Tls Cipher List
> + //
> + Status = TlsConfigCipherList (HttpInstance);
> + if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
> + DEBUG ((EFI_D_ERROR, "TlsConfigCipherList: return %r error.\n", Status));
> + return Status;
> + }
> +
> + //
> // Tls Config Certificate
> //
> Status = TlsConfigCertificate (HttpInstance);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "TlsConfigCertificate: return %r error.\n", Status));
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-09 10:11 ` Laszlo Ersek
@ 2018-02-11 2:33 ` Wu, Jiaxin
2018-02-12 18:53 ` Laszlo Ersek
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Jiaxin @ 2018-02-11 2:33 UTC (permalink / raw)
To: Laszlo Ersek, Fu, Siyuan, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Zimmer, Vincent, Yao, Jiewen, Ye, Ting
Hi Laszlo,
Besides the compatibility consideration, we'd better *not* put CipherList and CaCertificate into one variable. In the future, we prefer to manage the CaCertificate with other cert configuration items together (e.g. HostPublicCert, HostPrivateCert, etc ) rather than the parameters like CipherList. You know we can't save the host cert pairs as variable due to the security consideration.
So, case by case, let's keep current solution to define the variable named as "HttpTlsCipherList".
Thanks,
Jiaxin
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, February 9, 2018 6:12 PM
> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
> edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ye,
> Ting <ting.ye@intel.com>
> Subject: Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> CipherList.
>
> On 02/09/18 06:22, Fu, Siyuan wrote:
> > Hi, Jiaxin
> >
> > I think we can remove the "TlsCipherList.h" to another name like
> > "HttpTlsCipherListVariable.h" to highlight that the variable is only
> > used for HTTP configuration. And also the variable name and GUID
> > name.
> If we are renaming gEfiTlsCaCertificateGuid, can we pick a generic term
> as new name, something like "gHttpTlsVariableGuid"? And then put both
> variables, the CA List and the Cipher List, in that (same) namespace GUID?
>
> It's not that we'll run out of GUIDs any time soon :) , but I think
> these variables belong closely together.
>
> Thanks,
> Laszlo
>
> >> -----Original Message-----
> >> From: Wu, Jiaxin
> >> Sent: Friday, February 9, 2018 12:00 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>;
> >> Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> >> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> >> Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
> >> CipherList.
> >>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Cc: Kinney Michael D <michael.d.kinney@intel.com>
> >> Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> >> Cc: Yao Jiewen <jiewen.yao@intel.com>
> >> Cc: Ye Ting <ting.ye@intel.com>
> >> Cc: Fu Siyuan <siyuan.fu@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> >>
> >> Jiaxin Wu (2):
> >> NetworkPkg: Define one private variable for TLS CipherList
> >> configuration.
> >> NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
> >> session.
> >>
> >> NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> >> NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> >> NetworkPkg/HttpDxe/HttpsSupport.c | 92
> >> ++++++++++++++++++++++++++++++++-
> >> NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
> >> NetworkPkg/NetworkPkg.dec | 3 ++
> >> 5 files changed, 136 insertions(+), 3 deletions(-)
> >> create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
> >>
> >> --
> >> 1.9.5.msysgit.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session.
2018-02-09 10:16 ` Laszlo Ersek
@ 2018-02-11 2:45 ` Wu, Jiaxin
0 siblings, 0 replies; 12+ messages in thread
From: Wu, Jiaxin @ 2018-02-11 2:45 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Zimmer, Vincent, Yao, Jiewen, Ye, Ting,
Fu, Siyuan
Thanks Laszlo, I will integrate your comments into the new patch.
Best Regards!
Jiaxin
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, February 9, 2018 6:16 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ye,
> Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>
> Subject: Re: [Patch 2/2] NetworkPkg: Read TlsCipherList variable and
> configure it for HTTPS session.
>
> On 02/09/18 04:59, Jiaxin Wu wrote:
> > This patch is to read the TlsCipherList variable and configure it for the
> > later HTTPS session.
> >
> > If the variable is not set by any platform, EFI_NOT_FOUND will be returned
> > from GetVariable service. In such a case, the default CipherList created in
> > TlsDxe driver will be used.
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Kinney Michael D <michael.d.kinney@intel.com>
> > Cc: Zimmer Vincent <vincent.zimmer@intel.com>
> > Cc: Yao Jiewen <jiewen.yao@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> > NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
> > NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
> > NetworkPkg/HttpDxe/HttpsSupport.c | 92
> ++++++++++++++++++++++++++++++++++++++-
> > 3 files changed, 95 insertions(+), 3 deletions(-)
> >
> > diff --git a/NetworkPkg/HttpDxe/HttpDriver.h
> b/NetworkPkg/HttpDxe/HttpDriver.h
> > index 93a412a..eba7d32 100644
> > --- a/NetworkPkg/HttpDxe/HttpDriver.h
> > +++ b/NetworkPkg/HttpDxe/HttpDriver.h
> > @@ -1,9 +1,9 @@
> > /** @file
> > The header files of the driver binding and service binding protocol for
> HttpDxe driver.
> >
> > - Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> > + Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> > (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> >
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the
> BSD License
> > which accompanies this distribution. The full text of the license may be
> found at
> > @@ -59,10 +59,11 @@
> > // Produced Protocols
> > //
> > #include <Protocol/Http.h>
> >
> > #include <Guid/TlsAuthentication.h>
> > +#include <Guid/TlsCipherList.h>
> >
> > #include <IndustryStandard/Tls1.h>
> >
> > //
> > // Driver Version
> > diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf
> b/NetworkPkg/HttpDxe/HttpDxe.inf
> > index 20075f5..b1d7bd2 100644
> > --- a/NetworkPkg/HttpDxe/HttpDxe.inf
> > +++ b/NetworkPkg/HttpDxe/HttpDxe.inf
> > @@ -1,9 +1,9 @@
> > ## @file
> > # Implementation of EFI HTTP protocol interfaces.
> > #
> > -# Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.<BR>
> > +# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> > #
> > # This program and the accompanying materials
> > # are licensed and made available under the terms and conditions of the
> BSD License
> > # which accompanies this distribution. The full text of the license may be
> found at
> > # http://opensource.org/licenses/bsd-license.php.
> > @@ -72,10 +72,11 @@
> > gEfiTlsProtocolGuid ## SOMETIMES_CONSUMES
> > gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
> >
> > [Guids]
> > gEfiTlsCaCertificateGuid ## SOMETIMES_CONSUMES ##
> Variable:L"TlsCaCertificate"
> > + gTlsCipherListGuid ## SOMETIMES_CONSUMES ##
> Variable:L"TlsCipherList"
> >
> > [Pcd]
> > gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ##
> CONSUMES
> > gEfiNetworkPkgTokenSpaceGuid.PcdHttpsAuthenticationMode ##
> SOMETIMES_CONSUMES
> > gEfiNetworkPkgTokenSpaceGuid.PcdHttpsHostPublicCert ##
> SOMETIMES_CONSUMES
> > diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c
> b/NetworkPkg/HttpDxe/HttpsSupport.c
> > index 288082a..62cb867 100644
> > --- a/NetworkPkg/HttpDxe/HttpsSupport.c
> > +++ b/NetworkPkg/HttpDxe/HttpsSupport.c
> > @@ -1,9 +1,9 @@
> > /** @file
> > Miscellaneous routines specific to Https for HttpDxe driver.
> >
> > -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> > (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> > This program and the accompanying materials
> > are licensed and made available under the terms and conditions of the BSD
> License
> > which accompanies this distribution. The full text of the license may be
> found at
> > http://opensource.org/licenses/bsd-license.php
> > @@ -492,10 +492,91 @@ TlsConfigCertificate (
> >
> > return Status;
> > }
> >
> > /**
> > + Read the TlsCipherList variable and configure it for HTTPS session.
> > +
> > + @param[in, out] HttpInstance The HTTP instance private data.
> > +
> > + @retval EFI_SUCCESS The prefered TLS CipherList is configured.
> > + @retval EFI_NOT_FOUND Fail to get 'TlsCipherList' variable.
> > + @retval EFI_INVALID_PARAMETER The contents of variable are invalid.
> > + @retval EFI_OUT_OF_RESOURCES Can't allocate memory resources.
> > +
> > + @retval Others Other error as indicated.
> > +
> > +**/
> > +EFI_STATUS
> > +TlsConfigCipherList (
> > + IN OUT HTTP_PROTOCOL *HttpInstance
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT8 *CipherList;
> > + UINTN CipherListSize;
> > +
> > + CipherList = NULL;
> > + CipherListSize = 0;
> > +
> > + //
> > + // Try to read the TlsCipherList variable.
> > + //
> > + Status = gRT->GetVariable (
> > + EDKII_TLS_CIPHER_LIST_VARIABLE,
> > + &gTlsCipherListGuid,
> > + NULL,
> > + &CipherListSize,
> > + NULL
> > + );
> > +
> > + if (EFI_ERROR (Status) && Status != EFI_BUFFER_TOO_SMALL) {
> > + return Status;
> > + }
>
> I think the above GetVariable service call can never succeed. What about:
>
> ASSERT (EFI_ERROR (Status));
> if (Status != EFI_BUFFER_TOO_SMALL) {
> return Status;
> }
>
> Not very important (the behavior won't change), but I think this is
> better for documentation purposes.
>
> The patch looks good to me otherwise.
>
> Thank you!
> Laszlo
>
>
> > +
> > + if (CipherListSize % sizeof (EFI_TLS_CIPHER) != 0) {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + //
> > + // Allocate buffer and read the config variable.
> > + //
> > + CipherList = AllocatePool (CipherListSize);
> > + if (CipherList == NULL) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + Status = gRT->GetVariable (
> > + EDKII_TLS_CIPHER_LIST_VARIABLE,
> > + &gTlsCipherListGuid,
> > + NULL,
> > + &CipherListSize,
> > + CipherList
> > + );
> > + if (EFI_ERROR (Status)) {
> > + //
> > + // GetVariable still error or the variable is corrupted.
> > + //
> > + goto ON_EXIT;
> > + }
> > +
> > + ASSERT (CipherList != NULL);
> > +
> > + Status = HttpInstance->Tls->SetSessionData (
> > + HttpInstance->Tls,
> > + EfiTlsCipherList,
> > + CipherList,
> > + CipherListSize
> > + );
> > +
> > +ON_EXIT:
> > + FreePool (CipherList);
> > +
> > + return Status;
> > +}
> > +
> > +/**
> > Configure TLS session data.
> >
> > @param[in, out] HttpInstance The HTTP instance private data.
> >
> > @retval EFI_SUCCESS TLS session data is configured.
> > @@ -551,10 +632,19 @@ TlsConfigureSession (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> >
> > //
> > + // Tls Cipher List
> > + //
> > + Status = TlsConfigCipherList (HttpInstance);
> > + if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
> > + DEBUG ((EFI_D_ERROR, "TlsConfigCipherList: return %r error.\n",
> Status));
> > + return Status;
> > + }
> > +
> > + //
> > // Tls Config Certificate
> > //
> > Status = TlsConfigCertificate (HttpInstance);
> > if (EFI_ERROR (Status)) {
> > DEBUG ((EFI_D_ERROR, "TlsConfigCertificate: return %r error.\n",
> Status));
> >
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList.
2018-02-11 2:33 ` Wu, Jiaxin
@ 2018-02-12 18:53 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2018-02-12 18:53 UTC (permalink / raw)
To: Wu, Jiaxin, Fu, Siyuan, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Zimmer, Vincent, Yao, Jiewen, Ye, Ting
On 02/11/18 03:33, Wu, Jiaxin wrote:
> Hi Laszlo,
>
> Besides the compatibility consideration, we'd better *not* put
> CipherList and CaCertificate into one variable.
I didn't suggest to put them in the same variable -- I meant to put them
in separate variables, just the two variables should belong to the same
namespace GUID.
> In the future, we prefer to manage the CaCertificate with other cert
> configuration items together (e.g. HostPublicCert, HostPrivateCert,
> etc ) rather than the parameters like CipherList. You know we can't
> save the host cert pairs as variable due to the security
> consideration.
>
> So, case by case, let's keep current solution to define the variable
> named as "HttpTlsCipherList".
Sure, that works for me.
Thanks,
Laszlo
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, February 9, 2018 6:12 PM
>> To: Fu, Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>;
>> edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent
>> <vincent.zimmer@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ye,
>> Ting <ting.ye@intel.com>
>> Subject: Re: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
>> CipherList.
>>
>> On 02/09/18 06:22, Fu, Siyuan wrote:
>>> Hi, Jiaxin
>>>
>>> I think we can remove the "TlsCipherList.h" to another name like
>>> "HttpTlsCipherListVariable.h" to highlight that the variable is only
>>> used for HTTP configuration. And also the variable name and GUID
>>> name.
>> If we are renaming gEfiTlsCaCertificateGuid, can we pick a generic term
>> as new name, something like "gHttpTlsVariableGuid"? And then put both
>> variables, the CA List and the Cipher List, in that (same) namespace GUID?
>>
>> It's not that we'll run out of GUIDs any time soon :) , but I think
>> these variables belong closely together.
>>
>> Thanks,
>> Laszlo
>>
>>>> -----Original Message-----
>>>> From: Wu, Jiaxin
>>>> Sent: Friday, February 9, 2018 12:00 PM
>>>> To: edk2-devel@lists.01.org
>>>> Cc: Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Zimmer, Vincent
>> <vincent.zimmer@intel.com>;
>>>> Yao, Jiewen <jiewen.yao@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
>>>> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
>>>> Subject: [Patch 0/2] NetworkPkg: Support the platform to configure TLS
>>>> CipherList.
>>>>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Cc: Kinney Michael D <michael.d.kinney@intel.com>
>>>> Cc: Zimmer Vincent <vincent.zimmer@intel.com>
>>>> Cc: Yao Jiewen <jiewen.yao@intel.com>
>>>> Cc: Ye Ting <ting.ye@intel.com>
>>>> Cc: Fu Siyuan <siyuan.fu@intel.com>
>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>>>>
>>>> Jiaxin Wu (2):
>>>> NetworkPkg: Define one private variable for TLS CipherList
>>>> configuration.
>>>> NetworkPkg: Read TlsCipherList variable and configure it for HTTPS
>>>> session.
>>>>
>>>> NetworkPkg/HttpDxe/HttpDriver.h | 3 +-
>>>> NetworkPkg/HttpDxe/HttpDxe.inf | 3 +-
>>>> NetworkPkg/HttpDxe/HttpsSupport.c | 92
>>>> ++++++++++++++++++++++++++++++++-
>>>> NetworkPkg/Include/Guid/TlsCipherList.h | 38 ++++++++++++++
>>>> NetworkPkg/NetworkPkg.dec | 3 ++
>>>> 5 files changed, 136 insertions(+), 3 deletions(-)
>>>> create mode 100644 NetworkPkg/Include/Guid/TlsCipherList.h
>>>>
>>>> --
>>>> 1.9.5.msysgit.1
>>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-02-12 18:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-09 3:59 [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Jiaxin Wu
2018-02-09 3:59 ` [Patch 1/2] NetworkPkg: Define one private variable for TLS CipherList configuration Jiaxin Wu
2018-02-09 3:59 ` [Patch 2/2] NetworkPkg: Read TlsCipherList variable and configure it for HTTPS session Jiaxin Wu
2018-02-09 10:16 ` Laszlo Ersek
2018-02-11 2:45 ` Wu, Jiaxin
2018-02-09 5:22 ` [Patch 0/2] NetworkPkg: Support the platform to configure TLS CipherList Fu, Siyuan
2018-02-09 5:25 ` Wu, Jiaxin
2018-02-09 7:08 ` Li, Ruth
2018-02-09 7:10 ` Wu, Jiaxin
2018-02-09 10:11 ` Laszlo Ersek
2018-02-11 2:33 ` Wu, Jiaxin
2018-02-12 18:53 ` Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox