[edk2-devel] [PATCH 0/5] Refactor HTTP DXE to provide TLS reconfiguration capability

[edk2-devel] [PATCH 0/5] Refactor HTTP DXE to provide TLS reconfiguration capability

[Chang, Abner via groups.io]

From: Abner Chang <abner.chang@amd.com>

For the HTTPS connetion that doesn't require TLS peer verification,
We introduce a new HTTP event HttpEventTlsConfigured to notify HTTP callback
functions after TlsConfigureSession(). With this event, the upper layer
HTTP application can listen to HttpEventTlsConfigured HTTP event and
reconfigure TLS configuration data in the callback function to set TLS verify
method to TLS_VERIFY_NONE.
The use case such as Redfish service connction which doesn't require the
TLS peer verification on the cetificate, especially to the Redfish service
connection through the in-band network interface.
The root cause we are fixing is the TLS config data is hardcoded in
TlsConfigureSession() for any TLS child in HttpSupport.c. With this hardcoded
code, HTTP application has no way to give the proper TLS config data when TLS
session is initiated.

In Patch 1/5: We agreed to refactor TlsCreateChild and install TLS on
              the given HTTP handle.
In Patch 2/5: We consider TLS certificate not found as a success case to
              ensures HTTP session is still initiated and HttpInitSession()
              returns EFI_SUCCESS to the caller. The failure is pushed back to
              TLS DXE driver if the HTTP communication actually requires
              certificate.
In Patch 3/5: Introduce HttpEventTlsConfigured HTTP callback event
In Patch 4/5: REST EX DXE provides HTTP event callback protocol
In Patch 5/5: REST EX DXE fix.

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>

Abner Chang (5):
  NetwokrPkg/HttpDxe: Refactor TlsCreateChild
  NetwokrPkg/HttpDxe: Consider TLS certificate not found as a success
    case
  NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured HTTP callback event
  RedfishPkg/RedfishRestExDxe: Implement EDKII_HTTP_CALLBACK_PROTOCOL
  RedfishPkg/RedfishRestExDxe: Update Supported function

 .../RedfishRestExDxe/RedfishRestExDxe.inf     |  2 +
 NetworkPkg/HttpDxe/HttpProto.h                |  3 +-
 NetworkPkg/HttpDxe/HttpsSupport.h             | 18 ++--
 NetworkPkg/Include/Protocol/HttpCallback.h    | 11 +++
 .../RedfishRestExDxe/RedfishRestExDriver.h    | 31 ++++--
 NetworkPkg/HttpDxe/HttpImpl.c                 | 23 +----
 NetworkPkg/HttpDxe/HttpProto.c                |  8 +-
 NetworkPkg/HttpDxe/HttpsSupport.c             | 95 ++++++++++++-------
 .../RedfishRestExDxe/RedfishRestExDriver.c    | 84 ++++++++++++++++
 9 files changed, 198 insertions(+), 77 deletions(-)

-- 
2.37.1.windows.1



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

[edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

[Chang, Abner via groups.io]

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>
---
 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 (#113225): https://edk2.groups.io/g/devel/message/113225
Mute This Topic: https://groups.io/mt/103539578/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH 2/5] NetwokrPkg/HttpDxe: Consider TLS certificate not found as a success case

[Chang, Abner via groups.io]

From: Abner Chang <abner.chang@amd.com>

We still return EFI_SUCCESS to the caller when TlsConfigCertificate
returns error, for the use case the platform doesn't require
certificate for the specific HTTP session. This ensures
HttpInitSession function still initiated and returns EFI_SUCCESS to
the caller. The failure is pushed back to TLS DXE driver if the
HTTP communication actually requires certificate.

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>
---
 NetworkPkg/HttpDxe/HttpsSupport.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index a07323ff0bd..04a830f7152 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -722,8 +722,21 @@ TlsConfigureSession (
   //
   Status = TlsConfigCertificate (HttpInstance);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
-    return Status;
+    if (Status == EFI_NOT_FOUND) {
+      DEBUG((DEBUG_WARN, "TLS Certificate is not found on the system!\n"));
+      //
+      // We still return EFI_SUCCESS to the caller when TlsConfigCertificate
+      // returns error, for the use case the platform doesn't require
+      // certificate for the specific HTTP session. This ensures
+      // HttpInitSession function still initiated and returns EFI_SUCCESS to
+      // the caller. The failure is pushed back to TLS DXE driver if the
+      // HTTP communication actually requires certificate.
+      //
+      Status = EFI_SUCCESS;
+    } else {
+      DEBUG((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
+      return Status;
+    }
   }
 
   //
-- 
2.37.1.windows.1



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

[edk2-devel] [PATCH 3/5] NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured HTTP callback event

[Chang, Abner via groups.io]

From: Abner Chang <abner.chang@amd.com>

Add HttpEventTlsConfigured HTTP callback event and notify
callback functions when TlsConfigureSession () returns.

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>
---
 NetworkPkg/Include/Protocol/HttpCallback.h | 11 +++++++++++
 NetworkPkg/HttpDxe/HttpProto.c             |  1 +
 NetworkPkg/HttpDxe/HttpsSupport.c          |  4 ++--
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/Include/Protocol/HttpCallback.h b/NetworkPkg/Include/Protocol/HttpCallback.h
index 3accd2f8424..a785c972f8f 100644
--- a/NetworkPkg/Include/Protocol/HttpCallback.h
+++ b/NetworkPkg/Include/Protocol/HttpCallback.h
@@ -39,6 +39,17 @@ typedef enum {
   ///
   HttpEventConnectTcp,
 
+  ///
+  /// The Status of Event to configure TLS configuration data.
+  /// EventStatus:
+  /// EFI_SUCCESS            The TLS is configured successfully with the default value.
+  /// EFI_INVALID_PARAMETER  One or more input parameters to SetSessionData() is invalid.
+  /// EFI_NOT_READY          Current TLS session state is NOT EfiTlsSessionStateNotStarted.
+  /// EFI_NOT_FOUND          Fail to get 'HttpTlsCipherList' variable.
+  /// Others                 Other error as indicated.
+  ///
+  HttpEventTlsConfigured,
+
   ///
   /// The Status of Event to connect one TLS session by finishing the TLS handshake process.
   /// EventStatus:
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index 94900328ba9..9c3b497dce2 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1418,6 +1418,7 @@ HttpInitSession (
   //
   if (TlsConfigure) {
     Status = TlsConfigureSession (HttpInstance);
+    HttpNotify (HttpEventTlsConfigured, Status);
     if (EFI_ERROR (Status)) {
       return Status;
     }
diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index 04a830f7152..8d7bffe1e9c 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -723,7 +723,7 @@ TlsConfigureSession (
   Status = TlsConfigCertificate (HttpInstance);
   if (EFI_ERROR (Status)) {
     if (Status == EFI_NOT_FOUND) {
-      DEBUG((DEBUG_WARN, "TLS Certificate is not found on the system!\n"));
+      DEBUG ((DEBUG_WARN, "TLS Certificate is not found on the system!\n"));
       //
       // We still return EFI_SUCCESS to the caller when TlsConfigCertificate
       // returns error, for the use case the platform doesn't require
@@ -734,7 +734,7 @@ TlsConfigureSession (
       //
       Status = EFI_SUCCESS;
     } else {
-      DEBUG((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
+      DEBUG ((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
       return Status;
     }
   }
-- 
2.37.1.windows.1



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

[edk2-devel] [PATCH 4/5] RedfishPkg/RedfishRestExDxe: Implement EDKII_HTTP_CALLBACK_PROTOCOL

[Chang, Abner via groups.io]

From: Abner Chang <abner.chang@amd.com>

Implement EDKII_HTTP_CALLBACK_PROTOCOL that listens to
HttpEventTlsConfigured event for reconfiguring TLS configuration
data.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
---
 .../RedfishRestExDxe/RedfishRestExDxe.inf     |  2 +
 .../RedfishRestExDxe/RedfishRestExDriver.h    | 31 ++++++---
 .../RedfishRestExDxe/RedfishRestExDriver.c    | 69 +++++++++++++++++++
 3 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
index 64e6343bfbf..706d5a5f15e 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
@@ -57,6 +57,8 @@
   gEfiHttpServiceBindingProtocolGuid              ## TO_START
   gEfiHttpProtocolGuid                            ## TO_START
   gEfiDevicePathProtocolGuid                      ## TO_START
+  gEdkiiHttpCallbackProtocolGuid                  ## CONSUMES
+  gEfiTlsProtocolGuid                             ## CONSUMES
 
 [Pcd]
   gEfiRedfishPkgTokenSpaceGuid.PcdRedfishRestExServiceAccessModeInBand ## CONSUMES
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
index 6b94e5814c4..8d2b9ecb80b 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.h
@@ -32,6 +32,8 @@
 #include <Protocol/DriverBinding.h>
 #include <Protocol/RestEx.h>
 #include <Protocol/ServiceBinding.h>
+#include <Protocol/HttpCallback.h>
+#include <Protocol/Tls.h>
 
 ///
 /// Protocol instances
@@ -67,6 +69,9 @@ typedef struct _RESTEX_INSTANCE RESTEX_INSTANCE;
 #define RESTEX_INSTANCE_FROM_THIS(a)  \
   CR (a, RESTEX_INSTANCE, RestEx, RESTEX_INSTANCE_SIGNATURE)
 
+#define RESTEX_INSTANCE_FROM_HTTP_CALLBACK(a)  \
+  CR (a, RESTEX_INSTANCE, HttpCallbakFunction, RESTEX_INSTANCE_SIGNATURE)
+
 #define RESTEX_STATE_UNCONFIGED  0
 #define RESTEX_STATE_CONFIGED    1
 
@@ -94,25 +99,31 @@ struct _RESTEX_SERVICE {
 #define RESTEX_INSTANCE_FLAGS_TCP_ERROR_RETRY  0x00000002
 
 struct _RESTEX_INSTANCE {
-  UINT32                     Signature;
-  LIST_ENTRY                 Link;
+  UINT32                        Signature;
+  LIST_ENTRY                    Link;
 
-  EFI_REST_EX_PROTOCOL       RestEx;
+  EFI_REST_EX_PROTOCOL          RestEx;
 
-  INTN                       State;
-  BOOLEAN                    InDestroy;
+  INTN                          State;
+  BOOLEAN                       InDestroy;
 
-  RESTEX_SERVICE             *Service;
-  EFI_HANDLE                 ChildHandle;
+  RESTEX_SERVICE                *Service;
+  EFI_HANDLE                    ChildHandle;
 
-  EFI_REST_EX_CONFIG_DATA    ConfigData;
+  EFI_REST_EX_CONFIG_DATA       ConfigData;
 
   //
   // HTTP_IO to access the HTTP service
   //
-  HTTP_IO                    HttpIo;
+  HTTP_IO                       HttpIo;
+
+  //
+  // EDKII_HTTP_CALLBACK_PROTOCOL that listens to
+  // HttpEventInitSession event.
+  //
+  EDKII_HTTP_CALLBACK_PROTOCOL  HttpCallbakFunction;
 
-  UINT32                     Flags;
+  UINT32                        Flags;
 };
 
 typedef struct {
diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c
index 7036aed4268..9a20c90d49a 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c
@@ -585,6 +585,53 @@ RedfishRestExDriverBindingStop (
   return Status;
 }
 
+/**
+  Callback function that is invoked when HTTP event occurs.
+
+  @param[in]  This                Pointer to the EDKII_HTTP_CALLBACK_PROTOCOL instance.
+  @param[in]  Event               The event that occurs in the current state.
+  @param[in]  EventStatus         The Status of Event, EFI_SUCCESS or other errors.
+**/
+VOID
+EFIAPI
+RestExHttpCallback (
+  IN EDKII_HTTP_CALLBACK_PROTOCOL  *This,
+  IN EDKII_HTTP_CALLBACK_EVENT     Event,
+  IN EFI_STATUS                    EventStatus
+  )
+{
+  EFI_STATUS        Status;
+  EFI_TLS_PROTOCOL  *TlsProtocol;
+  RESTEX_INSTANCE   *Instance;
+  EFI_TLS_VERIFY    TlsVerifyMethod;
+
+  if ((Event == HttpEventTlsConfigured) && (EventStatus == EFI_SUCCESS)) {
+    // Reconfigure TLS configuration data.
+    Instance = RESTEX_INSTANCE_FROM_HTTP_CALLBACK (This);
+    Status   = gBS->HandleProtocol (
+                      Instance->HttpIo.Handle,
+                      &gEfiTlsProtocolGuid,
+                      (VOID **)&TlsProtocol
+                      );
+    if (EFI_ERROR (Status)) {
+      return;
+    }
+
+    TlsVerifyMethod = EFI_TLS_VERIFY_NONE;
+    Status          = TlsProtocol->SetSessionData (
+                                     TlsProtocol,
+                                     EfiTlsVerifyMethod,
+                                     &TlsVerifyMethod,
+                                     sizeof (EFI_TLS_VERIFY)
+                                     );
+    if (!EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_MANAGEABILITY, "%a: REST EX reconfigures TLS verify method.\n", __func__));
+    }
+  }
+
+  return;
+}
+
 /**
   Creates a child handle and installs a protocol.
 
@@ -699,6 +746,19 @@ RedfishRestExServiceBindingCreateChild (
     goto ON_ERROR;
   }
 
+  // Initial HTTP callback funciton on this REST EX instance
+  Instance->HttpCallbakFunction.Callback = RestExHttpCallback;
+  Status                                 = gBS->InstallProtocolInterface (
+                                                  &Instance->HttpIo.Handle,
+                                                  &gEdkiiHttpCallbackProtocolGuid,
+                                                  EFI_NATIVE_INTERFACE,
+                                                  &Instance->HttpCallbakFunction
+                                                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Fail to install HttpCallbakFunction.\n", __func__));
+    goto ON_ERROR;
+  }
+
   //
   // Add it to the parent's child list.
   //
@@ -812,6 +872,15 @@ RedfishRestExServiceBindingDestroyChild (
                   RestEx
                   );
 
+  //
+  // Uninstall the HTTP callback protocol.
+  //
+  Status = gBS->UninstallProtocolInterface (
+                  Instance->HttpIo.Handle,
+                  &gEdkiiHttpCallbackProtocolGuid,
+                  &Instance->HttpCallbakFunction
+                  );
+
   OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
 
   if (EFI_ERROR (Status)) {
-- 
2.37.1.windows.1



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

[edk2-devel] [PATCH 5/5] RedfishPkg/RedfishRestExDxe: Update Supported function

[Chang, Abner via groups.io]

From: Abner Chang <abner.chang@amd.com>

Update Supported function to check it the given
controller handle is already started.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Cc: Nickle Wang <nicklew@nvidia.com>
Cc: Igor Kulchytskyy <igork@ami.com>
---
 RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c
index 9a20c90d49a..38bcf28d993 100644
--- a/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c
+++ b/RedfishPkg/RedfishRestExDxe/RedfishRestExDriver.c
@@ -356,6 +356,21 @@ RedfishRestExDriverBindingSupported (
   IN EFI_DEVICE_PATH_PROTOCOL     *RemainingDevicePath OPTIONAL
   )
 {
+  EFI_STATUS  Status;
+  UINT32      *Id;
+
+  Status = gBS->OpenProtocol (
+                  ControllerHandle,
+                  &gEfiCallerIdGuid,
+                  (VOID **)&Id,
+                  This->DriverBindingHandle,
+                  ControllerHandle,
+                  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                  );
+  if (!EFI_ERROR (Status)) {
+    return EFI_ALREADY_STARTED;
+  }
+
   //
   // Test for the HttpServiceBinding Protocol.
   //
-- 
2.37.1.windows.1



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

Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

[Michael Brown]

On 05/01/2024 08:37, abner.chang@amd.com wrote:
> 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.

Logic looks good to me, just some minor cosmetic comments.

Commit title has "NetwokrPkg" typo, should be "NetworkPkg".

> -  @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.

Comment refers to TlsChildHandle, which no longer exists after this patch.

> -  @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.

As above.

Reviewed-by: Michael Brown <mcb30@ipxe.org>

Michael



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

Re: [edk2-devel] [PATCH 2/5] NetwokrPkg/HttpDxe: Consider TLS certificate not found as a success case

[Michael Brown]

On 05/01/2024 08:37, abner.chang@amd.com wrote:
> We still return EFI_SUCCESS to the caller when TlsConfigCertificate
> returns error, for the use case the platform doesn't require
> certificate for the specific HTTP session. This ensures
> HttpInitSession function still initiated and returns EFI_SUCCESS to
> the caller. The failure is pushed back to TLS DXE driver if the
> HTTP communication actually requires certificate.

Reviewed-by: Michael Brown <mcb30@ipxe.org>

Thanks,

Michael



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

Re: [edk2-devel] [PATCH 3/5] NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured HTTP callback event

[Michael Brown]

On 05/01/2024 08:37, abner.chang@amd.com wrote:
> +  ///
> +  /// The Status of Event to configure TLS configuration data.
> +  /// EventStatus:
> +  /// EFI_SUCCESS            The TLS is configured successfully with the default value.
> +  /// EFI_INVALID_PARAMETER  One or more input parameters to SetSessionData() is invalid.
> +  /// EFI_NOT_READY          Current TLS session state is NOT EfiTlsSessionStateNotStarted.
> +  /// EFI_NOT_FOUND          Fail to get 'HttpTlsCipherList' variable.
> +  /// Others                 Other error as indicated.
> +  ///
> +  HttpEventTlsConfigured,
> +

Since this changes the ABI, you may want to also update the protocol 
GUID to prevent strange errors if old and new binaries are used on the 
same system.

Reviewed-by: Michael Brown <mcb30@ipxe.org>

Thanks,

Michael



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

Re: [edk2-devel] [PATCH 2/5] NetwokrPkg/HttpDxe: Consider TLS certificate not found as a success case

[Saloni Kasbekar]

Reviewed-by: Saloni Kasbekar <saloni.kasbekar@intel.com>

-----Original Message-----
From: abner.chang@amd.com <abner.chang@amd.com> 
Sent: Friday, January 5, 2024 12:37 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: [PATCH 2/5] NetwokrPkg/HttpDxe: Consider TLS certificate not found as a success case

From: Abner Chang <abner.chang@amd.com>

We still return EFI_SUCCESS to the caller when TlsConfigCertificate returns error, for the use case the platform doesn't require certificate for the specific HTTP session. This ensures HttpInitSession function still initiated and returns EFI_SUCCESS to the caller. The failure is pushed back to TLS DXE driver if the HTTP communication actually requires certificate.

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>
---
 NetworkPkg/HttpDxe/HttpsSupport.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c b/NetworkPkg/HttpDxe/HttpsSupport.c
index a07323ff0bd..04a830f7152 100644
--- a/NetworkPkg/HttpDxe/HttpsSupport.c
+++ b/NetworkPkg/HttpDxe/HttpsSupport.c
@@ -722,8 +722,21 @@ TlsConfigureSession (
   //
   Status = TlsConfigCertificate (HttpInstance);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
-    return Status;
+    if (Status == EFI_NOT_FOUND) {
+      DEBUG((DEBUG_WARN, "TLS Certificate is not found on the system!\n"));
+      //
+      // We still return EFI_SUCCESS to the caller when TlsConfigCertificate
+      // returns error, for the use case the platform doesn't require
+      // certificate for the specific HTTP session. This ensures
+      // HttpInitSession function still initiated and returns EFI_SUCCESS to
+      // the caller. The failure is pushed back to TLS DXE driver if the
+      // HTTP communication actually requires certificate.
+      //
+      Status = EFI_SUCCESS;
+    } else {
+      DEBUG((DEBUG_ERROR, "TLS Certificate Config Error!\n"));
+      return Status;
+    }
   }
 
   //
--
2.37.1.windows.1



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

Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

[Saloni Kasbekar]

     //
     // Check whether we need to create Tls child and open the TLS protocol.
     //
+    if (HttpInstance->UseHttps && !HttpInstance->TlsAlreadyCreated) {
+      // Create TLS child for this HTTP instance.
+      Status = TlsCreateChild (HttpInstance);
+      if (EFI_ERROR (Status)) {
         return EFI_DEVICE_ERROR;
       }

Considering we're not creating TlsChildHandle anymore, we should probably rename TlsCreateChild() to TlsCreateHandle(). Also update the comments accordingly.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Brown
Sent: Friday, January 5, 2024 9:12 AM
To: abner.chang@amd.com; devel@edk2.groups.io
Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

On 05/01/2024 08:37, abner.chang@amd.com wrote:
> 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.

Logic looks good to me, just some minor cosmetic comments.

Commit title has "NetwokrPkg" typo, should be "NetworkPkg".

> -  @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.

Comment refers to TlsChildHandle, which no longer exists after this patch.

> -  @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.

As above.

Reviewed-by: Michael Brown <mcb30@ipxe.org>

Michael








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

Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Hi Saloni,
We still create TLS child via service binding protocol, the difference is we give HTTP handle to TLS service binding for installing TLS protocol on the HTTP handle instead of a on a new EFI handle.
The logic is still the same just the code is moved around, I think we can just keep the function name the same..

Thanks
Abner

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni
> Kasbekar via groups.io
> Sent: Saturday, January 6, 2024 5:32 AM
> To: devel@edk2.groups.io; mcb30@ipxe.org; Chang, Abner
> <Abner.Chang@amd.com>
> Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Nickle Wang
> <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> TlsCreateChild
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
>      //
>      // Check whether we need to create Tls child and open the TLS protocol.
>      //
> +    if (HttpInstance->UseHttps && !HttpInstance->TlsAlreadyCreated) {
> +      // Create TLS child for this HTTP instance.
> +      Status = TlsCreateChild (HttpInstance);
> +      if (EFI_ERROR (Status)) {
>          return EFI_DEVICE_ERROR;
>        }
>
> Considering we're not creating TlsChildHandle anymore, we should probably
> rename TlsCreateChild() to TlsCreateHandle(). Also update the comments
> accordingly.
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Brown
> Sent: Friday, January 5, 2024 9:12 AM
> To: abner.chang@amd.com; devel@edk2.groups.io
> Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary
> <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> TlsCreateChild
>
> On 05/01/2024 08:37, abner.chang@amd.com wrote:
> > 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.
>
> Logic looks good to me, just some minor cosmetic comments.
>
> Commit title has "NetwokrPkg" typo, should be "NetworkPkg".
>
> > -  @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.
>
> Comment refers to TlsChildHandle, which no longer exists after this patch.
>
> > -  @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.
>
> As above.
>
> Reviewed-by: Michael Brown <mcb30@ipxe.org>
>
> Michael
>
>
>
>
>
>
>
>
> 
>



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

Re: [edk2-devel] [PATCH 3/5] NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured HTTP callback event

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Michael Brown <mcb30@ipxe.org>
> Sent: Saturday, January 6, 2024 1:15 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>; Zachary Clark-williams
> <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: Re: [PATCH 3/5] NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured
> HTTP callback event
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> On 05/01/2024 08:37, abner.chang@amd.com wrote:
> > +  ///
> > +  /// The Status of Event to configure TLS configuration data.
> > +  /// EventStatus:
> > +  /// EFI_SUCCESS            The TLS is configured successfully with the default
> value.
> > +  /// EFI_INVALID_PARAMETER  One or more input parameters to
> SetSessionData() is invalid.
> > +  /// EFI_NOT_READY          Current TLS session state is NOT
> EfiTlsSessionStateNotStarted.
> > +  /// EFI_NOT_FOUND          Fail to get 'HttpTlsCipherList' variable.
> > +  /// Others                 Other error as indicated.
> > +  ///
> > +  HttpEventTlsConfigured,
> > +
>
> Since this changes the ABI, you may want to also update the protocol
> GUID to prevent strange errors if old and new binaries are used on the
> same system.
This makes sense, will send out V2.

Thanks
Abner

>
> Reviewed-by: Michael Brown <mcb30@ipxe.org>
>
> Thanks,
>
> Michael



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

Re: [edk2-devel] [PATCH 3/5] NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured HTTP callback event

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

> -----Original Message-----
> From: Chang, Abner
> Sent: Sunday, January 7, 2024 8:19 PM
> To: Michael Brown <mcb30@ipxe.org>; devel@edk2.groups.io
> Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>; Zachary Clark-williams
> <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>; Igor
> Kulchytskyy <igork@ami.com>
> Subject: RE: [PATCH 3/5] NetwokrPkg/HttpDxe: Add HttpEventTlsConfigured
> HTTP callback event
>
>
> > -----Original Message-----
> > From: Michael Brown <mcb30@ipxe.org>
> > Sent: Saturday, January 6, 2024 1:15 AM
> > To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> > Cc: Saloni Kasbekar <saloni.kasbekar@intel.com>; Zachary Clark-williams
> > <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>;
> Igor
> > Kulchytskyy <igork@ami.com>
> > Subject: Re: [PATCH 3/5] NetwokrPkg/HttpDxe: Add
> HttpEventTlsConfigured
> > HTTP callback event
> >
> > Caution: This message originated from an External Source. Use proper
> caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On 05/01/2024 08:37, abner.chang@amd.com wrote:
> > > +  ///
> > > +  /// The Status of Event to configure TLS configuration data.
> > > +  /// EventStatus:
> > > +  /// EFI_SUCCESS            The TLS is configured successfully with the default
> > value.
> > > +  /// EFI_INVALID_PARAMETER  One or more input parameters to
> > SetSessionData() is invalid.
> > > +  /// EFI_NOT_READY          Current TLS session state is NOT
> > EfiTlsSessionStateNotStarted.
> > > +  /// EFI_NOT_FOUND          Fail to get 'HttpTlsCipherList' variable.
> > > +  /// Others                 Other error as indicated.
> > > +  ///
> > > +  HttpEventTlsConfigured,
> > > +
> >
> > Since this changes the ABI, you may want to also update the protocol
> > GUID to prevent strange errors if old and new binaries are used on the
> > same system.
> This makes sense, will send out V2.
Hi Michael,
In order to minimize the change, I decided to relocate HttpEventTlsConfigured to the end of enum declaration to keep the enum number backward compatible.

Please check it in  V2 patch.
Thanks
Abner

>
> Thanks
> Abner
>
> >
> > Reviewed-by: Michael Brown <mcb30@ipxe.org>
> >
> > Thanks,
> >
> > Michael



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

Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor TlsCreateChild

[Chang, Abner via groups.io]

[AMD Official Use Only - General]

Hi @saloni.kasbekar@intel.com,
Could you please check patch 1/6 and 3/6 in V2 and give your RB if there is no more concerns from you? Then I can merge this patch set. Thank you.

Abner


> -----Original Message-----
> From: Chang, Abner
> Sent: Sunday, January 7, 2024 8:08 PM
> To: devel@edk2.groups.io; saloni.kasbekar@intel.com; mcb30@ipxe.org
> Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Nickle Wang
> <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> Subject: RE: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> TlsCreateChild
>
> Hi Saloni,
> We still create TLS child via service binding protocol, the difference is we give
> HTTP handle to TLS service binding for installing TLS protocol on the HTTP
> handle instead of a on a new EFI handle.
> The logic is still the same just the code is moved around, I think we can just
> keep the function name the same..
>
> Thanks
> Abner
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Saloni
> > Kasbekar via groups.io
> > Sent: Saturday, January 6, 2024 5:32 AM
> > To: devel@edk2.groups.io; mcb30@ipxe.org; Chang, Abner
> > <Abner.Chang@amd.com>
> > Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Nickle Wang
> > <nicklew@nvidia.com>; Igor Kulchytskyy <igork@ami.com>
> > Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> > TlsCreateChild
> >
> > Caution: This message originated from an External Source. Use proper
> caution
> > when opening attachments, clicking links, or responding.
> >
> >
> >      //
> >      // Check whether we need to create Tls child and open the TLS protocol.
> >      //
> > +    if (HttpInstance->UseHttps && !HttpInstance->TlsAlreadyCreated) {
> > +      // Create TLS child for this HTTP instance.
> > +      Status = TlsCreateChild (HttpInstance);
> > +      if (EFI_ERROR (Status)) {
> >          return EFI_DEVICE_ERROR;
> >        }
> >
> > Considering we're not creating TlsChildHandle anymore, we should probably
> > rename TlsCreateChild() to TlsCreateHandle(). Also update the comments
> > accordingly.
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> > Brown
> > Sent: Friday, January 5, 2024 9:12 AM
> > To: abner.chang@amd.com; devel@edk2.groups.io
> > Cc: Kasbekar, Saloni <saloni.kasbekar@intel.com>; Clark-williams, Zachary
> > <zachary.clark-williams@intel.com>; Nickle Wang <nicklew@nvidia.com>;
> Igor
> > Kulchytskyy <igork@ami.com>
> > Subject: Re: [edk2-devel] [PATCH 1/5] NetwokrPkg/HttpDxe: Refactor
> > TlsCreateChild
> >
> > On 05/01/2024 08:37, abner.chang@amd.com wrote:
> > > 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.
> >
> > Logic looks good to me, just some minor cosmetic comments.
> >
> > Commit title has "NetwokrPkg" typo, should be "NetworkPkg".
> >
> > > -  @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.
> >
> > Comment refers to TlsChildHandle, which no longer exists after this patch.
> >
> > > -  @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.
> >
> > As above.
> >
> > Reviewed-by: Michael Brown <mcb30@ipxe.org>
> >
> > Michael
> >
> >
> >
> >
> >
> >
> >
> >
> > 
> >



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