diff --git a/CHANGELOG.md b/CHANGELOG.md index c1cae74bbec..28c9b863f3a 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/doc/operations/start_network.rst b/doc/operations/start_network.rst index fb38bc2512b..d17df6b6a3e 100644 --- a/doc/operations/start_network.rst +++ b/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 `. diff --git a/python/pyproject.toml b/python/pyproject.toml index d1e6734c490..3baeb3875cf 100644 --- a/python/pyproject.toml +++ b/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" }, ] diff --git a/src/enclave/client_session.h b/src/enclave/client_session.h deleted file mode 100644 index b2419719076..00000000000 --- a/src/enclave/client_session.h +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the Apache 2.0 License. -#pragma once - -#include "http/http_builder.h" -#include "tcp/msg_types.h" - -namespace ccf -{ - class ClientSession - { - public: - virtual ~ClientSession() = default; - - using HandleDataCallback = std::function&& body)>; - - using HandleErrorCallback = - std::function; - - protected: - HandleDataCallback handle_data_cb; - HandleErrorCallback handle_error_cb; - - private: - int64_t client_session_id; - ringbuffer::WriterPtr to_host; - - public: - ClientSession( - int64_t client_session_id, - ringbuffer::AbstractWriterFactory& writer_factory) : - client_session_id(client_session_id), - to_host(writer_factory.create_writer_to_outside()) - {} - - virtual void send_request(::http::Request&& request) = 0; - - virtual void connect( - const std::string& hostname, - const std::string& service, - const HandleDataCallback f, - const HandleErrorCallback e = nullptr) - { - RINGBUFFER_WRITE_MESSAGE( - ::tcp::tcp_connect, to_host, client_session_id, hostname, service); - handle_data_cb = f; - handle_error_cb = e; - } - }; -} diff --git a/src/enclave/rpc_sessions.h b/src/enclave/rpc_sessions.h index 2177b3cc05d..ec919cda804 100644 --- a/src/enclave/rpc_sessions.h +++ b/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 next_client_session_id = -1; - template 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 create_client( - const std::shared_ptr<::tls::Cert>& cert, - const std::string& app_protocol = "HTTP1") - { - std::lock_guard 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 create_unencrypted_client() - { - std::lock_guard 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& disp) { diff --git a/src/http/curl.h b/src/http/curl.h index 3998848dcd6..c060c5893c8 100644 --- a/src/http/curl.h +++ b/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(std::vector()); } + // 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(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(std::vector()); + } + // 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(request_body->size())); + } + break; default: throw std::logic_error( fmt::format("Unsupported HTTP method: {}", method.c_str())); diff --git a/src/http/http2_session.h b/src/http/http2_session.h index dac2ebd1b97..bffa330d4bf 100644 --- a/src/http/http2_session.h +++ b/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 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 data) { - send_data(std::vector(data.begin(), data.end())); - }); - } - - bool parse(std::span 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(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&& body) override - { - handle_data_cb(status, std::move(headers), std::move(body)); - - LOG_TRACE_FMT("Closing connection, message handled"); - close_session(); - } - }; } diff --git a/src/http/http_session.h b/src/http/http_session.h index 143beb3636c..0b22cd89096 100644 --- a/src/http/http_session.h +++ b/src/http/http_session.h @@ -3,7 +3,6 @@ #pragma once #include "ds/internal_logger.h" -#include "enclave/client_session.h" #include "enclave/rpc_handler.h" #include "enclave/rpc_map.h" #include "error_reporter.h" @@ -303,149 +302,4 @@ namespace http std::move(body)); } }; - - class HTTPClientSession : public HTTPSession, - public ccf::ClientSession, - public ::http::ResponseProcessor - { - private: - ::http::ResponseParser response_parser; - - public: - HTTPClientSession( - ::tcp::ConnID session_id_, - ringbuffer::AbstractWriterFactory& writer_factory, - std::unique_ptr ctx) : - HTTPSession(session_id_, writer_factory, std::move(ctx)), - ClientSession(session_id_, writer_factory), - response_parser(*this) - {} - - bool parse(std::span data) override - { - // Catch response parsing errors and log them - try - { - response_parser.execute(data.data(), data.size()); - - return true; - } - catch (const std::exception& e) - { - LOG_FAIL_FMT("Error parsing HTTP response on session {}", session_id); - LOG_DEBUG_FMT("Error parsing HTTP response: {}", e.what()); - LOG_DEBUG_FMT( - "Error occurred while parsing fragment {} byte fragment:\n{}", - data.size(), - std::string_view( - reinterpret_cast(data.data()), data.size())); - - close_session(); - } - return false; - } - - void send_request(http::Request&& request) override - { - auto data = request.build_request(); - send_data(std::move(data)); - } - - void connect( - const std::string& hostname, - const std::string& service, - const HandleDataCallback f, - const HandleErrorCallback e) override - { - tls_io->set_handshake_error_cb([e](std::string&& error_msg) { - if (e) - { - e(error_msg); - } - else - { - LOG_FAIL_FMT("{}", error_msg); - } - }); - - ccf::ClientSession::connect(hostname, service, f, e); - } - - void handle_response( - ccf::http_status status, - ccf::http::HeaderMap&& headers, - std::vector&& body) override - { - handle_data_cb(status, std::move(headers), std::move(body)); - - LOG_TRACE_FMT("Closing connection, message handled"); - close_session(); - } - }; - - using UnencryptedHTTPSession = ccf::UnencryptedSession; - - class UnencryptedHTTPClientSession : public UnencryptedHTTPSession, - public ccf::ClientSession, - public ::http::ResponseProcessor - { - private: - ::http::ResponseParser response_parser; - - public: - UnencryptedHTTPClientSession( - ::tcp::ConnID session_id_, - ringbuffer::AbstractWriterFactory& writer_factory) : - UnencryptedHTTPSession(session_id_, writer_factory), - ClientSession(session_id_, writer_factory), - response_parser(*this) - {} - - bool parse(std::span data) override - { - try - { - response_parser.execute(data.data(), data.size()); - return true; - } - catch (const std::exception& e) - { - LOG_FAIL_FMT("Error parsing HTTP response on session {}", session_id); - LOG_DEBUG_FMT("Error parsing HTTP response: {}", e.what()); - LOG_DEBUG_FMT( - "Error occurred while parsing fragment {} byte fragment:\n{}", - data.size(), - std::string_view( - reinterpret_cast(data.data()), data.size())); - - close_session(); - } - return false; - } - - void send_request(http::Request&& request) override - { - auto data = request.build_request(); - send_data(std::move(data)); - } - - void connect( - const std::string& hostname, - const std::string& service, - const HandleDataCallback f, - const HandleErrorCallback e) override - { - ccf::ClientSession::connect(hostname, service, f, e); - } - - void handle_response( - ccf::http_status status, - ccf::http::HeaderMap&& headers, - std::vector&& body) override - { - handle_data_cb(status, std::move(headers), std::move(body)); - LOG_TRACE_FMT("Closing connection, message handled"); - close_session(); - } - }; } diff --git a/src/http/test/curl_test.cpp b/src/http/test/curl_test.cpp index 76343cb4908..53a5e85967c 100644 --- a/src/http/test/curl_test.cpp +++ b/src/http/test/curl_test.cpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -33,6 +34,52 @@ struct Data DECLARE_JSON_TYPE(Data); DECLARE_JSON_REQUIRED_FIELDS(Data, foo, bar, iter); +TEST_CASE("is_transient_transport_error classifies curl errors") +{ + // Transport/protocol-layer failures that a retry loop (e.g. the node join + // client) should retry rather than treat as fatal. + const std::vector transient = { + CURLE_COULDNT_RESOLVE_PROXY, + CURLE_COULDNT_RESOLVE_HOST, + CURLE_COULDNT_CONNECT, + CURLE_OPERATION_TIMEDOUT, + CURLE_GOT_NOTHING, + CURLE_RECV_ERROR, + CURLE_SEND_ERROR, + CURLE_PARTIAL_FILE, + CURLE_WEIRD_SERVER_REPLY, + CURLE_HTTP2, + CURLE_HTTP2_STREAM, + }; + for (const auto code : transient) + { + INFO("code = " << static_cast(code)); + CHECK(ccf::curl::is_transient_transport_error(code)); + } + + // Errors that must be treated as fatal (never retried): TLS/certificate + // failures, application-level errors, and our own response size-cap + // rejection (CURLE_WRITE_ERROR). CURLE_OK and CURLE_ABORTED_BY_CALLBACK are + // not transport errors either. + const std::vector fatal = { + CURLE_OK, + CURLE_PEER_FAILED_VERIFICATION, + CURLE_SSL_CACERT_BADFILE, + CURLE_SSL_CONNECT_ERROR, + CURLE_SSL_CERTPROBLEM, + CURLE_USE_SSL_FAILED, + CURLE_WRITE_ERROR, + CURLE_TOO_MANY_REDIRECTS, + CURLE_UNSUPPORTED_PROTOCOL, + CURLE_ABORTED_BY_CALLBACK, + }; + for (const auto code : fatal) + { + INFO("code = " << static_cast(code)); + CHECK_FALSE(ccf::curl::is_transient_transport_error(code)); + } +} + TEST_CASE("ResponseHeaders rejects oversized headers") { ccf::curl::ResponseHeaders headers; @@ -174,6 +221,163 @@ TEST_CASE("Synchronous") REQUIRE(response_count == sync_number_requests); } +TEST_CASE("Synchronous POST echoes body") +{ + // Exercises POST-with-body support in the curl wrapper: the echo server + // reflects the request method and body, so we can assert that both were + // transmitted correctly. + const std::string sent_body = R"({"message":"join","iter":42})"; + std::vector body_bytes(sent_body.begin(), sent_body.end()); + auto body = std::make_unique(std::move(body_bytes)); + + auto headers = ccf::curl::UniqueSlist(); + headers.append("Content-Type", "application/json"); + + auto curl_handle = ccf::curl::UniqueCURL(); + std::string url = fmt::format("http://{}/join", server_address); + + CURLcode curl_code = CURLE_FAILED_INIT; + long status_code = 0; + std::string response_body; + + auto response = [&curl_code, &status_code, &response_body]( + std::unique_ptr&& request, + CURLcode curl_response, + long status) { + curl_code = curl_response; + status_code = status; + auto* rb = request->get_response_body(); + response_body = std::string(rb->buffer.begin(), rb->buffer.end()); + }; + + auto request = std::make_unique( + std::move(curl_handle), + HTTP_POST, + std::move(url), + std::move(headers), + std::move(body), + std::make_unique(SIZE_MAX), + response); + + ccf::curl::CurlRequest::synchronous_perform(std::move(request)); + + constexpr long HTTP_SUCCESS = 200; + REQUIRE(curl_code == CURLE_OK); + REQUIRE(status_code == HTTP_SUCCESS); + + const auto parsed = nlohmann::json::parse(response_body); + REQUIRE(parsed.at("metadata").at("method") == "POST"); + REQUIRE(parsed.at("body") == sent_body); +} + +TEST_CASE("VERIFYHOST rejects a certificate SAN mismatch") +{ + // Guards the TLS hostname-verification hardening the node join client relies + // on: with the same pinned CA and the same connection, + // CURLOPT_SSL_VERIFYHOST == 2 must reject a certificate whose SAN does not + // cover the dialed host, and accept one that does. Requires the HTTPS test + // server started by e2e_curl.py (self-signed cert with a single dNSName SAN, + // served on the loopback IP). + const char* tls_addr_env = std::getenv("TLS_SERVER_ADDR"); + const char* tls_san_env = std::getenv("TLS_SERVER_SAN"); + const char* tls_ca_env = std::getenv("TLS_SERVER_CA"); + if ( + tls_addr_env == nullptr || tls_san_env == nullptr || tls_ca_env == nullptr) + { + MESSAGE("Skipping: TLS_SERVER_* env not set (run via e2e_curl.py)"); + return; + } + + const std::string tls_addr = tls_addr_env; + const std::string tls_san = tls_san_env; + + std::string ca_pem; + { + std::ifstream ca_file(tls_ca_env, std::ios::binary); + REQUIRE(ca_file.good()); + ca_pem.assign( + std::istreambuf_iterator(ca_file), + std::istreambuf_iterator()); + } + REQUIRE(!ca_pem.empty()); + + // TLS_SERVER_ADDR is ":". + const auto colon = tls_addr.rfind(':'); + REQUIRE(colon != std::string::npos); + const std::string tls_host = tls_addr.substr(0, colon); + const std::string tls_port = tls_addr.substr(colon + 1); + + auto perform_get = [&]( + const std::string& url, + long verifyhost, + const std::optional& resolve_entry) { + auto curl_handle = ccf::curl::UniqueCURL(); + curl_handle.set_opt(CURLOPT_SSL_VERIFYPEER, 1L); + curl_handle.set_opt(CURLOPT_SSL_VERIFYHOST, verifyhost); + curl_handle.set_opt(CURLOPT_PROTOCOLS_STR, "https"); + curl_handle.set_blob_opt( + CURLOPT_CAINFO_BLOB, + reinterpret_cast(ca_pem.data()), + ca_pem.size()); + curl_handle.set_opt(CURLOPT_CAPATH, nullptr); + curl_handle.set_opt(CURLOPT_CONNECTTIMEOUT, 5L); + curl_handle.set_opt(CURLOPT_TIMEOUT, 10L); + + auto resolve = ccf::curl::UniqueSlist(); + if (resolve_entry.has_value()) + { + resolve.append(resolve_entry->c_str()); + curl_handle.set_opt(CURLOPT_RESOLVE, resolve.get()); + } + + CURLcode result = CURLE_FAILED_INIT; + auto callback = [&result]( + std::unique_ptr&& /*request*/, + CURLcode curl_response, + long /*status*/) { result = curl_response; }; + + ccf::curl::CurlRequest::synchronous_perform( + std::make_unique( + std::move(curl_handle), + HTTP_GET, + url, + ccf::curl::UniqueSlist(), + nullptr, + std::make_unique(SIZE_MAX), + callback)); + return result; + }; + + SUBCASE("VERIFYHOST=2 rejects a host absent from the certificate SAN") + { + // The certificate's only SAN is a dNSName, so dialing the loopback IP + // directly must fail hostname verification. + const auto result = + perform_get(fmt::format("https://{}/", tls_addr), 2L, std::nullopt); + REQUIRE(result == CURLE_PEER_FAILED_VERIFICATION); + } + + SUBCASE("VERIFYHOST=2 accepts the certificate SAN host") + { + // Dial the SAN name, resolved to the server's loopback address. + const auto result = perform_get( + fmt::format("https://{}:{}/", tls_san, tls_port), + 2L, + fmt::format("{}:{}:{}", tls_san, tls_port, tls_host)); + REQUIRE(result == CURLE_OK); + } + + SUBCASE("VERIFYHOST=0 accepts the mismatched host (control)") + { + // With hostname verification disabled the same mismatched connection + // succeeds, proving the CA/cert/connection are otherwise valid and that + // the hostname check is the sole discriminator. + const auto result = + perform_get(fmt::format("https://{}/", tls_addr), 0L, std::nullopt); + REQUIRE(result == CURLE_OK); + } +} + TEST_CASE("CurlmLibuvContext") { size_t response_count = 0; diff --git a/src/node/node_state.h b/src/node/node_state.h index a6587897aa3..2d7f2f84dee 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -30,6 +30,7 @@ #include "enclave/rpc_sessions.h" #include "encryptor.h" #include "history.h" +#include "http/curl.h" #include "http/http_parser.h" #include "indexing/indexer.h" #include "js/global_class_ids.h" @@ -478,6 +479,16 @@ namespace ccf ccf::tasks::Task snapshot_fetch_task; ccf::tasks::Task backup_snapshot_fetch_task; + // Set while a join request is in flight so the periodic join timer does + // not issue overlapping requests. The shared CurlmLibuvContextSingleton is + // also used by other clients (JWT refresh, endorsements, snapshot fetch); + // because config.join.retry_timeout (default 1s) is far shorter than the + // per-attempt timeout (CONNECTTIMEOUT 5s / TIMEOUT 60s), unguarded retries + // could accumulate in-flight requests and starve those other users. Reset + // atomically from the response callback, which runs on the libuv thread + // and so must not take NodeState::lock. + std::atomic join_request_in_flight = false; + // Number of times we have fetched the latest snapshot from the primary size_t join_fetch_count = 0; @@ -1050,339 +1061,522 @@ namespace ccf { sm.expect(NodeStartupState::pending); - auto network_ca = std::make_shared<::tls::CA>(std::string( - config.join.service_cert.begin(), config.join.service_cert.end())); + // Only allow a single join request to be in flight at a time. The + // periodic join timer fires every config.join.retry_timeout (default + // 1s), but a single attempt can remain in flight for much longer (up to + // CONNECTTIMEOUT/TIMEOUT). Without this gate, a slow or unresponsive + // target would cause join requests to pile up on the shared curl + // singleton, starving its other users. The flag is cleared when the + // request completes (see the response callback below). + if (join_request_in_flight.load()) + { + LOG_DEBUG_FMT( + "A join request to {} is already in flight; skipping this retry", + config.join.target_rpc_address); + return; + } - auto [target_host, target_port] = - split_net_address(config.join.target_rpc_address); + // Assemble the join request body. + JoinNetworkNodeToNode::In join_params; + join_params.node_info_network = config.network; + join_params.public_encryption_key = node_encrypt_kp->public_key_pem(); + join_params.quote_info = quote_info; + join_params.startup_seqno = startup_seqno; + if (config.join.fetch_recent_snapshot) + { + join_params.join_fetch_count = join_fetch_count; + } + else + { + join_params.join_fetch_count = 1; + } + join_params.certificate_signing_request = node_sign_kp->create_csr( + config.node_certificate.subject_name, subject_alt_names); + join_params.node_data = config.node_data; + join_params.ledger_sign_mode = ccf::get_ledger_sign_mode(); + if (config.sealing_recovery.has_value() && snp_tcb_version.has_value()) + { + join_params.sealing_recovery_data = std::make_pair( + sealing::get_snp_sealed_recovery_key(snp_tcb_version.value()), + config.sealing_recovery->location.name); + } + if (config.join.host_data_transparent_statement_path.has_value()) + { + LOG_INFO_FMT( + "Reading code_transparent_statement from file: {}", + config.join.host_data_transparent_statement_path.value()); + auto ts = files::slurp( + config.join.host_data_transparent_statement_path.value()); + join_params.code_transparent_statement = std::move(ts); + } - auto join_client_cert = std::make_unique<::tls::Cert>( - network_ca, - self_signed_node_cert, - node_sign_kp->private_key_pem(), - target_host); - - // Create RPC client and connect to remote node - // Note: For now, assume that target node accepts same application - // protocol as this node's main RPC interface - auto join_client = rpcsessions->create_client( - std::move(join_client_cert), - rpcsessions->get_app_protocol_main_interface()); - - join_client->connect( - target_host, - target_port, - // Capture target_address by value, and use them when - // logging about this response. Do not use config target address, which - // may have updated in the interim. + LOG_DEBUG_FMT( + "Sending join request to {}", config.join.target_rpc_address); + const auto body = nlohmann::json(join_params).dump(); + LOG_DEBUG_FMT("Sending join request body: {}", body); + + // The service certificate is the sole trust anchor for the join + // connection. CURLOPT_CAINFO_BLOB installs it and CURLOPT_CAPATH=nullptr + // prevents any fallback to the system CA store, so the set of accepted + // certificate authorities is identical to the legacy tls::CA path. The + // joining node presents its self-signed node certificate for mutual TLS + // (it is not yet endorsed at join time). CURLOPT_SSL_VERIFYHOST=2 + // additionally checks that the target certificate matches the address we + // connected to. + ccf::curl::UniqueCURL curl_handle; + curl_handle.set_opt(CURLOPT_SSL_VERIFYPEER, 1L); + curl_handle.set_opt(CURLOPT_SSL_VERIFYHOST, 2L); + curl_handle.set_opt(CURLOPT_PROTOCOLS_STR, "https"); + curl_handle.set_blob_opt( + CURLOPT_CAINFO_BLOB, + config.join.service_cert.data(), + config.join.service_cert.size()); + curl_handle.set_opt(CURLOPT_CAPATH, nullptr); + + // Bound each attempt so a stalled connection is eventually abandoned, + // releasing the single-in-flight gate above so the periodic join timer + // can issue a fresh attempt. A timeout surfaces as a transient error and + // is retried. + curl_handle.set_opt(CURLOPT_CONNECTTIMEOUT, 5L); + curl_handle.set_opt(CURLOPT_TIMEOUT, 60L); + + const auto client_key_pem = node_sign_kp->private_key_pem(); + curl_handle.set_blob_opt( + CURLOPT_SSLCERT_BLOB, + self_signed_node_cert.data(), + self_signed_node_cert.size()); + curl_handle.set_opt(CURLOPT_SSLCERTTYPE, "PEM"); + curl_handle.set_blob_opt( + CURLOPT_SSLKEY_BLOB, client_key_pem.data(), client_key_pem.size()); + curl_handle.set_opt(CURLOPT_SSLKEYTYPE, "PEM"); + + ccf::curl::UniqueSlist request_headers; + request_headers.append( + http::headers::CONTENT_TYPE, http::headervalues::contenttype::JSON); + + const auto url = fmt::format( + "https://{}/{}/{}", + config.join.target_rpc_address, + get_actor_prefix(ActorsType::nodes), + "join"); + + auto request_body = std::make_unique( + std::vector(body.begin(), body.end())); + + // Generous cap on the join response body (service identity, endorsed + // node certificate, and full ledger-secret history). Bounds memory use + // without risking rejection of a legitimate response. + static constexpr size_t max_join_response_size = 100UL * 1024 * 1024; + + // Capture target_address by value, and use it when logging about this + // response. Do not use the config target address, which may have been + // updated by a redirect in the interim. + // NOLINTBEGIN(readability-function-cognitive-complexity) + ccf::curl::CurlRequest::ResponseCallback response_callback = [this, target_address = config.join.target_rpc_address]( - ccf::http_status status, - http::HeaderMap&& headers, - std::vector&& data) { - std::lock_guard guard(lock); - if (!sm.check(NodeStartupState::pending)) + std::unique_ptr&& request, + CURLcode curl_response, + long status_code) { + // The request has completed (with a response, a transport error, or + // an abort during shutdown), so it is no longer in flight. Clear the + // gate here, before any early return, so the periodic join timer can + // issue the next attempt. This runs on the libuv thread and so must + // not take NodeState::lock; an atomic store is used instead. Join + // liveness depends on the curl singleton invoking this callback + // exactly once per attached request. + join_request_in_flight.store(false); + + if (curl_response == CURLE_ABORTED_BY_CALLBACK) { + // Aborted, e.g. during node shutdown. Nothing to process, and + // the task board may be stopping, so do not schedule a task. return; } - if (is_http_status_client_error(status)) - { - std::optional error_response = - std::nullopt; - - try + // This callback runs on the shared host libuv loop thread. Keep it + // minimal: capture the response and defer all node-state processing + // to a task, matching the JWT refresh client. That processing can + // deserialise a large snapshot and acquires NodeState::lock; + // running it on the libuv thread would stall every other user of + // the shared curl loop and risk a ringbuffer back-pressure deadlock + // (the blocking host writer is drained on this same thread). + // NodeState outlives the curl singleton and the task board (both + // are torn down during enclave shutdown, before NodeState is + // destroyed), so capturing raw `this` is safe. + auto response_headers = + std::make_shared( + request->get_response_headers()); + auto response_body = std::make_shared>( + request->get_response_body() != nullptr ? + std::move(request->get_response_body()->buffer) : + std::vector{}); + + ccf::tasks::add_task(ccf::tasks::make_basic_task([this, + target_address, + curl_response, + status_code, + response_headers, + response_body]() { + std::lock_guard guard(lock); + if (!sm.check(NodeStartupState::pending)) { - auto j = ccf::parse_json_safe(data); - error_response = j.get(); - } - catch (const ccf::JsonParseError& e) - { - LOG_FAIL_FMT( - "Join request returned {}, body exceeds permitted JSON nesting " - "depth: {}", - status, - e.what()); - } - catch (const nlohmann::json::exception& e) - { - // Leave error_response == nullopt - LOG_FAIL_FMT( - "Join request returned {}, body is not ODataErrorResponse: {}", - status, - std::string(data.begin(), data.end())); + return; } - if ( - error_response.has_value() && - error_response->error.code == ccf::errors::StartupSeqnoIsOld && - config.join.fetch_recent_snapshot) + try { - LOG_INFO_FMT( - "Join request to {} returned {} error. Attempting to fetch " - "fresher snapshot", - target_address, - ccf::errors::StartupSeqnoIsOld); - - // If we've followed a redirect, it will have been updated in - // config.join. Note that this is fire-and-forget, it is assumed - // that it proceeds in the background, updating state when it - // completes, and the join timer separately re-attempts join after - // this succeeds - if ( - snapshot_fetch_task != nullptr && - !snapshot_fetch_task->is_cancelled()) + if (curl_response != CURLE_OK) { - LOG_INFO_FMT("Snapshot fetch already in progress, skipping"); + // The legacy httpclient path silently dropped a failed + // connection and relied on the periodic join timer to retry + // when the target could not yet be reached, while treating TLS + // handshake failures (e.g. an untrusted service certificate) as + // fatal. Preserve both behaviours: transient transport errors + // are retried, everything else is fatal. + if (ccf::curl::is_transient_transport_error(curl_response)) + { + LOG_INFO_FMT( + "Transient error contacting {} to join: {} ({}). The join " + "timer will retry.", + target_address, + curl_easy_strerror(curl_response), + static_cast(curl_response)); + return; + } + + // CURLE_WRITE_ERROR here means our own write callback rejected + // the response body, which for the join can only be the body + // exceeding max_join_response_size. Surface a clear, actionable + // message rather than curl's generic "write error". + if (curl_response == CURLE_WRITE_ERROR) + { + auto error_msg = fmt::format( + "Join response from {} exceeded the maximum permitted size " + "of {} bytes. Shutting down node gracefully...", + target_address, + max_join_response_size); + LOG_FAIL_FMT("{}", error_msg); + RINGBUFFER_WRITE_MESSAGE( + AdminMessage::fatal_error_msg, to_host, error_msg); + return; + } + + // Fatal TLS/protocol-layer failure. Certificate trust could + // not be established: either the peer certificate failed + // verification (an untrusted or expired service certificate, + // a hostname/SAN mismatch under VERIFYHOST=2, or any other + // peer verification failure), or the configured service + // certificate could not be loaded (CURLE_SSL_CACERT_BADFILE). + // Flag these with a stable marker so they can be told apart + // from other fatal errors in logs and tests. + const bool tls_certificate_trust_check_failed = + curl_response == CURLE_PEER_FAILED_VERIFICATION || + curl_response == CURLE_SSL_CACERT_BADFILE; + auto error_msg = fmt::format( + "Early error when joining existing network at {}: {}{} ({}). " + "Shutting down node gracefully...", + target_address, + tls_certificate_trust_check_failed ? + "TLS certificate trust check failed: " : + "", + curl_easy_strerror(curl_response), + static_cast(curl_response)); + LOG_FAIL_FMT("{}", error_msg); + RINGBUFFER_WRITE_MESSAGE( + AdminMessage::fatal_error_msg, to_host, error_msg); + return; } - else + + const auto status = static_cast(status_code); + const auto& headers = *response_headers; + const auto& data = *response_body; + + if (is_http_status_client_error(status)) { - snapshot_fetch_task = std::make_shared( - config.join, config.snapshots, this); - ccf::tasks::add_task(snapshot_fetch_task); - } - return; - } + std::optional error_response = + std::nullopt; - auto error_msg = fmt::format( - "Join request to {} returned {} Bad Request: {}. Shutting " - "down node gracefully.", - target_address, - status, - std::string(data.begin(), data.end())); - LOG_FAIL_FMT("{}", error_msg); - RINGBUFFER_WRITE_MESSAGE( - AdminMessage::fatal_error_msg, to_host, error_msg); - return; - } + try + { + auto j = ccf::parse_json_safe(data); + error_response = j.get(); + } + catch (const ccf::JsonParseError& e) + { + LOG_FAIL_FMT( + "Join request returned {}, body exceeds permitted JSON " + "nesting " + "depth: {}", + status, + e.what()); + } + catch (const nlohmann::json::exception& e) + { + // Leave error_response == nullopt + LOG_FAIL_FMT( + "Join request returned {}, body is not ODataErrorResponse: " + "{}", + status, + std::string(data.begin(), data.end())); + } - if (status != HTTP_STATUS_OK) - { - const auto& location = headers.find(http::headers::LOCATION); - if ( - config.join.follow_redirect && - (status == HTTP_STATUS_PERMANENT_REDIRECT || - status == HTTP_STATUS_TEMPORARY_REDIRECT) && - location != headers.end()) - { - const auto& url = ::http::parse_url_full(location->second); - config.join.target_rpc_address = - make_net_address(url.host, url.port); - LOG_INFO_FMT("Target node redirected to {}", location->second); - } - else - { - LOG_FAIL_FMT( - "An error occurred while joining the network: {} {}{}", - status, - ccf::http_status_str(status), - data.empty() ? - "" : - fmt::format(" '{}'", std::string(data.begin(), data.end()))); - } - return; - } + if ( + error_response.has_value() && + error_response->error.code == + ccf::errors::StartupSeqnoIsOld && + config.join.fetch_recent_snapshot) + { + LOG_INFO_FMT( + "Join request to {} returned {} error. Attempting to fetch " + "fresher snapshot", + target_address, + ccf::errors::StartupSeqnoIsOld); + + // If we've followed a redirect, it will have been updated in + // config.join. Note that this is fire-and-forget, it is + // assumed that it proceeds in the background, updating state + // when it completes, and the join timer separately + // re-attempts join after this succeeds + if ( + snapshot_fetch_task != nullptr && + !snapshot_fetch_task->is_cancelled()) + { + LOG_INFO_FMT( + "Snapshot fetch already in progress, skipping"); + } + else + { + snapshot_fetch_task = std::make_shared( + config.join, config.snapshots, this); + ccf::tasks::add_task(snapshot_fetch_task); + } + return; + } - JoinNetworkNodeToNode::Out resp; - try - { - auto j = ccf::parse_json_safe(data); - resp = j.get(); - } - catch (const std::exception& e) - { - LOG_FAIL_FMT( - "An error occurred while parsing the join network response"); + auto error_msg = fmt::format( + "Join request to {} returned {} Bad Request: {}. Shutting " + "down node gracefully.", + target_address, + status, + std::string(data.begin(), data.end())); + LOG_FAIL_FMT("{}", error_msg); + RINGBUFFER_WRITE_MESSAGE( + AdminMessage::fatal_error_msg, to_host, error_msg); + return; + } + + if (status != HTTP_STATUS_OK) + { + const auto& location = headers.find(http::headers::LOCATION); + if ( + config.join.follow_redirect && + (status == HTTP_STATUS_PERMANENT_REDIRECT || + status == HTTP_STATUS_TEMPORARY_REDIRECT) && + location != headers.end()) + { + const auto& url = ::http::parse_url_full(location->second); + config.join.target_rpc_address = + make_net_address(url.host, url.port); + LOG_INFO_FMT( + "Target node redirected to {}", location->second); + } + else + { + LOG_FAIL_FMT( + "An error occurred while joining the network: {} {}{}", + status, + ccf::http_status_str(status), + data.empty() ? + "" : + fmt::format( + " '{}'", std::string(data.begin(), data.end()))); + } + return; + } - LOG_DEBUG_FMT("Join network response error: {}", e.what()); - LOG_DEBUG_FMT( - "Join network response body: {}", - std::string(data.begin(), data.end())); + JoinNetworkNodeToNode::Out resp; + try + { + auto j = ccf::parse_json_safe(data); + resp = j.get(); + } + catch (const std::exception& e) + { + LOG_FAIL_FMT( + "An error occurred while parsing the join network response"); - return; - } + LOG_DEBUG_FMT("Join network response error: {}", e.what()); + LOG_DEBUG_FMT( + "Join network response body: {}", + std::string(data.begin(), data.end())); - // Set network secrets, node id and become part of network. - if (resp.node_status == NodeStatus::TRUSTED) - { - if (!resp.network_info.has_value()) - { - throw std::logic_error("Expected network info in join response"); - } + return; + } - network.identity = std::make_unique( - resp.network_info->identity); - network.ledger_secrets->init_from_map( - std::move(resp.network_info->ledger_secrets)); + // Set network secrets, node id and become part of network. + if (resp.node_status == NodeStatus::TRUSTED) + { + if (!resp.network_info.has_value()) + { + throw std::logic_error( + "Expected network info in join response"); + } - history->set_service_signing_identity( - network.identity->get_key_pair(), - resp.network_info->cose_signatures_config.value_or( - ccf::COSESignaturesConfig{})); + network.identity = std::make_unique( + resp.network_info->identity); + network.ledger_secrets->init_from_map( + std::move(resp.network_info->ledger_secrets)); - ccf::crypto::Pem n2n_channels_cert; - if (!resp.network_info->endorsed_certificate.has_value()) - { - // Endorsed certificate was added to join response in 2.x - throw std::logic_error( - "Expected endorsed certificate in join response"); - } - n2n_channels_cert = resp.network_info->endorsed_certificate.value(); + history->set_service_signing_identity( + network.identity->get_key_pair(), + resp.network_info->cose_signatures_config.value_or( + ccf::COSESignaturesConfig{})); - setup_consensus(resp.network_info->public_only, n2n_channels_cert); - auto_refresh_jwt_keys(); + ccf::crypto::Pem n2n_channels_cert; + if (!resp.network_info->endorsed_certificate.has_value()) + { + // Endorsed certificate was added to join response in 2.x + throw std::logic_error( + "Expected endorsed certificate in join response"); + } + n2n_channels_cert = + resp.network_info->endorsed_certificate.value(); - if (resp.network_info->public_only) - { - last_recovered_signed_idx = - resp.network_info->last_recovered_signed_idx; - setup_recovery_hook(); - snapshotter->set_snapshot_generation(false); - } + setup_consensus( + resp.network_info->public_only, n2n_channels_cert); + auto_refresh_jwt_keys(); - View view = VIEW_UNKNOWN; - std::vector view_history_ = {}; - if (startup_snapshot_info) - { - // It is only possible to deserialise the entire snapshot now, - // once the ledger secrets have been passed in by the network - ccf::kv::ConsensusHookPtrs hooks; - deserialise_snapshot( - network.tables, - startup_snapshot_info->raw, - hooks, - &view_history_, - resp.network_info->public_only); - - for (auto& hook : hooks) - { - hook->call(consensus.get()); - } + if (resp.network_info->public_only) + { + last_recovered_signed_idx = + resp.network_info->last_recovered_signed_idx; + setup_recovery_hook(); + snapshotter->set_snapshot_generation(false); + } - auto tx = network.tables->create_read_only_tx(); - view = resolve_latest_sig_view(tx); + View view = VIEW_UNKNOWN; + std::vector view_history_ = {}; + if (startup_snapshot_info) + { + // It is only possible to deserialise the entire snapshot now, + // once the ledger secrets have been passed in by the network + ccf::kv::ConsensusHookPtrs hooks; + deserialise_snapshot( + network.tables, + startup_snapshot_info->raw, + hooks, + &view_history_, + resp.network_info->public_only); + + for (auto& hook : hooks) + { + hook->call(consensus.get()); + } + + auto tx = network.tables->create_read_only_tx(); + view = resolve_latest_sig_view(tx); + + if (!resp.network_info->public_only) + { + // Only clear snapshot if not recovering. When joining the + // public network the snapshot is used later to initialise + // the recovery store + startup_snapshot_info.reset(); + } + + LOG_INFO_FMT( + "Joiner successfully resumed from snapshot at seqno {} and " + "view {}", + network.tables->current_version(), + view); + } - if (!resp.network_info->public_only) - { - // Only clear snapshot if not recovering. When joining the - // public network the snapshot is used later to initialise the - // recovery store - startup_snapshot_info.reset(); - } + consensus->init_as_backup( + network.tables->current_version(), + view, + view_history_, + last_recovered_signed_idx); - LOG_INFO_FMT( - "Joiner successfully resumed from snapshot at seqno {} and " - "view {}", - network.tables->current_version(), - view); - } + { + auto snap_tx = network.tables->create_read_only_tx(); + auto snapshot_status = + snap_tx.ro(Tables::SNAPSHOT_STATUS) + ->get(); + if (snapshot_status.has_value()) + { + snapshotter->init_from_snapshot_status( + snapshot_status.value()); + } + } + history->start_signature_emit_timer(); - consensus->init_as_backup( - network.tables->current_version(), - view, - view_history_, - last_recovered_signed_idx); + if (resp.network_info->public_only) + { + sm.advance(NodeStartupState::partOfPublicNetwork); + } + else + { + reset_data(quote_info.quote); + reset_data(quote_info.endorsements); + sm.advance(NodeStartupState::partOfNetwork); + } - { - auto snap_tx = network.tables->create_read_only_tx(); - auto snapshot_status = - snap_tx.ro(Tables::SNAPSHOT_STATUS)->get(); - if (snapshot_status.has_value()) + if (join_periodic_task != nullptr) + { + join_periodic_task->cancel_task(); + join_periodic_task = nullptr; + } + + LOG_INFO_FMT( + "Node has now joined the network as node {}: {}", + self, + (resp.network_info->public_only ? "public only" : + "all domains")); + } + else if (resp.node_status == NodeStatus::PENDING) { - snapshotter->init_from_snapshot_status(snapshot_status.value()); + LOG_INFO_FMT( + "Node {} is waiting for votes of members to be trusted", + self); } } - history->start_signature_emit_timer(); - - if (resp.network_info->public_only) - { - sm.advance(NodeStartupState::partOfPublicNetwork); - } - else - { - reset_data(quote_info.quote); - reset_data(quote_info.endorsements); - sm.advance(NodeStartupState::partOfNetwork); - } - - if (join_periodic_task != nullptr) + catch (const std::exception& e) { - join_periodic_task->cancel_task(); - join_periodic_task = nullptr; + LOG_FAIL_FMT( + "Unhandled error while processing join response from {}: {}", + target_address, + e.what()); } + })); + }; + // NOLINTEND(readability-function-cognitive-complexity) - LOG_INFO_FMT( - "Node has now joined the network as node {}: {}", - self, - (resp.network_info->public_only ? "public only" : "all domains")); - } - else if (resp.node_status == NodeStatus::PENDING) - { - LOG_INFO_FMT( - "Node {} is waiting for votes of members to be trusted", self); - } - }, - [this](const std::string& error_msg) { - std::lock_guard guard(lock); - auto long_error_msg = fmt::format( - "Early error when joining existing network at {}: {}. Shutting " - "down node gracefully...", - config.join.target_rpc_address, - error_msg); - LOG_FAIL_FMT("{}", long_error_msg); - RINGBUFFER_WRITE_MESSAGE( - AdminMessage::fatal_error_msg, to_host, long_error_msg); - }); - - // Send RPC request to remote node to join the network. - JoinNetworkNodeToNode::In join_params; + auto join_request = std::make_unique( + std::move(curl_handle), + HTTP_POST, + url, + std::move(request_headers), + std::move(request_body), + std::make_unique(max_join_response_size), + std::move(response_callback)); - join_params.node_info_network = config.network; - join_params.public_encryption_key = node_encrypt_kp->public_key_pem(); - join_params.quote_info = quote_info; - join_params.startup_seqno = startup_seqno; - if (config.join.fetch_recent_snapshot) - { - join_params.join_fetch_count = join_fetch_count; - } - else - { - join_params.join_fetch_count = 1; - } - join_params.certificate_signing_request = node_sign_kp->create_csr( - config.node_certificate.subject_name, subject_alt_names); - join_params.node_data = config.node_data; - join_params.ledger_sign_mode = ccf::get_ledger_sign_mode(); - if (config.sealing_recovery.has_value() && snp_tcb_version.has_value()) + // Mark a request as in flight before handing it to the shared curl + // singleton. If attach_request aborts synchronously (e.g. the singleton + // is shutting down) the response callback runs inline and clears this + // again. If attach_request instead throws (e.g. bad_alloc) the callback + // never runs, so reset the flag here to avoid gating out every future + // retry and stranding the node in pending. + join_request_in_flight.store(true); + try { - join_params.sealing_recovery_data = std::make_pair( - sealing::get_snp_sealed_recovery_key(snp_tcb_version.value()), - config.sealing_recovery->location.name); + ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request( + std::move(join_request)); } - - if (config.join.host_data_transparent_statement_path.has_value()) + catch (...) { - LOG_INFO_FMT( - "Reading code_transparent_statement from file: {}", - config.join.host_data_transparent_statement_path.value()); - auto ts = files::slurp( - config.join.host_data_transparent_statement_path.value()); - join_params.code_transparent_statement = std::move(ts); + join_request_in_flight.store(false); + throw; } - - LOG_DEBUG_FMT( - "Sending join request to {}", config.join.target_rpc_address); - - const auto body = nlohmann::json(join_params).dump(); - - LOG_DEBUG_FMT("Sending join request body: {}", body); - - ::http::Request r( - fmt::format("/{}/{}", get_actor_prefix(ActorsType::nodes), "join")); - r.set_header( - http::headers::CONTENT_TYPE, http::headervalues::contenttype::JSON); - r.set_body(body); - - join_client->send_request(std::move(r)); } void initiate_join() @@ -1393,6 +1587,9 @@ namespace ccf void start_join_timer() { + // The initial attempt runs under NodeState::lock held by the caller + // (launch_node, via create), satisfying the initiate_join_unsafe + // precondition; the periodic task below re-acquires the lock per retry. initiate_join_unsafe(); join_periodic_task = ccf::tasks::make_basic_task([this]() { @@ -3300,42 +3497,6 @@ namespace ccf return network.identity->cert; } - // Stop-gap until it becomes easier to use other HTTP clients - void make_http_request( - const ::http::URL& url, - ::http::Request&& req, - std::function&&)> - callback, - const std::vector& ca_certs = {}, - const std::string& app_protocol = "HTTP1", - bool authenticate_as_node_client_certificate = false) override - { - std::optional client_cert = std::nullopt; - std::optional client_cert_key = std::nullopt; - if (authenticate_as_node_client_certificate) - { - client_cert = - endorsed_node_cert ? *endorsed_node_cert : self_signed_node_cert; - client_cert_key = node_sign_kp->private_key_pem(); - } - - auto ca = std::make_shared<::tls::CA>(ca_certs, true); - std::shared_ptr<::tls::Cert> ca_cert = - std::make_shared<::tls::Cert>(ca, client_cert, client_cert_key); - auto client = rpcsessions->create_client(ca_cert, app_protocol); - client->connect( - url.host, - url.port, - [callback]( - ccf::http_status status, - http::HeaderMap&& headers, - std::vector&& data) { - return callback(status, std::move(headers), std::move(data)); - }); - client->send_request(std::move(req)); - } - std::shared_ptr get_store() override { return network.tables; diff --git a/src/node/rpc/node_interface.h b/src/node/rpc/node_interface.h index 82fab1610d9..82d8fd27742 100644 --- a/src/node/rpc/node_interface.h +++ b/src/node/rpc/node_interface.h @@ -66,17 +66,6 @@ namespace ccf virtual bool is_user_frontend_open() = 0; [[nodiscard]] virtual bool is_accessible_to_members() const = 0; - virtual void make_http_request( - const ::http::URL& url, - ::http::Request&& req, - std::function&&)> callback, - const std::vector& ca_certs = {}, - const std::string& app_protocol = "HTTP1", - bool use_node_client_certificate = false) = 0; - virtual std::shared_ptr get_store() = 0; virtual ringbuffer::AbstractWriterFactory& get_writer_factory() = 0; }; diff --git a/tests/e2e_curl.py b/tests/e2e_curl.py index 93b0409b4c1..e39958a2625 100644 --- a/tests/e2e_curl.py +++ b/tests/e2e_curl.py @@ -1,10 +1,16 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the Apache 2.0 License. from aiohttp import web -from datetime import datetime, UTC +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import ec +from cryptography.x509.oid import NameOID +from datetime import datetime, timedelta, UTC import asyncio -import random import os +import random +import ssl +import tempfile async def echo_handler(request): @@ -35,6 +41,33 @@ async def echo_handler(request): return web.json_response(response_data) +def make_self_signed_cert(san_dns): + key = ec.generate_private_key(ec.SECP256R1()) + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, san_dns)]) + now = datetime.now(UTC) + cert = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - timedelta(days=1)) + .not_valid_after(now + timedelta(days=365)) + .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True) + .add_extension( + x509.SubjectAlternativeName([x509.DNSName(san_dns)]), critical=False + ) + .sign(key, hashes.SHA256()) + ) + cert_pem = cert.public_bytes(serialization.Encoding.PEM).decode("ascii") + key_pem = key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ).decode("ascii") + return cert_pem, key_pem + + async def main(): app = web.Application() app.router.add_route("*", "/{path:.*}", echo_handler) @@ -54,13 +87,47 @@ async def main(): print(f"Echo server running on http://{addr}") - env = os.environ.copy() - env["ECHO_SERVER_ADDR"] = str(addr) - - cmd = "./curl_test" - process = await asyncio.create_subprocess_shell(cmd, env=env) - await process.wait() - exit(process.returncode) + # A second, TLS-enabled endpoint used to exercise curl's certificate + # hostname verification (CURLOPT_SSL_VERIFYHOST). Its self-signed + # certificate has a single dNSName SAN that intentionally does not cover + # the loopback IP it is served on, so a client dialing the IP with + # VERIFYHOST=2 must reject it, while one dialing the SAN name (resolved to + # the same address) accepts it. + tls_san = "ccf-curl-test.invalid" + tls_cert_pem, tls_key_pem = make_self_signed_cert(tls_san) + + with tempfile.TemporaryDirectory() as tls_dir: + cert_path = os.path.join(tls_dir, "tls_cert.pem") + key_path = os.path.join(tls_dir, "tls_key.pem") + with open(cert_path, "w", encoding="utf-8") as cert_file: + cert_file.write(tls_cert_pem) + with open(key_path, "w", encoding="utf-8") as key_file: + key_file.write(tls_key_pem) + + ssl_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + ssl_context.load_cert_chain(cert_path, key_path) + + tls_site = web.TCPSite(runner, base_addr, 0, ssl_context=ssl_context) + await tls_site.start() + + tls_sockets = tls_site._server.sockets + if not tls_sockets: + raise RuntimeError("Failed to start TLS server") + tls_port = tls_sockets[0].getsockname()[1] + tls_addr = f"{base_addr}:{tls_port}" + + print(f"TLS server running on https://{tls_addr} (cert SAN {tls_san})") + + env = os.environ.copy() + env["ECHO_SERVER_ADDR"] = str(addr) + env["TLS_SERVER_ADDR"] = str(tls_addr) + env["TLS_SERVER_SAN"] = tls_san + env["TLS_SERVER_CA"] = cert_path + + cmd = "./curl_test" + process = await asyncio.create_subprocess_shell(cmd, env=env) + await process.wait() + exit(process.returncode) if __name__ == "__main__": diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index 984e1a5ca3c..d64ccec5c9f 100644 --- a/tests/e2e_operations.py +++ b/tests/e2e_operations.py @@ -1666,6 +1666,72 @@ def run_tls_san_checks(const_args): ), f"Expected SANs do not match: {ip_sans} vs {dummy_public_rpc_hosts}" +def run_tls_san_join_mismatch(const_args): + # Since the join client was migrated to libcurl, a joining node verifies + # (CURLOPT_SSL_VERIFYHOST=2) that the target node's certificate covers the + # address it dialled. Check that a join is rejected when the dialled address + # is absent from the target node's certificate SANs, even though the service + # certificate itself is trusted. This exercises the hostname/SAN check + # specifically, as opposed to the untrusted-service-certificate path already + # covered by reconfiguration.test_add_node_invalid_service_cert. + args = copy.deepcopy(const_args) + args.label += "_tls_san_join_mismatch" + + # Bind the target node's primary and file-serving interfaces to distinct + # loopback addresses, and restrict the node certificate SANs to the primary + # address only. The test harness reaches the node through the primary + # interface (in the SAN, so trusted), while a joining node dials the + # file-serving interface (not in the SAN, so rejected). + primary_host = "127.0.0.1" + file_serving_host = "127.0.0.2" + + args.nodes = infra.e2e_args.nodes(args, 1) + target_spec = args.nodes[0] + target_spec.get_primary_interface().host = primary_host + target_spec.get_file_serving_interface().host = file_serving_host + args.subject_alt_names = [f"iPAddress:{primary_host}"] + + with infra.network.network( + args.nodes, + args.binary_dir, + args.debug_nodes, + pdb=args.pdb, + ) as network: + network.start_and_open(args) + + LOG.info( + "A join is rejected when the target address is not covered by the " + "target node's certificate SANs" + ) + new_node = network.create_node() + try: + network.join_node( + new_node, + args.package, + args, + timeout=3, + stop_on_error=True, + from_snapshot=False, + # Skip the proactive snapshot fetch so the node goes straight to + # the join request. The snapshot fetch treats a certificate + # verification failure as a retryable (non-fatal) error, whereas + # the join request itself treats it as fatal, which is the + # behaviour under test. + fetch_recent_snapshot=False, + ) + except infra.network.ServiceCertificateInvalid: + LOG.info( + f"Node {new_node.local_node_id} correctly rejected the target, " + "whose certificate SANs do not cover the dialled file-serving " + "address (VERIFYHOST)" + ) + else: + assert False, ( + f"Node {new_node.local_node_id} unexpectedly joined despite a " + "target certificate SAN mismatch" + ) + + def run_config_timeout_check(const_args): args = copy.deepcopy(const_args) args.nodes = infra.e2e_args.nodes(args, 1) @@ -4649,6 +4715,7 @@ def run(args): run_ledger_cleanup_no_read_only_dir_check(args) run_ledger_chunk_cleanup_tests(args) run_tls_san_checks(args) + run_tls_san_join_mismatch(args) run_config_timeout_check(args) run_configuration_file_checks(args) run_pid_file_check(args) diff --git a/tests/infra/network.py b/tests/infra/network.py index d972a5311a6..e96d7d816b7 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -105,7 +105,11 @@ def __init__(self, *args, endorsement_server, retries, **kwargs): class ServiceCertificateInvalid(NodeJoinException): - pass + """Raised when a joining node cannot establish TLS certificate trust with + the target service. This covers any join-time certificate trust failure + (an untrusted or wrong service certificate, a hostname/SAN mismatch under + VERIFYHOST, or a service certificate that cannot be loaded), not only a + literally invalid certificate.""" class NetworkShutdownError(Exception): @@ -1420,7 +1424,20 @@ def run_join_node( ) from e if "StartupSeqnoIsOld" in error: raise StartupSeqnoIsOld(node, has_stopped, error) from e - if "invalid cert on handshake" in error: + # The joining node now connects to the target via the + # curl client, which reports any failure to establish + # certificate trust as "TLS certificate trust check + # failed": a rejected or untrusted service certificate, + # a hostname/SAN mismatch (VERIFYHOST=2) or any other + # peer verification failure, or a configured service + # certificate that could not be loaded. The legacy + # TLS-session wording ("invalid cert on handshake") is + # retained for compatibility with logs from older nodes + # during mixed-version tests. + if ( + "TLS certificate trust check failed" in error + or "invalid cert on handshake" in error + ): raise ServiceCertificateInvalid( node, has_stopped, error ) from e