Migrate node join client from httpclient to libcurl

CHANGELOG.md

@@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [7.0.8]
+
+[7.0.8]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.8
+
+### Changed
+
+- The node join protocol client now uses the curl multi singleton client (introduced in #7102) instead of the legacy enclave `RPCSessions::create_client()` HTTP client, matching the JWT refresh and snapshot-fetch clients. The service certificate remains the sole trust anchor for the join connection (the host certificate store is never consulted) (#8040).
+- **Node joins now check the target RPC address against the target node's certificate SANs.** TLS certificate hostname verification (`CURLOPT_SSL_VERIFYHOST`) is now enforced on the join connection: the host in `join.target_rpc_address` must be covered by one of the target node's certificate Subject Alternative Names (SANs), and a join to an address absent from the target's SANs is now rejected (the previous join client did not check the target certificate name at all). CCF derives node-certificate SANs from `node_certificate.subject_alt_names`, or by default from each RPC interface's `published_address`, so standard deployments are unaffected; operators that configure a bespoke `join.target_rpc_address` must ensure it is present in the target node's certificate SANs (#8040).
+
+### Removed
+
+- The unused enclave-side HTTP client infrastructure (`RPCSessions::create_client`, `HTTPClientSession`, `HTTP2ClientSession`, `UnencryptedHTTPClientSession`, and the `ClientSession` base) has been removed following the migration of the node join client to curl, completing the legacy HTTP client removal tracked in #7262 (#8040).
+
 ## [7.0.7]
 
 [7.0.7]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.7

doc/operations/start_network.rst

@@ -52,6 +52,8 @@ To add a new node to an existing opening network, other nodes should be started
 
 The joining node takes the certificate of the existing network to join via ``service_certificate_file`` configuration entry and initiates an enclave-to-enclave TLS connection to an existing node of the network as specified by ``join.target_rpc_address`` configuration entry.
 
+.. note:: The joining node verifies the target node's TLS certificate against the ``service_certificate_file`` (which is the only trust anchor used for this connection) and checks that it matches the ``join.target_rpc_address`` host. Operators must therefore ensure that the target node's certificate subject alternative names - derived from its RPC interface ``published_address`` values, or set explicitly via ``node_certificate.subject_alt_names`` - include the address used in ``join.target_rpc_address``.
+
 The join configuration option should be set in the :ref:`operations/configuration:``command.join``` section of the JSON configuration.
 
 A new node can only join an existing CCF network if its hardware attestation is valid  [#remote_attestation]_. and runs an enclave application that is :ref:`trusted by the consortium <governance/common_member_operations:Updating Code Version>`.

python/pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "ccf"
-version = "7.0.7"
+version = "7.0.8"
 authors = [
   { name="CCF Team", email="CCF-Sec@microsoft.com" },
 ]

src/enclave/rpc_sessions.h

@@ -15,7 +15,6 @@
 #include "node/session_metrics.h"
 #include "rpc_handler.h"
 #include "tls/cert.h"
-#include "tls/client.h"
 #include "tls/context.h"
 #include "tls/plaintext_server.h"
 #include "tls/server.h"
@@ -72,10 +71,6 @@ namespace ccf
       sessions;
     size_t sessions_peak = 0;
 
-    // Negative sessions are reserved for those originating from
-    // the enclave via create_client().
-    std::atomic<ccf::tls::ConnID> next_client_session_id = -1;
-
     template <typename Base>
     class NoMoreSessionsImpl : public Base
     {
@@ -102,35 +97,6 @@ namespace ccf
       }
     };
 
-    ccf::tls::ConnID get_next_client_id()
-    {
-      auto id = next_client_session_id--;
-      const auto initial = id;
-
-      if (next_client_session_id > 0)
-      {
-        next_client_session_id = -1;
-      }
-
-      while (sessions.find(id) != sessions.end())
-      {
-        id--;
-
-        if (id > 0)
-        {
-          id = -1;
-        }
-
-        if (id == initial)
-        {
-          throw std::runtime_error(
-            "Exhausted all IDs for enclave client sessions");
-        }
-      }
-
-      return id;
-    }
-
     ListenInterface& get_interface_from_interface_id(
       const ccf::ListenInterfaceID& id)
     {
@@ -557,50 +523,6 @@ namespace ccf
       }
     }
 
-    std::shared_ptr<ClientSession> create_client(
-      const std::shared_ptr<::tls::Cert>& cert,
-      const std::string& app_protocol = "HTTP1")
-    {
-      std::lock_guard<ccf::pal::Mutex> guard(lock);
-      auto ctx = std::make_unique<::tls::Client>(cert);
-      auto id = get_next_client_id();
-
-      LOG_DEBUG_FMT("Creating a new client session inside the enclave: {}", id);
-
-      // There are no limits on outbound client sessions (we do not check any
-      // session caps here). We expect this type of session to be rare and
-      // want it to succeed even when we are busy.
-      if (app_protocol == "HTTP2")
-      {
-        auto session = std::make_shared<::http::HTTP2ClientSession>(
-          id, writer_factory, std::move(ctx));
-        sessions.insert(std::make_pair(id, std::make_pair("", session)));
-        sessions_peak = std::max(sessions_peak, sessions.size());
-        return session;
-      }
-      if (app_protocol == "HTTP1")
-      {
-        auto session = std::make_shared<::http::HTTPClientSession>(
-          id, writer_factory, std::move(ctx));
-        sessions.insert(std::make_pair(id, std::make_pair("", session)));
-        sessions_peak = std::max(sessions_peak, sessions.size());
-        return session;
-      }
-
-      throw std::runtime_error("unsupported client application protocol");
-    }
-
-    std::shared_ptr<ClientSession> create_unencrypted_client()
-    {
-      std::lock_guard<ccf::pal::Mutex> guard(lock);
-      auto id = get_next_client_id();
-      auto session = std::make_shared<::http::UnencryptedHTTPClientSession>(
-        id, writer_factory);
-      sessions.insert(std::make_pair(id, std::make_pair("", session)));
-      sessions_peak = std::max(sessions_peak, sessions.size());
-      return session;
-    }
-
     void register_message_handlers(
       messaging::Dispatcher<ringbuffer::Message>& disp)
     {

src/http/curl.h

@@ -61,6 +61,26 @@
 
 namespace ccf::curl
 {
+  // Returns true for libcurl transfer failures at the transport/protocol layer
+  // that are generally safe to retry: the peer may not be ready yet, a
+  // connection was dropped, or a transient HTTP/2 framing error occurred.
+  // Callers that run a retry loop (e.g. the node join client) use this to
+  // distinguish retryable transport failures from fatal TLS/certificate or
+  // application errors.
+  //
+  // This deliberately excludes CURLE_WRITE_ERROR: that indicates our own write
+  // callback rejected the response (e.g. it exceeded the caller's size cap),
+  // which is an anomalous response the caller should treat as fatal rather
+  // than retry indefinitely.
+  inline bool is_transient_transport_error(CURLcode code)
+  {
+    return code == CURLE_COULDNT_RESOLVE_PROXY ||
+      code == CURLE_COULDNT_RESOLVE_HOST || code == CURLE_COULDNT_CONNECT ||
+      code == CURLE_OPERATION_TIMEDOUT || code == CURLE_GOT_NOTHING ||
+      code == CURLE_RECV_ERROR || code == CURLE_SEND_ERROR ||
+      code == CURLE_PARTIAL_FILE || code == CURLE_WEIRD_SERVER_REPLY ||
+      code == CURLE_HTTP2 || code == CURLE_HTTP2_STREAM;
+  }
 
   class UniqueCURL
   {
@@ -238,6 +258,11 @@ namespace ccf::curl
       return bytes_to_copy;
     }
 
+    [[nodiscard]] size_t size() const
+    {
+      return unsent.size();
+    }
+
     void attach_to_curl(CURL* curl)
     {
       if (curl == nullptr)
@@ -247,7 +272,9 @@ namespace ccf::curl
       }
       CHECK_CURL_EASY_SETOPT(curl, CURLOPT_READDATA, this);
       CHECK_CURL_EASY_SETOPT(curl, CURLOPT_READFUNCTION, send_data);
-      CHECK_CURL_EASY_SETOPT(curl, CURLOPT_INFILESIZE, unsent.size());
+      // The body size is declared by the caller in a method-specific way
+      // (CURLOPT_POSTFIELDSIZE_LARGE for POST, CURLOPT_INFILESIZE_LARGE for a
+      // PUT upload), so it is intentionally not set here.
     }
   };
 
@@ -497,13 +524,39 @@ namespace ccf::curl
             request_body =
               std::make_unique<RequestBody>(std::vector<uint8_t>());
           }
+          // For an upload (PUT), declare the body size via
+          // CURLOPT_INFILESIZE_LARGE so a Content-Length is sent rather than
+          // switching to chunked transfer encoding. (POST declares its size
+          // via CURLOPT_POSTFIELDSIZE_LARGE below.)
+          CHECK_CURL_EASY_SETOPT(
+            curl_handle,
+            CURLOPT_INFILESIZE_LARGE,
+            static_cast<curl_off_t>(request_body->size()));
         }
         break;
         case HTTP_POST:
-          // libcurl sets the post verb when CURLOPT_POSTFIELDS is set, so we
-          // skip doing so here, and we assume that the user has already set
-          // these fields
-          break;
+        {
+          // CURLOPT_POST takes a long: a non-zero value (1L) selects a
+          // regular HTTP POST request.
+          // See https://curl.se/libcurl/c/CURLOPT_POST.html
+          CHECK_CURL_EASY_SETOPT(curl_handle, CURLOPT_POST, 1L);
+          if (request_body == nullptr)
+          {
+            // If no request body is provided, curl will try reading from
+            // stdin, which causes a blockage
+            request_body =
+              std::make_unique<RequestBody>(std::vector<uint8_t>());
+          }
+          // With CURLOPT_POST set and no CURLOPT_POSTFIELDS, libcurl obtains
+          // the request body from the read callback attached below. Declare
+          // the size so a Content-Length is sent rather than switching to
+          // chunked transfer encoding.
+          CHECK_CURL_EASY_SETOPT(
+            curl_handle,
+            CURLOPT_POSTFIELDSIZE_LARGE,
+            static_cast<curl_off_t>(request_body->size()));
+        }
+        break;
         default:
           throw std::logic_error(
             fmt::format("Unsupported HTTP method: {}", method.c_str()));

src/http/http2_session.h

@@ -3,7 +3,6 @@
 #pragma once
 
 #include "ds/internal_logger.h"
-#include "enclave/client_session.h"
 #include "enclave/rpc_map.h"
 #include "error_reporter.h"
 #include "http/http2_types.h"
@@ -434,72 +433,4 @@ namespace http
         ->set_on_stream_close_callback(cb);
     }
   };
-
-  class HTTP2ClientSession : public HTTP2Session,
-                             public ccf::ClientSession,
-                             public ::http::ResponseProcessor
-  {
-  private:
-    http2::ClientParser client_parser;
-
-  public:
-    HTTP2ClientSession(
-      int64_t session_id_,
-      ringbuffer::AbstractWriterFactory& writer_factory,
-      std::unique_ptr<ccf::tls::Context> ctx) :
-      HTTP2Session(session_id_, writer_factory, std::move(ctx)),
-      ccf::ClientSession(session_id_, writer_factory),
-      client_parser(*this)
-    {
-      client_parser.set_outgoing_data_handler(
-        [this](std::span<const uint8_t> data) {
-          send_data(std::vector<uint8_t>(data.begin(), data.end()));
-        });
-    }
-
-    bool parse(std::span<const uint8_t> data) override
-    {
-      // Catch response parsing errors and log them
-      try
-      {
-        client_parser.execute(data.data(), data.size());
-
-        return true;
-      }
-      catch (const std::exception& e)
-      {
-        LOG_FAIL_FMT("Error parsing HTTP2 response on session {}", session_id);
-        LOG_DEBUG_FMT("Error parsing HTTP2 response: {}", e.what());
-        LOG_DEBUG_FMT(
-          "Error occurred while parsing fragment {} byte fragment:\n{}",
-          data.size(),
-          std::string_view(
-            reinterpret_cast<char const*>(data.data()), data.size()));
-
-        close_session();
-      }
-      return false;
-    }
-
-    void send_request(http::Request&& request) override
-    {
-      client_parser.send_structured_request(
-        request.get_method(),
-        request.get_path(),
-        request.get_headers(),
-        {request.get_content_data(),
-         request.get_content_data() + request.get_content_length()});
-    }
-
-    void handle_response(
-      ccf::http_status status,
-      ccf::http::HeaderMap&& headers,
-      std::vector<uint8_t>&& body) override
-    {
-      handle_data_cb(status, std::move(headers), std::move(body));
-
-      LOG_TRACE_FMT("Closing connection, message handled");
-      close_session();
-    }
-  };
 }