From bab88059868a62686e5f8769f325acb97c421a01 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Jun 2026 20:27:12 +0000 Subject: [PATCH 01/20] Initial plan From f96215d9f409464f869acf35c678f47d218296ca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Jun 2026 20:50:30 +0000 Subject: [PATCH 02/20] Migrate JWT/JWK auto-refresh from RPCSessions to curl multi singleton Replace rpcsessions->create_client() HTTP fetches in JwtKeyAutoRefresh with ccf::curl::CurlRequest attached to CurlmLibuvContextSingleton, following the pattern established in #7102 for QuoteEndorsementsClient. Changes: - jwt_key_auto_refresh.h: inherit enable_shared_from_this, add send_curl_get() helper with CURLOPT_CAINFO_BLOB CA bundle support, migrate both metadata and JWKS fetches to curl with task-deferred response callbacks - node_state.h: remove rpcsessions parameter from JwtKeyAutoRefresh constructor call - CHANGELOG.md: add [Unreleased] entry for this change Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- CHANGELOG.md | 6 ++ src/node/jwt_key_auto_refresh.h | 177 +++++++++++++++++++++----------- src/node/node_state.h | 1 - 3 files changed, 121 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e2ef85f32c8..924b05a98425 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ 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). +## [Unreleased] + +### 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). + ## [7.0.6] [7.0.6]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.6 diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 9738c36d70db..cb181cd2f58f 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -4,22 +4,26 @@ #include "ccf/ds/json.h" #include "ccf/service/tables/jwt.h" +#include "http/curl.h" #include "http/http_builder.h" #include "http/http_rpc_context.h" #include "node/rpc/node_frontend.h" +#include "tasks/basic_task.h" +#include "tasks/task_system.h" #define FMT_HEADER_ONLY +#include #include namespace ccf { class JwtKeyAutoRefresh + : public std::enable_shared_from_this { private: size_t refresh_interval_s; NetworkState& network; std::shared_ptr consensus; - std::shared_ptr rpcsessions; std::shared_ptr rpc_map; ccf::crypto::ECKeyPairPtr node_sign_kp; ccf::crypto::Pem node_cert; @@ -27,19 +31,49 @@ namespace ccf ccf::tasks::Task periodic_refresh_task; + // Maximum response body size for JWK/metadata responses (1 MB) + static constexpr size_t max_response_size = 1024 * 1024; + + void send_curl_get( + const std::string& url, + const std::string& ca_bundle_pem, + ccf::curl::CurlRequest::ResponseCallback callback) + { + ccf::curl::UniqueCURL curl_handle; + curl_handle.set_opt(CURLOPT_HTTPGET, 1L); + curl_handle.set_opt(CURLOPT_SSL_VERIFYPEER, 1L); + curl_handle.set_opt(CURLOPT_SSL_VERIFYHOST, 2L); + curl_handle.set_blob_opt( + CURLOPT_CAINFO_BLOB, + reinterpret_cast(ca_bundle_pem.data()), + ca_bundle_pem.size()); + + ccf::curl::UniqueSlist headers; + + auto request = std::make_unique( + std::move(curl_handle), + HTTP_GET, + url, + std::move(headers), + nullptr, + std::make_unique(max_response_size), + std::move(callback)); + + ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request( + std::move(request)); + } + public: JwtKeyAutoRefresh( size_t refresh_interval_s, NetworkState& network, const std::shared_ptr& consensus, - const std::shared_ptr& rpcsessions, const std::shared_ptr& rpc_map, ccf::crypto::ECKeyPairPtr node_sign_kp, ccf::crypto::Pem node_cert) : refresh_interval_s(refresh_interval_s), network(network), consensus(consensus), - rpcsessions(rpcsessions), rpc_map(rpc_map), node_sign_kp(std::move(node_sign_kp)), node_cert(std::move(node_cert)), @@ -187,7 +221,7 @@ namespace ccf void handle_jwt_metadata_response( const std::string& issuer, - std::shared_ptr<::tls::CA> ca, + std::string ca_bundle_pem, ccf::http_status status, std::vector&& data) { @@ -226,12 +260,11 @@ namespace ccf send_refresh_jwt_keys_error(); return; } - ::http::URL jwks_url; try { - jwks_url = ::http::parse_url_full(jwks_url_str); + ::http::parse_url_full(jwks_url_str); } - catch (const std::invalid_argument& e) + catch (const std::invalid_argument&) { LOG_FAIL_FMT( "JWT key auto-refresh: Cannot parse jwks_uri for issuer '{}': {}", @@ -240,10 +273,6 @@ namespace ccf send_refresh_jwt_keys_error(); return; } - auto jwks_url_port = !jwks_url.port.empty() ? jwks_url.port : "443"; - - auto ca_cert = std::make_shared<::tls::Cert>( - ca, std::nullopt, std::nullopt, jwks_url.host); std::optional issuer_constraint{std::nullopt}; const auto constraint = metadata.find("issuer"); @@ -253,27 +282,39 @@ namespace ccf } LOG_DEBUG_FMT( - "JWT key auto-refresh: Requesting JWKS at https://{}:{}{}", - jwks_url.host, - jwks_url_port, - jwks_url.path); - auto http_client = rpcsessions->create_client(ca_cert); - // Note: Connection errors are not signalled and hence not tracked in - // endpoint metrics currently. - http_client->connect( - std::string(jwks_url.host), - std::string(jwks_url_port), - [this, issuer, issuer_constraint]( - ccf::http_status status, - http::HeaderMap&&, - std::vector&& data) { - handle_jwt_jwks_response( - issuer, issuer_constraint, status, std::move(data)); - return true; - }); - ::http::Request r(jwks_url.path, HTTP_GET); - r.set_header(ccf::http::headers::HOST, std::string(jwks_url.host)); - http_client->send_request(std::move(r)); + "JWT key auto-refresh: Requesting JWKS at {}", jwks_url_str); + + auto response_callback = + [self = shared_from_this(), issuer, issuer_constraint]( + std::unique_ptr&& request, + CURLcode curl_response, + long status_code) { + auto response_body_sp = 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( + [self, issuer, issuer_constraint, curl_response, status_code, response_body_sp]() { + if (curl_response != CURLE_OK) + { + LOG_FAIL_FMT( + "JWT key auto-refresh: Failed to fetch JWKS for issuer '{}': " + "{} ({})", + issuer, + curl_easy_strerror(curl_response), + curl_response); + self->send_refresh_jwt_keys_error(); + return; + } + self->handle_jwt_jwks_response( + issuer, + issuer_constraint, + static_cast(status_code), + std::move(*response_body_sp)); + })); + }; + + send_curl_get(jwks_url_str, ca_bundle_pem, std::move(response_callback)); } void refresh_jwt_keys() @@ -312,38 +353,50 @@ namespace ccf return true; } - auto metadata_url_str = issuer + "/.well-known/openid-configuration"; - auto metadata_url = ::http::parse_url_full(metadata_url_str); - auto metadata_url_port = - !metadata_url.port.empty() ? metadata_url.port : "443"; - - auto ca_pems = - crypto::split_x509_cert_bundle(ca_cert_bundle_pem.value()); - auto ca = std::make_shared<::tls::CA>(ca_pems); - auto ca_cert = std::make_shared<::tls::Cert>( - ca, std::nullopt, std::nullopt, metadata_url.host); + auto metadata_url = issuer + "/.well-known/openid-configuration"; LOG_DEBUG_FMT( - "JWT key auto-refresh: Requesting OpenID metadata at https://{}:{}{}", - metadata_url.host, - metadata_url_port, - metadata_url.path); - auto http_client = rpcsessions->create_client(ca_cert); - // Note: Connection errors are not signalled and hence not tracked in - // endpoint metrics currently. - http_client->connect( - std::string(metadata_url.host), - std::string(metadata_url_port), - [this, issuer, ca]( - ccf::http_status status, - ccf::http::HeaderMap&&, - std::vector&& data) { - handle_jwt_metadata_response(issuer, ca, status, std::move(data)); - return true; - }); - ::http::Request r(metadata_url.path, HTTP_GET); - r.set_header(ccf::http::headers::HOST, std::string(metadata_url.host)); - http_client->send_request(std::move(r)); + "JWT key auto-refresh: Requesting OpenID metadata at {}", + metadata_url); + + auto ca_bundle_pem = ca_cert_bundle_pem.value(); + + auto response_callback = + [self = shared_from_this(), issuer, ca_bundle_pem]( + std::unique_ptr&& request, + CURLcode curl_response, + long status_code) { + auto response_body_sp = 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( + [self, + issuer, + ca_bundle_pem, + curl_response, + status_code, + response_body_sp]() { + if (curl_response != CURLE_OK) + { + LOG_FAIL_FMT( + "JWT key auto-refresh: Failed to fetch OpenID metadata for " + "issuer '{}': {} ({})", + issuer, + curl_easy_strerror(curl_response), + curl_response); + self->send_refresh_jwt_keys_error(); + return; + } + self->handle_jwt_metadata_response( + issuer, + ca_bundle_pem, + static_cast(status_code), + std::move(*response_body_sp)); + })); + }; + + send_curl_get(metadata_url, ca_bundle_pem, std::move(response_callback)); return true; }); } diff --git a/src/node/node_state.h b/src/node/node_state.h index 8e66c00b159b..d0690218bc40 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1419,7 +1419,6 @@ namespace ccf config.jwt.key_refresh_interval.count_s(), network, consensus, - rpcsessions, rpc_map, node_sign_kp, self_signed_node_cert); From fd1936f0a0a956326dc134ecc7bbca2781170948 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Jun 2026 20:53:16 +0000 Subject: [PATCH 03/20] Address code review feedback: improve error messages and type safety Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index cb181cd2f58f..de8816999aa6 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -264,12 +264,13 @@ namespace ccf { ::http::parse_url_full(jwks_url_str); } - catch (const std::invalid_argument&) + catch (const std::invalid_argument& e) { LOG_FAIL_FMT( - "JWT key auto-refresh: Cannot parse jwks_uri for issuer '{}': {}", + "JWT key auto-refresh: Cannot parse jwks_uri for issuer '{}': {} ({})", issuer, - jwks_url_str); + jwks_url_str, + e.what()); send_refresh_jwt_keys_error(); return; } @@ -289,12 +290,13 @@ namespace ccf std::unique_ptr&& request, CURLcode curl_response, long status_code) { + auto http_status = static_cast(status_code); auto response_body_sp = 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( - [self, issuer, issuer_constraint, curl_response, status_code, response_body_sp]() { + [self, issuer, issuer_constraint, curl_response, http_status, response_body_sp]() { if (curl_response != CURLE_OK) { LOG_FAIL_FMT( @@ -309,7 +311,7 @@ namespace ccf self->handle_jwt_jwks_response( issuer, issuer_constraint, - static_cast(status_code), + http_status, std::move(*response_body_sp)); })); }; @@ -366,17 +368,13 @@ namespace ccf std::unique_ptr&& request, CURLcode curl_response, long status_code) { + auto http_status = static_cast(status_code); auto response_body_sp = 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( - [self, - issuer, - ca_bundle_pem, - curl_response, - status_code, - response_body_sp]() { + [self, issuer, ca_bundle_pem, curl_response, http_status, response_body_sp]() { if (curl_response != CURLE_OK) { LOG_FAIL_FMT( @@ -391,7 +389,7 @@ namespace ccf self->handle_jwt_metadata_response( issuer, ca_bundle_pem, - static_cast(status_code), + http_status, std::move(*response_body_sp)); })); }; From 976bc54a7d373db3398d1fa9f1de7e03a57390a6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 25 Jun 2026 20:54:01 +0000 Subject: [PATCH 04/20] Add comment explaining parse_url_full is validation-only Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index de8816999aa6..ee94a85989bc 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -260,6 +260,8 @@ namespace ccf send_refresh_jwt_keys_error(); return; } + // Validate jwks_uri before handing it to libcurl; the parsed result is + // not used directly since the full URL string is passed to curl. try { ::http::parse_url_full(jwks_url_str); From 98ab6405a9c4e77fca56236f06cff33a150d4149 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 26 Jun 2026 07:00:39 +0000 Subject: [PATCH 05/20] Fix SSL hostname verification for JWT issuer TLS certs The JWT issuer test server creates TLS certs using generate_cert which hashes the CN (e.g. sha256("localhost")), so libcurl's hostname verification fails when connecting to the server because neither the CN nor any SAN matches the actual hostname. Add an optional `san` parameter to `generate_cert` that, when provided, adds a DNS Subject Alternative Name extension to the certificate. Update JwtIssuer._generate_auth_data to pass the hostname as the SAN so that libcurl can successfully verify the server certificate. Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- tests/infra/crypto.py | 6 ++++++ tests/infra/jwt_issuer.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/infra/crypto.py b/tests/infra/crypto.py index 8602562560da..ad48fb44dfb2 100644 --- a/tests/infra/crypto.py +++ b/tests/infra/crypto.py @@ -133,6 +133,7 @@ def generate_cert( ca=False, valid_from=None, validity_days=10, + san: Optional[str] = None, ) -> str: cn = cn or "dummy" if issuer_priv_key_pem is None: @@ -175,6 +176,11 @@ def generate_cert( x509.BasicConstraints(ca=True, path_length=None), critical=True, ) + if san is not None: + builder = builder.add_extension( + x509.SubjectAlternativeName([x509.DNSName(san)]), + critical=False, + ) cert = builder.sign(issuer_priv, hashes.SHA256(), default_backend()) diff --git a/tests/infra/jwt_issuer.py b/tests/infra/jwt_issuer.py index 76f38ab70681..1ef125ee8c27 100644 --- a/tests/infra/jwt_issuer.py +++ b/tests/infra/jwt_issuer.py @@ -158,7 +158,7 @@ def _generate_auth_data(self, cn=None): else: raise ValueError(f"Unsupported algorithm: {self._alg}") - cert = infra.crypto.generate_cert(key_priv, cn=cn, ca=True) + cert = infra.crypto.generate_cert(key_priv, cn=cn, ca=True, san=cn) return (key_priv, key_pub), cert def __init__( From 4012868972103c503c69619eaeaad37cc9ac91b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 26 Jun 2026 08:44:36 +0000 Subject: [PATCH 06/20] Fix CI failures: clang-format, CHANGELOG version, JWT test baseline metrics Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- CHANGELOG.md | 10 ++++------ src/node/jwt_key_auto_refresh.h | 24 ++++++++++++++++++------ tests/jwt_test.py | 13 +++++++++---- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 924b05a98425..02f34ca81826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,12 +5,6 @@ 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). -## [Unreleased] - -### 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). - ## [7.0.6] [7.0.6]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.6 @@ -19,6 +13,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Experimental support for IPv6. Node RPC and node-to-node interface hosts may now be specified as IPv6 literals in bracketed form (e.g. `[::1]:8000`), and addresses are consistently parsed, bound, connected (with fallback across mixed IPv4/IPv6 resolved addresses), serialised, and embedded in redirect URLs for IPv6 (#7671). +### 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). + ### Fixed - Forwarded commands are no longer processed until the node is part of the network, matching the existing behaviour for other node-to-node messages. Previously a forwarded command could be executed while the node was in an earlier startup state, which could lead to undefined behaviour for some commands (#7936). diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index ee94a85989bc..81176542b692 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -269,7 +269,8 @@ namespace ccf catch (const std::invalid_argument& e) { LOG_FAIL_FMT( - "JWT key auto-refresh: Cannot parse jwks_uri for issuer '{}': {} ({})", + "JWT key auto-refresh: Cannot parse jwks_uri for issuer '{}': {} " + "({})", issuer, jwks_url_str, e.what()); @@ -297,8 +298,13 @@ namespace ccf request->get_response_body() != nullptr ? std::move(request->get_response_body()->buffer) : std::vector{}); - ccf::tasks::add_task(ccf::tasks::make_basic_task( - [self, issuer, issuer_constraint, curl_response, http_status, response_body_sp]() { + ccf::tasks::add_task( + ccf::tasks::make_basic_task([self, + issuer, + issuer_constraint, + curl_response, + http_status, + response_body_sp]() { if (curl_response != CURLE_OK) { LOG_FAIL_FMT( @@ -375,8 +381,13 @@ namespace ccf request->get_response_body() != nullptr ? std::move(request->get_response_body()->buffer) : std::vector{}); - ccf::tasks::add_task(ccf::tasks::make_basic_task( - [self, issuer, ca_bundle_pem, curl_response, http_status, response_body_sp]() { + ccf::tasks::add_task( + ccf::tasks::make_basic_task([self, + issuer, + ca_bundle_pem, + curl_response, + http_status, + response_body_sp]() { if (curl_response != CURLE_OK) { LOG_FAIL_FMT( @@ -396,7 +407,8 @@ namespace ccf })); }; - send_curl_get(metadata_url, ca_bundle_pem, std::move(response_callback)); + send_curl_get( + metadata_url, ca_bundle_pem, std::move(response_callback)); return true; }); } diff --git a/tests/jwt_test.py b/tests/jwt_test.py index ca7d67f7769e..ea571faa7a00 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -415,6 +415,11 @@ def test_jwt_key_auto_refresh(network, args): ) LOG.info("Start OpenID endpoint server") + # Capture baseline metrics before the server starts: any connection failures + # from a prior test run (e.g. after a primary failover when the server was + # briefly unavailable) will already be reflected here and must not be counted + # as failures introduced by this test. + baseline_m = get_jwt_refresh_endpoint_metrics(primary) with issuer.start_openid_server(issuer_port, kid) as server: # Send oversized headers with the payload that will cause the CCF client to # fail parsing and log an error. @@ -454,9 +459,9 @@ def assert_request_count_increased(): LOG.info("Check that JWT refresh has attempts and successes and no failures") m = get_jwt_refresh_endpoint_metrics(primary) - assert m["attempts"] > 0, m - assert m["successes"] > 0, m - assert m["failures"] == 0, m + assert m["attempts"] > baseline_m["attempts"], m + assert m["successes"] > baseline_m["successes"], m + assert m["failures"] == baseline_m["failures"], m LOG.info("Serve invalid JWKS") server.jwks = {"foo": "bar"} @@ -465,7 +470,7 @@ def assert_request_count_increased(): def check_has_failures(): m = get_jwt_refresh_endpoint_metrics(primary) - assert m["failures"] > 0, m + assert m["failures"] > baseline_m["failures"], m with_timeout(check_has_failures, timeout=5) From 146d6d4fc3bd7006195362de0782a27aa7d79a03 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 26 Jun 2026 09:17:26 +0000 Subject: [PATCH 07/20] Harden JWT JWK curl refresh Use HTTPS-only curl requests with bounded timeouts for JWT/JWK auto-refresh, and add coverage for connection and TLS refresh failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../jwk_refresh_curl_multi_migration_plan.md | 256 ++++++------------ src/node/jwt_key_auto_refresh.h | 24 +- tests/jwt_test.py | 111 +++++++- 3 files changed, 208 insertions(+), 183 deletions(-) diff --git a/doc/dev/jwk_refresh_curl_multi_migration_plan.md b/doc/dev/jwk_refresh_curl_multi_migration_plan.md index 5df21a83a738..289a420c6840 100644 --- a/doc/dev/jwk_refresh_curl_multi_migration_plan.md +++ b/doc/dev/jwk_refresh_curl_multi_migration_plan.md @@ -1,182 +1,84 @@ # JWK refresh migration to curl multi singleton -This note captures the implementation plan for migrating JWT/JWK auto-refresh in CCF away from the current `RPCSessions::create_client()` outbound HTTP client and onto the curl multi singleton infrastructure introduced in microsoft/CCF#7102. - -## Reference PR - -PR microsoft/CCF#7102 introduced: - -- `ccf::curl::UniqueCURL` -- `ccf::curl::UniqueSlist` -- `ccf::curl::RequestBody` -- `ccf::curl::ResponseBody` -- `ccf::curl::ResponseHeaders` -- `ccf::curl::CurlRequest` -- `ccf::curl::CurlmLibuvContextSingleton` - -It migrated `src/node/quote_endorsements_client.h` from the enclave/host RPC client path to asynchronous curl requests attached to the singleton curl multi/libuv context. - -## Current JWK refresh flow - -The current JWK refresh implementation is in: - -- `src/node/jwt_key_auto_refresh.h` - -It uses `rpcsessions->create_client(...)` in two places: - -1. OpenID metadata fetch: - - GET `issuer + "/.well-known/openid-configuration"` - - parse `jwks_uri` -2. JWKS fetch: - - GET the parsed `jwks_uri` - - parse `JsonWebKeySet` - - call the internal `/node/jwt_keys/refresh` endpoint via `send_refresh_jwt_keys(...)` - -The internal update path should remain unchanged. - -## Implementation goals - -- Replace both external HTTP fetches with `ccf::curl::CurlRequest`. -- Enqueue asynchronous requests with `ccf::curl::CurlmLibuvContextSingleton::get_instance()->attach_request(...)`. -- Preserve the existing refresh, parsing, issuer constraint, and metrics behaviour. -- Improve observability where possible: curl connection/TLS failures should call `send_refresh_jwt_keys_error()` so they are reflected in refresh failure metrics. -- Keep changes bite-sized and testable. - -## Bite-sized implementation steps - -### 1. Add curl dependency to JWK refresh - -- Include `http/curl.h` from `src/node/jwt_key_auto_refresh.h`. -- Keep the existing implementation compiling unchanged. - -Verification: - -- Build affected targets. - -### 2. Add request-building helpers - -Add private helpers to `JwtKeyAutoRefresh` for: - -- defaulting URL ports to `443` for HTTPS -- building a full request URL from `http::URL` -- setting common curl options -- attaching CA bundle material to curl -- creating a bounded `ccf::curl::ResponseBody` - -Verification: - -- Unit-level compile checks. -- Existing JWT tests still pass before call sites are migrated. - -### 3. Implement CA bundle handling for curl - -The existing code reads a PEM CA bundle from KV and constructs `tls::CA`/`tls::Cert` for the old client. - -For curl, prefer in-memory CA bundle support if available: - -- `CURLOPT_CAINFO_BLOB` via `UniqueCURL::set_blob_opt(...)` - -If unsupported by the project's supported libcurl version, introduce a request-owned fallback that keeps temporary CA material alive for the async request lifetime. - -Verification: - -- Valid issuer cert signed by configured CA bundle succeeds. -- Wrong CA bundle fails and is counted as a refresh failure. - -### 4. Migrate OpenID metadata fetch first - -In `refresh_jwt_keys()`: - -- Preserve issuer iteration, `auto_refresh` checks, attempts accounting, and missing CA bundle handling. -- Replace `rpcsessions->create_client(...)`, `connect(...)`, and `send_request(...)` with a curl GET request. -- Callback should: - - call `send_refresh_jwt_keys_error()` on `curl_response != CURLE_OK` - - otherwise pass status code and response body to `handle_jwt_metadata_response(...)` - -Expected signature change: - -```cpp -void handle_jwt_metadata_response( - const std::string& issuer, - std::string ca_cert_bundle_pem, - ccf::http_status status, - std::vector&& data); -``` - -Verification: - -- Metadata happy path still reaches JWKS fetch. -- Invalid metadata still increments failure metrics. -- Metadata connection failure increments failure metrics. - -### 5. Migrate JWKS fetch - -In `handle_jwt_metadata_response(...)`: - -- Preserve HTTP status handling, metadata parsing, `jwks_uri` parsing, and issuer constraint extraction. -- Replace the old client with a curl GET request using the same CA bundle material. -- Callback should: - - call `send_refresh_jwt_keys_error()` on `curl_response != CURLE_OK` - - otherwise pass status code and body to `handle_jwt_jwks_response(...)` - -Verification: - -- Existing JWT auto-refresh happy path passes. -- Invalid JWKS response increments failure metrics. -- HTTP non-200 response still triggers existing error handling. - -### 6. Clean up dependencies - -Once both external fetches use curl: - -- Remove the `rpcsessions` member from `JwtKeyAutoRefresh` if unused. -- Remove the constructor parameter if unused. -- Update all construction sites. -- Remove stale includes and comments about connection errors not being tracked. - -Verification: - -- Full build catches all construction-site updates. -- No `rpcsessions->create_client` usage remains in `jwt_key_auto_refresh.h`. - -### 7. Tests - -Add or update focused tests around: - -- successful JWT key auto-refresh -- initial one-off refresh after adding an issuer -- invalid OpenID metadata -- invalid JWKS -- unavailable issuer endpoint / connection failure -- TLS trust failure with an incorrect CA bundle - -Existing tests to keep passing include: - -- `test_jwt_key_auto_refresh` -- `test_jwt_key_initial_refresh` -- `test_jwt_key_auto_refresh_entries` - -### 8. Validation order - -1. Build affected C++ targets. -2. Run curl tests introduced by microsoft/CCF#7102 where available. -3. Run JWT auto-refresh tests. -4. Run broader JWT test file. -5. Check shutdown for libuv handle leaks or async lifetime issues. - -## Risks and things to watch - -- Async callback captures must own all data they use. -- CA bundle material must outlive async curl requests. -- Hostname/SNI verification should use the actual target host, especially for `jwks_uri`. -- Response bodies should be bounded rather than using `SIZE_MAX` where possible. -- Curl connection failures will now be counted as refresh failures; this is an intentional observability improvement but should be noted in the PR. -- The internal `/node/jwt_keys/refresh` update path should remain unchanged. +Status note for migrating JWT/JWK auto-refresh away from the enclave/host +`RPCSessions::create_client()` outbound HTTP client and onto the curl multi +singleton infrastructure introduced in microsoft/CCF#7102. + +Last reviewed: 2026-06-26. + +## Current status + +Implementation is complete on this branch pending validation. + +- `src/node/jwt_key_auto_refresh.h` uses `ccf::curl::CurlRequest` with + `CurlmLibuvContextSingleton::get_instance()->attach_request(...)` for both + external OpenID metadata and JWKS fetches. +- `JwtKeyAutoRefresh` no longer stores an `RPCSessions` pointer, and + `src/node/node_state.h` constructs it without passing `rpcsessions`. +- The internal `/node/jwt_keys/refresh` update path still goes through + `send_refresh_jwt_keys(...)` and is intentionally unchanged. +- Curl requests now set peer and host verification, use the configured + in-memory CA bundle, restrict protocols to HTTPS only, and bound both connect + and total transfer timeouts. +- OpenID metadata is still fetched from the configured issuer URL. The JWKS URL + from metadata is parsed and must use the `https` scheme before it is handed to + curl. +- Response bodies are bounded to 1 MB. +- Curl connection and TLS failures now call `send_refresh_jwt_keys_error()` and + are counted in refresh failure metrics. `CHANGELOG.md` documents this + observability change. +- `tests/jwt_test.py` includes focused connection-failure and TLS-trust-failure + coverage and wires these into `run_manual()`. +- The new failure tests remove existing JWT issuers before recording the + baseline failure count, so their metric-delta checks are isolated from other + auto-refresh issuers. +- `src/http/test/curl_test.cpp` already covers generic curl singleton behavior; + JWT behavior is covered by the Python end-to-end tests. + +Unrelated local changes exist in `.github/workflows/README.md`, +`perf_artifacts/`, and `perf_data/`; leave them untouched. + +## Remaining validation + +Run these before considering the migration complete: + +1. Build affected C++ targets, at minimum the node target and `curl_test`. +2. Run `curl_test` to catch curl multi singleton regressions. +3. Run JWT manual suite coverage containing: + - `test_jwt_key_initial_refresh` + - `test_jwt_key_auto_refresh_connection_failure` + - `test_jwt_key_auto_refresh_tls_failure` +4. Run broader JWT auto-refresh coverage containing: + - `test_jwt_key_auto_refresh` + - backup-primary variant of `test_jwt_key_auto_refresh` + - `test_jwt_key_auto_refresh_entries` +5. Confirm shutdown has no libuv handle leaks or async lifetime issues. +6. Optional follow-up: add a direct invalid OpenID metadata auto-refresh test if + the test harness can serve malformed metadata without duplicating large parts + of `infra.jwt_issuer`. Current coverage exercises invalid JWKS and curl-level + connection/TLS failures, but not malformed metadata from a reachable issuer. + +## Risks to keep watching + +- `JwtKeyAutoRefresh` now inherits `enable_shared_from_this`; callers must keep + constructing it as a shared pointer before any refresh is scheduled. +- Async callback captures must own all data they use. Current callbacks capture + `self`, issuer strings, CA bundle strings, and response bodies by value/shared + ownership. +- CA bundle material must remain valid until libcurl has consumed it. Current + code passes the CA bundle into `set_blob_opt(...)`; verify this remains copied + or otherwise owned by the curl wrapper. +- The 1 MB response limit and 5 second connect/transfer timeouts are intentional + bounds, but can reject very large or slow metadata/JWKS responses. +- The HTTPS-only checks are intentional hardening. Any future support for other + schemes should be explicit and tested. ## Definition of done -- `src/node/jwt_key_auto_refresh.h` no longer uses `rpcsessions->create_client(...)` for OpenID/JWKS external fetches. -- Metadata and JWKS requests use `ccf::curl::CurlRequest` and `CurlmLibuvContextSingleton`. -- Existing JWT auto-refresh behaviour remains intact. -- New failure-mode tests cover connection and TLS failures. -- The PR description explains the relationship to microsoft/CCF#7102 and calls out the metrics-observability improvement. +- Both external fetches use curl multi singleton requests. +- No JWT auto-refresh external fetch uses `RPCSessions::create_client(...)`. +- JWKS URIs must be HTTPS, and curl is restricted to HTTPS protocols. +- Curl requests have bounded connect and total transfer timeouts. +- Existing auto-refresh behavior remains intact. +- New connection and TLS failure tests pass with isolated metric baselines. +- Remaining validation above has been run and recorded in the PR or commit notes. diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 81176542b692..b7045b2ba142 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -32,7 +32,9 @@ namespace ccf ccf::tasks::Task periodic_refresh_task; // Maximum response body size for JWK/metadata responses (1 MB) - static constexpr size_t max_response_size = 1024 * 1024; + static constexpr size_t max_response_size = static_cast(1024) * 1024; + static constexpr long request_connection_timeout_s = 5; + static constexpr long request_response_timeout_s = 5; void send_curl_get( const std::string& url, @@ -41,8 +43,15 @@ namespace ccf { ccf::curl::UniqueCURL curl_handle; curl_handle.set_opt(CURLOPT_HTTPGET, 1L); + curl_handle.set_opt(CURLOPT_CONNECTTIMEOUT, request_connection_timeout_s); + curl_handle.set_opt(CURLOPT_TIMEOUT, request_response_timeout_s); curl_handle.set_opt(CURLOPT_SSL_VERIFYPEER, 1L); curl_handle.set_opt(CURLOPT_SSL_VERIFYHOST, 2L); +#if LIBCURL_VERSION_NUM >= 0x075500 + curl_handle.set_opt(CURLOPT_PROTOCOLS_STR, "https"); +#else + curl_handle.set_opt(CURLOPT_PROTOCOLS, CURLPROTO_HTTPS); +#endif curl_handle.set_blob_opt( CURLOPT_CAINFO_BLOB, reinterpret_cast(ca_bundle_pem.data()), @@ -262,9 +271,10 @@ namespace ccf } // Validate jwks_uri before handing it to libcurl; the parsed result is // not used directly since the full URL string is passed to curl. + ::http::URL jwks_url; try { - ::http::parse_url_full(jwks_url_str); + jwks_url = ::http::parse_url_full(jwks_url_str); } catch (const std::invalid_argument& e) { @@ -278,6 +288,16 @@ namespace ccf return; } + if (jwks_url.scheme != "https") + { + LOG_FAIL_FMT( + "JWT key auto-refresh: jwks_uri for issuer '{}' must use https: {}", + issuer, + jwks_url_str); + send_refresh_jwt_keys_error(); + return; + } + std::optional issuer_constraint{std::nullopt}; const auto constraint = metadata.find("issuer"); if (constraint != metadata.end()) diff --git a/tests/jwt_test.py b/tests/jwt_test.py index ea571faa7a00..db9d8e80fcd8 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -4,6 +4,7 @@ import json import time import base64 +import socket import infra.network import infra.path import infra.proc @@ -393,6 +394,106 @@ def get_jwt_refresh_endpoint_metrics(primary) -> dict: return r.body.json() +def get_unused_local_port(): + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("localhost", 0)) + return s.getsockname()[1] + + +def add_auto_refresh_jwt_issuer(network, primary, issuer, ca_cert_bundle_name): + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as ca_cert_bundle_fp: + ca_cert_bundle_fp.write(issuer.tls_cert) + ca_cert_bundle_fp.flush() + network.consortium.set_ca_cert_bundle( + primary, ca_cert_bundle_name, ca_cert_bundle_fp.name + ) + + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: + json.dump( + { + "issuer": issuer.name, + "auto_refresh": True, + "ca_cert_bundle_name": ca_cert_bundle_name, + }, + metadata_fp, + ) + metadata_fp.flush() + network.consortium.set_jwt_issuer(primary, metadata_fp.name) + + +def remove_all_jwt_issuers(network, args, primary): + for issuer in list(get_jwt_issuers(args, primary)): + network.consortium.remove_jwt_issuer(primary, issuer) + + +def check_refresh_failures_increased(primary, failures_before): + m = get_jwt_refresh_endpoint_metrics(primary) + assert m["failures"] > failures_before, m + + +def test_jwt_key_auto_refresh_connection_failure(network, args): + primary, _ = network.find_nodes() + remove_all_jwt_issuers(network, args, primary) + failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] + issuer_host = "localhost" + issuer_port = get_unused_local_port() + issuer = infra.jwt_issuer.JwtIssuer( + f"https://{issuer_host}:{issuer_port}", cn=issuer_host + ) + + LOG.info("Add JWT issuer with auto-refresh pointing at an unavailable endpoint") + add_auto_refresh_jwt_issuer(network, primary, issuer, "jwt_connection_failure") + try: + with_timeout( + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, + ) + finally: + network.consortium.remove_jwt_issuer(primary, issuer.name) + + +def test_jwt_key_auto_refresh_tls_failure(network, args): + primary, _ = network.find_nodes() + remove_all_jwt_issuers(network, args, primary) + failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] + issuer = infra.jwt_issuer.JwtIssuer("https://localhost", cn="localhost") + wrong_ca_issuer = infra.jwt_issuer.JwtIssuer( + "https://localhost-wrong-ca", cn="localhost" + ) + ca_cert_bundle_name = "jwt_tls_failure" + + LOG.info("Add wrong CA cert for JWT issuer") + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as ca_cert_bundle_fp: + ca_cert_bundle_fp.write(wrong_ca_issuer.tls_cert) + ca_cert_bundle_fp.flush() + network.consortium.set_ca_cert_bundle( + primary, ca_cert_bundle_name, ca_cert_bundle_fp.name + ) + + LOG.info("Start OpenID endpoint server with a certificate signed by another CA") + with issuer.start_openid_server(0) as server: + issuer_name = f"https://localhost:{server.bind_port}" + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: + json.dump( + { + "issuer": issuer_name, + "auto_refresh": True, + "ca_cert_bundle_name": ca_cert_bundle_name, + }, + metadata_fp, + ) + metadata_fp.flush() + network.consortium.set_jwt_issuer(primary, metadata_fp.name) + + try: + with_timeout( + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, + ) + finally: + network.consortium.remove_jwt_issuer(primary, issuer_name) + + @reqs.description("JWT with auto_refresh enabled") def test_jwt_key_auto_refresh(network, args): primary, _ = network.find_nodes() @@ -457,20 +558,20 @@ def assert_request_count_increased(): timeout=5, ) - LOG.info("Check that JWT refresh has attempts and successes and no failures") + LOG.info("Check that JWT refresh has attempts and successes") m = get_jwt_refresh_endpoint_metrics(primary) assert m["attempts"] > baseline_m["attempts"], m assert m["successes"] > baseline_m["successes"], m - assert m["failures"] == baseline_m["failures"], m + failures = m["failures"] LOG.info("Serve invalid JWKS") server.jwks = {"foo": "bar"} - LOG.info("Check that JWT refresh endpoint has some failures") + LOG.info("Check that JWT refresh endpoint has more failures") def check_has_failures(): m = get_jwt_refresh_endpoint_metrics(primary) - assert m["failures"] > baseline_m["failures"], m + assert m["failures"] > failures, m with_timeout(check_has_failures, timeout=5) @@ -817,6 +918,8 @@ def run_manual(args): primary.stop() network.wait_for_new_primary(primary) test_jwt_key_initial_refresh(network, args) + test_jwt_key_auto_refresh_connection_failure(network, args) + test_jwt_key_auto_refresh_tls_failure(network, args) def run_ca_cert(args): From 18f06f9bcc580d425df6eb1202255898a043b8bb Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 26 Jun 2026 09:49:04 +0000 Subject: [PATCH 08/20] Accept case-insensitive JWKS HTTPS schemes Normalize the parsed jwks_uri scheme before enforcing HTTPS-only refreshes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index b7045b2ba142..6e4029ac388d 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -3,6 +3,7 @@ #pragma once #include "ccf/ds/json.h" +#include "ccf/ds/nonstd.h" #include "ccf/service/tables/jwt.h" #include "http/curl.h" #include "http/http_builder.h" @@ -288,6 +289,7 @@ namespace ccf return; } + ccf::nonstd::to_lower(jwks_url.scheme); if (jwks_url.scheme != "https") { LOG_FAIL_FMT( From 189495fa423ec6c0faf766bb77f41fcfe70b1b81 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 26 Jun 2026 18:05:20 +0000 Subject: [PATCH 09/20] Format JWT key auto-refresh code Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 6e4029ac388d..e6927fc6e908 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -33,7 +33,8 @@ namespace ccf ccf::tasks::Task periodic_refresh_task; // Maximum response body size for JWK/metadata responses (1 MB) - static constexpr size_t max_response_size = static_cast(1024) * 1024; + static constexpr size_t max_response_size = + static_cast(1024) * 1024; static constexpr long request_connection_timeout_s = 5; static constexpr long request_response_timeout_s = 5; From e703fbe270153cd922caf6065c911ed7b9141cc0 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 26 Jun 2026 18:23:54 +0000 Subject: [PATCH 10/20] Fix JWT auto-refresh issuer constraints and shutdown Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 77 +++++++++++++++++++++++++-------- tests/jwt_test.py | 10 +++++ 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index e6927fc6e908..b611a3feb9eb 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -29,6 +29,7 @@ namespace ccf ccf::crypto::ECKeyPairPtr node_sign_kp; ccf::crypto::Pem node_cert; std::atomic_size_t attempts; + std::atomic_bool stopped; ccf::tasks::Task periodic_refresh_task; @@ -88,7 +89,8 @@ namespace ccf rpc_map(rpc_map), node_sign_kp(std::move(node_sign_kp)), node_cert(std::move(node_cert)), - attempts(0) + attempts(0), + stopped(false) {} ~JwtKeyAutoRefresh() @@ -98,19 +100,28 @@ namespace ccf void start() { + stopped.store(false); LOG_DEBUG_FMT("JWT key initial auto-refresh"); - periodic_refresh_task = ccf::tasks::make_basic_task([this]() { - if (!this->consensus->can_replicate()) + const auto self = weak_from_this(); + periodic_refresh_task = ccf::tasks::make_basic_task([self]() { + const auto self_sp = self.lock(); + if (self_sp == nullptr || self_sp->stopped.load()) + { + return; + } + + if (!self_sp->consensus->can_replicate()) { LOG_DEBUG_FMT("JWT key auto-refresh: Node is not primary, skipping"); } else { - this->refresh_jwt_keys(); + self_sp->refresh_jwt_keys(); } LOG_DEBUG_FMT( - "JWT key auto-refresh: Scheduling in {}s", this->refresh_interval_s); + "JWT key auto-refresh: Scheduling in {}s", + self_sp->refresh_interval_s); }); const std::chrono::seconds period(refresh_interval_s); @@ -119,6 +130,7 @@ namespace ccf void stop() { + stopped.store(true); if (periodic_refresh_task != nullptr) { periodic_refresh_task->cancel_task(); @@ -128,15 +140,22 @@ namespace ccf void schedule_once() { LOG_DEBUG_FMT("JWT key one-off refresh: Scheduling without delay"); - ccf::tasks::add_task(ccf::tasks::make_basic_task([this]() { - if (!this->consensus->can_replicate()) + const auto self = weak_from_this(); + ccf::tasks::add_task(ccf::tasks::make_basic_task([self]() { + const auto self_sp = self.lock(); + if (self_sp == nullptr || self_sp->stopped.load()) + { + return; + } + + if (!self_sp->consensus->can_replicate()) { LOG_DEBUG_FMT( "JWT key one-off refresh: Node is not primary, skipping"); } else { - this->refresh_jwt_keys(); + self_sp->refresh_jwt_keys(); } })); } @@ -211,9 +230,6 @@ namespace ccf return; } - // call internal endpoint to update keys - auto msg = SetJwtPublicSigningKeys{issuer, jwks}; - // For each key we leave the specified issuer constraint or set a common // one otherwise (if present). if (issuer_constraint.has_value()) @@ -227,6 +243,9 @@ namespace ccf } } + // call internal endpoint to update keys + auto msg = SetJwtPublicSigningKeys{issuer, jwks}; + send_refresh_jwt_keys(msg); } @@ -311,8 +330,9 @@ namespace ccf LOG_DEBUG_FMT( "JWT key auto-refresh: Requesting JWKS at {}", jwks_url_str); + const auto self = weak_from_this(); auto response_callback = - [self = shared_from_this(), issuer, issuer_constraint]( + [self, issuer, issuer_constraint]( std::unique_ptr&& request, CURLcode curl_response, long status_code) { @@ -328,6 +348,12 @@ namespace ccf curl_response, http_status, response_body_sp]() { + const auto self_sp = self.lock(); + if (self_sp == nullptr || self_sp->stopped.load()) + { + return; + } + if (curl_response != CURLE_OK) { LOG_FAIL_FMT( @@ -336,10 +362,10 @@ namespace ccf issuer, curl_easy_strerror(curl_response), curl_response); - self->send_refresh_jwt_keys_error(); + self_sp->send_refresh_jwt_keys_error(); return; } - self->handle_jwt_jwks_response( + self_sp->handle_jwt_jwks_response( issuer, issuer_constraint, http_status, @@ -352,12 +378,22 @@ namespace ccf void refresh_jwt_keys() { + if (stopped.load()) + { + return; + } + auto tx = network.tables->create_read_only_tx(); auto* jwt_issuers = tx.ro(network.jwt_issuers); auto* ca_cert_bundles = tx.ro(network.ca_cert_bundles); jwt_issuers->foreach([this, &ca_cert_bundles]( const JwtIssuer& issuer, const JwtIssuerMetadata& metadata) { + if (stopped.load()) + { + return false; + } + if (!metadata.auto_refresh) { LOG_DEBUG_FMT( @@ -394,8 +430,9 @@ namespace ccf auto ca_bundle_pem = ca_cert_bundle_pem.value(); + const auto self = weak_from_this(); auto response_callback = - [self = shared_from_this(), issuer, ca_bundle_pem]( + [self, issuer, ca_bundle_pem]( std::unique_ptr&& request, CURLcode curl_response, long status_code) { @@ -411,6 +448,12 @@ namespace ccf curl_response, http_status, response_body_sp]() { + const auto self_sp = self.lock(); + if (self_sp == nullptr || self_sp->stopped.load()) + { + return; + } + if (curl_response != CURLE_OK) { LOG_FAIL_FMT( @@ -419,10 +462,10 @@ namespace ccf issuer, curl_easy_strerror(curl_response), curl_response); - self->send_refresh_jwt_keys_error(); + self_sp->send_refresh_jwt_keys_error(); return; } - self->handle_jwt_metadata_response( + self_sp->handle_jwt_metadata_response( issuer, ca_bundle_pem, http_status, diff --git a/tests/jwt_test.py b/tests/jwt_test.py index db9d8e80fcd8..e09b62a00c3b 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -372,6 +372,15 @@ def check_kv_jwt_key_matches(args, network, kid, key_pem): assert stored_key == key_pem, "input cert is not equal to stored cert" +def check_kv_jwt_key_constraint(args, network, kid, expected_constraint): + primary, _ = network.find_nodes() + latest_jwt_signing_keys = get_jwt_keys(args, primary) + + assert kid in latest_jwt_signing_keys + stored_constraint = latest_jwt_signing_keys[kid][0]["constraint"] + assert stored_constraint == expected_constraint + + def check_kv_jwt_keys_not_empty(args, network, issuer): primary, _ = network.find_nodes() latest_jwt_signing_keys = get_jwt_keys(args, primary) @@ -557,6 +566,7 @@ def assert_request_count_increased(): ), timeout=5, ) + check_kv_jwt_key_constraint(args, network, kid, issuer.name) LOG.info("Check that JWT refresh has attempts and successes") m = get_jwt_refresh_endpoint_metrics(primary) From eee088d79dee508de97c7fcc0a2b4d1c5d377bbb Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 26 Jun 2026 19:04:14 +0000 Subject: [PATCH 11/20] Harden JWT auto-refresh metadata handling Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/node/jwt_key_auto_refresh.h | 27 +++++++++-- tests/jwt_test.py | 86 +++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 4 deletions(-) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index b611a3feb9eb..a694aa5bf097 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -59,6 +59,7 @@ namespace ccf CURLOPT_CAINFO_BLOB, reinterpret_cast(ca_bundle_pem.data()), ca_bundle_pem.size()); + curl_handle.set_opt(CURLOPT_CAPATH, nullptr); ccf::curl::UniqueSlist headers; @@ -275,10 +276,16 @@ namespace ccf std::string jwks_url_str; nlohmann::json metadata; + std::optional issuer_constraint{std::nullopt}; try { metadata = ccf::parse_json_safe(data); jwks_url_str = metadata.at("jwks_uri").get(); + const auto constraint = metadata.find("issuer"); + if (constraint != metadata.end()) + { + issuer_constraint = constraint->get(); + } } catch (const std::exception& e) { @@ -292,9 +299,11 @@ namespace ccf } // Validate jwks_uri before handing it to libcurl; the parsed result is // not used directly since the full URL string is passed to curl. + ::http::URL issuer_url; ::http::URL jwks_url; try { + issuer_url = ::http::parse_url_full(issuer); jwks_url = ::http::parse_url_full(jwks_url_str); } catch (const std::invalid_argument& e) @@ -309,7 +318,10 @@ namespace ccf return; } + ccf::nonstd::to_lower(issuer_url.scheme); + ccf::nonstd::to_lower(issuer_url.host); ccf::nonstd::to_lower(jwks_url.scheme); + ccf::nonstd::to_lower(jwks_url.host); if (jwks_url.scheme != "https") { LOG_FAIL_FMT( @@ -320,11 +332,18 @@ namespace ccf return; } - std::optional issuer_constraint{std::nullopt}; - const auto constraint = metadata.find("issuer"); - if (constraint != metadata.end()) + const auto issuer_port = + issuer_url.port.empty() ? "443" : issuer_url.port; + const auto jwks_port = jwks_url.port.empty() ? "443" : jwks_url.port; + if (jwks_url.host != issuer_url.host || jwks_port != issuer_port) { - issuer_constraint = *constraint; + LOG_FAIL_FMT( + "JWT key auto-refresh: jwks_uri for issuer '{}' must use the issuer " + "authority: {}", + issuer, + jwks_url_str); + send_refresh_jwt_keys_error(); + return; } LOG_DEBUG_FMT( diff --git a/tests/jwt_test.py b/tests/jwt_test.py index e09b62a00c3b..955c832edfe2 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -503,6 +503,90 @@ def test_jwt_key_auto_refresh_tls_failure(network, args): network.consortium.remove_jwt_issuer(primary, issuer_name) +def test_jwt_key_auto_refresh_invalid_metadata_issuer(network, args): + primary, _ = network.find_nodes() + remove_all_jwt_issuers(network, args, primary) + failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] + issuer = infra.jwt_issuer.JwtIssuer("https://localhost", cn="localhost") + ca_cert_bundle_name = "jwt_invalid_metadata_issuer" + + LOG.info("Add CA cert for JWT issuer") + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as ca_cert_bundle_fp: + ca_cert_bundle_fp.write(issuer.tls_cert) + ca_cert_bundle_fp.flush() + network.consortium.set_ca_cert_bundle( + primary, ca_cert_bundle_name, ca_cert_bundle_fp.name + ) + + LOG.info("Start OpenID endpoint server with a non-string issuer metadata field") + with issuer.start_openid_server(0) as server: + issuer_name = f"https://localhost:{server.bind_port}" + server.metadata["issuer"] = {"unexpected": "object"} + + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: + json.dump( + { + "issuer": issuer_name, + "auto_refresh": True, + "ca_cert_bundle_name": ca_cert_bundle_name, + }, + metadata_fp, + ) + metadata_fp.flush() + network.consortium.set_jwt_issuer(primary, metadata_fp.name) + + try: + with_timeout( + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, + ) + finally: + network.consortium.remove_jwt_issuer(primary, issuer_name) + + +def test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args): + primary, _ = network.find_nodes() + remove_all_jwt_issuers(network, args, primary) + failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] + issuer = infra.jwt_issuer.JwtIssuer("https://localhost", cn="localhost") + ca_cert_bundle_name = "jwt_cross_authority_jwks_uri" + + LOG.info("Add CA cert for JWT issuer") + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as ca_cert_bundle_fp: + ca_cert_bundle_fp.write(issuer.tls_cert) + ca_cert_bundle_fp.flush() + network.consortium.set_ca_cert_bundle( + primary, ca_cert_bundle_name, ca_cert_bundle_fp.name + ) + + LOG.info("Start OpenID endpoint server with cross-authority JWKS URI") + with issuer.start_openid_server(0) as server: + issuer_name = f"https://localhost:{server.bind_port}" + server.metadata["jwks_uri"] = ( + f"https://localhost:{get_unused_local_port()}/keys" + ) + + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: + json.dump( + { + "issuer": issuer_name, + "auto_refresh": True, + "ca_cert_bundle_name": ca_cert_bundle_name, + }, + metadata_fp, + ) + metadata_fp.flush() + network.consortium.set_jwt_issuer(primary, metadata_fp.name) + + try: + with_timeout( + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, + ) + finally: + network.consortium.remove_jwt_issuer(primary, issuer_name) + + @reqs.description("JWT with auto_refresh enabled") def test_jwt_key_auto_refresh(network, args): primary, _ = network.find_nodes() @@ -930,6 +1014,8 @@ def run_manual(args): test_jwt_key_initial_refresh(network, args) test_jwt_key_auto_refresh_connection_failure(network, args) test_jwt_key_auto_refresh_tls_failure(network, args) + test_jwt_key_auto_refresh_invalid_metadata_issuer(network, args) + test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args) def run_ca_cert(args): From c55a93f25d63aead63b96d5ddb5fc1c904266633 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 10:26:44 +0000 Subject: [PATCH 12/20] Make JWT refresh response size configurable --- CHANGELOG.md | 1 + doc/build_apps/auth/jwt.rst | 2 +- doc/host_config_schema/host_config.json | 5 ++++ include/ccf/node/startup_config.h | 1 + src/common/configuration.h | 3 ++- src/node/jwt_key_auto_refresh.h | 10 +++---- src/node/node_state.h | 3 ++- tests/config.jinja | 3 ++- tests/infra/e2e_args.py | 5 ++++ tests/infra/network.py | 1 + tests/infra/remote.py | 2 ++ tests/jwt_test.py | 36 +++++++++++++++++++++++++ tests/programmability.py | 1 + 13 files changed, 64 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02f34ca81826..2e96ea49b470 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### 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). ### Fixed diff --git a/doc/build_apps/auth/jwt.rst b/doc/build_apps/auth/jwt.rst index 48dd1fcdd4c9..92e9856d877a 100644 --- a/doc/build_apps/auth/jwt.rst +++ b/doc/build_apps/auth/jwt.rst @@ -107,7 +107,7 @@ Now the issuer can be created with auto-refresh enabled: .. note:: - The key refresh interval is set via the ``jwt.key_refresh_interval_s`` configuration entry, where the default is 30 min (1800 seconds). + The key refresh interval is set via the ``jwt.key_refresh_interval`` configuration entry, where the default is 30 min (1800 seconds). The maximum response body size accepted when fetching OpenID metadata and JWKS is set via ``jwt.key_refresh_max_response_size``, where the default is 1 MB. Removing a token issuer ----------------------- diff --git a/doc/host_config_schema/host_config.json b/doc/host_config_schema/host_config.json index 30a48cdeaf69..ec7c776871ca 100644 --- a/doc/host_config_schema/host_config.json +++ b/doc/host_config_schema/host_config.json @@ -653,6 +653,11 @@ "type": "string", "default": "30min", "description": "Interval at which JWT keys for issuers registered with auto-refresh are automatically refreshed" + }, + "key_refresh_max_response_size": { + "type": "string", + "default": "1MB", + "description": "Maximum response body size accepted when fetching OpenID metadata and JWKS for JWT issuer auto-refresh" } }, "description": "This section includes configuration for JWT issuers automatic refresh", diff --git a/include/ccf/node/startup_config.h b/include/ccf/node/startup_config.h index 457b418947eb..3d6cef413798 100644 --- a/include/ccf/node/startup_config.h +++ b/include/ccf/node/startup_config.h @@ -65,6 +65,7 @@ namespace ccf struct JWT { ccf::ds::TimeString key_refresh_interval = {"30min"}; + ccf::ds::SizeString key_refresh_max_response_size = {"1MB"}; bool operator==(const JWT&) const = default; }; diff --git a/src/common/configuration.h b/src/common/configuration.h index 29dcc2e08aca..d11b775b26e6 100644 --- a/src/common/configuration.h +++ b/src/common/configuration.h @@ -73,7 +73,8 @@ namespace ccf DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::JWT); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::JWT); - DECLARE_JSON_OPTIONAL_FIELDS(CCFConfig::JWT, key_refresh_interval); + DECLARE_JSON_OPTIONAL_FIELDS( + CCFConfig::JWT, key_refresh_interval, key_refresh_max_response_size); DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(CCFConfig::Attestation::Environment); DECLARE_JSON_REQUIRED_FIELDS(CCFConfig::Attestation::Environment); diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index a694aa5bf097..72aeb895bf1a 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -30,12 +30,10 @@ namespace ccf ccf::crypto::Pem node_cert; std::atomic_size_t attempts; std::atomic_bool stopped; + size_t max_response_size; ccf::tasks::Task periodic_refresh_task; - // Maximum response body size for JWK/metadata responses (1 MB) - static constexpr size_t max_response_size = - static_cast(1024) * 1024; static constexpr long request_connection_timeout_s = 5; static constexpr long request_response_timeout_s = 5; @@ -83,7 +81,8 @@ namespace ccf const std::shared_ptr& consensus, const std::shared_ptr& rpc_map, ccf::crypto::ECKeyPairPtr node_sign_kp, - ccf::crypto::Pem node_cert) : + ccf::crypto::Pem node_cert, + size_t max_response_size) : refresh_interval_s(refresh_interval_s), network(network), consensus(consensus), @@ -91,7 +90,8 @@ namespace ccf node_sign_kp(std::move(node_sign_kp)), node_cert(std::move(node_cert)), attempts(0), - stopped(false) + stopped(false), + max_response_size(max_response_size) {} ~JwtKeyAutoRefresh() diff --git a/src/node/node_state.h b/src/node/node_state.h index d0690218bc40..738520612040 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1421,7 +1421,8 @@ namespace ccf consensus, rpc_map, node_sign_kp, - self_signed_node_cert); + self_signed_node_cert, + config.jwt.key_refresh_max_response_size); jwt_key_auto_refresh->start(); network.tables->set_map_hook( diff --git a/tests/config.jinja b/tests/config.jinja index d4a232fe0cec..b6a0bf18b51d 100644 --- a/tests/config.jinja +++ b/tests/config.jinja @@ -98,7 +98,8 @@ }, "jwt": { - "key_refresh_interval": "{{ jwt_key_refresh_interval }}" + "key_refresh_interval": "{{ jwt_key_refresh_interval }}", + "key_refresh_max_response_size": "{{ jwt_key_refresh_max_response_size }}" }, "output_files": { "node_certificate_file": "{{ node_certificate_file or "node.pem" }}", diff --git a/tests/infra/e2e_args.py b/tests/infra/e2e_args.py index 9a135f836fe2..260fb9c924fa 100644 --- a/tests/infra/e2e_args.py +++ b/tests/infra/e2e_args.py @@ -112,6 +112,11 @@ def cli_args( action="append", default=[], ) + parser.add_argument( + "--jwt-key-refresh-max-response-size", + help="Maximum response body size accepted when fetching JWT issuer OpenID metadata and JWKS", + default="1MB", + ) parser.add_argument( "-o", "--network-only", diff --git a/tests/infra/network.py b/tests/infra/network.py index 12ce0e16f9e8..16c6bf33274e 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -203,6 +203,7 @@ class Network: "max_open_sessions_hard", "forwarding_timeout_ms", "jwt_key_refresh_interval_s", + "jwt_key_refresh_max_response_size", "common_read_only_ledger_dir", "curve_id", "initial_node_cert_validity_days", diff --git a/tests/infra/remote.py b/tests/infra/remote.py index bc7d5769ab53..471445c1277e 100644 --- a/tests/infra/remote.py +++ b/tests/infra/remote.py @@ -313,6 +313,7 @@ def __init__( join_timer_s=None, sig_ms_interval=None, jwt_key_refresh_interval_s=None, + jwt_key_refresh_max_response_size="1MB", election_timeout_ms=None, consensus_update_timeout_ms=None, node_data_json_file=None, @@ -518,6 +519,7 @@ def __init__( join_timer=f"{join_timer_s}s" if join_timer_s else None, signature_interval_duration=f"{sig_ms_interval}ms", jwt_key_refresh_interval=f"{jwt_key_refresh_interval_s}s", + jwt_key_refresh_max_response_size=jwt_key_refresh_max_response_size, election_timeout=f"{election_timeout_ms}ms", message_timeout=f"{consensus_update_timeout_ms}ms", node_data_json_file=node_data_json_file, diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 955c832edfe2..858ca3e3d550 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -587,6 +587,41 @@ def test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args): network.consortium.remove_jwt_issuer(primary, issuer_name) +def test_jwt_key_auto_refresh_response_size_limit(network, args): + primary, _ = network.find_nodes() + remove_all_jwt_issuers(network, args, primary) + failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] + issuer = infra.jwt_issuer.JwtIssuer("https://localhost", cn="localhost") + ca_cert_bundle_name = "jwt_response_size_limit" + kid = "response_size_limit" + + LOG.info("Start OpenID endpoint server with oversized metadata") + with issuer.start_openid_server(0, kid) as server: + issuer.name = f"https://localhost:{server.bind_port}" + server.metadata["oversized_response"] = "x" * 4096 + add_auto_refresh_jwt_issuer(network, primary, issuer, ca_cert_bundle_name) + + try: + with_timeout( + lambda: check_refresh_failures_increased(primary, failures_before), + timeout=5, + ) + + LOG.info("Restore OpenID metadata and re-add JWT issuer") + del server.metadata["oversized_response"] + network.consortium.remove_jwt_issuer(primary, issuer.name) + add_auto_refresh_jwt_issuer(network, primary, issuer, ca_cert_bundle_name) + + with_timeout( + lambda: check_kv_jwt_key_matches( + args, network, kid, issuer.key_pub_pem + ), + timeout=5, + ) + finally: + network.consortium.remove_jwt_issuer(primary, issuer.name) + + @reqs.description("JWT with auto_refresh enabled") def test_jwt_key_auto_refresh(network, args): primary, _ = network.find_nodes() @@ -1016,6 +1051,7 @@ def run_manual(args): test_jwt_key_auto_refresh_tls_failure(network, args) test_jwt_key_auto_refresh_invalid_metadata_issuer(network, args) test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args) + test_jwt_key_auto_refresh_response_size_limit(network, args) def run_ca_cert(args): diff --git a/tests/programmability.py b/tests/programmability.py index 7af0236ee453..804c69c363b2 100644 --- a/tests/programmability.py +++ b/tests/programmability.py @@ -652,6 +652,7 @@ def run(args): package="samples/apps/logging/logging", nodes=infra.e2e_args.min_nodes(cr.args, f=1), jwt_key_refresh_interval_s=100000, + jwt_key_refresh_max_response_size="4KB", issuer_port=12346, forwarding_timeout_ms=jwt_forwarding_timeout_ms, ) From b53d4a37efca7e6a3cc7a019d8cff497a7536d8d Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 10:56:45 +0000 Subject: [PATCH 13/20] Format files required by CI --- samples/constitutions/default/actions.js | 10 ++++------ samples/minimal_ccf/app/actions.js | 10 ++++------ tests/npm-app/src/endpoints/crypto.ts | 4 +--- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 20c9bd69bb0e..d1d3477ccd1e 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -505,12 +505,10 @@ function checkReconfigurationType(config, new_config) { const from = config.reconfiguration_type; const to = new_config.reconfiguration_type; if (from !== to && to !== undefined) { - if ( - !( - (from === undefined || from === "OneTransaction") && - to === "TwoTransaction" - ) - ) { + if (!( + (from === undefined || from === "OneTransaction") && + to === "TwoTransaction" + )) { throw new Error( `Cannot change reconfiguration type from ${from} to ${to}.`, ); diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index d04e54c83494..2df7c76d4fc5 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -485,12 +485,10 @@ function checkReconfigurationType(config, new_config) { const from = config.reconfiguration_type; const to = new_config.reconfiguration_type; if (from !== to && to !== undefined) { - if ( - !( - (from === undefined || from === "OneTransaction") && - to === "TwoTransaction" - ) - ) { + if (!( + (from === undefined || from === "OneTransaction") && + to === "TwoTransaction" + )) { throw new Error( `Cannot change reconfiguration type from ${from} to ${to}.`, ); diff --git a/tests/npm-app/src/endpoints/crypto.ts b/tests/npm-app/src/endpoints/crypto.ts index 94afa9730728..2bbadad891cd 100644 --- a/tests/npm-app/src/endpoints/crypto.ts +++ b/tests/npm-app/src/endpoints/crypto.ts @@ -97,9 +97,7 @@ interface RsaOaepAesKwpParams { } type WrapAlgoParams = - | RsaOaepParams - | RsaOaepAesKwpParams - | ccfcrypto.AesKwpParams; + RsaOaepParams | RsaOaepAesKwpParams | ccfcrypto.AesKwpParams; interface WrapKeyRequest { key: Base64; // typically an AES key From 9e6060501a9e96f2b8c08018c373493d1f7d47c1 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 11:05:36 +0000 Subject: [PATCH 14/20] Drop legacy libcurl protocol option fallback --- src/node/jwt_key_auto_refresh.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 72aeb895bf1a..f8e9368544ed 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -48,11 +48,7 @@ namespace ccf curl_handle.set_opt(CURLOPT_TIMEOUT, request_response_timeout_s); curl_handle.set_opt(CURLOPT_SSL_VERIFYPEER, 1L); curl_handle.set_opt(CURLOPT_SSL_VERIFYHOST, 2L); -#if LIBCURL_VERSION_NUM >= 0x075500 curl_handle.set_opt(CURLOPT_PROTOCOLS_STR, "https"); -#else - curl_handle.set_opt(CURLOPT_PROTOCOLS, CURLPROTO_HTTPS); -#endif curl_handle.set_blob_opt( CURLOPT_CAINFO_BLOB, reinterpret_cast(ca_bundle_pem.data()), From 0371cfcdba52e175c07a4cb65c948fe4918d81f9 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 15:08:40 +0000 Subject: [PATCH 15/20] Lower curl stop-path logs to debug --- src/http/curl.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/http/curl.h b/src/http/curl.h index 5d41e52d10fa..fb3041294f71 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -664,7 +664,7 @@ namespace ccf::curl if (self->is_stopping) { - LOG_FAIL_FMT("async_requests_callback called while stopping"); + LOG_DEBUG_FMT("async_requests_callback called while stopping"); return; } @@ -694,7 +694,7 @@ namespace ccf::curl if (self->is_stopping) { - LOG_FAIL_FMT("libuv_timeout_callback called while stopping"); + LOG_DEBUG_FMT("libuv_timeout_callback called while stopping"); return; } @@ -722,7 +722,7 @@ namespace ccf::curl if (self->is_stopping) { - LOG_FAIL_FMT("curl_timeout_callback called while stopping"); + LOG_DEBUG_FMT("curl_timeout_callback called while stopping"); return 0; } @@ -763,7 +763,7 @@ namespace ccf::curl if (self->is_stopping) { - LOG_FAIL_FMT( + LOG_DEBUG_FMT( "libuv_socket_poll_callback called on {} while stopped", socket_context->socket); return; From 1da62b6054d40159f185215da4809fc03b329bfe Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 15:12:41 +0000 Subject: [PATCH 16/20] Remove JWK refresh migration plan --- .../jwk_refresh_curl_multi_migration_plan.md | 84 ------------------- 1 file changed, 84 deletions(-) delete mode 100644 doc/dev/jwk_refresh_curl_multi_migration_plan.md diff --git a/doc/dev/jwk_refresh_curl_multi_migration_plan.md b/doc/dev/jwk_refresh_curl_multi_migration_plan.md deleted file mode 100644 index 289a420c6840..000000000000 --- a/doc/dev/jwk_refresh_curl_multi_migration_plan.md +++ /dev/null @@ -1,84 +0,0 @@ -# JWK refresh migration to curl multi singleton - -Status note for migrating JWT/JWK auto-refresh away from the enclave/host -`RPCSessions::create_client()` outbound HTTP client and onto the curl multi -singleton infrastructure introduced in microsoft/CCF#7102. - -Last reviewed: 2026-06-26. - -## Current status - -Implementation is complete on this branch pending validation. - -- `src/node/jwt_key_auto_refresh.h` uses `ccf::curl::CurlRequest` with - `CurlmLibuvContextSingleton::get_instance()->attach_request(...)` for both - external OpenID metadata and JWKS fetches. -- `JwtKeyAutoRefresh` no longer stores an `RPCSessions` pointer, and - `src/node/node_state.h` constructs it without passing `rpcsessions`. -- The internal `/node/jwt_keys/refresh` update path still goes through - `send_refresh_jwt_keys(...)` and is intentionally unchanged. -- Curl requests now set peer and host verification, use the configured - in-memory CA bundle, restrict protocols to HTTPS only, and bound both connect - and total transfer timeouts. -- OpenID metadata is still fetched from the configured issuer URL. The JWKS URL - from metadata is parsed and must use the `https` scheme before it is handed to - curl. -- Response bodies are bounded to 1 MB. -- Curl connection and TLS failures now call `send_refresh_jwt_keys_error()` and - are counted in refresh failure metrics. `CHANGELOG.md` documents this - observability change. -- `tests/jwt_test.py` includes focused connection-failure and TLS-trust-failure - coverage and wires these into `run_manual()`. -- The new failure tests remove existing JWT issuers before recording the - baseline failure count, so their metric-delta checks are isolated from other - auto-refresh issuers. -- `src/http/test/curl_test.cpp` already covers generic curl singleton behavior; - JWT behavior is covered by the Python end-to-end tests. - -Unrelated local changes exist in `.github/workflows/README.md`, -`perf_artifacts/`, and `perf_data/`; leave them untouched. - -## Remaining validation - -Run these before considering the migration complete: - -1. Build affected C++ targets, at minimum the node target and `curl_test`. -2. Run `curl_test` to catch curl multi singleton regressions. -3. Run JWT manual suite coverage containing: - - `test_jwt_key_initial_refresh` - - `test_jwt_key_auto_refresh_connection_failure` - - `test_jwt_key_auto_refresh_tls_failure` -4. Run broader JWT auto-refresh coverage containing: - - `test_jwt_key_auto_refresh` - - backup-primary variant of `test_jwt_key_auto_refresh` - - `test_jwt_key_auto_refresh_entries` -5. Confirm shutdown has no libuv handle leaks or async lifetime issues. -6. Optional follow-up: add a direct invalid OpenID metadata auto-refresh test if - the test harness can serve malformed metadata without duplicating large parts - of `infra.jwt_issuer`. Current coverage exercises invalid JWKS and curl-level - connection/TLS failures, but not malformed metadata from a reachable issuer. - -## Risks to keep watching - -- `JwtKeyAutoRefresh` now inherits `enable_shared_from_this`; callers must keep - constructing it as a shared pointer before any refresh is scheduled. -- Async callback captures must own all data they use. Current callbacks capture - `self`, issuer strings, CA bundle strings, and response bodies by value/shared - ownership. -- CA bundle material must remain valid until libcurl has consumed it. Current - code passes the CA bundle into `set_blob_opt(...)`; verify this remains copied - or otherwise owned by the curl wrapper. -- The 1 MB response limit and 5 second connect/transfer timeouts are intentional - bounds, but can reject very large or slow metadata/JWKS responses. -- The HTTPS-only checks are intentional hardening. Any future support for other - schemes should be explicit and tested. - -## Definition of done - -- Both external fetches use curl multi singleton requests. -- No JWT auto-refresh external fetch uses `RPCSessions::create_client(...)`. -- JWKS URIs must be HTTPS, and curl is restricted to HTTPS protocols. -- Curl requests have bounded connect and total transfer timeouts. -- Existing auto-refresh behavior remains intact. -- New connection and TLS failure tests pass with isolated metric baselines. -- Remaining validation above has been run and recorded in the PR or commit notes. From ae5114058ff38718765e7c09c173de4517a65064 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 15:19:08 +0000 Subject: [PATCH 17/20] Document curl TLS verification options --- CHANGELOG.md | 14 +++++++++----- src/node/jwt_key_auto_refresh.h | 5 +++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e96ea49b470..db2b7ea8fe4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,15 @@ 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.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). + ## [7.0.6] [7.0.6]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.6 @@ -13,11 +22,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Experimental support for IPv6. Node RPC and node-to-node interface hosts may now be specified as IPv6 literals in bracketed form (e.g. `[::1]:8000`), and addresses are consistently parsed, bound, connected (with fallback across mixed IPv4/IPv6 resolved addresses), serialised, and embedded in redirect URLs for IPv6 (#7671). -### 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). - ### Fixed - Forwarded commands are no longer processed until the node is part of the network, matching the existing behaviour for other node-to-node messages. Previously a forwarded command could be executed while the node was in an earlier startup state, which could lead to undefined behaviour for some commands (#7936). diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index f8e9368544ed..bba696581879 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -46,7 +46,12 @@ namespace ccf curl_handle.set_opt(CURLOPT_HTTPGET, 1L); curl_handle.set_opt(CURLOPT_CONNECTTIMEOUT, request_connection_timeout_s); curl_handle.set_opt(CURLOPT_TIMEOUT, request_response_timeout_s); + // 1L enables peer certificate verification. See libcurl docs: + // https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYPEER.html curl_handle.set_opt(CURLOPT_SSL_VERIFYPEER, 1L); + // 2L requires the certificate name to match the requested host. + // See libcurl docs: + // https://curl.se/libcurl/c/CURLOPT_SSL_VERIFYHOST.html curl_handle.set_opt(CURLOPT_SSL_VERIFYHOST, 2L); curl_handle.set_opt(CURLOPT_PROTOCOLS_STR, "https"); curl_handle.set_blob_opt( From e5b3e35d824071888ae6bb0b314352d94d053ff8 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 16:22:56 +0000 Subject: [PATCH 18/20] Harden curl cleanup error handling --- python/pyproject.toml | 2 +- src/http/curl.h | 74 ++++++++++++++++++++++++++++++++----------- 2 files changed, 56 insertions(+), 20 deletions(-) diff --git a/python/pyproject.toml b/python/pyproject.toml index bcfe2aae22da..bde210cda2b8 100644 --- a/python/pyproject.toml +++ b/python/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "ccf" -version = "7.0.6" +version = "7.0.7" authors = [ { name="CCF Team", email="CCF-Sec@microsoft.com" }, ] diff --git a/src/http/curl.h b/src/http/curl.h index fb3041294f71..a4c89f6b1175 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -586,10 +586,18 @@ namespace ccf::curl // retrieve the request data and attach a lifetime to it ccf::curl::CurlRequest* request = nullptr; - curl_easy_getinfo(easy, CURLINFO_PRIVATE, &request); + try + { + CHECK_CURL_EASY_GETINFO(easy, CURLINFO_PRIVATE, &request); + } + catch (const std::runtime_error&) + { + CHECK_CURL_MULTI(curl_multi_remove_handle, p.get(), easy); + throw; + } if (request == nullptr) { - curl_multi_remove_handle(p.get(), easy); + CHECK_CURL_MULTI(curl_multi_remove_handle, p.get(), easy); throw std::runtime_error( "CURLMSG_DONE received with no associated request data"); } @@ -597,7 +605,7 @@ namespace ccf::curl // detach the easy handle such that it can be cleaned up with the // destructor of CurlRequest - curl_multi_remove_handle(p.get(), easy); + CHECK_CURL_MULTI(curl_multi_remove_handle, p.get(), easy); // handle response inline. Note that if this is expensive, it should // defer its work to a task @@ -957,27 +965,55 @@ namespace ccf::curl is_stopping = true; // remove, stop and cleanup all curl easy handles - std::unique_ptr easy_handles( - curl_multi_get_handles(curl_request_curlm), - // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion) - [](CURL** handles) { curl_free(static_cast(handles)); }); - // curl_multi_get_handles returns the handles as a null-terminated array - for (size_t i = 0; easy_handles.get()[i] != nullptr; ++i) + auto* easy_handles_raw = curl_multi_get_handles(curl_request_curlm); + if (easy_handles_raw == nullptr) + { + LOG_FAIL_FMT("Error calling curl_multi_get_handles while closing"); + } + else { - auto* easy = easy_handles.get()[i]; - curl_multi_remove_handle(curl_request_curlm, easy); - if (easy != nullptr) + std::unique_ptr easy_handles( + easy_handles_raw, + // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion) + [](CURL** handles) { curl_free(static_cast(handles)); }); + // curl_multi_get_handles returns the handles as a null-terminated array + for (size_t i = 0; easy_handles.get()[i] != nullptr; ++i) { - // attach a lifetime to the request - ccf::curl::CurlRequest* request = nullptr; - curl_easy_getinfo(easy, CURLINFO_PRIVATE, &request); - if (request == nullptr) + auto* easy = easy_handles.get()[i]; + const auto remove_res = curl_multi_remove_handle( + curl_request_curlm, easy); + if (remove_res != CURLM_OK) { LOG_FAIL_FMT( - "CURLMSG_DONE received with no associated request data"); + "Error calling curl_multi_remove_handle while closing: {} ({})", + remove_res, + curl_multi_strerror(remove_res)); + } + if (easy != nullptr) + { + // attach a lifetime to the request + ccf::curl::CurlRequest* request = nullptr; + const auto getinfo_res = curl_easy_getinfo( + easy, CURLINFO_PRIVATE, &request); + if (getinfo_res != CURLE_OK) + { + LOG_FAIL_FMT( + "Error calling curl_easy_getinfo while closing: {} ({})", + getinfo_res, + curl_easy_strerror(getinfo_res)); + curl_easy_cleanup(easy); + continue; + } + if (request == nullptr) + { + LOG_FAIL_FMT( + "CURL easy handle had no associated request data while closing"); + curl_easy_cleanup(easy); + continue; + } + std::unique_ptr request_data_ptr(request); + curl_easy_cleanup(easy); } - std::unique_ptr request_data_ptr(request); - curl_easy_cleanup(easy); } } // Drain the deque rather than letting it destruct From f6ccb61ee84decd7ca41186224cb2315ae553154 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 16:34:30 +0000 Subject: [PATCH 19/20] Check curl slist append failures --- src/http/curl.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/http/curl.h b/src/http/curl.h index a4c89f6b1175..e1bc9d4d4195 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -164,7 +164,13 @@ namespace ccf::curl void append(const char* str) { - p.reset(curl_slist_append(p.release(), str)); + auto* updated = curl_slist_append(p.get(), str); + if (updated == nullptr) + { + throw std::runtime_error("Error calling curl_slist_append"); + } + p.release(); + p.reset(updated); } void append(const std::string& key, const std::string& value) From 3668bb967a531c9a692987d287834dc7aa3225f4 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 29 Jun 2026 17:15:22 +0000 Subject: [PATCH 20/20] Allow cross-authority JWKS URIs --- src/http/curl.h | 22 ++++++++++++++-------- src/node/jwt_key_auto_refresh.h | 23 +++-------------------- tests/jwt_test.py | 32 +++++++++++++++++++++++--------- 3 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/http/curl.h b/src/http/curl.h index e1bc9d4d4195..9d822db864b6 100644 --- a/src/http/curl.h +++ b/src/http/curl.h @@ -164,13 +164,18 @@ namespace ccf::curl void append(const char* str) { - auto* updated = curl_slist_append(p.get(), str); + auto* current = p.get(); + auto* updated = curl_slist_append(current, str); if (updated == nullptr) { throw std::runtime_error("Error calling curl_slist_append"); } - p.release(); - p.reset(updated); + if (updated != current) + { + auto* released = p.release(); + (void)released; + p.reset(updated); + } } void append(const std::string& key, const std::string& value) @@ -986,8 +991,8 @@ namespace ccf::curl for (size_t i = 0; easy_handles.get()[i] != nullptr; ++i) { auto* easy = easy_handles.get()[i]; - const auto remove_res = curl_multi_remove_handle( - curl_request_curlm, easy); + const auto remove_res = + curl_multi_remove_handle(curl_request_curlm, easy); if (remove_res != CURLM_OK) { LOG_FAIL_FMT( @@ -999,8 +1004,8 @@ namespace ccf::curl { // attach a lifetime to the request ccf::curl::CurlRequest* request = nullptr; - const auto getinfo_res = curl_easy_getinfo( - easy, CURLINFO_PRIVATE, &request); + const auto getinfo_res = + curl_easy_getinfo(easy, CURLINFO_PRIVATE, &request); if (getinfo_res != CURLE_OK) { LOG_FAIL_FMT( @@ -1013,7 +1018,8 @@ namespace ccf::curl if (request == nullptr) { LOG_FAIL_FMT( - "CURL easy handle had no associated request data while closing"); + "CURL easy handle had no associated request data while " + "closing"); curl_easy_cleanup(easy); continue; } diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index bba696581879..34724b614522 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -299,12 +299,12 @@ namespace ccf return; } // Validate jwks_uri before handing it to libcurl; the parsed result is - // not used directly since the full URL string is passed to curl. - ::http::URL issuer_url; + // not used directly since the full URL string is passed to curl. The + // JWKS host/port may differ from the issuer authority; OIDC Discovery + // requires HTTPS here, but does not require matching authorities. ::http::URL jwks_url; try { - issuer_url = ::http::parse_url_full(issuer); jwks_url = ::http::parse_url_full(jwks_url_str); } catch (const std::invalid_argument& e) @@ -319,10 +319,7 @@ namespace ccf return; } - ccf::nonstd::to_lower(issuer_url.scheme); - ccf::nonstd::to_lower(issuer_url.host); ccf::nonstd::to_lower(jwks_url.scheme); - ccf::nonstd::to_lower(jwks_url.host); if (jwks_url.scheme != "https") { LOG_FAIL_FMT( @@ -333,20 +330,6 @@ namespace ccf return; } - const auto issuer_port = - issuer_url.port.empty() ? "443" : issuer_url.port; - const auto jwks_port = jwks_url.port.empty() ? "443" : jwks_url.port; - if (jwks_url.host != issuer_url.host || jwks_port != issuer_port) - { - LOG_FAIL_FMT( - "JWT key auto-refresh: jwks_uri for issuer '{}' must use the issuer " - "authority: {}", - issuer, - jwks_url_str); - send_refresh_jwt_keys_error(); - return; - } - LOG_DEBUG_FMT( "JWT key auto-refresh: Requesting JWKS at {}", jwks_url_str); diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 858ca3e3d550..4d40916fc883 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -6,6 +6,7 @@ import base64 import socket import infra.network +import infra.jwt_issuer import infra.path import infra.proc import infra.net @@ -13,7 +14,11 @@ import infra.e2e_args import infra.proposal import suite.test_requirements as reqs -from infra.jwt_issuer import get_jwt_issuers, get_jwt_keys +from infra.jwt_issuer import ( + OpenIDProviderServer, + get_jwt_issuers, + get_jwt_keys, +) import ca_certs import ccf.ledger from ccf.tx_id import TxID @@ -547,9 +552,14 @@ def test_jwt_key_auto_refresh_invalid_metadata_issuer(network, args): def test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args): primary, _ = network.find_nodes() remove_all_jwt_issuers(network, args, primary) - failures_before = get_jwt_refresh_endpoint_metrics(primary)["failures"] - issuer = infra.jwt_issuer.JwtIssuer("https://localhost", cn="localhost") + issuer_host = "localhost" + issuer_port = get_unused_local_port() + jwks_port = get_unused_local_port() + issuer = infra.jwt_issuer.JwtIssuer( + f"https://{issuer_host}:{issuer_port}", cn=issuer_host + ) ca_cert_bundle_name = "jwt_cross_authority_jwks_uri" + kid = "cross_authority_jwks_uri" LOG.info("Add CA cert for JWT issuer") with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as ca_cert_bundle_fp: @@ -560,11 +570,13 @@ def test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args): ) LOG.info("Start OpenID endpoint server with cross-authority JWKS URI") - with issuer.start_openid_server(0) as server: - issuer_name = f"https://localhost:{server.bind_port}" - server.metadata["jwks_uri"] = ( - f"https://localhost:{get_unused_local_port()}/keys" - ) + with issuer.start_openid_server(issuer_port, kid) as server, OpenIDProviderServer( + jwks_port, issuer.tls_priv, issuer.tls_cert, issuer.create_jwks(kid) + ) as jwks_server: + issuer_name = issuer.name + # Exercise OIDC-compatible metadata where JWKS are served from a + # different authority than the issuer metadata. + server.metadata["jwks_uri"] = f"https://localhost:{jwks_server.bind_port}/keys" with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: json.dump( @@ -580,7 +592,9 @@ def test_jwt_key_auto_refresh_cross_authority_jwks_uri(network, args): try: with_timeout( - lambda: check_refresh_failures_increased(primary, failures_before), + lambda: check_kv_jwt_key_matches( + args, network, kid, issuer.key_pub_pem + ), timeout=5, ) finally: