From 4b167078d3ee7463b8311f74e075ba8780c736ef Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 23 Sep 2024 20:07:21 +0100 Subject: [PATCH] Only the KV-defined set of UVM roots of trust should be used to accept joining nodes (#6489) --- .snpcc_canary | 1 + CHANGELOG.md | 3 ++- include/ccf/http_status.h | 5 +++++ src/node/node_state.h | 5 +++-- src/node/quote.cpp | 44 +++++++++++++++++++++------------------ tests/code_update.py | 44 +++++++++++++++++++++++++++++++++------ tests/infra/network.py | 6 ++++++ 7 files changed, 79 insertions(+), 29 deletions(-) diff --git a/.snpcc_canary b/.snpcc_canary index 08708bcac61a..e9eb0ba33a62 100644 --- a/.snpcc_canary +++ b/.snpcc_canary @@ -4,3 +4,4 @@ /-xXx--//-----x=x--/-xXx--/---x---->>>--/ ... /\/\d(-_-)b/\/\ +-- \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index ee64883cbd64..eeab73b6ac8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - The `set_jwt_issuer` governance action has been updated, and no longer accepts `key_filter` or `key_policy` arguments (#6450). -- Nodes started in `Join` mode will shut down if they receive and unrecoverable condition such as `StartupSeqnoIsOld` when attempting to join (#6471). +- Nodes started in `Join` mode will shut down if they receive an unrecoverable condition such as `StartupSeqnoIsOld` or `InvalidQuote` when attempting to join (#6471, #6489). - In configuration, `attestation.snp_endorsements_servers` can specify a `max_retries_count`. If the count has been exhausted without success for all configured servers, the node will shut down (#6478). +- When deciding which nodes are allowed to join, only UVM roots of trust defined in `public:ccf.gov.nodes.snp.uvm_endorsements` are considered (#6489). ### Removed diff --git a/include/ccf/http_status.h b/include/ccf/http_status.h index 4fa3d3a4a780..10006494c722 100644 --- a/include/ccf/http_status.h +++ b/include/ccf/http_status.h @@ -10,4 +10,9 @@ using http_status = llhttp_status; static inline const char* http_status_str(http_status s) { return llhttp_status_name(s); +} + +static inline bool is_http_status_client_error(http_status s) +{ + return s >= 400 && s < 500; } \ No newline at end of file diff --git a/src/node/node_state.h b/src/node/node_state.h index 5671f313a0e3..02ea36454e9d 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -595,12 +595,13 @@ namespace ccf return; } - if (status == HTTP_STATUS_BAD_REQUEST) + if (is_http_status_client_error(status)) { auto error_msg = fmt::format( - "Join request to {} returned 400 Bad Request: {}. Shutting " + "Join request to {} returned {} Bad Request: {}. Shutting " "down node gracefully.", config.join.target_rpc_address, + status, std::string(data.begin(), data.end())); LOG_FAIL_FMT("{}", error_msg); RINGBUFFER_WRITE_MESSAGE( diff --git a/src/node/quote.cpp b/src/node/quote.cpp index f475576ddcee..68afc95f733c 100644 --- a/src/node/quote.cpp +++ b/src/node/quote.cpp @@ -16,31 +16,35 @@ namespace ccf const pal::PlatformAttestationMeasurement& quote_measurement, const std::vector& uvm_endorsements) { - auto uvm_endorsements_data = - verify_uvm_endorsements(uvm_endorsements, quote_measurement); + // Uses KV-defined roots of trust (did -> (feed, svn)) to verify the + // UVM measurement against endorsements in the quote. + std::vector uvm_roots_of_trust_from_kv; auto uvmes = tx.ro(Tables::NODE_SNP_UVM_ENDORSEMENTS); - if (uvmes == nullptr) + if (uvmes) { - // No recorded trusted UVM endorsements - return false; + uvmes->foreach( + [&uvm_roots_of_trust_from_kv]( + const DID& did, const FeedToEndorsementsDataMap& endorsements_map) { + for (const auto& [feed, data] : endorsements_map) + { + uvm_roots_of_trust_from_kv.push_back( + UVMEndorsements{did, feed, data.svn}); + } + return true; + }); } - bool match = false; - uvmes->foreach([&match, &uvm_endorsements_data]( - const DID& did, const FeedToEndorsementsDataMap& value) { - if (uvm_endorsements_data.did == did) - { - auto search = value.find(uvm_endorsements_data.feed); - if (search != value.end()) - { - match = true; - return false; - } - } + try + { + auto uvm_endorsements_data = verify_uvm_endorsements( + uvm_endorsements, quote_measurement, uvm_roots_of_trust_from_kv); return true; - }); - - return match; + } + catch (const std::logic_error& e) + { + LOG_FAIL_FMT("Failed to verify UVM endorsements: {}", e.what()); + return false; + } } QuoteVerificationResult verify_enclave_measurement_against_store( diff --git a/tests/code_update.py b/tests/code_update.py index 3a40f29957eb..8c8186b27e81 100644 --- a/tests/code_update.py +++ b/tests/code_update.py @@ -52,6 +52,13 @@ def test_verify_quotes(network, args): return network +def get_trusted_uvm_endorsements(node): + with node.api_versioned_client(api_version=args.gov_api_version) as client: + r = client.get("/gov/service/join-policy") + assert r.status_code == http.HTTPStatus.OK, r + return r.body.json()["snp"]["uvmEndorsements"] + + @reqs.description("Test the SNP measurements table") @reqs.snp_only() def test_snp_measurements_tables(network, args): @@ -88,12 +95,6 @@ def get_trusted_measurements(node): LOG.info("SNP UVM endorsement table") - def get_trusted_uvm_endorsements(node): - with node.api_versioned_client(api_version=args.gov_api_version) as client: - r = client.get("/gov/service/join-policy") - assert r.status_code == http.HTTPStatus.OK, r - return r.body.json()["snp"]["uvmEndorsements"] - uvm_endorsements = get_trusted_uvm_endorsements(primary) assert ( len(uvm_endorsements) == 1 @@ -478,6 +479,34 @@ def test_proposal_invalidation(network, args): return network +@reqs.description( + "Node fails to join if KV contains no UVM endorsements roots of trust" +) +@reqs.snp_only() +def test_add_node_with_no_uvm_endorsements_in_kv(network, args): + LOG.info("Remove KV endorsements roots of trust (expect failure)") + primary, _ = network.find_nodes() + + uvm_endorsements = get_trusted_uvm_endorsements(primary) + assert ( + len(uvm_endorsements) == 1 + ), f"Expected one UVM endorsement, {uvm_endorsements}" + did, value = next(iter(uvm_endorsements.items())) + feed, data = next(iter(value.items())) + + network.consortium.remove_snp_uvm_endorsement(primary, did, feed) + + try: + new_node = network.create_node("local://localhost") + network.join_node(new_node, args.package, args, timeout=3) + except infra.network.UVMEndorsementsNotAuthorised: + LOG.info("As expected, node with no UVM endorsements failed to join") + else: + raise AssertionError("Node join unexpectedly succeeded") + + return network + + def run(args): with infra.network.network( args.nodes, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb @@ -503,6 +532,9 @@ def run(args): # Run again at the end to confirm current nodes are acceptable test_verify_quotes(network, args) + if snp.IS_SNP: + test_add_node_with_no_uvm_endorsements_in_kv(network, args) + if __name__ == "__main__": args = infra.e2e_args.cli_args() diff --git a/tests/infra/network.py b/tests/infra/network.py index b454435a50bf..81ba6f766d3f 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -73,6 +73,10 @@ class CodeIdNotFound(Exception): pass +class UVMEndorsementsNotAuthorised(Exception): + pass + + class StartupSeqnoIsOld(Exception): pass @@ -928,6 +932,8 @@ def run_join_node( for error in errors: if "Quote does not contain known enclave measurement" in error: raise CodeIdNotFound from e + if "UVM endorsements are not authorised" in error: + raise UVMEndorsementsNotAuthorised from e if "StartupSeqnoIsOld" in error: raise StartupSeqnoIsOld(has_stopped) from e if "invalid cert on handshake" in error: