Re: [edk2-devel] [PATCH V2 1/6] NetworkPkg/HttpDxe: Refactor TlsCreateChild

["Chang, Abner via groups.io" <abner.chang=amd.com@groups.io>]

[AMD Official Use Only - General]

Thanks Saloni, I will create a PR and merge it.

Abner

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni
> Kasbekar via groups.io
> Sent: Wednesday, January 10, 2024 7:26 AM
> To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Michael
> Brown <mcb30@ipxe.org>; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [edk2-devel] [PATCH V2 1/6] NetworkPkg/HttpDxe: Refactor
> TlsCreateChild
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang,
> Abner via groups.io
> Sent: Sunday, January 7, 2024 5:27 AM
> To: devel@edk2.groups.io
> Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary
> <zachary.clark-williams@intel.com>; Michael Brown <mcb30@ipxe.org>;
> Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: [edk2-devel] [PATCH V2 1/6] NetworkPkg/HttpDxe: Refactor
> TlsCreateChild
>
> From: Abner Chang <abner.chang@amd.com>
>
> - Use HTTP instance as the parameter for TlsCreateChild function.
> - Install TLS protocol on the HTTP instance thats create TLS child.
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>
> Cc: Zachary Clark-williams <zachary.clark-williams@intel.com>
> Cc: Michael Brown <mcb30@ipxe.org>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Reviewed-by: Michael Brown <mcb30@ipxe.org>
> ---
>  NetworkPkg/HttpDxe/HttpProto.h    |  3 +-
>  NetworkPkg/HttpDxe/HttpsSupport.h | 18 ++++---
>  NetworkPkg/HttpDxe/HttpImpl.c     | 23 ++-------
>  NetworkPkg/HttpDxe/HttpProto.c    |  7 +--
>  NetworkPkg/HttpDxe/HttpsSupport.c | 78 ++++++++++++++++++------------
> -
>  5 files changed, 64 insertions(+), 65 deletions(-)
>
> diff --git a/NetworkPkg/HttpDxe/HttpProto.h
> b/NetworkPkg/HttpDxe/HttpProto.h index 012f1f4b467..7e77b389a78
> 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.h
> +++ b/NetworkPkg/HttpDxe/HttpProto.h
> @@ -3,6 +3,7 @@
>
>  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
> @@ -171,7 +172,7 @@ typedef struct _HTTP_PROTOCOL {
>    BOOLEAN                           UseHttps;
>
>    EFI_SERVICE_BINDING_PROTOCOL      *TlsSb;
> -  EFI_HANDLE                        TlsChildHandle; /// Tls ChildHandle
> +  BOOLEAN                           TlsAlreadyCreated;
>    TLS_CONFIG_DATA                   TlsConfigData;
>    EFI_TLS_PROTOCOL                  *Tls;
>    EFI_TLS_CONFIGURATION_PROTOCOL    *TlsConfiguration;
> diff --git a/NetworkPkg/HttpDxe/HttpsSupport.h
> b/NetworkPkg/HttpDxe/HttpsSupport.h
> index 3c70825e8c3..5b44c7ac395 100644
> --- a/NetworkPkg/HttpDxe/HttpsSupport.h
> +++ b/NetworkPkg/HttpDxe/HttpsSupport.h
> @@ -2,6 +2,7 @@
>    The header files of miscellaneous routines specific to Https for HttpDxe
> driver.
>
>  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
> @@ -30,21 +31,18 @@ IsHttpsUrl (
>  /**
>    Creates a Tls child handle, open EFI_TLS_PROTOCOL and
> EFI_TLS_CONFIGURATION_PROTOCOL.
>
> -  @param[in]  ImageHandle           The firmware allocated handle for the UEFI
> image.
> -  @param[out] TlsSb                 Pointer to the TLS
> SERVICE_BINDING_PROTOCOL.
> -  @param[out] TlsProto              Pointer to the EFI_TLS_PROTOCOL instance.
> -  @param[out] TlsConfiguration      Pointer to the
> EFI_TLS_CONFIGURATION_PROTOCOL instance.
> +  @param[in]  HttpInstance  Pointer to HTTP_PROTOCOL structure.
>
> -  @return  The child handle with opened EFI_TLS_PROTOCOL and
> EFI_TLS_CONFIGURATION_PROTOCOL.
> +  @return  EFI_SUCCESS        TLS child handle is returned in HttpInstance-
> >TlsChildHandle
> +                              with opened EFI_TLS_PROTOCOL and
> EFI_TLS_CONFIGURATION_PROTOCOL.
> +           EFI_DEVICE_ERROR   TLS service binding protocol is not found.
> +           Otherwise          Fail to create TLS chile handle.
>
>  **/
> -EFI_HANDLE
> +EFI_STATUS
>  EFIAPI
>  TlsCreateChild (
> -  IN  EFI_HANDLE                      ImageHandle,
> -  OUT EFI_SERVICE_BINDING_PROTOCOL    **TlsSb,
> -  OUT EFI_TLS_PROTOCOL                **TlsProto,
> -  OUT EFI_TLS_CONFIGURATION_PROTOCOL  **TlsConfiguration
> +  IN  HTTP_PROTOCOL  *HttpInstance
>    );
>
>  /**
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c index 7c5c925cf78..6606c293421
> 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -3,6 +3,7 @@
>
>    Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
>    (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
> +  Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> + reserved.<BR>
>
>    SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -248,7 +249,6 @@ EfiHttpRequest (
>    HTTP_TOKEN_WRAP        *Wrap;
>    CHAR8                  *FileUrl;
>    UINTN                  RequestMsgSize;
> -  EFI_HANDLE             ImageHandle;
>
>    //
>    // Initializations
> @@ -371,23 +371,10 @@ EfiHttpRequest (
>      //
>      // Check whether we need to create Tls child and open the TLS protocol.
>      //
> -    if (HttpInstance->UseHttps && (HttpInstance->TlsChildHandle == NULL)) {
> -      //
> -      // Use TlsSb to create Tls child and open the TLS protocol.
> -      //
> -      if (HttpInstance->LocalAddressIsIPv6) {
> -        ImageHandle = HttpInstance->Service->Ip6DriverBindingHandle;
> -      } else {
> -        ImageHandle = HttpInstance->Service->Ip4DriverBindingHandle;
> -      }
> -
> -      HttpInstance->TlsChildHandle = TlsCreateChild (
> -                                       ImageHandle,
> -                                       &(HttpInstance->TlsSb),
> -                                       &(HttpInstance->Tls),
> -                                       &(HttpInstance->TlsConfiguration)
> -                                       );
> -      if (HttpInstance->TlsChildHandle == NULL) {
> +    if (HttpInstance->UseHttps && !HttpInstance->TlsAlreadyCreated) {
> +      // Create TLS child for this HTTP instance.
> +      Status = TlsCreateChild (HttpInstance);
> +      if (EFI_ERROR (Status)) {
>          return EFI_DEVICE_ERROR;
>        }
>
> diff --git a/NetworkPkg/HttpDxe/HttpProto.c
> b/NetworkPkg/HttpDxe/HttpProto.c index 7dfb82dd2e5..94900328ba9
> 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.c
> +++ b/NetworkPkg/HttpDxe/HttpProto.c
> @@ -3,6 +3,7 @@
>
>  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
> @@ -852,12 +853,12 @@ HttpCleanProtocol (
>    NetMapClean (&HttpInstance->TxTokens);
>    NetMapClean (&HttpInstance->RxTokens);
>
> -  if ((HttpInstance->TlsSb != NULL) && (HttpInstance->TlsChildHandle !=
> NULL)) {
> +  if ((HttpInstance->TlsSb != NULL) && HttpInstance->TlsAlreadyCreated)
> + {
>      //
>      // Destroy the TLS instance.
>      //
> -    HttpInstance->TlsSb->DestroyChild (HttpInstance->TlsSb, HttpInstance-
> >TlsChildHandle);
> -    HttpInstance->TlsChildHandle = NULL;
> +    HttpInstance->TlsSb->DestroyChild (HttpInstance->TlsSb, HttpInstance-
> >Handle);
> +    HttpInstance->TlsAlreadyCreated = FALSE;
>    }
>
>    if (HttpInstance->Tcp4ChildHandle != NULL) { diff --git
> a/NetworkPkg/HttpDxe/HttpsSupport.c
> b/NetworkPkg/HttpDxe/HttpsSupport.c
> index 7330be42c00..a07323ff0bd 100644
> --- a/NetworkPkg/HttpDxe/HttpsSupport.c
> +++ b/NetworkPkg/HttpDxe/HttpsSupport.c
> @@ -3,6 +3,7 @@
>
>  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
>  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
> +Copyright (C) 2024 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>
>  **/
> @@ -134,27 +135,31 @@ IsHttpsUrl (
>  /**
>    Creates a Tls child handle, open EFI_TLS_PROTOCOL and
> EFI_TLS_CONFIGURATION_PROTOCOL.
>
> -  @param[in]  ImageHandle           The firmware allocated handle for the UEFI
> image.
> -  @param[out] TlsSb                 Pointer to the TLS
> SERVICE_BINDING_PROTOCOL.
> -  @param[out] TlsProto              Pointer to the EFI_TLS_PROTOCOL instance.
> -  @param[out] TlsConfiguration      Pointer to the
> EFI_TLS_CONFIGURATION_PROTOCOL instance.
> +  @param[in]  HttpInstance  Pointer to HTTP_PROTOCOL structure.
>
> -  @return  The child handle with opened EFI_TLS_PROTOCOL and
> EFI_TLS_CONFIGURATION_PROTOCOL.
> +  @return  EFI_SUCCESS        TLS child handle is returned in HttpInstance-
> >TlsChildHandle
> +                              with opened EFI_TLS_PROTOCOL and
> EFI_TLS_CONFIGURATION_PROTOCOL.
> +           EFI_DEVICE_ERROR   TLS service binding protocol is not found.
> +           Otherwise          Fail to create TLS chile handle.
>
>  **/
> -EFI_HANDLE
> +EFI_STATUS
>  EFIAPI
>  TlsCreateChild (
> -  IN  EFI_HANDLE                      ImageHandle,
> -  OUT EFI_SERVICE_BINDING_PROTOCOL    **TlsSb,
> -  OUT EFI_TLS_PROTOCOL                **TlsProto,
> -  OUT EFI_TLS_CONFIGURATION_PROTOCOL  **TlsConfiguration
> +  IN  HTTP_PROTOCOL  *HttpInstance
>    )
>  {
> +  EFI_HANDLE  ImageHandle;
>    EFI_STATUS  Status;
> -  EFI_HANDLE  TlsChildHandle;
>
> -  TlsChildHandle = 0;
> +  //
> +  // Use TlsSb to create Tls child and open the TLS protocol.
> +  //
> +  if (HttpInstance->LocalAddressIsIPv6) {
> +    ImageHandle = HttpInstance->Service->Ip6DriverBindingHandle;
> +  } else {
> +    ImageHandle = HttpInstance->Service->Ip4DriverBindingHandle;
> +  }
>
>    //
>    // Locate TlsServiceBinding protocol.
> @@ -162,44 +167,51 @@ TlsCreateChild (
>    gBS->LocateProtocol (
>           &gEfiTlsServiceBindingProtocolGuid,
>           NULL,
> -         (VOID **)TlsSb
> +         (VOID **)&HttpInstance->TlsSb
>           );
> -  if (*TlsSb == NULL) {
> -    return NULL;
> +  if (HttpInstance->TlsSb == NULL) {
> +    return EFI_DEVICE_ERROR;
>    }
>
> -  Status = (*TlsSb)->CreateChild (*TlsSb, &TlsChildHandle);
> +  //
> +  // Create TLS protocol on HTTP handle, this creates the association
> + between HTTP and TLS  // for HTTP driver external usages.
> +  //
> +  Status = HttpInstance->TlsSb->CreateChild (HttpInstance->TlsSb,
> + &HttpInstance->Handle);
>    if (EFI_ERROR (Status)) {
> -    return NULL;
> +    return Status;
>    }
>
> -  Status = gBS->OpenProtocol (
> -                  TlsChildHandle,
> -                  &gEfiTlsProtocolGuid,
> -                  (VOID **)TlsProto,
> -                  ImageHandle,
> -                  TlsChildHandle,
> -                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
> -                  );
> +  HttpInstance->TlsAlreadyCreated = TRUE;
> +  Status                          = gBS->OpenProtocol (
> +                                           HttpInstance->Handle,
> +                                           &gEfiTlsProtocolGuid,
> +                                           (VOID **)&HttpInstance->Tls,
> +                                           ImageHandle,
> +                                           HttpInstance->Handle,
> +                                           EFI_OPEN_PROTOCOL_GET_PROTOCOL
> +                                           );
>    if (EFI_ERROR (Status)) {
> -    (*TlsSb)->DestroyChild (*TlsSb, TlsChildHandle);
> -    return NULL;
> +    HttpInstance->TlsSb->DestroyChild (HttpInstance->TlsSb, HttpInstance-
> >Handle);
> +    HttpInstance->TlsAlreadyCreated = FALSE;
> +    return Status;
>    }
>
>    Status = gBS->OpenProtocol (
> -                  TlsChildHandle,
> +                  HttpInstance->Handle,
>                    &gEfiTlsConfigurationProtocolGuid,
> -                  (VOID **)TlsConfiguration,
> +                  (VOID **)&HttpInstance->TlsConfiguration,
>                    ImageHandle,
> -                  TlsChildHandle,
> +                  HttpInstance->Handle,
>                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (EFI_ERROR (Status)) {
> -    (*TlsSb)->DestroyChild (*TlsSb, TlsChildHandle);
> -    return NULL;
> +    HttpInstance->TlsSb->DestroyChild (HttpInstance->TlsSb, HttpInstance-
> >Handle);
> +    HttpInstance->TlsAlreadyCreated = FALSE;
> +    return Status;
>    }
>
> -  return TlsChildHandle;
> +  return EFI_SUCCESS;
>  }
>
>  /**
> --
> 2.37.1.windows.1
>
>
>
>
>
>
>
>
> 
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113507): https://edk2.groups.io/g/devel/message/113507
Mute This Topic: https://groups.io/mt/103577242/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-