From af8353f271df223051a78dfb35d30c45108638e3 Mon Sep 17 00:00:00 2001 From: Siddharth More Date: Thu, 27 Jun 2024 14:15:22 -0700 Subject: [PATCH 1/8] Remove Old JWT Tables --- include/ccf/service/tables/jwt.h | 12 ------------ samples/constitutions/default/actions.js | 18 +++++++++++++++++ src/endpoints/authentication/jwt_auth.cpp | 17 ---------------- src/node/rpc/jwt_management.h | 24 ----------------------- src/service/network_tables.h | 9 +-------- tests/infra/consortium.py | 7 +++++++ 6 files changed, 26 insertions(+), 61 deletions(-) diff --git a/include/ccf/service/tables/jwt.h b/include/ccf/service/tables/jwt.h index d4e6a7866cc1..b72288a0cb21 100644 --- a/include/ccf/service/tables/jwt.h +++ b/include/ccf/service/tables/jwt.h @@ -78,18 +78,6 @@ namespace ccf static constexpr auto JWT_PUBLIC_SIGNING_KEYS_METADATA = "public:ccf.gov.jwt.public_signing_keys_metadata"; - - namespace Legacy - { - static constexpr auto JWT_PUBLIC_SIGNING_KEYS = - "public:ccf.gov.jwt.public_signing_key"; - static constexpr auto JWT_PUBLIC_SIGNING_KEY_ISSUER = - "public:ccf.gov.jwt.public_signing_key_issuer"; - - using JwtPublicSigningKeys = kv::RawCopySerialisedMap; - using JwtPublicSigningKeyIssuer = - kv::RawCopySerialisedMap; - } } struct JsonWebKeySet diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index fa380f3c9364..4b0e5d3e8a1d 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -1494,4 +1494,22 @@ const actions = new Map([ function (args) {}, ), ], + [ + "remove_old_jwt_tables", + new Action( + function (args) { + // Validate that no arguments are passed + if (args !== undefined && args !== null && Object.keys(args).length > 0) { + throw new Error("remove_old_jwt_tables does not accept any arguments"); + } + }, + function (args) { + // Clear the JWT public signing key table + ccf.kv["public:ccf.gov.jwt.public_signing_key"].clear(); + + // Clear the JWT public signing key issuer table + ccf.kv["public:ccf.gov.jwt.public_signing_key_issuer"].clear(); + } + ), + ], ]); diff --git a/src/endpoints/authentication/jwt_auth.cpp b/src/endpoints/authentication/jwt_auth.cpp index 758f65972c8b..e378eaaef1c7 100644 --- a/src/endpoints/authentication/jwt_auth.cpp +++ b/src/endpoints/authentication/jwt_auth.cpp @@ -136,23 +136,6 @@ namespace ccf const auto key_id = token.header_typed.kid; auto token_keys = keys->get(key_id); - if (!token_keys) - { - auto fallback_keys = tx.ro( - ccf::Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS); - auto fallback_issuers = tx.ro( - ccf::Tables::Legacy::JWT_PUBLIC_SIGNING_KEY_ISSUER); - - auto fallback_key = fallback_keys->get(key_id); - if (fallback_key) - { - token_keys = std::vector{OpenIDJWKMetadata{ - .cert = *fallback_key, - .issuer = *fallback_issuers->get(key_id), - .constraint = std::nullopt}}; - } - } - if (!token_keys) { error_reason = diff --git a/src/node/rpc/jwt_management.h b/src/node/rpc/jwt_management.h index 4d10d1ab5e0b..ec021b65e0c3 100644 --- a/src/node/rpc/jwt_management.h +++ b/src/node/rpc/jwt_management.h @@ -23,25 +23,6 @@ namespace ccf { - static void legacy_remove_jwt_public_signing_keys( - kv::Tx& tx, std::string issuer) - { - auto keys = - tx.rw(Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS); - auto key_issuer = tx.rw( - Tables::Legacy::JWT_PUBLIC_SIGNING_KEY_ISSUER); - - key_issuer->foreach( - [&issuer, &keys, &key_issuer](const auto& k, const auto& v) { - if (v == issuer) - { - keys->remove(k); - key_issuer->remove(k); - } - return true; - }); - } - static bool check_issuer_constraint( const std::string& issuer, const std::string& constraint) { @@ -75,11 +56,6 @@ namespace ccf static void remove_jwt_public_signing_keys(kv::Tx& tx, std::string issuer) { - // Unlike resetting JWT keys for a particular issuer, removing keys can be - // safely done on both table revisions, as soon as the application shouldn't - // use them anyway after being ask about that explicitly. - legacy_remove_jwt_public_signing_keys(tx, issuer); - auto keys = tx.rw(Tables::JWT_PUBLIC_SIGNING_KEYS_METADATA); diff --git a/src/service/network_tables.h b/src/service/network_tables.h index 57c6894a735e..4d32be03a9d0 100644 --- a/src/service/network_tables.h +++ b/src/service/network_tables.h @@ -157,20 +157,13 @@ namespace ccf const JwtIssuers jwt_issuers = {Tables::JWT_ISSUERS}; const JwtPublicSigningKeys jwt_public_signing_keys_metadata = { Tables::JWT_PUBLIC_SIGNING_KEYS_METADATA}; - const Tables::Legacy::JwtPublicSigningKeys legacy_jwt_public_signing_keys = - {Tables::Legacy::JWT_PUBLIC_SIGNING_KEYS}; - const Tables::Legacy::JwtPublicSigningKeyIssuer - legacy_jwt_public_signing_key_issuer = { - Tables::Legacy::JWT_PUBLIC_SIGNING_KEY_ISSUER}; inline auto get_all_jwt_tables() const { return std::make_tuple( ca_cert_bundles, jwt_issuers, - jwt_public_signing_keys_metadata, - legacy_jwt_public_signing_keys, - legacy_jwt_public_signing_key_issuer); + jwt_public_signing_keys_metadata); } // diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index e20864086d7c..891a1f8d5c3c 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -875,3 +875,10 @@ def check_for_service(self, remote_node, status, recovery_count=None): assert ( recovery_count is None or current_recovery_count == recovery_count ), f"Current recovery count {current_recovery_count} is not expected {recovery_count}" + + def remove_old_jwt_tables(self, remote_node, issuer): + proposal_body, careful_vote = self.make_proposal( + "remove_old_jwt_tables", issuer=issuer + ) + proposal = self.get_any_active_member().propose(remote_node, proposal_body) + return self.vote_using_majority(remote_node, proposal, careful_vote) \ No newline at end of file From c938fa5a484ac5e3be138eb2da551e5926fc3509 Mon Sep 17 00:00:00 2001 From: Siddharth More Date: Thu, 27 Jun 2024 14:22:43 -0700 Subject: [PATCH 2/8] fix format error --- src/service/network_tables.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/service/network_tables.h b/src/service/network_tables.h index 4d32be03a9d0..6ef422a12b71 100644 --- a/src/service/network_tables.h +++ b/src/service/network_tables.h @@ -161,9 +161,7 @@ namespace ccf inline auto get_all_jwt_tables() const { return std::make_tuple( - ca_cert_bundles, - jwt_issuers, - jwt_public_signing_keys_metadata); + ca_cert_bundles, jwt_issuers, jwt_public_signing_keys_metadata); } // From f3492bd51089e9e214d3a8933eeef9516a47a3f0 Mon Sep 17 00:00:00 2001 From: Siddharth More Date: Thu, 27 Jun 2024 14:26:00 -0700 Subject: [PATCH 3/8] fix prettier error --- samples/constitutions/default/actions.js | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 4b0e5d3e8a1d..7dc8373e36ff 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -1499,17 +1499,23 @@ const actions = new Map([ new Action( function (args) { // Validate that no arguments are passed - if (args !== undefined && args !== null && Object.keys(args).length > 0) { - throw new Error("remove_old_jwt_tables does not accept any arguments"); + if ( + args !== undefined && + args !== null && + Object.keys(args).length > 0 + ) { + throw new Error( + "remove_old_jwt_tables does not accept any arguments", + ); } }, function (args) { // Clear the JWT public signing key table ccf.kv["public:ccf.gov.jwt.public_signing_key"].clear(); - + // Clear the JWT public signing key issuer table ccf.kv["public:ccf.gov.jwt.public_signing_key_issuer"].clear(); - } + }, ), ], ]); From 4d90934903641c07ac40826e7d0e29df85f8be50 Mon Sep 17 00:00:00 2001 From: smore Date: Mon, 1 Jul 2024 08:25:49 -0700 Subject: [PATCH 4/8] Update samples/constitutions/default/actions.js Co-authored-by: Eddy Ashton --- samples/constitutions/default/actions.js | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 7dc8373e36ff..319d529c0eb0 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -1499,15 +1499,7 @@ const actions = new Map([ new Action( function (args) { // Validate that no arguments are passed - if ( - args !== undefined && - args !== null && - Object.keys(args).length > 0 - ) { - throw new Error( - "remove_old_jwt_tables does not accept any arguments", - ); - } + checkNone(args); }, function (args) { // Clear the JWT public signing key table From f99b75691d140e52b2424835212e683ba805cd32 Mon Sep 17 00:00:00 2001 From: Siddharth More Date: Tue, 2 Jul 2024 08:35:47 -0700 Subject: [PATCH 5/8] Add Test for verification of JWT tables --- tests/infra/consortium.py | 4 +- tests/recovery.py | 87 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index 891a1f8d5c3c..4c9ec2ed6df9 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -876,9 +876,9 @@ def check_for_service(self, remote_node, status, recovery_count=None): recovery_count is None or current_recovery_count == recovery_count ), f"Current recovery count {current_recovery_count} is not expected {recovery_count}" - def remove_old_jwt_tables(self, remote_node, issuer): + def remove_old_jwt_tables(self, remote_node): proposal_body, careful_vote = self.make_proposal( - "remove_old_jwt_tables", issuer=issuer + "remove_old_jwt_tables" ) proposal = self.get_any_active_member().propose(remote_node, proposal_body) return self.vote_using_majority(remote_node, proposal, careful_vote) \ No newline at end of file diff --git a/tests/recovery.py b/tests/recovery.py index 72040d543b8f..6cdafa214050 100644 --- a/tests/recovery.py +++ b/tests/recovery.py @@ -528,6 +528,93 @@ def test_share_resilience(network, args, from_snapshot=False): recovered_network.service_load.set_network(recovered_network) return recovered_network +@reqs.description("Remove JWT Tables") +def test_recovered_ledger_remove_jwt_tables( + args, directory, expected_recovery_count, test_receipt=True +): + service_dir = os.path.join( + os.path.dirname(os.path.realpath(__file__)), "testdata", directory + ) + + old_common = os.path.join(service_dir, "common") + new_common = infra.network.get_common_folder_name(args.workspace, args.label) + copy_tree(old_common, new_common) + + network = infra.network.Network(args.nodes, args.binary_dir) + + args.previous_service_identity_file = os.path.join(old_common, "service_cert.pem") + + network.start_in_recovery( + args, + ledger_dir=os.path.join(service_dir, "ledger"), + committed_ledger_dirs=[os.path.join(service_dir, "ledger")], + snapshots_dir=os.path.join(service_dir, "snapshots"), + common_dir=new_common, + ) + + network.recover(args, expected_recovery_count=expected_recovery_count) + + primary, _ = network.find_primary() + + # The member and user certs stored on this service are all currently expired. + # Remove user certs and add new users before attempting any user requests + primary, _ = network.find_primary() + + user_certs = [ + os.path.join(old_common, file) + for file in os.listdir(old_common) + if file.startswith("user") and file.endswith("_cert.pem") + ] + user_ids = [ + infra.crypto.compute_cert_der_hash_hex_from_pem(open(cert).read()) + for cert in user_certs + ] + for user_id in user_ids: + LOG.info(f"Removing expired user {user_id}") + network.consortium.remove_user(primary, user_id) + + new_user_local_id = "recovery_user" + new_user = network.create_user(new_user_local_id, args.participants_curve) + LOG.info(f"Adding new user {new_user.service_id}") + network.consortium.add_user(primary, new_user.local_id) + + infra.checker.check_can_progress(primary, local_user_id=new_user_local_id) + + if test_receipt: + r = primary.get_receipt(2, 3) + verify_receipt(r.json(), network.cert) + + # Get State + network.get_latest_ledger_public_state() + ledger_directories = primary.remote.ledger_paths() + ledger = ccf.ledger.Ledger(ledger_directories) + + for chunk in ledger: + for tx in chunk: + txid = TxID(tx.gcm_header.view, tx.gcm_header.seqno) + tables = tx.get_public_domain().get_tables() + pub_keys = tables["public:ccf.gov.jwt.public_signing_key"] + assert (pub_keys is not None), "PubKeys is not None" + pub_key_issuer = tables["public:ccf.gov.jwt.public_signing_key_issuer"] + assert (pub_key_issuer is not None), "PubKeys is not None" + + # Submit Proposal to remove old JWT Tables + network.consortium.remove_old_jwt_tables(primary) + + # Get New State after submitting proposal to remove older JWT tables + network.get_latest_ledger_public_state() + ledger_directories = primary.remote.ledger_paths() + ledger = ccf.ledger.Ledger(ledger_directories) + + for chunk in ledger: + for tx in chunk: + txid = TxID(tx.gcm_header.view, tx.gcm_header.seqno) + tables = tx.get_public_domain().get_tables() + pub_keys = tables["public:ccf.gov.jwt.public_signing_key"] + assert (pub_keys is None), "PubKeys is None" + pub_key_issuer = tables["public:ccf.gov.jwt.public_signing_key_issuer"] + assert (pub_key_issuer is None), "PubKeys is None" + @reqs.description("Recover a service from malformed ledger") @reqs.recover(number_txs=2) From 6314bbc473287188167062c053dbe470c14c283f Mon Sep 17 00:00:00 2001 From: Siddharth More Date: Tue, 2 Jul 2024 09:18:31 -0700 Subject: [PATCH 6/8] update doc version --- doc/schemas/gov_openapi.json | 44 ---------------------------------- src/node/rpc/member_frontend.h | 2 +- tests/suite/test_suite.py | 1 + 3 files changed, 2 insertions(+), 45 deletions(-) diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index 855144deeb4a..3ba96b6e6e0c 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -1737,50 +1737,6 @@ } } }, - "/gov/kv/jwt/public_signing_key": { - "get": { - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/string_to_base64string" - } - } - }, - "description": "Default response description" - }, - "default": { - "$ref": "#/components/responses/default" - } - }, - "x-ccf-forwarding": { - "$ref": "#/components/x-ccf-forwarding/sometimes" - } - } - }, - "/gov/kv/jwt/public_signing_key_issuer": { - "get": { - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/string_to_string" - } - } - }, - "description": "Default response description" - }, - "default": { - "$ref": "#/components/responses/default" - } - }, - "x-ccf-forwarding": { - "$ref": "#/components/x-ccf-forwarding/sometimes" - } - } - }, "/gov/kv/jwt/public_signing_keys_metadata": { "get": { "responses": { diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index 6d6262b8ae0b..8348ff14a3fd 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -595,7 +595,7 @@ namespace ccf openapi_info.description = "This API is used to submit and query proposals which affect CCF's " "public governance tables."; - openapi_info.document_version = "4.1.8"; + openapi_info.document_version = "4.1.9"; } static std::optional get_caller_member_id( diff --git a/tests/suite/test_suite.py b/tests/suite/test_suite.py index 1016f8498b9d..96ba39b89435 100644 --- a/tests/suite/test_suite.py +++ b/tests/suite/test_suite.py @@ -110,6 +110,7 @@ # recovery: recovery.test_recover_service, recovery.test_recover_service_aborted, + recovery.test_recovered_ledger_remove_jwt_tables, # rekey: e2e_logging.test_rekey, # election: From b13fd66a23184222c8e3b5201a127e31cd0762e3 Mon Sep 17 00:00:00 2001 From: Siddharth More Date: Fri, 5 Jul 2024 09:34:20 -0700 Subject: [PATCH 7/8] Update ChangeLog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77cff1fd9b2e..3506fde6b1bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Removed the existing metrics endpoint and API (`GET /api/metrics`, `get_metrics_v1`). Stats for request execution can instead be gathered by overriding the `EndpointRegistry::handle_event_request_completed()` method. - Removed automatic msgpack support from JSON endpoint adapters, and related `include/ccf/serdes.h` file. +- Removed endpoint for `/gov/kv/jwt/public_signing_key` and `/gov/kv/jwt/public_signing_key_issuer` ## [5.0.0-dev18] From f9cd14d083655579aaa352054d53bd268d47ff6a Mon Sep 17 00:00:00 2001 From: smore Date: Wed, 10 Jul 2024 08:59:39 -0700 Subject: [PATCH 8/8] Update gov_openapi.json --- doc/schemas/gov_openapi.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index 3ba96b6e6e0c..01374e16e7af 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -1351,7 +1351,7 @@ "info": { "description": "This API is used to submit and query proposals which affect CCF's public governance tables.", "title": "CCF Governance API", - "version": "4.1.8" + "version": "4.1.9" }, "openapi": "3.0.0", "paths": { @@ -2654,4 +2654,4 @@ } }, "servers": [] -} \ No newline at end of file +}