From c9f4864e6feb6d6137da7d9d1ea17f36e298feef Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 18:14:44 +0000 Subject: [PATCH 01/19] Document node join curl migration plan --- doc/dev/join_curl_migration_plan.md | 241 ++++++++++++++++++++++++++++ 1 file changed, 241 insertions(+) create mode 100644 doc/dev/join_curl_migration_plan.md diff --git a/doc/dev/join_curl_migration_plan.md b/doc/dev/join_curl_migration_plan.md new file mode 100644 index 00000000000..142ea29b916 --- /dev/null +++ b/doc/dev/join_curl_migration_plan.md @@ -0,0 +1,241 @@ +# Migrating the node join client from the legacy HTTP client to libcurl + +Tracking issue: [#7262](https://github.com/microsoft/CCF/issues/7262) - "Migrate +httpclients over to libcurl". The node join sequence is the **last** remaining +user of `RPCSessions::create_client()` (the legacy in-enclave TLS HTTP client). +Migrating it lets us delete the remaining legacy HTTP client infrastructure. + +Reference implementation: [#8005](https://github.com/microsoft/CCF/pull/8005), +which migrated JWT/JWK OpenID discovery and key refresh to the shared curl-multi +client. The endorsements client ([#7102]) and snapshot fetch already use the same +curl client, and the snapshot fetch runs inside the join flow, so there is a +directly comparable node-to-node precedent. + +## Goals + +- Replace the legacy HTTP client in `NodeState::initiate_join_unsafe()` with the + async curl-multi client (`ccf::curl::CurlmLibuvContextSingleton`). +- Preserve the join trust model exactly: the **service certificate is the only + trust anchor**, and the joining node presents its self-signed node certificate + as a client certificate (mutual TLS). +- **Strengthen** TLS peer verification by enabling certificate hostname + verification (`CURLOPT_SSL_VERIFYHOST=2`), which the legacy path did not do. +- Preserve every existing join control-flow branch (redirects, snapshot re-fetch + on `StartupSeqnoIsOld`, pending/trusted handling, fatal errors). +- Remove the now-dead legacy HTTP client infrastructure. +- Lean on existing test coverage as validation gates, and plug the gaps that the + behavioural change introduces. + +## Non-goals + +- No change to the `/node/join` server endpoint or the join wire protocol. +- No change to the in-process node client (`NodeClient` / `HTTPNodeClient`), + which is a loopback handler dispatch, not a network/TLS client. +- No change to the server-side TLS/HTTP session stack (nodes still serve RPC). + +## Current implementation (baseline) + +`NodeState::initiate_join_unsafe()` in `src/node/node_state.h`: + +```cpp +auto network_ca = std::make_shared<::tls::CA>(std::string( + config.join.service_cert.begin(), config.join.service_cert.end())); + +auto join_client_cert = std::make_unique<::tls::Cert>( + network_ca, + self_signed_node_cert, + node_sign_kp->private_key_pem(), + target_host); + +auto join_client = rpcsessions->create_client( + std::move(join_client_cert), + rpcsessions->get_app_protocol_main_interface()); + +join_client->connect(target_host, target_port, /* response cb */, /* error cb */); +join_client->send_request(POST /node/join with JSON body); +``` + +Relevant properties of the baseline: + +- **Trust anchor**: `config.join.service_cert` only. `::tls::CA` builds an + `X509_STORE` containing exactly that certificate; the system CA bundle is never + consulted. `partial_ok` is `false` (full chain to the service cert required). +- **Peer verification**: `SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT` with + standard OpenSSL chain validation. +- **Hostname verification**: **none**. `::tls::Cert` uses `peer_hostname` only for + SNI (`SSL_set_tlsext_host_name`); there is no `X509_VERIFY_PARAM_set1_host` + call, so the target's certificate name is not checked. +- **Client authentication**: the node presents `self_signed_node_cert` + + `node_sign_kp` private key (at join time the node is not yet endorsed, so + `endorsed_node_cert` is `std::nullopt`). +- **Concurrency**: asynchronous. `connect()` returns immediately; the response + callback runs later and processes the result under `NodeState::lock` guarded by + `sm.check(NodeStartupState::pending)`. The lock is **not** held during network + I/O. +- **Retry**: `join_periodic_task` re-invokes `initiate_join_unsafe()` on a timer. + +### Join response-handling branches to preserve + +1. Transport/handshake failure -> fatal: write `AdminMessage::fatal_error_msg`, + node shuts down gracefully (not retried). +2. 4xx with `ODataError` code `StartupSeqnoIsOld` and `fetch_recent_snapshot` -> + schedule a `FetchSnapshot` task, return, and let the periodic timer retry. +3. Other 4xx -> fatal shutdown. +4. `307`/`308` with `follow_redirect` and a `Location` header -> update + `config.join.target_rpc_address` and let the periodic timer retry. +5. Other non-200 -> log failure, wait for retry. +6. `200 OK`, `node_status == TRUSTED` -> initialise identity / ledger secrets / + consensus, become part of (public) network, cancel the join timer. +7. `200 OK`, `node_status == PENDING` -> wait for member votes (retry). + +## Target implementation + +Model on `JwtKeyAutoRefresh::send_curl_get()` (`src/node/jwt_key_auto_refresh.h`) +for the TLS options and async dispatch, and on `recovery_decision_protocol.cpp` +for the client-certificate (mTLS) options. + +curl easy-handle options for the join request: + +| Option | Value | Rationale | +| --- | --- | --- | +| `CURLOPT_CAINFO_BLOB` | `config.join.service_cert` | Trust anchor = service cert only (matches baseline). | +| `CURLOPT_CAPATH` | `nullptr` | Do **not** fall back to the system CA bundle. Critical: keeps the accepted-CA set identical to the baseline. | +| `CURLOPT_SSL_VERIFYPEER` | `1L` | Verify the peer chain (matches baseline). | +| `CURLOPT_SSL_VERIFYHOST` | `2L` | Verify the peer certificate name (hardening; see below). | +| `CURLOPT_PROTOCOLS_STR` | `"https"` | Restrict to HTTPS (matches JWT refresh). | +| `CURLOPT_SSLCERT_BLOB` | self-signed node cert | Client cert for mTLS (matches baseline). | +| `CURLOPT_SSLCERTTYPE` | `"PEM"` | | +| `CURLOPT_SSLKEY_BLOB` | node signing key | Client key for mTLS (matches baseline). | +| `CURLOPT_SSLKEYTYPE` | `"PEM"` | | +| `CURLOPT_CONNECTTIMEOUT` / `CURLOPT_TIMEOUT` | small fixed values | Bound transport time (matches JWT refresh). | + +- **Method/body**: POST `https://{target_rpc_address}/node/join` with the JSON + `JoinNetworkNodeToNode::In` body and `Content-Type: application/json`. +- **Response body cap**: a fixed, generous `ccf::curl::ResponseBody` maximum + (large enough for any realistic join response - identity, ledger secrets - but + far below a snapshot). No new operator-facing config option. +- **Redirects**: do **not** set `CURLOPT_FOLLOWLOCATION`. Handle redirects + manually exactly as today (inspect status + `Location`, update + `target_rpc_address`), so the trust anchor is re-applied to the new target and + the snapshot fetch picks up the redirected address. +- **Dispatch**: build the request under `lock`, then + `CurlmLibuvContextSingleton::get_instance()->attach_request(...)` and return. + The single `ResponseCallback` re-acquires `lock`, re-checks + `sm.check(pending)`, and maps `CURLcode` + HTTP status to the seven branches + above. Transport/TLS failures (`CURLcode != CURLE_OK`, e.g. + `CURLE_PEER_FAILED_VERIFICATION`, `CURLE_SSL_CACERT`) map to the current fatal + path and **must log a single stable, greppable message** so the test suite can + detect a rejected service certificate. + +### POST-with-body support in the curl wrapper (prerequisite) + +`ccf::curl::CurlRequest` currently wires GET, HEAD and PUT (upload) fully, but the +`HTTP_POST` branch assumes the caller pre-set `CURLOPT_POSTFIELDS` and does not +enable `CURLOPT_POST`. Add first-class POST-with-body support (set `CURLOPT_POST` +and `CURLOPT_POSTFIELDSIZE`, feeding the existing `RequestBody` read callback, or +an equivalent `COPYPOSTFIELDS` helper) so the join can send a JSON body cleanly, +and cover it in `src/http/test/curl_test.cpp`. + +## Security analysis: which root CAs are accepted + +| Property | Baseline (legacy client) | Migrated (curl) | Verdict | +| --- | --- | --- | --- | +| Accepted trust anchors | service cert only (`::tls::CA` store) | `CAINFO_BLOB` = service cert, `CAPATH` = `nullptr` | Identical | +| System CA fallback | never | disabled via `CAPATH=nullptr` | Identical | +| Chain building | full chain, `partial_ok=false` | curl default (no partial chain) | Identical (service identity is a self-signed root; snapshot fetch already proves this against the same peer) | +| Peer chain verification | `SSL_VERIFY_PEER` | `SSL_VERIFYPEER=1` | Identical | +| Certificate name check | none (SNI only) | `SSL_VERIFYHOST=2` | **Strengthened** | +| Client auth (mTLS) | self-signed node cert | `SSLCERT_BLOB` self-signed node cert | Identical | + +**Hostname verification hardening.** Enabling `VERIFYHOST=2` means a join fails if +the host in `join.target_rpc_address` is not present in the target node's +certificate SANs. CCF derives node-cert SANs from +`config.node_certificate.subject_alt_names`, or, by default, from each RPC +interface's `published_address` (`get_subject_alternative_names()`), with IPs +emitted as `iPAddress` SANs and names as `dNSName`. Redirect targets are built +from those same published addresses. The snapshot fetch inside the join flow +already uses curl with the default `VERIFYHOST=2` against the *same* +`target_rpc_address` and *same* `service_cert`, so standard deployments and the +test suite already satisfy this constraint. The residual risk is a deployment that +connects to an address deliberately absent from the target SANs; this is a +behavioural change and must be documented in `CHANGELOG.md`. + +## Test strategy and validation gates + +### Existing coverage to lean on + +- **Security gate**: `tests/reconfiguration.py::test_add_node_invalid_service_cert` + joins with the wrong service certificate and expects + `infra.network.ServiceCertificateInvalid`. That exception is raised in + `tests/infra/network.py::run_join_node()` by matching the log string + `"invalid cert on handshake"` (emitted by `src/enclave/tls_session.h`). The curl + path emits a **different** message, so this detection string must be updated to + match the new stable TLS-failure log. This is the primary gap. +- **Happy paths**: `test_add_node`, `test_add_node_from_snapshot`, + `test_add_node_from_backup`, `test_add_as_many_pending_nodes`. +- **Redirects**: `test_join_straddling_primary_replacement`, and + `start_network.py --redirection_kind node-by-role`. +- **Fake/duplicate joins**: `test_issue_fake_join`, `node_frontend_test.cpp`. +- **Attestation error mapping**: `code_update.py` SNP join tests exercise the + `MeasurementNotFound` / `HostDataNotFound` / `UVMEndorsementsNotAuthorised` + detection in `run_join_node()`; these must still work with curl error handling. +- **Recovery / public-network join**: `recovery.py`. +- **curl mechanics**: `src/http/test/curl_test.cpp`. + +### Gaps to plug + +1. Update the `ServiceCertificateInvalid` detection in `tests/infra/network.py` + to the new curl TLS-failure log line. +2. Add `curl_test.cpp` coverage for POST-with-body and mTLS client certificates. +3. Add/confirm coverage that `VERIFYHOST=2` succeeds for both DNS-name and IP + published addresses, and after a redirect. +4. (Recommended) Add a negative test that a join to an address **not** in the + target SANs is now rejected, to regression-protect the hardening. + +## Removal scope (after the join no longer uses the legacy client) + +Remove (confirmed to have no other users once the join and the dead +`make_http_request` are gone): + +- `NodeState::make_http_request()` (`src/node/node_state.h`) and the + `AbstractNodeState::make_http_request` declaration + (`src/node/rpc/node_interface.h`). Already dead (no callers). +- `RPCSessions::create_client()` and `RPCSessions::create_unencrypted_client()` + (`src/enclave/rpc_sessions.h`). The latter is already dead. +- `ClientSession` (`src/enclave/client_session.h`), `HTTPClientSession` + (`src/http/http_session.h`), `HTTP2ClientSession` (`src/http/http2_session.h`), + `UnencryptedHTTPClientSession` (`src/http/http_session.h`), plus their includes. + +Keep: + +- `NodeClient` / `HTTPNodeClient` (`src/node/node_client.h`, + `src/node/http_node_client.h`): in-process handler dispatch + (`RpcHandler::process`), not a network/TLS client. Used by + `retired_nodes_cleanup.h` and `raft.h`. +- `::tls::Client` (`src/tls/client.h`): a TLS-layer primitive still exercised by + the TLS unit test (`src/tls/test/main.cpp`); the server side still relies on the + TLS layer. (Optional stretch: remove it too and trim the test.) +- All server-side TLS/HTTP session classes. + +## Staged delivery (each stage is independently reviewable and gated) + +- **Stage 0 - Branch + this plan.** Branch `join-client-curl-migration`. +- **Stage 1 - curl wrapper POST-with-body.** Extend `ccf::curl::CurlRequest` + + `curl_test.cpp` coverage (POST body, mTLS). Gate: `curl_test`. +- **Stage 2 - curl-based join.** Rewrite the client half of + `initiate_join_unsafe()`; remove the legacy `tls::CA`/`tls::Cert`/`create_client` + join code. Preserve all seven branches. Gate: build + `node_frontend_test`. +- **Stage 3 - Test infra + e2e gates.** Update the `ServiceCertificateInvalid` + detection string; run the join e2e gates; add the hostname-verification + coverage. Gate: e2e join suite green. +- **Stage 4 - Remove dead httpclient infra.** Delete the removal set above; grep + for zero references; build all targets and run affected unit tests. Gate: full + build + unit tests. +- **Stage 5 - Docs, changelog, compatibility.** `CHANGELOG.md` (Changed: curl + join migration + `VERIFYHOST` hardening migration note + infra removal), bump + `python/pyproject.toml` if a new version is cut, review + `doc/operations/start_network.rst` wording, run `ci-checks.sh` and formatting/ + linting, and run `lts_compatibility` with `LONG_TESTS=1` (join touches + cross-version TLS and protocol paths). + +[#7102]: https://github.com/microsoft/CCF/pull/7102 From 26b6d0ce5063e68194949d0284c2a2b6a6fac41b Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 18:21:11 +0000 Subject: [PATCH 02/19] Add POST-with-body support to curl wrapper The CurlRequest HTTP_POST branch previously assumed the caller had pre-set CURLOPT_POSTFIELDS and did not enable CURLOPT_POST, so passing a RequestBody for a POST did not transmit the body. Enable CURLOPT_POST and declare the body size via CURLOPT_POSTFIELDSIZE_LARGE so the existing RequestBody read callback is used and a Content-Length is sent. Adds a curl_test case that posts a body to the echo server and asserts the method and body round-trip. --- src/http/curl.h | 28 ++++++++++++++++++--- src/http/test/curl_test.cpp | 49 +++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/src/http/curl.h b/src/http/curl.h index 3998848dcd6..e42a7040315 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -238,6 +238,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) @@ -500,10 +505,25 @@ namespace ccf::curl } 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; + { + 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/test/curl_test.cpp b/src/http/test/curl_test.cpp index 76343cb4908..ccca80727a6 100644 --- a/src/http/test/curl_test.cpp +++ b/src/http/test/curl_test.cpp @@ -174,6 +174,55 @@ 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("CurlmLibuvContext") { size_t response_count = 0; From c6422d37b25848b5620a8d0d74c7b2f3f962db7a Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 18:59:22 +0000 Subject: [PATCH 03/19] Migrate node join request from httpclient to curl client Replace the RPCSessions::create_client join path with an async curl request via CurlmLibuvContextSingleton, matching the JWT refresh and snapshot fetch clients. The service certificate remains the sole trust anchor (CURLOPT_CAINFO_BLOB + CURLOPT_CAPATH=nullptr), peer verification is enforced (VERIFYPEER=1), and hostname verification is now enabled (VERIFYHOST=2). The joining node presents its self-signed node certificate for mutual TLS. All existing response-handling branches (redirect, StartupSeqnoIsOld snapshot fetch, TRUSTED/PENDING, fatal errors) are preserved. --- src/node/node_state.h | 613 +++++++++++++++++++++++------------------- 1 file changed, 340 insertions(+), 273 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 89dce2dd936..561259e88b3 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" @@ -1048,339 +1049,405 @@ 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())); + // 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 [target_host, target_port] = - split_net_address(config.join.target_rpc_address); + 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); - 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. + // 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); + + 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. + 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::unique_ptr&& request, + CURLcode curl_response, + long status_code) { std::lock_guard guard(lock); if (!sm.check(NodeStartupState::pending)) { return; } - if (is_http_status_client_error(status)) + try { - std::optional error_response = - std::nullopt; - - try + if (curl_response == CURLE_ABORTED_BY_CALLBACK) { - auto j = ccf::parse_json_safe(data); - error_response = j.get(); + // The request was aborted, e.g. during node shutdown. + return; } - catch (const ccf::JsonParseError& e) + + if (curl_response != CURLE_OK) { - LOG_FAIL_FMT( - "Join request returned {}, body exceeds permitted JSON nesting " - "depth: {}", - status, - e.what()); + // Transport or TLS-layer failure. A wrong or expired service + // certificate surfaces here as a peer verification failure, + // which we flag distinctly so it can be told apart from generic + // connectivity errors. + const bool invalid_service_certificate = + 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, + invalid_service_certificate ? "invalid service certificate: " : + "", + 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; } - catch (const nlohmann::json::exception& e) + + const auto status = static_cast(status_code); + const auto& headers = request->get_response_headers(); + const auto& data = request->get_response_body()->buffer; + + if (is_http_status_client_error(status)) { - // Leave error_response == nullopt - LOG_FAIL_FMT( - "Join request returned {}, body is not ODataErrorResponse: {}", + std::optional error_response = + std::nullopt; + + 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 ( + 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; + } + + 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 ( - error_response.has_value() && - error_response->error.code == ccf::errors::StartupSeqnoIsOld && - config.join.fetch_recent_snapshot) + if (status != HTTP_STATUS_OK) { - 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 + const auto& location = headers.find(http::headers::LOCATION); if ( - snapshot_fetch_task != nullptr && - !snapshot_fetch_task->is_cancelled()) + config.join.follow_redirect && + (status == HTTP_STATUS_PERMANENT_REDIRECT || + status == HTTP_STATUS_TEMPORARY_REDIRECT) && + location != headers.end()) { - LOG_INFO_FMT("Snapshot fetch already in progress, skipping"); + 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 { - snapshot_fetch_task = std::make_shared( - config.join, config.snapshots, this); - ccf::tasks::add_task(snapshot_fetch_task); + 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; } - 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()) + JoinNetworkNodeToNode::Out resp; + try { - 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); + auto j = ccf::parse_json_safe(data); + resp = j.get(); } - else + catch (const std::exception& e) { 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; - } + "An error occurred while parsing the join network response"); - 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"); - - LOG_DEBUG_FMT("Join network response error: {}", e.what()); - LOG_DEBUG_FMT( - "Join network response body: {}", - 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())); - // 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)); - - history->set_service_signing_identity( - network.identity->get_key_pair(), - resp.network_info->cose_signatures_config.value_or( - ccf::COSESignaturesConfig{})); - - ccf::crypto::Pem n2n_channels_cert; - if (!resp.network_info->endorsed_certificate.has_value()) + // Set network secrets, node id and become part of network. + if (resp.node_status == NodeStatus::TRUSTED) { - // 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.has_value()) + { + throw std::logic_error( + "Expected network info in join response"); + } - setup_consensus(resp.network_info->public_only, n2n_channels_cert); - auto_refresh_jwt_keys(); + network.identity = std::make_unique( + resp.network_info->identity); + network.ledger_secrets->init_from_map( + std::move(resp.network_info->ledger_secrets)); - 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); - } + history->set_service_signing_identity( + network.identity->get_key_pair(), + resp.network_info->cose_signatures_config.value_or( + ccf::COSESignaturesConfig{})); - 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) + ccf::crypto::Pem n2n_channels_cert; + if (!resp.network_info->endorsed_certificate.has_value()) { - hook->call(consensus.get()); + // 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(); - auto tx = network.tables->create_read_only_tx(); - view = resolve_latest_sig_view(tx); + setup_consensus( + resp.network_info->public_only, n2n_channels_cert); + auto_refresh_jwt_keys(); - if (!resp.network_info->public_only) + 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(); + last_recovered_signed_idx = + resp.network_info->last_recovered_signed_idx; + setup_recovery_hook(); + snapshotter->set_snapshot_generation(false); } - LOG_INFO_FMT( - "Joiner successfully resumed from snapshot at seqno {} and " - "view {}", + 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); + } + + consensus->init_as_backup( network.tables->current_version(), - view); - } + view, + view_history_, + last_recovered_signed_idx); - consensus->init_as_backup( - network.tables->current_version(), - view, - view_history_, - last_recovered_signed_idx); + { + 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(); - { - 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 (resp.network_info->public_only) { - snapshotter->init_from_snapshot_status(snapshot_status.value()); + sm.advance(NodeStartupState::partOfPublicNetwork); + } + else + { + reset_data(quote_info.quote); + reset_data(quote_info.endorsements); + sm.advance(NodeStartupState::partOfNetwork); } - } - 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) + { + join_periodic_task->cancel_task(); + join_periodic_task = nullptr; + } - if (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) { - join_periodic_task->cancel_task(); - join_periodic_task = nullptr; + LOG_INFO_FMT( + "Node {} is waiting for votes of members to be trusted", self); } - - 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) + catch (const std::exception& e) { - LOG_INFO_FMT( - "Node {} is waiting for votes of members to be trusted", self); + LOG_FAIL_FMT( + "Unhandled error while processing join response from {}: {}", + target_address, + e.what()); } - }, - [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; - - 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); - } - - 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)); + }; + + 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)); + + ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request( + std::move(join_request)); } void initiate_join() From 61d22ebacb3593cfca15c62400ccbb897b2f8067 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 19:41:38 +0000 Subject: [PATCH 04/19] Retry transient join transport errors instead of failing fatally The legacy httpclient join path only invoked its fatal error callback for TLS handshake failures; transport-level failures (connection refused, unresolved host, timeouts) silently closed the connection and were retried by the periodic join timer. The initial curl migration treated every non-OK CURLcode as fatal, which caused nodes to shut down on a transient 'could not connect' during normal startup races. Categorise transient transport errors (COULDNT_RESOLVE_*, COULDNT_CONNECT, OPERATION_TIMEDOUT, GOT_NOTHING, RECV/SEND_ERROR, PARTIAL_FILE) as retryable, and keep TLS/protocol failures (including peer verification failures for an untrusted service certificate) fatal. Add connect and total timeouts so stalled attempts are abandoned for the timer to retry. --- src/node/node_state.h | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 561259e88b3..39831b305b9 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1106,6 +1106,12 @@ namespace ccf config.join.service_cert.size()); curl_handle.set_opt(CURLOPT_CAPATH, nullptr); + // Bound each attempt so a stalled connection is abandoned and the + // periodic join timer can retry, rather than accumulating in-flight + // requests. 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, @@ -1137,6 +1143,7 @@ namespace ccf // 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]( std::unique_ptr&& request, @@ -1158,10 +1165,36 @@ namespace ccf if (curl_response != CURLE_OK) { - // Transport or TLS-layer failure. A wrong or expired service + // 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. + const bool transient_transport_error = + curl_response == CURLE_COULDNT_RESOLVE_PROXY || + curl_response == CURLE_COULDNT_RESOLVE_HOST || + curl_response == CURLE_COULDNT_CONNECT || + curl_response == CURLE_OPERATION_TIMEDOUT || + curl_response == CURLE_GOT_NOTHING || + curl_response == CURLE_RECV_ERROR || + curl_response == CURLE_SEND_ERROR || + curl_response == CURLE_PARTIAL_FILE; + if (transient_transport_error) + { + 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; + } + + // Fatal TLS/protocol-layer failure. A wrong or expired service // certificate surfaces here as a peer verification failure, - // which we flag distinctly so it can be told apart from generic - // connectivity errors. + // which we flag distinctly so it can be told apart from other + // fatal errors. const bool invalid_service_certificate = curl_response == CURLE_PEER_FAILED_VERIFICATION || curl_response == CURLE_SSL_CACERT_BADFILE; @@ -1436,6 +1469,7 @@ namespace ccf e.what()); } }; + // NOLINTEND(readability-function-cognitive-complexity) auto join_request = std::make_unique( std::move(curl_handle), From 0aa42e77efddd6846d670d307f2a82d15b397b29 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 19:41:45 +0000 Subject: [PATCH 05/19] Detect curl service-certificate rejection in join test infra The curl-based join client reports an untrusted service certificate as "invalid service certificate" rather than the legacy TLS session's "invalid cert on handshake". Update the ServiceCertificateInvalid detection to match the new wording, retaining the old string for mixed-version test compatibility. --- tests/infra/network.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/infra/network.py b/tests/infra/network.py index ddaa33a0768..9dbcb5350e9 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -1442,7 +1442,16 @@ 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 a rejected service + # certificate as "invalid service certificate". The + # legacy TLS-session wording ("invalid cert on + # handshake") is retained for compatibility with logs + # from older nodes during mixed-version tests. + if ( + "invalid service certificate" in error + or "invalid cert on handshake" in error + ): raise ServiceCertificateInvalid( node, has_stopped, error ) from e From 059ff75b1370597942b85fc4f6531247cd1f857c Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 19:58:40 +0000 Subject: [PATCH 06/19] Remove unused legacy httpclient infrastructure With the node join migrated to the curl client, the enclave-side HTTP client sessions are no longer used. Remove make_http_request (and its AbstractNodeState declaration), RPCSessions::create_client and create_unencrypted_client (and the now-dead client session id helper), and the HTTPClientSession, HTTP2ClientSession and UnencryptedHTTPClientSession classes along with the ClientSession base. The in-process NodeClient/HTTPNodeClient (used by retired node cleanup) and the tls::Client primitive (still exercised by the TLS unit tests and underpinning server-side TLS) are retained. Completes the httpclient removal tracked in #7262. --- src/enclave/client_session.h | 53 ------------ src/enclave/rpc_sessions.h | 78 ------------------ src/http/http2_session.h | 69 ---------------- src/http/http_session.h | 146 ---------------------------------- src/node/node_state.h | 36 --------- src/node/rpc/node_interface.h | 11 --- 6 files changed, 393 deletions(-) delete mode 100644 src/enclave/client_session.h 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/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/node/node_state.h b/src/node/node_state.h index 39831b305b9..7365c626a5e 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -3399,42 +3399,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; }; From 0ec4c1035f7744504d1eb18cc9ee3b997db12e02 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 20:06:22 +0000 Subject: [PATCH 07/19] Document node join curl migration and hostname verification Add CHANGELOG entries for the join client migration to curl (including the new TLS hostname-verification requirement on the join connection) and the removal of the legacy httpclient infrastructure. Add an operator note to the start-network docs explaining that the target node's certificate SANs must cover join.target_rpc_address. --- CHANGELOG.md | 5 ++++ doc/dev/join_curl_migration_plan.md | 44 ++++++++++++++--------------- doc/operations/start_network.rst | 2 ++ 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1cae74bbec..c2afd8b7262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,10 +11,15 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### 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). TLS certificate hostname verification is now enforced on the join connection: operators must ensure the target node's certificate subject alternative names cover the address configured in `join.target_rpc_address` (#XXXX). - JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). - JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). - Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). +### 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 (#XXXX). + ### Fixed - JS endpoint string values (e.g. response bodies, KV keys/values) containing embedded NUL bytes are no longer silently truncated at the first NUL when converted to their C++ representation (#8033). diff --git a/doc/dev/join_curl_migration_plan.md b/doc/dev/join_curl_migration_plan.md index 142ea29b916..bfd54b4a8b0 100644 --- a/doc/dev/join_curl_migration_plan.md +++ b/doc/dev/join_curl_migration_plan.md @@ -96,18 +96,18 @@ for the client-certificate (mTLS) options. curl easy-handle options for the join request: -| Option | Value | Rationale | -| --- | --- | --- | -| `CURLOPT_CAINFO_BLOB` | `config.join.service_cert` | Trust anchor = service cert only (matches baseline). | -| `CURLOPT_CAPATH` | `nullptr` | Do **not** fall back to the system CA bundle. Critical: keeps the accepted-CA set identical to the baseline. | -| `CURLOPT_SSL_VERIFYPEER` | `1L` | Verify the peer chain (matches baseline). | -| `CURLOPT_SSL_VERIFYHOST` | `2L` | Verify the peer certificate name (hardening; see below). | -| `CURLOPT_PROTOCOLS_STR` | `"https"` | Restrict to HTTPS (matches JWT refresh). | -| `CURLOPT_SSLCERT_BLOB` | self-signed node cert | Client cert for mTLS (matches baseline). | -| `CURLOPT_SSLCERTTYPE` | `"PEM"` | | -| `CURLOPT_SSLKEY_BLOB` | node signing key | Client key for mTLS (matches baseline). | -| `CURLOPT_SSLKEYTYPE` | `"PEM"` | | -| `CURLOPT_CONNECTTIMEOUT` / `CURLOPT_TIMEOUT` | small fixed values | Bound transport time (matches JWT refresh). | +| Option | Value | Rationale | +| -------------------------------------------- | -------------------------- | ------------------------------------------------------------------------------------------------------------ | +| `CURLOPT_CAINFO_BLOB` | `config.join.service_cert` | Trust anchor = service cert only (matches baseline). | +| `CURLOPT_CAPATH` | `nullptr` | Do **not** fall back to the system CA bundle. Critical: keeps the accepted-CA set identical to the baseline. | +| `CURLOPT_SSL_VERIFYPEER` | `1L` | Verify the peer chain (matches baseline). | +| `CURLOPT_SSL_VERIFYHOST` | `2L` | Verify the peer certificate name (hardening; see below). | +| `CURLOPT_PROTOCOLS_STR` | `"https"` | Restrict to HTTPS (matches JWT refresh). | +| `CURLOPT_SSLCERT_BLOB` | self-signed node cert | Client cert for mTLS (matches baseline). | +| `CURLOPT_SSLCERTTYPE` | `"PEM"` | | +| `CURLOPT_SSLKEY_BLOB` | node signing key | Client key for mTLS (matches baseline). | +| `CURLOPT_SSLKEYTYPE` | `"PEM"` | | +| `CURLOPT_CONNECTTIMEOUT` / `CURLOPT_TIMEOUT` | small fixed values | Bound transport time (matches JWT refresh). | - **Method/body**: POST `https://{target_rpc_address}/node/join` with the JSON `JoinNetworkNodeToNode::In` body and `Content-Type: application/json`. @@ -138,14 +138,14 @@ and cover it in `src/http/test/curl_test.cpp`. ## Security analysis: which root CAs are accepted -| Property | Baseline (legacy client) | Migrated (curl) | Verdict | -| --- | --- | --- | --- | -| Accepted trust anchors | service cert only (`::tls::CA` store) | `CAINFO_BLOB` = service cert, `CAPATH` = `nullptr` | Identical | -| System CA fallback | never | disabled via `CAPATH=nullptr` | Identical | -| Chain building | full chain, `partial_ok=false` | curl default (no partial chain) | Identical (service identity is a self-signed root; snapshot fetch already proves this against the same peer) | -| Peer chain verification | `SSL_VERIFY_PEER` | `SSL_VERIFYPEER=1` | Identical | -| Certificate name check | none (SNI only) | `SSL_VERIFYHOST=2` | **Strengthened** | -| Client auth (mTLS) | self-signed node cert | `SSLCERT_BLOB` self-signed node cert | Identical | +| Property | Baseline (legacy client) | Migrated (curl) | Verdict | +| ----------------------- | ------------------------------------- | -------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | +| Accepted trust anchors | service cert only (`::tls::CA` store) | `CAINFO_BLOB` = service cert, `CAPATH` = `nullptr` | Identical | +| System CA fallback | never | disabled via `CAPATH=nullptr` | Identical | +| Chain building | full chain, `partial_ok=false` | curl default (no partial chain) | Identical (service identity is a self-signed root; snapshot fetch already proves this against the same peer) | +| Peer chain verification | `SSL_VERIFY_PEER` | `SSL_VERIFYPEER=1` | Identical | +| Certificate name check | none (SNI only) | `SSL_VERIFYHOST=2` | **Strengthened** | +| Client auth (mTLS) | self-signed node cert | `SSLCERT_BLOB` self-signed node cert | Identical | **Hostname verification hardening.** Enabling `VERIFYHOST=2` means a join fails if the host in `join.target_rpc_address` is not present in the target node's @@ -154,8 +154,8 @@ certificate SANs. CCF derives node-cert SANs from interface's `published_address` (`get_subject_alternative_names()`), with IPs emitted as `iPAddress` SANs and names as `dNSName`. Redirect targets are built from those same published addresses. The snapshot fetch inside the join flow -already uses curl with the default `VERIFYHOST=2` against the *same* -`target_rpc_address` and *same* `service_cert`, so standard deployments and the +already uses curl with the default `VERIFYHOST=2` against the _same_ +`target_rpc_address` and _same_ `service_cert`, so standard deployments and the test suite already satisfy this constraint. The residual risk is a deployment that connects to an address deliberately absent from the target SANs; this is a behavioural change and must be documented in `CHANGELOG.md`. 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 `. From 87753c719b7cd611e2a8f476dd0366233c329830 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 20:48:57 +0000 Subject: [PATCH 08/19] Reference PR #8040 in join curl migration changelog entries --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2afd8b7262..3db81e19405 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,14 +11,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### 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). TLS certificate hostname verification is now enforced on the join connection: operators must ensure the target node's certificate subject alternative names cover the address configured in `join.target_rpc_address` (#XXXX). +- 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). TLS certificate hostname verification is now enforced on the join connection: operators must ensure the target node's certificate subject alternative names cover the address configured in `join.target_rpc_address` (#8040). - JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). - JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). - Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). ### 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 (#XXXX). +- 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). ### Fixed From 66667fdfca46a4401f4b2c9993b2acba1cf41076 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 22:15:11 +0000 Subject: [PATCH 09/19] Extract and test join transient-error classifier; retry more transport errors Move the join client's transient-vs-fatal CURLcode decision into a reusable, unit-tested ccf::curl::is_transient_transport_error() helper, and add CURLE_WEIRD_SERVER_REPLY, CURLE_HTTP2 and CURLE_HTTP2_STREAM to the retryable set. The legacy httpclient join silently retried these transfer/parse failures via the periodic timer, whereas the initial curl migration treated everything outside a small allow-list as fatal, so a transient garbled reply or HTTP/2 framing error during a startup race could shut a node down. CURLE_WRITE_ERROR (our own response size-cap rejection) remains fatal by design. Adds a table-driven curl_test that pins the full classification. --- src/http/curl.h | 20 ++++++++++++++++ src/http/test/curl_test.cpp | 46 +++++++++++++++++++++++++++++++++++++ src/node/node_state.h | 11 +-------- 3 files changed, 67 insertions(+), 10 deletions(-) diff --git a/src/http/curl.h b/src/http/curl.h index e42a7040315..d891071ea46 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 { diff --git a/src/http/test/curl_test.cpp b/src/http/test/curl_test.cpp index ccca80727a6..807ec6d6193 100644 --- a/src/http/test/curl_test.cpp +++ b/src/http/test/curl_test.cpp @@ -33,6 +33,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; diff --git a/src/node/node_state.h b/src/node/node_state.h index 7365c626a5e..57f596c66c5 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1171,16 +1171,7 @@ namespace ccf // handshake failures (e.g. an untrusted service certificate) as // fatal. Preserve both behaviours: transient transport errors // are retried, everything else is fatal. - const bool transient_transport_error = - curl_response == CURLE_COULDNT_RESOLVE_PROXY || - curl_response == CURLE_COULDNT_RESOLVE_HOST || - curl_response == CURLE_COULDNT_CONNECT || - curl_response == CURLE_OPERATION_TIMEDOUT || - curl_response == CURLE_GOT_NOTHING || - curl_response == CURLE_RECV_ERROR || - curl_response == CURLE_SEND_ERROR || - curl_response == CURLE_PARTIAL_FILE; - if (transient_transport_error) + if (ccf::curl::is_transient_transport_error(curl_response)) { LOG_INFO_FMT( "Transient error contacting {} to join: {} ({}). The join " From 8afa7ccd3817d2fa7cd745ab5a73d989ff996780 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Tue, 7 Jul 2026 22:33:48 +0000 Subject: [PATCH 10/19] Process join response on a task thread, not the host libuv loop The curl response callback runs on the shared host libuv event loop thread. The previous code did all join processing there under NodeState::lock, including deserialising a potentially large snapshot and issuing KV/ledger writes. That stalls every other user of the shared curl loop (JWT refresh, quote endorsements, recovery decision) and risks a deadlock: the blocking host ring-buffer writer spin-waits for space while its only consumer runs on that same libuv thread. Restructure the callback to mirror the JWT refresh client: on the libuv thread it only captures the response (headers and body) and schedules a ccf::tasks task; all NodeState::lock-holding work then runs on a task thread, as it did before the curl migration (when the callback ran on the enclave RPC thread). This also removes the latent self-deadlock where a synchronous abort during shutdown could re-enter the callback while lock was held. Aborted requests are dropped on the libuv thread without scheduling a task. --- src/node/node_state.h | 523 ++++++++++++++++++++++-------------------- 1 file changed, 277 insertions(+), 246 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 57f596c66c5..cc2192ba56a 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1149,316 +1149,347 @@ namespace ccf std::unique_ptr&& request, CURLcode curl_response, long status_code) { - std::lock_guard guard(lock); - if (!sm.check(NodeStartupState::pending)) + 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; } - try - { - if (curl_response == CURLE_ABORTED_BY_CALLBACK) + // 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)) { - // The request was aborted, e.g. during node shutdown. return; } - if (curl_response != CURLE_OK) + try { - // 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)) + if (curl_response != CURLE_OK) { - LOG_INFO_FMT( - "Transient error contacting {} to join: {} ({}). The join " - "timer will retry.", + // 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; + } + + // Fatal TLS/protocol-layer failure. A wrong or expired service + // certificate surfaces here as a peer verification failure, + // which we flag distinctly so it can be told apart from other + // fatal errors. + const bool invalid_service_certificate = + 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, + invalid_service_certificate ? + "invalid service certificate: " : + "", 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; } - // Fatal TLS/protocol-layer failure. A wrong or expired service - // certificate surfaces here as a peer verification failure, - // which we flag distinctly so it can be told apart from other - // fatal errors. - const bool invalid_service_certificate = - 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, - invalid_service_certificate ? "invalid service certificate: " : - "", - 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; - } + const auto status = static_cast(status_code); + const auto& headers = *response_headers; + const auto& data = *response_body; - const auto status = static_cast(status_code); - const auto& headers = request->get_response_headers(); - const auto& data = request->get_response_body()->buffer; + if (is_http_status_client_error(status)) + { + std::optional error_response = + std::nullopt; - if (is_http_status_client_error(status)) - { - std::optional error_response = - std::nullopt; + 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())); + } - 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: " - "{}", + 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; + } + + 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 ( - error_response.has_value() && - error_response->error.code == ccf::errors::StartupSeqnoIsOld && - config.join.fetch_recent_snapshot) + if (status != HTTP_STATUS_OK) { - 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 + const auto& location = headers.find(http::headers::LOCATION); if ( - snapshot_fetch_task != nullptr && - !snapshot_fetch_task->is_cancelled()) + config.join.follow_redirect && + (status == HTTP_STATUS_PERMANENT_REDIRECT || + status == HTTP_STATUS_TEMPORARY_REDIRECT) && + location != headers.end()) { - LOG_INFO_FMT("Snapshot fetch already in progress, skipping"); + 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 { - snapshot_fetch_task = std::make_shared( - config.join, config.snapshots, this); - ccf::tasks::add_task(snapshot_fetch_task); + 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; } - 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()) + JoinNetworkNodeToNode::Out resp; + try { - 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); + auto j = ccf::parse_json_safe(data); + resp = j.get(); } - else + catch (const std::exception& e) { 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; - } - - 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"); + "An error occurred while parsing the join network response"); - LOG_DEBUG_FMT("Join network response error: {}", e.what()); - LOG_DEBUG_FMT( - "Join network response body: {}", - 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())); - // 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)); - - history->set_service_signing_identity( - network.identity->get_key_pair(), - resp.network_info->cose_signatures_config.value_or( - ccf::COSESignaturesConfig{})); - - ccf::crypto::Pem n2n_channels_cert; - if (!resp.network_info->endorsed_certificate.has_value()) + // Set network secrets, node id and become part of network. + if (resp.node_status == NodeStatus::TRUSTED) { - // 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.has_value()) + { + throw std::logic_error( + "Expected network info in join response"); + } - setup_consensus( - resp.network_info->public_only, n2n_channels_cert); - auto_refresh_jwt_keys(); + network.identity = std::make_unique( + resp.network_info->identity); + network.ledger_secrets->init_from_map( + std::move(resp.network_info->ledger_secrets)); - 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); - } + history->set_service_signing_identity( + network.identity->get_key_pair(), + resp.network_info->cose_signatures_config.value_or( + ccf::COSESignaturesConfig{})); - 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) + ccf::crypto::Pem n2n_channels_cert; + if (!resp.network_info->endorsed_certificate.has_value()) { - hook->call(consensus.get()); + // 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(); - auto tx = network.tables->create_read_only_tx(); - view = resolve_latest_sig_view(tx); + setup_consensus( + resp.network_info->public_only, n2n_channels_cert); + auto_refresh_jwt_keys(); - if (!resp.network_info->public_only) + 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(); + last_recovered_signed_idx = + resp.network_info->last_recovered_signed_idx; + setup_recovery_hook(); + snapshotter->set_snapshot_generation(false); } - LOG_INFO_FMT( - "Joiner successfully resumed from snapshot at seqno {} and " - "view {}", + 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); + } + + consensus->init_as_backup( network.tables->current_version(), - view); - } + view, + view_history_, + last_recovered_signed_idx); - consensus->init_as_backup( - network.tables->current_version(), - view, - view_history_, - last_recovered_signed_idx); + { + 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(); - { - 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 (resp.network_info->public_only) { - snapshotter->init_from_snapshot_status( - snapshot_status.value()); + sm.advance(NodeStartupState::partOfPublicNetwork); + } + else + { + reset_data(quote_info.quote); + reset_data(quote_info.endorsements); + sm.advance(NodeStartupState::partOfNetwork); } - } - 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) + { + join_periodic_task->cancel_task(); + join_periodic_task = nullptr; + } - if (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) { - join_periodic_task->cancel_task(); - join_periodic_task = nullptr; + LOG_INFO_FMT( + "Node {} is waiting for votes of members to be trusted", + self); } - - 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) + catch (const std::exception& e) { - LOG_INFO_FMT( - "Node {} is waiting for votes of members to be trusted", self); + LOG_FAIL_FMT( + "Unhandled error while processing join response from {}: {}", + target_address, + e.what()); } - } - catch (const std::exception& e) - { - LOG_FAIL_FMT( - "Unhandled error while processing join response from {}: {}", - target_address, - e.what()); - } + })); }; // NOLINTEND(readability-function-cognitive-complexity) From 242d31c0e4c5fa88ddf7fb8dc1cd57b0a940cfed Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 8 Jul 2026 06:26:33 +0000 Subject: [PATCH 11/19] Add curl test for TLS hostname verification (VERIFYHOST) Guard the TLS hostname-verification hardening the node join client relies on. The e2e_curl harness now also serves HTTPS with a self-signed certificate whose only SAN is a dNSName that does not cover the loopback IP it is served on. A new curl_test then asserts, with that certificate pinned as the CA, that CURLOPT_SSL_VERIFYHOST=2 rejects a request to the IP (CURLE_PEER_FAILED_VERIFICATION), accepts one to the SAN name resolved to the same address, and that VERIFYHOST=0 accepts the mismatch (control, proving the hostname check is the sole discriminator). The test skips cleanly when run outside the harness. --- src/http/test/curl_test.cpp | 109 ++++++++++++++++++++++++++++++++++++ tests/e2e_curl.py | 85 +++++++++++++++++++++++++--- 2 files changed, 185 insertions(+), 9 deletions(-) diff --git a/src/http/test/curl_test.cpp b/src/http/test/curl_test.cpp index 807ec6d6193..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 @@ -269,6 +270,114 @@ TEST_CASE("Synchronous POST echoes body") 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/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__": From a399037ab8770e2ea438a7a581095f5757bf4172 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 8 Jul 2026 16:45:16 +0000 Subject: [PATCH 12/19] Gate node join retries to a single in-flight request The periodic join timer fires every config.join.retry_timeout (default 1s), but a join attempt can remain in flight for up to CONNECTTIMEOUT/TIMEOUT. Because the join client shares the CurlmLibuvContextSingleton with the JWT refresh, endorsements and snapshot-fetch clients, unguarded retries against a slow or unresponsive target could accumulate in-flight requests and starve those other users. Track an atomic join_request_in_flight flag: initiate_join_unsafe() skips the retry if a request is already in flight, sets the flag before attaching the request, and the response callback clears it (on the libuv thread, so via an atomic store rather than NodeState::lock) for every completion path including synchronous abort during shutdown. --- src/node/node_state.h | 44 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index cc2192ba56a..9d49bbe110d 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -479,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; @@ -1049,6 +1059,21 @@ namespace ccf { sm.expect(NodeStartupState::pending); + // 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; + } + // Assemble the join request body. JoinNetworkNodeToNode::In join_params; join_params.node_info_network = config.network; @@ -1106,9 +1131,10 @@ namespace ccf config.join.service_cert.size()); curl_handle.set_opt(CURLOPT_CAPATH, nullptr); - // Bound each attempt so a stalled connection is abandoned and the - // periodic join timer can retry, rather than accumulating in-flight - // requests. A timeout surfaces as a transient error and is retried. + // 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); @@ -1149,6 +1175,13 @@ namespace ccf 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_request_in_flight.store(false); + if (curl_response == CURLE_ABORTED_BY_CALLBACK) { // Aborted, e.g. during node shutdown. Nothing to process, and @@ -1502,6 +1535,11 @@ namespace ccf std::make_unique(max_join_response_size), std::move(response_callback)); + // 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. + join_request_in_flight.store(true); ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request( std::move(join_request)); } From d6a5e4bc0ca08a41adc5d99e38fc55d365025865 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Wed, 8 Jul 2026 17:07:53 +0000 Subject: [PATCH 13/19] Harden join in-flight gate: reset flag if attach_request throws Follow-up to the single-in-flight join gate. If attach_request (or the singleton accessor) throws after the in-flight flag is set - realistically only under an OOM-class bad_alloc - the response callback never runs, so the flag would remain set and the periodic timer would gate out every future retry, stranding the node in pending. Wrap the handoff so the flag is reset on the exceptional path. Also add two clarifying comments: that join liveness depends on the curl singleton invoking each request callback exactly once, and that start_join_timer's initial initiate_join_unsafe call relies on the caller holding NodeState::lock. --- src/node/node_state.h | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 9d49bbe110d..f20a189b40a 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1179,7 +1179,9 @@ namespace ccf // 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. + // 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) @@ -1538,10 +1540,20 @@ namespace ccf // 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. + // 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); - ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request( - std::move(join_request)); + try + { + ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request( + std::move(join_request)); + } + catch (...) + { + join_request_in_flight.store(false); + throw; + } } void initiate_join() @@ -1552,6 +1564,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]() { From e04b34d44e2e60b7aa8947ddd24c4aca68344da7 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Thu, 9 Jul 2026 06:38:30 +0000 Subject: [PATCH 14/19] Use accurate TLS verification marker in join failure message The join client flagged CURLE_PEER_FAILED_VERIFICATION (and CURLE_SSL_CACERT_BADFILE) as an "invalid service certificate", but with VERIFYHOST=2 that CURLcode is also returned for a hostname/SAN mismatch or any other peer certificate verification failure, so the wording could mislead operators. Replace the marker with the accurate "TLS certificate verification failed", rename the local bool, and broaden the code comment. Update the test infra matcher and comment in network.py to the new marker (still matching the legacy "invalid cert on handshake" wording for mixed-version tests). The marker was introduced by this unreleased PR, so there is no released node emitting the old text. --- src/node/node_state.h | 16 +++++++++------- tests/infra/network.py | 15 +++++++++------ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index 180f318a052..a560c6d9724 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1244,19 +1244,21 @@ namespace ccf return; } - // Fatal TLS/protocol-layer failure. A wrong or expired service - // certificate surfaces here as a peer verification failure, - // which we flag distinctly so it can be told apart from other - // fatal errors. - const bool invalid_service_certificate = + // Fatal TLS/protocol-layer failure. Certificate verification + // failures surface here: an untrusted or expired service + // certificate, but equally a hostname/SAN mismatch + // (VERIFYHOST=2) or any other peer verification failure. Flag + // them with a stable marker so they can be told apart from + // other fatal errors in logs and tests. + const bool tls_certificate_verification_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, - invalid_service_certificate ? - "invalid service certificate: " : + tls_certificate_verification_failed ? + "TLS certificate verification failed: " : "", curl_easy_strerror(curl_response), static_cast(curl_response)); diff --git a/tests/infra/network.py b/tests/infra/network.py index 45ba64d58c5..ed92f7bd49d 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -1421,13 +1421,16 @@ def run_join_node( if "StartupSeqnoIsOld" in error: raise StartupSeqnoIsOld(node, has_stopped, error) from e # The joining node now connects to the target via the - # curl client, which reports a rejected service - # certificate as "invalid service certificate". The - # legacy TLS-session wording ("invalid cert on - # handshake") is retained for compatibility with logs - # from older nodes during mixed-version tests. + # curl client, which reports any TLS peer certificate + # verification failure as "TLS certificate verification + # failed": a rejected or untrusted service certificate, + # but also a hostname/SAN mismatch (VERIFYHOST=2) or any + # other peer verification failure. The legacy + # TLS-session wording ("invalid cert on handshake") is + # retained for compatibility with logs from older nodes + # during mixed-version tests. if ( - "invalid service certificate" in error + "TLS certificate verification failed" in error or "invalid cert on handshake" in error ): raise ServiceCertificateInvalid( From e154062dbc53cc5eec0c8f5ec84a2ca254f10c42 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Thu, 9 Jul 2026 07:05:26 +0000 Subject: [PATCH 15/19] Clarify join TLS failure marker to cover CA-load failures Address review nits on the join certificate-failure path. The fatal branch fires for both CURLE_PEER_FAILED_VERIFICATION and CURLE_SSL_CACERT_BADFILE, but the previous "TLS certificate verification failed" marker and comments only described peer verification, not the case where the configured service certificate blob cannot be loaded (CURLE_SSL_CACERT_BADFILE). Reword the marker to the more accurate "TLS certificate trust check failed" (updating the local bool and the test-infra matcher to match), expand both code comments to enumerate both underlying causes, and document the ServiceCertificateInvalid test exception, which now covers all join-time certificate trust failures rather than only a literally invalid certificate. --- src/node/node_state.h | 20 +++++++++++--------- tests/infra/network.py | 17 +++++++++++------ 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/node/node_state.h b/src/node/node_state.h index a560c6d9724..ee9f35c6f05 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1244,21 +1244,23 @@ namespace ccf return; } - // Fatal TLS/protocol-layer failure. Certificate verification - // failures surface here: an untrusted or expired service - // certificate, but equally a hostname/SAN mismatch - // (VERIFYHOST=2) or any other peer verification failure. Flag - // them with a stable marker so they can be told apart from - // other fatal errors in logs and tests. - const bool tls_certificate_verification_failed = + // 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_verification_failed ? - "TLS certificate verification failed: " : + tls_certificate_trust_check_failed ? + "TLS certificate trust check failed: " : "", curl_easy_strerror(curl_response), static_cast(curl_response)); diff --git a/tests/infra/network.py b/tests/infra/network.py index ed92f7bd49d..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): @@ -1421,16 +1425,17 @@ def run_join_node( if "StartupSeqnoIsOld" in error: raise StartupSeqnoIsOld(node, has_stopped, error) from e # The joining node now connects to the target via the - # curl client, which reports any TLS peer certificate - # verification failure as "TLS certificate verification + # curl client, which reports any failure to establish + # certificate trust as "TLS certificate trust check # failed": a rejected or untrusted service certificate, - # but also a hostname/SAN mismatch (VERIFYHOST=2) or any - # other peer verification failure. The legacy + # 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 verification failed" in error + "TLS certificate trust check failed" in error or "invalid cert on handshake" in error ): raise ServiceCertificateInvalid( From de9765fe56aa56138e3ca766d9d34f33c952f35d Mon Sep 17 00:00:00 2001 From: achamayou <4016369+achamayou@users.noreply.github.com> Date: Thu, 9 Jul 2026 09:58:07 +0000 Subject: [PATCH 16/19] Drop migration plan doc; document CURLOPT_POST; highlight SAN check - Remove doc/dev/join_curl_migration_plan.md; the plan is preserved as a comment on PR #8040 rather than shipped in the tree. - Add an explanatory comment for CURLOPT_POST=1L in the curl wrapper, linking to the libcurl documentation. - Split the CHANGELOG entry so the new join-time target RPC address SAN check (CURLOPT_SSL_VERIFYHOST) is highlighted as its own item. --- CHANGELOG.md | 3 +- doc/dev/join_curl_migration_plan.md | 241 ---------------------------- src/http/curl.h | 3 + 3 files changed, 5 insertions(+), 242 deletions(-) delete mode 100644 doc/dev/join_curl_migration_plan.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 3db81e19405..7a929b82b9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### 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). TLS certificate hostname verification is now enforced on the join connection: operators must ensure the target node's certificate subject alternative names cover the address configured in `join.target_rpc_address` (#8040). +- 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). - JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). - JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). - Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). diff --git a/doc/dev/join_curl_migration_plan.md b/doc/dev/join_curl_migration_plan.md deleted file mode 100644 index bfd54b4a8b0..00000000000 --- a/doc/dev/join_curl_migration_plan.md +++ /dev/null @@ -1,241 +0,0 @@ -# Migrating the node join client from the legacy HTTP client to libcurl - -Tracking issue: [#7262](https://github.com/microsoft/CCF/issues/7262) - "Migrate -httpclients over to libcurl". The node join sequence is the **last** remaining -user of `RPCSessions::create_client()` (the legacy in-enclave TLS HTTP client). -Migrating it lets us delete the remaining legacy HTTP client infrastructure. - -Reference implementation: [#8005](https://github.com/microsoft/CCF/pull/8005), -which migrated JWT/JWK OpenID discovery and key refresh to the shared curl-multi -client. The endorsements client ([#7102]) and snapshot fetch already use the same -curl client, and the snapshot fetch runs inside the join flow, so there is a -directly comparable node-to-node precedent. - -## Goals - -- Replace the legacy HTTP client in `NodeState::initiate_join_unsafe()` with the - async curl-multi client (`ccf::curl::CurlmLibuvContextSingleton`). -- Preserve the join trust model exactly: the **service certificate is the only - trust anchor**, and the joining node presents its self-signed node certificate - as a client certificate (mutual TLS). -- **Strengthen** TLS peer verification by enabling certificate hostname - verification (`CURLOPT_SSL_VERIFYHOST=2`), which the legacy path did not do. -- Preserve every existing join control-flow branch (redirects, snapshot re-fetch - on `StartupSeqnoIsOld`, pending/trusted handling, fatal errors). -- Remove the now-dead legacy HTTP client infrastructure. -- Lean on existing test coverage as validation gates, and plug the gaps that the - behavioural change introduces. - -## Non-goals - -- No change to the `/node/join` server endpoint or the join wire protocol. -- No change to the in-process node client (`NodeClient` / `HTTPNodeClient`), - which is a loopback handler dispatch, not a network/TLS client. -- No change to the server-side TLS/HTTP session stack (nodes still serve RPC). - -## Current implementation (baseline) - -`NodeState::initiate_join_unsafe()` in `src/node/node_state.h`: - -```cpp -auto network_ca = std::make_shared<::tls::CA>(std::string( - config.join.service_cert.begin(), config.join.service_cert.end())); - -auto join_client_cert = std::make_unique<::tls::Cert>( - network_ca, - self_signed_node_cert, - node_sign_kp->private_key_pem(), - target_host); - -auto join_client = rpcsessions->create_client( - std::move(join_client_cert), - rpcsessions->get_app_protocol_main_interface()); - -join_client->connect(target_host, target_port, /* response cb */, /* error cb */); -join_client->send_request(POST /node/join with JSON body); -``` - -Relevant properties of the baseline: - -- **Trust anchor**: `config.join.service_cert` only. `::tls::CA` builds an - `X509_STORE` containing exactly that certificate; the system CA bundle is never - consulted. `partial_ok` is `false` (full chain to the service cert required). -- **Peer verification**: `SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT` with - standard OpenSSL chain validation. -- **Hostname verification**: **none**. `::tls::Cert` uses `peer_hostname` only for - SNI (`SSL_set_tlsext_host_name`); there is no `X509_VERIFY_PARAM_set1_host` - call, so the target's certificate name is not checked. -- **Client authentication**: the node presents `self_signed_node_cert` + - `node_sign_kp` private key (at join time the node is not yet endorsed, so - `endorsed_node_cert` is `std::nullopt`). -- **Concurrency**: asynchronous. `connect()` returns immediately; the response - callback runs later and processes the result under `NodeState::lock` guarded by - `sm.check(NodeStartupState::pending)`. The lock is **not** held during network - I/O. -- **Retry**: `join_periodic_task` re-invokes `initiate_join_unsafe()` on a timer. - -### Join response-handling branches to preserve - -1. Transport/handshake failure -> fatal: write `AdminMessage::fatal_error_msg`, - node shuts down gracefully (not retried). -2. 4xx with `ODataError` code `StartupSeqnoIsOld` and `fetch_recent_snapshot` -> - schedule a `FetchSnapshot` task, return, and let the periodic timer retry. -3. Other 4xx -> fatal shutdown. -4. `307`/`308` with `follow_redirect` and a `Location` header -> update - `config.join.target_rpc_address` and let the periodic timer retry. -5. Other non-200 -> log failure, wait for retry. -6. `200 OK`, `node_status == TRUSTED` -> initialise identity / ledger secrets / - consensus, become part of (public) network, cancel the join timer. -7. `200 OK`, `node_status == PENDING` -> wait for member votes (retry). - -## Target implementation - -Model on `JwtKeyAutoRefresh::send_curl_get()` (`src/node/jwt_key_auto_refresh.h`) -for the TLS options and async dispatch, and on `recovery_decision_protocol.cpp` -for the client-certificate (mTLS) options. - -curl easy-handle options for the join request: - -| Option | Value | Rationale | -| -------------------------------------------- | -------------------------- | ------------------------------------------------------------------------------------------------------------ | -| `CURLOPT_CAINFO_BLOB` | `config.join.service_cert` | Trust anchor = service cert only (matches baseline). | -| `CURLOPT_CAPATH` | `nullptr` | Do **not** fall back to the system CA bundle. Critical: keeps the accepted-CA set identical to the baseline. | -| `CURLOPT_SSL_VERIFYPEER` | `1L` | Verify the peer chain (matches baseline). | -| `CURLOPT_SSL_VERIFYHOST` | `2L` | Verify the peer certificate name (hardening; see below). | -| `CURLOPT_PROTOCOLS_STR` | `"https"` | Restrict to HTTPS (matches JWT refresh). | -| `CURLOPT_SSLCERT_BLOB` | self-signed node cert | Client cert for mTLS (matches baseline). | -| `CURLOPT_SSLCERTTYPE` | `"PEM"` | | -| `CURLOPT_SSLKEY_BLOB` | node signing key | Client key for mTLS (matches baseline). | -| `CURLOPT_SSLKEYTYPE` | `"PEM"` | | -| `CURLOPT_CONNECTTIMEOUT` / `CURLOPT_TIMEOUT` | small fixed values | Bound transport time (matches JWT refresh). | - -- **Method/body**: POST `https://{target_rpc_address}/node/join` with the JSON - `JoinNetworkNodeToNode::In` body and `Content-Type: application/json`. -- **Response body cap**: a fixed, generous `ccf::curl::ResponseBody` maximum - (large enough for any realistic join response - identity, ledger secrets - but - far below a snapshot). No new operator-facing config option. -- **Redirects**: do **not** set `CURLOPT_FOLLOWLOCATION`. Handle redirects - manually exactly as today (inspect status + `Location`, update - `target_rpc_address`), so the trust anchor is re-applied to the new target and - the snapshot fetch picks up the redirected address. -- **Dispatch**: build the request under `lock`, then - `CurlmLibuvContextSingleton::get_instance()->attach_request(...)` and return. - The single `ResponseCallback` re-acquires `lock`, re-checks - `sm.check(pending)`, and maps `CURLcode` + HTTP status to the seven branches - above. Transport/TLS failures (`CURLcode != CURLE_OK`, e.g. - `CURLE_PEER_FAILED_VERIFICATION`, `CURLE_SSL_CACERT`) map to the current fatal - path and **must log a single stable, greppable message** so the test suite can - detect a rejected service certificate. - -### POST-with-body support in the curl wrapper (prerequisite) - -`ccf::curl::CurlRequest` currently wires GET, HEAD and PUT (upload) fully, but the -`HTTP_POST` branch assumes the caller pre-set `CURLOPT_POSTFIELDS` and does not -enable `CURLOPT_POST`. Add first-class POST-with-body support (set `CURLOPT_POST` -and `CURLOPT_POSTFIELDSIZE`, feeding the existing `RequestBody` read callback, or -an equivalent `COPYPOSTFIELDS` helper) so the join can send a JSON body cleanly, -and cover it in `src/http/test/curl_test.cpp`. - -## Security analysis: which root CAs are accepted - -| Property | Baseline (legacy client) | Migrated (curl) | Verdict | -| ----------------------- | ------------------------------------- | -------------------------------------------------- | ------------------------------------------------------------------------------------------------------------ | -| Accepted trust anchors | service cert only (`::tls::CA` store) | `CAINFO_BLOB` = service cert, `CAPATH` = `nullptr` | Identical | -| System CA fallback | never | disabled via `CAPATH=nullptr` | Identical | -| Chain building | full chain, `partial_ok=false` | curl default (no partial chain) | Identical (service identity is a self-signed root; snapshot fetch already proves this against the same peer) | -| Peer chain verification | `SSL_VERIFY_PEER` | `SSL_VERIFYPEER=1` | Identical | -| Certificate name check | none (SNI only) | `SSL_VERIFYHOST=2` | **Strengthened** | -| Client auth (mTLS) | self-signed node cert | `SSLCERT_BLOB` self-signed node cert | Identical | - -**Hostname verification hardening.** Enabling `VERIFYHOST=2` means a join fails if -the host in `join.target_rpc_address` is not present in the target node's -certificate SANs. CCF derives node-cert SANs from -`config.node_certificate.subject_alt_names`, or, by default, from each RPC -interface's `published_address` (`get_subject_alternative_names()`), with IPs -emitted as `iPAddress` SANs and names as `dNSName`. Redirect targets are built -from those same published addresses. The snapshot fetch inside the join flow -already uses curl with the default `VERIFYHOST=2` against the _same_ -`target_rpc_address` and _same_ `service_cert`, so standard deployments and the -test suite already satisfy this constraint. The residual risk is a deployment that -connects to an address deliberately absent from the target SANs; this is a -behavioural change and must be documented in `CHANGELOG.md`. - -## Test strategy and validation gates - -### Existing coverage to lean on - -- **Security gate**: `tests/reconfiguration.py::test_add_node_invalid_service_cert` - joins with the wrong service certificate and expects - `infra.network.ServiceCertificateInvalid`. That exception is raised in - `tests/infra/network.py::run_join_node()` by matching the log string - `"invalid cert on handshake"` (emitted by `src/enclave/tls_session.h`). The curl - path emits a **different** message, so this detection string must be updated to - match the new stable TLS-failure log. This is the primary gap. -- **Happy paths**: `test_add_node`, `test_add_node_from_snapshot`, - `test_add_node_from_backup`, `test_add_as_many_pending_nodes`. -- **Redirects**: `test_join_straddling_primary_replacement`, and - `start_network.py --redirection_kind node-by-role`. -- **Fake/duplicate joins**: `test_issue_fake_join`, `node_frontend_test.cpp`. -- **Attestation error mapping**: `code_update.py` SNP join tests exercise the - `MeasurementNotFound` / `HostDataNotFound` / `UVMEndorsementsNotAuthorised` - detection in `run_join_node()`; these must still work with curl error handling. -- **Recovery / public-network join**: `recovery.py`. -- **curl mechanics**: `src/http/test/curl_test.cpp`. - -### Gaps to plug - -1. Update the `ServiceCertificateInvalid` detection in `tests/infra/network.py` - to the new curl TLS-failure log line. -2. Add `curl_test.cpp` coverage for POST-with-body and mTLS client certificates. -3. Add/confirm coverage that `VERIFYHOST=2` succeeds for both DNS-name and IP - published addresses, and after a redirect. -4. (Recommended) Add a negative test that a join to an address **not** in the - target SANs is now rejected, to regression-protect the hardening. - -## Removal scope (after the join no longer uses the legacy client) - -Remove (confirmed to have no other users once the join and the dead -`make_http_request` are gone): - -- `NodeState::make_http_request()` (`src/node/node_state.h`) and the - `AbstractNodeState::make_http_request` declaration - (`src/node/rpc/node_interface.h`). Already dead (no callers). -- `RPCSessions::create_client()` and `RPCSessions::create_unencrypted_client()` - (`src/enclave/rpc_sessions.h`). The latter is already dead. -- `ClientSession` (`src/enclave/client_session.h`), `HTTPClientSession` - (`src/http/http_session.h`), `HTTP2ClientSession` (`src/http/http2_session.h`), - `UnencryptedHTTPClientSession` (`src/http/http_session.h`), plus their includes. - -Keep: - -- `NodeClient` / `HTTPNodeClient` (`src/node/node_client.h`, - `src/node/http_node_client.h`): in-process handler dispatch - (`RpcHandler::process`), not a network/TLS client. Used by - `retired_nodes_cleanup.h` and `raft.h`. -- `::tls::Client` (`src/tls/client.h`): a TLS-layer primitive still exercised by - the TLS unit test (`src/tls/test/main.cpp`); the server side still relies on the - TLS layer. (Optional stretch: remove it too and trim the test.) -- All server-side TLS/HTTP session classes. - -## Staged delivery (each stage is independently reviewable and gated) - -- **Stage 0 - Branch + this plan.** Branch `join-client-curl-migration`. -- **Stage 1 - curl wrapper POST-with-body.** Extend `ccf::curl::CurlRequest` + - `curl_test.cpp` coverage (POST body, mTLS). Gate: `curl_test`. -- **Stage 2 - curl-based join.** Rewrite the client half of - `initiate_join_unsafe()`; remove the legacy `tls::CA`/`tls::Cert`/`create_client` - join code. Preserve all seven branches. Gate: build + `node_frontend_test`. -- **Stage 3 - Test infra + e2e gates.** Update the `ServiceCertificateInvalid` - detection string; run the join e2e gates; add the hostname-verification - coverage. Gate: e2e join suite green. -- **Stage 4 - Remove dead httpclient infra.** Delete the removal set above; grep - for zero references; build all targets and run affected unit tests. Gate: full - build + unit tests. -- **Stage 5 - Docs, changelog, compatibility.** `CHANGELOG.md` (Changed: curl - join migration + `VERIFYHOST` hardening migration note + infra removal), bump - `python/pyproject.toml` if a new version is cut, review - `doc/operations/start_network.rst` wording, run `ci-checks.sh` and formatting/ - linting, and run `lts_compatibility` with `LONG_TESTS=1` (join touches - cross-version TLS and protocol paths). - -[#7102]: https://github.com/microsoft/CCF/pull/7102 diff --git a/src/http/curl.h b/src/http/curl.h index d891071ea46..00029c056d6 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -526,6 +526,9 @@ namespace ccf::curl break; case HTTP_POST: { + // 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) { From e145f807143b9ab0c4a709092fc436e614b20758 Mon Sep 17 00:00:00 2001 From: achamayou <4016369+achamayou@users.noreply.github.com> Date: Thu, 9 Jul 2026 10:03:11 +0000 Subject: [PATCH 17/19] Move #8040 changelog entries to new 7.0.8 section 7.0.7 has been released, so this PR's entries belong in a new dev version. Open [7.0.8] with the #8040 Changed and Removed entries and bump python/pyproject.toml to 7.0.8 to match. --- CHANGELOG.md | 17 ++++++++++++----- python/pyproject.toml | 2 +- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a929b82b9b..28c9b863f3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,22 +5,29 @@ 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.7] +## [7.0.8] -[7.0.7]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.7 +[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). -- JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). -- JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). -- Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). ### 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 + +### Changed + +- JWT/JWK auto-refresh outbound HTTP fetches (OpenID metadata and JWKS) now use the curl multi singleton client introduced in #7102, replacing the previous `RPCSessions::create_client()` path. Connection and TLS failures are now counted in refresh failure metrics via `send_refresh_jwt_keys_error()`, improving observability of network-level refresh errors (#7989). +- JWT/JWK auto-refresh now supports configuring the maximum response body size for fetched OpenID metadata and JWKS via the `jwt.key_refresh_max_response_size` node startup config setting (#7989). +- Fatal task worker stack traces now use libbacktrace for improved function and source-location resolution. Building CCF now requires the libbacktrace development package, and the RPM development package depends on `libbacktrace-static` (#7721). + ### Fixed - JS endpoint string values (e.g. response bodies, KV keys/values) containing embedded NUL bytes are no longer silently truncated at the first NUL when converted to their C++ representation (#8033). 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" }, ] From 11f0581ddf42eb3bff10338b2473a319d9cb0a27 Mon Sep 17 00:00:00 2001 From: achamayou <4016369+achamayou@users.noreply.github.com> Date: Thu, 9 Jul 2026 11:11:29 +0000 Subject: [PATCH 18/19] Clarify curl body-size options and oversized join response error - Set CURLOPT_INFILESIZE only for PUT uploads; POST already declares its body size via CURLOPT_POSTFIELDSIZE_LARGE, so the shared attach_to_curl no longer sets it redundantly. - Map CURLE_WRITE_ERROR on the join to a clear fatal message naming the response size cap, instead of libcurl's generic write error. --- src/http/curl.h | 12 +++++++++++- src/node/node_state.h | 17 +++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/http/curl.h b/src/http/curl.h index 00029c056d6..c060c5893c8 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -272,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. } }; @@ -522,6 +524,14 @@ 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: diff --git a/src/node/node_state.h b/src/node/node_state.h index ee9f35c6f05..2d7f2f84dee 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1244,6 +1244,23 @@ namespace ccf 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, From 4a416baf8fb54ba8fe1f4526e8710d891090d947 Mon Sep 17 00:00:00 2001 From: achamayou <4016369+achamayou@users.noreply.github.com> Date: Thu, 9 Jul 2026 15:07:12 +0000 Subject: [PATCH 19/19] Add e2e test for join target certificate SAN (VERIFYHOST) check 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. Add run_tls_san_join_mismatch, a dedicated-network test where the target's file-serving interface binds an address deliberately absent from the node certificate SANs, so a joining node dialling it is rejected with ServiceCertificateInvalid. This exercises the hostname/SAN check specifically, distinct from the untrusted-service-certificate path already covered by test_add_node_invalid_service_cert. --- tests/e2e_operations.py | 67 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/e2e_operations.py b/tests/e2e_operations.py index e170bda7231..88d418baf17 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)