Re: [PATCH v2 2/2] NetworkPkg: Read HttpTlsCipherList variable and configure it for HTTPS session.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Thanks the comment, I will integrate it when I commit the patch.



> -----Original Message-----
> From: Ye, Ting
> Sent: Monday, February 12, 2018 11:06 AM
> 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>; Fu,
> Siyuan <siyuan.fu@intel.com>
> Subject: RE: [PATCH v2 2/2] NetworkPkg: Read HttpTlsCipherList variable and
> configure it for HTTPS session.
> 
> Hi Jiaxin,
> 
> In following code, how about use "gEdkiiHttpTlsCipherListGuid" as the
> variable GUID as to make it consistent with the variable name?
> 
> +  Status  = gRT->GetVariable (
> +                   EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE,
> +                   &gHttpTlsCipherListGuid,
> +                   NULL,
> +                   &CipherListSize,
> +                   NULL
> +                   );
> 
> Others are good to me.
> Reviewed-by:  Ye Ting <ting.ye@intel.com>
> 
> Best Regards,
> Ting
> 
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Sunday, February 11, 2018 11:15 AM
> 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 v2 2/2] NetworkPkg: Read HttpTlsCipherList variable and
> configure it for HTTPS session.
> 
> v2:
> * Refine the error handling returned from GetVariable.
> 
> This patch is to read the HttpTlsCipherList 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..3b7a7a2 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/HttpTlsCipherList.h>
> 
>  #include <IndustryStandard/Tls1.h>
> 
>  //
>  // Driver Version
> diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf
> b/NetworkPkg/HttpDxe/HttpDxe.inf index 20075f5..56a2472 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"
> +  gHttpTlsCipherListGuid                           ## SOMETIMES_CONSUMES  ##
> Variable:L"HttpTlsCipherList"
> 
>  [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..fbe4087 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 HttpTlsCipherList variable and configure it for HTTPS session.
> +
> +  @param[in, out]  HttpInstance  The HTTP instance private data.
> +
> +  @retval EFI_SUCCESS            The prefered HTTP TLS CipherList is configured.
> +  @retval EFI_NOT_FOUND          Fail to get 'HttpTlsCipherList' 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 HttpTlsCipherList variable.
> +  //
> +  Status  = gRT->GetVariable (
> +                   EDKII_HTTP_TLS_CIPHER_LIST_VARIABLE,
> +                   &gHttpTlsCipherListGuid,
> +                   NULL,
> +                   &CipherListSize,
> +                   NULL
> +                   );
> +  ASSERT (EFI_ERROR (Status));
> +  if (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_HTTP_TLS_CIPHER_LIST_VARIABLE,
> +                  &gHttpTlsCipherListGuid,
> +                  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