diff --git a/CHANGELOG.md b/CHANGELOG.md index dcab163c99ce..ca612b099e4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,13 +10,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - `x-ccf-tx-view` and `x-ccf-tx-seqno` response headers have been removed, and replaced with `x-ms-ccf-transaction-id`. This includes both original fields, separated by a single `.`. Historical queries using `ccf::historical::adapter` should also pass a single combined `x-ms-ccf-transaction-id` header (#2257). -- Node unique identifier is the hex-encoded string of the SHA-256 digest of the node's identity public key, which is also used as the node's quote report data (#2241). +- Node unique identifier is the hex-encoded string of the SHA-256 digest of the node's DER-encoded identity public key, which is also used as the node's quote report data (#2241). +- Members and users unique identifier is the hex-encoded string of the SHA-256 digest of their DER-encoded identity certificate (i.e. fingerprint), which has to be specified as the `keyId` field for signed HTTP requests (#2279). - The receipt interface has changed, `/app/receipt?commit=23` is replaced by `/app/receipt?transaction_id=2.23`. Receipt fetching is now implemented as a historical query, which means that the first reponse(s) may be 202 with a Retry-After header. Receipts are now structured JSON, as opposed to a flat byte sequence, and `/app/receipt/verify` has been removed in favour of an [offline verification sample](https://microsoft.github.io/CCF/ccf-0.19.0/use_apps/verify_tx.html#transaction-receipts). - `ccfapp::get_rpc_handler()` now takes a reference to a `ccf::AbstractNodeContext` rather than `ccf::AbstractNodeState`. The node state can be obtained from the context via `get_node_state()`. ### Removed - `get_receipt_for_seqno_v1` has been removed. Handlers wanting to return receipts must now use the historical API, and can obtain a receipt via `ccf::historical::StatePtr`. See the [historical query with receipt sample](https://microsoft.github.io/CCF/ccf-0.19.0/build_apps/logging_cpp.html#receipts) for reference. +- `caller_id` endpoint. Members and users can now compute their unique identifier without interacting with CCF (#2279). +- `public:ccf.internal.members.certs_der`, `public:ccf.internal.users.certs_der`, `public:ccf.internal.members.digests` and `public:ccf.internal.users.digests` KV tables (#2279). ## [0.18.5] diff --git a/CMakeLists.txt b/CMakeLists.txt index 14f0d419e683..1f6a0d29f7a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -388,6 +388,12 @@ if(BUILD_TESTS) http_parser.host sss.host ) + set_tests_properties( + member_voting_test + PROPERTIES ENVIRONMENT + RUNTIME_CONFIG_DIR=${CMAKE_SOURCE_DIR}/src/runtime_config + ) + add_unit_test( proposal_id_test ${CMAKE_CURRENT_SOURCE_DIR}/src/node/rpc/test/proposal_id_test.cpp @@ -406,14 +412,6 @@ if(BUILD_TESTS) http_parser.host sss.host ) - if(NOT ENV{RUNTIME_CONFIG_DIR}) - set_tests_properties( - member_voting_test - PROPERTIES ENVIRONMENT - RUNTIME_CONFIG_DIR=${CMAKE_SOURCE_DIR}/src/runtime_config - ) - endif() - add_unit_test( lua_test ${CMAKE_CURRENT_SOURCE_DIR}/src/lua_interp/test/lua_test.cpp ${CMAKE_CURRENT_SOURCE_DIR}/src/lua_interp/test/lua_kv.cpp diff --git a/doc/governance/adding_member.rst b/doc/governance/adding_member.rst index dd286c1e9b53..cb3090871711 100644 --- a/doc/governance/adding_member.rst +++ b/doc/governance/adding_member.rst @@ -30,6 +30,13 @@ Members that are registered in CCF `with` a public encryption key are recovery m The member’s identity and encryption private keys (e.g. ``member_name_privk.pem`` and ``member_name_enc_privk.pem``) should be stored on a trusted device (e.g. HSM) while the certificate (e.g. ``member_name_cert.pem``) and public encryption key (e.g. ``member_name_enc_pubk.pem``) should be registered in CCF by members. +The CCF unique member identity is the hex-encoded string of the SHA-256 hash of the DER-encoded certificate, and can be computed from the certificate alone, without interacting with CCF: + +.. code-block:: bash + + $ identity_cert_path=/path/to/member/cert + $ openssl x509 -in "$identity_cert_path" -noout -fingerprint -sha256 | cut -d "=" -f 2 | sed 's/://g' | awk '{print tolower($0)}' + .. note:: See :ref:`overview/cryptography:Algorithms and Curves` for the list of supported cryptographic curves for member identity. Registering a New Member @@ -63,4 +70,4 @@ Then, the new member should sign the state digest returned by the ``/gov/ack/upd Once the command completes, the new member becomes active and can take part in governance operations (e.g. creating a new proposal or voting for an existing one). -.. note:: The newly-activated member is also given a recovery share that can be used :ref:`to recover a defunct service `. \ No newline at end of file +.. note:: The newly-activated member is also given a recovery share that can be used :ref:`to recover a defunct service `. diff --git a/doc/schemas/app_openapi.json b/doc/schemas/app_openapi.json index ad41fd72ac2f..f831beba874b 100644 --- a/doc/schemas/app_openapi.json +++ b/doc/schemas/app_openapi.json @@ -1,17 +1,6 @@ { "components": { "schemas": { - "CallerInfo": { - "properties": { - "caller_id": { - "$ref": "#/components/schemas/uint64" - } - }, - "required": [ - "caller_id" - ], - "type": "object" - }, "CodeStatus": { "enum": [ "ALLOWED_TO_JOIN" @@ -66,6 +55,10 @@ ], "type": "object" }, + "EntityId": { + "pattern": "^[a-f0-9]{64}$", + "type": "string" + }, "GetCode__Out": { "properties": { "versions": { @@ -136,7 +129,7 @@ "$ref": "#/components/schemas/string" }, "node_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "proof": { "$ref": "#/components/schemas/GetReceipt__Element_array" @@ -283,10 +276,6 @@ ], "type": "object" }, - "NodeId": { - "pattern": "^[a-f0-9]{64}$", - "type": "string" - }, "Report": { "properties": { "histogram": { @@ -391,40 +380,6 @@ } } }, - "/caller_id": { - "get": { - "parameters": [ - { - "in": "query", - "name": "cert", - "required": false, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CallerInfo" - } - } - }, - "description": "Default response description" - } - }, - "security": [ - { - "user_signature": [] - }, - { - "member_signature": [] - } - ] - } - }, "/code": { "get": { "responses": { diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index d89b1599fc08..a2e237cae8da 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -1,17 +1,6 @@ { "components": { "schemas": { - "CallerInfo": { - "properties": { - "caller_id": { - "$ref": "#/components/schemas/uint64" - } - }, - "required": [ - "caller_id" - ], - "type": "object" - }, "CodeStatus": { "enum": [ "ALLOWED_TO_JOIN" @@ -66,6 +55,28 @@ ], "type": "object" }, + "EntityId": { + "pattern": "^[a-f0-9]{64}$", + "type": "string" + }, + "EntityId_to_Script": { + "items": { + "items": { + "oneOf": [ + { + "$ref": "#/components/schemas/EntityId" + }, + { + "$ref": "#/components/schemas/Script" + } + ] + }, + "maxItems": 2, + "minItems": 2, + "type": "array" + }, + "type": "array" + }, "GetCode__Out": { "properties": { "versions": { @@ -136,7 +147,7 @@ "$ref": "#/components/schemas/string" }, "node_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "proof": { "$ref": "#/components/schemas/GetReceipt__Element_array" @@ -202,17 +213,13 @@ ], "type": "object" }, - "NodeId": { - "pattern": "^[a-f0-9]{64}$", - "type": "string" - }, "Proposal": { "properties": { "parameter": { "$ref": "#/components/schemas/json" }, "proposer": { - "$ref": "#/components/schemas/uint64" + "$ref": "#/components/schemas/EntityId" }, "script": { "$ref": "#/components/schemas/Script" @@ -221,7 +228,7 @@ "$ref": "#/components/schemas/ProposalState" }, "votes": { - "$ref": "#/components/schemas/uint64_to_Script" + "$ref": "#/components/schemas/EntityId_to_Script" } }, "required": [ @@ -239,7 +246,7 @@ "$ref": "#/components/schemas/string" }, "proposer_id": { - "$ref": "#/components/schemas/uint64" + "$ref": "#/components/schemas/EntityId" }, "state": { "$ref": "#/components/schemas/ProposalState" @@ -354,24 +361,6 @@ "minimum": 0, "type": "integer" }, - "uint64_to_Script": { - "items": { - "items": { - "oneOf": [ - { - "$ref": "#/components/schemas/uint64" - }, - { - "$ref": "#/components/schemas/Script" - } - ] - }, - "maxItems": 2, - "minItems": 2, - "type": "array" - }, - "type": "array" - }, "uint8": { "maximum": 255, "minimum": 0, @@ -389,11 +378,6 @@ "description": "Request must be signed according to the HTTP Signature scheme. The signer must be a member identity registered with this service.", "scheme": "signature", "type": "http" - }, - "user_signature": { - "description": "Request must be signed according to the HTTP Signature scheme. The signer must be a user identity registered with this service.", - "scheme": "signature", - "type": "http" } } }, @@ -481,40 +465,6 @@ } } }, - "/caller_id": { - "get": { - "parameters": [ - { - "in": "query", - "name": "cert", - "required": false, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CallerInfo" - } - } - }, - "description": "Default response description" - } - }, - "security": [ - { - "user_signature": [] - }, - { - "member_signature": [] - } - ] - } - }, "/code": { "get": { "responses": { diff --git a/doc/schemas/node_openapi.json b/doc/schemas/node_openapi.json index 5b5b362491a8..7077603f3cff 100644 --- a/doc/schemas/node_openapi.json +++ b/doc/schemas/node_openapi.json @@ -1,17 +1,6 @@ { "components": { "schemas": { - "CallerInfo": { - "properties": { - "caller_id": { - "$ref": "#/components/schemas/uint64" - } - }, - "required": [ - "caller_id" - ], - "type": "object" - }, "CodeStatus": { "enum": [ "ALLOWED_TO_JOIN" @@ -66,6 +55,10 @@ ], "type": "object" }, + "EntityId": { + "pattern": "^[a-f0-9]{64}$", + "type": "string" + }, "GetCode__Out": { "properties": { "versions": { @@ -119,7 +112,7 @@ "$ref": "#/components/schemas/int64" }, "primary_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "service_status": { "$ref": "#/components/schemas/ServiceStatus" @@ -144,7 +137,7 @@ "$ref": "#/components/schemas/string" }, "node_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "port": { "$ref": "#/components/schemas/string" @@ -218,7 +211,7 @@ "$ref": "#/components/schemas/string" }, "node_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "proof": { "$ref": "#/components/schemas/GetReceipt__Element_array" @@ -248,7 +241,7 @@ "$ref": "#/components/schemas/int64" }, "node_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "recovery_target_seqno": { "$ref": "#/components/schemas/int64" @@ -302,10 +295,6 @@ ], "type": "object" }, - "NodeId": { - "pattern": "^[a-f0-9]{64}$", - "type": "string" - }, "NodeStatus": { "enum": [ "PENDING", @@ -326,7 +315,7 @@ "$ref": "#/components/schemas/string" }, "node_id": { - "$ref": "#/components/schemas/NodeId" + "$ref": "#/components/schemas/EntityId" }, "raw": { "$ref": "#/components/schemas/string" @@ -400,18 +389,6 @@ "minimum": 0, "type": "integer" } - }, - "securitySchemes": { - "member_signature": { - "description": "Request must be signed according to the HTTP Signature scheme. The signer must be a member identity registered with this service.", - "scheme": "signature", - "type": "http" - }, - "user_signature": { - "description": "Request must be signed according to the HTTP Signature scheme. The signer must be a user identity registered with this service.", - "scheme": "signature", - "type": "http" - } } }, "info": { @@ -453,40 +430,6 @@ } } }, - "/caller_id": { - "get": { - "parameters": [ - { - "in": "query", - "name": "cert", - "required": false, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/CallerInfo" - } - } - }, - "description": "Default response description" - } - }, - "security": [ - { - "user_signature": [] - }, - { - "member_signature": [] - } - ] - } - }, "/code": { "get": { "responses": { diff --git a/doc/use_apps/issue_commands.rst b/doc/use_apps/issue_commands.rst index 95932bc1dba1..78e97d786301 100644 --- a/doc/use_apps/issue_commands.rst +++ b/doc/use_apps/issue_commands.rst @@ -36,7 +36,7 @@ In some situations CCF requires signed requests, for example for member votes. The signing scheme is compatible with the `IETF HTTP Signatures draft RFC `_. We provide a wrapper script (``scurl.sh``) around ``curl`` to submit signed requests from the command line. -CCF identifies the signing identity for a request via the SHA-256 digest of its certificate, represented as a hex string. +CCF identifies the signing identity for a request via the SHA-256 digest of its certificate (DER encoded), represented as a hex string. That value must be set in the ``keyId`` field of the ``Authorization`` HTTP header for a signed request. These commands can also be signed and transmitted by external libraries. diff --git a/python/ccf/clients.py b/python/ccf/clients.py index a7f6a52511ca..f90c9a2e66be 100644 --- a/python/ccf/clients.py +++ b/python/ccf/clients.py @@ -13,10 +13,10 @@ from io import BytesIO from cryptography import x509 from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import hashes import struct import base64 import re -import hashlib from typing import Union, Optional, List, Any import requests @@ -395,7 +395,13 @@ def __init__( self.session.cert = (self.session_auth.cert, self.session_auth.key) if self.signing_auth: with open(self.signing_auth.cert) as cert_file: - self.key_id = hashlib.sha256(cert_file.read().encode()).hexdigest() + self.key_id = ( + x509.load_pem_x509_certificate( + cert_file.read().encode(), default_backend() + ) + .fingerprint(hashes.SHA256()) + .hex() + ) def request( self, diff --git a/python/ccf/proposal_generator.py b/python/ccf/proposal_generator.py index 11408b79f29c..6aa4aef4aa9f 100644 --- a/python/ccf/proposal_generator.py +++ b/python/ccf/proposal_generator.py @@ -261,12 +261,12 @@ def new_member( @cli_proposal -def retire_member(member_id: int, **kwargs): +def retire_member(member_id: str, **kwargs): return build_proposal("retire_member", member_id, **kwargs) @cli_proposal -def set_member_data(member_id: int, member_data: Any, **kwargs): +def set_member_data(member_id: str, member_data: Any, **kwargs): proposal_args = {"member_id": member_id, "member_data": member_data} return build_proposal("set_member_data", proposal_args, **kwargs) @@ -280,12 +280,12 @@ def new_user(user_cert_path: str, user_data: Any = None, **kwargs): @cli_proposal -def remove_user(user_id: int, **kwargs): +def remove_user(user_id: str, **kwargs): return build_proposal("remove_user", user_id, **kwargs) @cli_proposal -def set_user_data(user_id: int, user_data: Any, **kwargs): +def set_user_data(user_id: str, user_data: Any, **kwargs): proposal_args = {"user_id": user_id, "user_data": user_data} return build_proposal("set_user_data", proposal_args, **kwargs) diff --git a/python/utils/scurl.sh b/python/utils/scurl.sh index 2c12e5074dda..7643359ac95d 100755 --- a/python/utils/scurl.sh +++ b/python/utils/scurl.sh @@ -45,7 +45,7 @@ for item in "$@" ; do fi if [ "$next_is_signing_cert" == true ]; then signing_cert=$item - next_is_signing_privk=false + next_is_signing_cert=false continue fi if [ "$item" == "--url" ]; then @@ -119,8 +119,8 @@ content-length: $content_length" # https://tools.ietf.org/html/draft-cavage-http-signatures-12#appendix-E.2 signature_algorithm="hs2019" -# Compute key ID -key_id=$(openssl dgst -sha256 "$signing_cert" | cut -d ' ' -f 2) +# Compute key ID, as the SHA-256 fingerprint of the signing certificate +key_id=$(openssl x509 -in "$signing_cert" -noout -fingerprint -sha256 | cut -d "=" -f 2 | sed 's/://g' | awk '{print tolower($0)}') if [ "$is_print_digest_to_sign" == true ]; then hash_to_sign=$(echo -n "$string_to_sign" | openssl dgst -binary -sha384 | openssl base64 -A) diff --git a/src/apps/js_generic/js_generic.cpp b/src/apps/js_generic/js_generic.cpp index eebd98c7b820..83fbdd434fed 100644 --- a/src/apps/js_generic/js_generic.cpp +++ b/src/apps/js_generic/js_generic.cpp @@ -889,7 +889,7 @@ namespace ccfapp } char const* policy_name = nullptr; - size_t id = ccf::INVALID_ID; + CallerId id; nlohmann::json data; std::string cert_s; @@ -936,7 +936,8 @@ namespace ccfapp } JS_SetPropertyStr(ctx, caller, "policy", JS_NewString(ctx, policy_name)); - JS_SetPropertyStr(ctx, caller, "id", JS_NewUint32(ctx, id)); + JS_SetPropertyStr( + ctx, caller, "id", JS_NewStringLen(ctx, id.data(), id.size())); JS_SetPropertyStr(ctx, caller, "data", create_json_obj(data, ctx)); JS_SetPropertyStr( ctx, diff --git a/src/consensus/aft/raft.h b/src/consensus/aft/raft.h index 6ab8d04d5a68..95426542b5b5 100644 --- a/src/consensus/aft/raft.h +++ b/src/consensus/aft/raft.h @@ -486,7 +486,7 @@ namespace aft LOG_DEBUG_FMT( "Replicated on leader {}: {}{} ({} hooks)", - state->my_node_id, + state->my_node_id.trim(), index, (globally_committable ? " committable" : ""), hooks->size()); @@ -519,7 +519,7 @@ namespace aft entry_size_not_limited = 0; for (const auto& it : nodes) { - LOG_DEBUG_FMT("Sending updates to follower {}", it.first); + LOG_DEBUG_FMT("Sending updates to follower {}", it.first.trim()); send_append_entries(it.first, it.second.sent_idx + 1); } } @@ -802,7 +802,8 @@ namespace aft ccf::ViewChangeRequest v = ccf::ViewChangeRequest::deserialize(data, size); - LOG_INFO_FMT("Received view change from:{}, view:{}", from, r.view); + LOG_INFO_FMT( + "Received view change from:{}, view:{}", from.trim(), r.view); auto progress_tracker = store->get_progress_tracker(); if (!progress_tracker->apply_view_change_message( @@ -987,8 +988,8 @@ namespace aft LOG_DEBUG_FMT( "Send append entries from {} to {}: {} to {} ({})", - state->my_node_id, - to, + state->my_node_id.trim(), + to.trim(), start_idx, end_idx, state->commit_idx); @@ -1055,7 +1056,7 @@ namespace aft r.prev_idx, r.term_of_idx, r.idx, - from, + from.trim(), r.term); // Don't check that the sender node ID is valid. Accept anything that @@ -1202,8 +1203,8 @@ namespace aft LOG_DEBUG_FMT( "Recv append entries to {} from {} for index {} and previous index {}", - state->my_node_id, - from, + state->my_node_id.trim(), + from.trim(), r.idx, r.prev_idx); @@ -1240,7 +1241,8 @@ namespace aft continue; } - LOG_DEBUG_FMT("Replicating on follower {}: {}", state->my_node_id, i); + LOG_DEBUG_FMT( + "Replicating on follower {}: {}", state->my_node_id.trim(), i); std::vector entry; try @@ -1262,7 +1264,11 @@ namespace aft auto ds = store->apply(entry, consensus_type, public_only); if (ds == nullptr) { - LOG_DEBUG_FMT("failed to deserialize we failed to apply"); + LOG_FAIL_FMT( + "Recv append entries to {} from {} but the entry could not be " + "deserialised", + state->my_node_id, + from); send_append_entries_response(from, AppendEntriesResponseType::FAIL); return; } @@ -1505,8 +1511,8 @@ namespace aft { LOG_DEBUG_FMT( "Send append entries response from {} to {} for index {}: {}", - state->my_node_id, - to, + state->my_node_id.trim(), + to.trim(), state->last_idx, answer); @@ -1524,8 +1530,8 @@ namespace aft { LOG_DEBUG_FMT( "Send append entries signed response from {} to {} for index {}", - state->my_node_id, - to, + state->my_node_id.trim(), + to.trim(), state->last_idx); auto progress_tracker = store->get_progress_tracker(); @@ -1647,7 +1653,7 @@ namespace aft CCF_ASSERT(progress_tracker != nullptr, "progress_tracker is not set"); LOG_TRACE_FMT( "processing recv_signature_received_ack, from:{} view:{}, seqno:{}", - from, + from.trim(), r.term, r.idx); @@ -1712,7 +1718,7 @@ namespace aft CCF_ASSERT(progress_tracker != nullptr, "progress_tracker is not set"); LOG_TRACE_FMT( "processing nonce_reveal, from:{} view:{}, seqno:{}", - from, + from.trim(), r.term, r.idx); progress_tracker->add_nonce_reveal( @@ -1817,8 +1823,8 @@ namespace aft LOG_DEBUG_FMT( "Recv append entries response to {} from {} for index {}: success", - state->my_node_id, - from, + state->my_node_id.trim(), + from.trim(), r.last_log_idx); update_commit(); } @@ -1828,8 +1834,8 @@ namespace aft auto last_committable_idx = last_committable_index(); LOG_INFO_FMT( "Send request vote from {} to {} at {}", - state->my_node_id, - to, + state->my_node_id.trim(), + to.trim(), last_committable_idx); CCF_ASSERT(last_committable_idx >= state->commit_idx, "lci < ci"); @@ -1929,8 +1935,8 @@ namespace aft { LOG_INFO_FMT( "Send request vote response from {} to {}: {}", - state->my_node_id, - to, + state->my_node_id.trim(), + to.trim(), answer); RequestVoteResponse response = { @@ -2249,7 +2255,7 @@ namespace aft store->compact(idx); ledger->commit(idx); - LOG_DEBUG_FMT("Commit on {}: {}", state->my_node_id, idx); + LOG_DEBUG_FMT("Commit on {}: {}", state->my_node_id.trim(), idx); // Examine all configurations that are followed by a globally committed // configuration. diff --git a/src/consensus/aft/raft_types.h b/src/consensus/aft/raft_types.h index cbe1247b953d..74712675777f 100644 --- a/src/consensus/aft/raft_types.h +++ b/src/consensus/aft/raft_types.h @@ -9,7 +9,7 @@ #include "enclave/rpc_handler.h" #include "kv/kv_types.h" #include "mbedtls/ecdsa.h" -#include "node/node_id.h" +#include "node/entity_id.h" #include "node/progress_tracker.h" #include diff --git a/src/consensus/aft/revealed_nonces.h b/src/consensus/aft/revealed_nonces.h index 5a379fdfc54b..cb193dd0ba2f 100644 --- a/src/consensus/aft/revealed_nonces.h +++ b/src/consensus/aft/revealed_nonces.h @@ -4,7 +4,7 @@ #include "crypto/hash.h" #include "kv/map.h" -#include "node/node_id.h" +#include "node/entity_id.h" #include #include @@ -43,5 +43,7 @@ namespace aft }; DECLARE_JSON_TYPE(RevealedNonces); DECLARE_JSON_REQUIRED_FIELDS(RevealedNonces, tx_id, nonces) - using RevealedNoncesMap = kv::Map; + + // Always recorded at key 0 + using RevealedNoncesMap = kv::Map; } \ No newline at end of file diff --git a/src/crypto/verifier.cpp b/src/crypto/verifier.cpp index c3bbdf892a02..a9770df66869 100644 --- a/src/crypto/verifier.cpp +++ b/src/crypto/verifier.cpp @@ -49,6 +49,11 @@ namespace crypto return make_verifier(der)->cert_pem(); } + std::vector cert_pem_to_der(const crypto::Pem& pem) + { + return make_verifier(pem)->cert_der(); + } + std::vector public_key_der_from_cert(const std::vector& der) { return make_unique_verifier(der)->public_key_der(); diff --git a/src/crypto/verifier.h b/src/crypto/verifier.h index 684345e803e5..f31c500921cb 100644 --- a/src/crypto/verifier.h +++ b/src/crypto/verifier.h @@ -139,6 +139,7 @@ namespace crypto VerifierPtr make_verifier(const Pem& pem); crypto::Pem cert_der_to_pem(const std::vector& der); + std::vector cert_pem_to_der(const Pem& pem); std::vector public_key_der_from_cert( const std::vector& der); diff --git a/src/http/authentication/cert_auth.h b/src/http/authentication/cert_auth.h index b843ec31dd1b..50cbb4e34c9b 100644 --- a/src/http/authentication/cert_auth.h +++ b/src/http/authentication/cert_auth.h @@ -31,23 +31,15 @@ namespace ccf const std::shared_ptr& ctx, std::string& error_reason) override { - const auto caller_cert = ctx->session->caller_cert; + const auto& caller_cert = ctx->session->caller_cert; + auto caller_id = crypto::Sha256Hash(caller_cert).hex_str(); - auto users_by_cert = tx.ro(Tables::USER_CERT_DERS); - const auto user_id = users_by_cert->get(caller_cert); - - if (user_id.has_value()) + auto users = tx.ro(Tables::USERS); + const auto user = users->get(caller_id); + if (user.has_value()) { - Users users_table(Tables::USERS); - auto users = tx.ro(users_table); - const auto user = users->get(user_id.value()); - if (!user.has_value()) - { - throw std::logic_error("Users and user certs tables do not match"); - } - auto identity = std::make_unique(); - identity->user_id = user_id.value(); + identity->user_id = caller_id; identity->user_cert = user->cert; identity->user_data = user->user_data; return identity; @@ -87,24 +79,15 @@ namespace ccf const std::shared_ptr& ctx, std::string& error_reason) override { - const auto caller_cert = ctx->session->caller_cert; + const auto& caller_cert = ctx->session->caller_cert; + auto caller_id = crypto::Sha256Hash(caller_cert).hex_str(); - auto members_by_cert = tx.ro(Tables::MEMBER_CERT_DERS); - const auto member_id = members_by_cert->get(caller_cert); - - if (member_id.has_value()) + auto members = tx.ro(Tables::MEMBERS); + const auto member = members->get(caller_id); + if (member.has_value()) { - Members members_table(Tables::MEMBERS); - auto members = tx.ro(members_table); - const auto member = members->get(member_id.value()); - if (!member.has_value()) - { - throw std::logic_error( - "Members and member certs tables do not match"); - } - auto identity = std::make_unique(); - identity->member_id = member_id.value(); + identity->member_id = caller_id; identity->member_cert = member->cert; identity->member_data = member->member_data; return identity; diff --git a/src/http/authentication/sig_auth.h b/src/http/authentication/sig_auth.h index f6dfad19d5dc..0598974376d0 100644 --- a/src/http/authentication/sig_auth.h +++ b/src/http/authentication/sig_auth.h @@ -78,25 +78,17 @@ namespace ccf const auto signed_request = parse_signed_request(ctx); if (signed_request.has_value()) { - auto digests = tx.ro(Tables::USER_DIGESTS); - auto user_id = digests->get(signed_request->key_id); - - if (user_id.has_value()) + Users users_table(Tables::USERS); + auto users = tx.ro(users_table); + auto user = users->get(signed_request->key_id); + if (user.has_value()) { - Users users_table(Tables::USERS); - auto users = tx.ro(users_table); - const auto user = users->get(user_id.value()); - if (!user.has_value()) - { - throw std::logic_error("Users and user certs tables do not match"); - } - auto verifier = verifiers.get_verifier(user->cert); if (verifier->verify( signed_request->req, signed_request->sig, signed_request->md)) { auto identity = std::make_unique(); - identity->user_id = user_id.value(); + identity->user_id = signed_request->key_id; identity->user_cert = user->cert; identity->user_data = user->user_data; identity->signed_request = signed_request.value(); @@ -179,20 +171,11 @@ namespace ccf const auto signed_request = parse_signed_request(ctx); if (signed_request.has_value()) { - auto digests = tx.ro(Tables::MEMBER_DIGESTS); - auto member_id = digests->get(signed_request->key_id); - - if (member_id.has_value()) + Members members_table(Tables::MEMBERS); + auto members = tx.ro(members_table); + auto member = members->get(signed_request->key_id); + if (member.has_value()) { - Members members_table(Tables::MEMBERS); - auto members = tx.ro(members_table); - const auto member = members->get(member_id.value()); - if (!member.has_value()) - { - throw std::logic_error( - "Members and member certs tables do not match"); - } - std::vector digest; auto verifier = verifiers.get_verifier(member->cert); if (verifier->verify( @@ -202,7 +185,7 @@ namespace ccf digest)) { auto identity = std::make_unique(); - identity->member_id = member_id.value(); + identity->member_id = signed_request->key_id; identity->member_cert = member->cert; identity->member_data = member->member_data; identity->signed_request = signed_request.value(); diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index c0dd6415610d..5dc9cd740703 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -6,7 +6,7 @@ #include "crypto/pem.h" #include "ds/nonstd.h" #include "enclave/consensus_type.h" -#include "node/node_id.h" +#include "node/entity_id.h" #include "serialiser_declare.h" #include diff --git a/src/lua_interp/lua_util.h b/src/lua_interp/lua_util.h index ae1a146f9a97..4447ee29e26e 100644 --- a/src/lua_interp/lua_util.h +++ b/src/lua_interp/lua_util.h @@ -3,11 +3,14 @@ #pragma once #define FMT_HEADER_ONLY +#include "node/entity_id.h" + #include #include #include #include #include + extern "C" { #include "../../3rdparty/lua/lauxlib.h" @@ -96,6 +99,11 @@ namespace ccf lua_pushnil(l); } + inline void push_raw(lua_State* l, const ccf::EntityId& entity) + { + lua_pushstring(l, entity.value().c_str()); + } + /** The base push case. Specialize this to push other types onto the lua * stack. */ diff --git a/src/node/backup_signatures.h b/src/node/backup_signatures.h index 5ce2821be728..dc085d5db4a7 100644 --- a/src/node/backup_signatures.h +++ b/src/node/backup_signatures.h @@ -33,5 +33,7 @@ namespace ccf }; DECLARE_JSON_TYPE(BackupSignatures); DECLARE_JSON_REQUIRED_FIELDS(BackupSignatures, view, seqno, root, signatures) - using BackupSignaturesMap = kv::Map; + + // Always recorded at key 0 + using BackupSignaturesMap = kv::Map; } \ No newline at end of file diff --git a/src/node/certs.h b/src/node/certs.h index 5a0d8caf2cbb..c4a6ed55e77c 100644 --- a/src/node/certs.h +++ b/src/node/certs.h @@ -1,12 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the Apache 2.0 License. #pragma once + #include "entities.h" namespace ccf { - using CertDERs = kv::Map; + using CertDERs = kv::Map; using CACertBundlePEMs = kv::Map; - // Mapping from hex-encoded digest of PEM cert to entity id - using CertDigests = kv::Map; } diff --git a/src/node/channels.h b/src/node/channels.h index ae1b5710ae08..8fd8101ae2d5 100644 --- a/src/node/channels.h +++ b/src/node/channels.h @@ -7,7 +7,7 @@ #include "ds/logger.h" #include "ds/spin_lock.h" #include "entities.h" -#include "node/node_id.h" +#include "node/entity_id.h" #include "node_types.h" #include "tls/key_exchange.h" diff --git a/src/node/client_signatures.h b/src/node/client_signatures.h index eef9b29f19c6..da54aefcea8e 100644 --- a/src/node/client_signatures.h +++ b/src/node/client_signatures.h @@ -54,6 +54,4 @@ namespace ccf }; DECLARE_JSON_TYPE(SignedReq) DECLARE_JSON_REQUIRED_FIELDS(SignedReq, sig, req, request_body, md, key_id) - // this maps client-id to latest SignedReq - using ClientSignatures = kv::Map; } \ No newline at end of file diff --git a/src/node/entities.h b/src/node/entities.h index ec85bd155d3c..f67ad21f8c56 100644 --- a/src/node/entities.h +++ b/src/node/entities.h @@ -2,23 +2,19 @@ // Licensed under the Apache 2.0 License. #pragma once +#include "entity_id.h" + #include #include #include +#include #include namespace ccf { - using ObjectId = uint64_t; - - constexpr ObjectId INVALID_ID = (std::numeric_limits::max)(); - using Index = int64_t; using Node2NodeMsg = uint64_t; - using MemberId = ObjectId; - using UserId = ObjectId; - using CallerId = ObjectId; using Cert = std::vector; // SGX MRENCLAVE is SHA256 digest @@ -64,16 +60,9 @@ namespace ccf // Member identities static constexpr auto MEMBERS = "public:ccf.gov.members.info"; static constexpr auto MEMBER_ACKS = "public:ccf.gov.members.acks"; - static constexpr auto MEMBER_CERT_DERS = - "public:ccf.internal.members.certs_der"; - static constexpr auto MEMBER_DIGESTS = - "public:ccf.internal.members.digests"; // User identities static constexpr auto USERS = "public:ccf.gov.users.info"; - static constexpr auto USER_CERT_DERS = - "public:ccf.internal.users.certs_der"; - static constexpr auto USER_DIGESTS = "public:ccf.internal.users.digests"; // Nodes identities and allowed code ids static constexpr auto NODES = "public:ccf.gov.nodes.info"; diff --git a/src/node/entity_id.h b/src/node/entity_id.h new file mode 100644 index 000000000000..b84be3b17e87 --- /dev/null +++ b/src/node/entity_id.h @@ -0,0 +1,152 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the Apache 2.0 License. +#pragma once + +#include "ds/json.h" + +#include +#include + +namespace ccf +{ + struct EntityId + { + public: + // The underlying value type should be blit-serialisable so that it can be + // written to and read from the ring buffer + static constexpr size_t LENGTH = 64; // hex-encoded SHA-256 hash + using Value = std::string; // < hex-encoded hash + + private: + Value id; + + public: + EntityId() = default; + EntityId(const Value& id_) : id(id_) {} + + void operator=(const EntityId& other) + { + id = other.id; + } + + void operator=(const Value& id_) + { + id = id_; + } + + bool operator==(const EntityId& other) const + { + return id == other.id; + } + + bool operator!=(const EntityId& other) const + { + return !(*this == other); + } + + bool operator<(const EntityId& other) const + { + return id < other.id; + } + + std::string trim() const + { + // Some entities (typically NodeIds) are printed in many places when + // VERBOSE_LOGGING is activated so trim them explicitly whenever possible + // in this case. Otherwise, return the entire value. +#ifdef VERBOSE_LOGGING + static constexpr size_t entity_id_truncation_max_char_count = 10; + return id.substr( + 0, std::min(size(), entity_id_truncation_max_char_count)); +#else + return id; +#endif + } + + Value& value() + { + return id; + } + + const Value& value() const + { + return id; + } + + char const* data() const + { + return id.data(); + } + + size_t size() const + { + return id.size(); + } + + MSGPACK_DEFINE(id); + }; + + inline void to_json(nlohmann::json& j, const EntityId& entity_id) + { + j = entity_id.value(); + } + + inline void from_json(const nlohmann::json& j, EntityId& entity_id) + { + if (j.is_string()) + { + entity_id = j.get(); + } + else + { + throw JsonParseError( + fmt::format("Entity id should be hex-encoded string: {}", j.dump())); + } + } + + inline std::string schema_name(const EntityId&) + { + return "EntityId"; + } + + inline void fill_json_schema(nlohmann::json& schema, const EntityId&) + { + schema["type"] = "string"; + + // According to the spec, "format is an open value, so you can use any + // formats, even not those defined by the OpenAPI Specification" + // https://swagger.io/docs/specification/data-models/data-types/#format + schema["format"] = "hex"; + } + + template + void add_schema_components(T&, nlohmann::json& j, const EntityId&) + { + j["type"] = "string"; + j["pattern"] = fmt::format("^[a-f0-9]{{{}}}$", EntityId::LENGTH); + } + + using CallerId = EntityId; + using MemberId = EntityId; + using UserId = EntityId; + using NodeId = EntityId; +} + +namespace std +{ + static inline std::ostream& operator<<( + std::ostream& os, const ccf::EntityId& entity_id) + { + os << entity_id.value(); + return os; + } + + template <> + struct hash + { + size_t operator()(const ccf::EntityId& entity_id) const + { + return std::hash{}(entity_id.value()); + } + }; +} \ No newline at end of file diff --git a/src/node/genesis_gen.h b/src/node/genesis_gen.h index ee157298afca..03c6b4d3319d 100644 --- a/src/node/genesis_gen.h +++ b/src/node/genesis_gen.h @@ -102,30 +102,20 @@ namespace ccf MemberId add_member(const MemberPubInfo& member_pub_info) { auto m = tx.rw(tables.members); - auto mc = tx.rw(tables.member_certs); - auto md = tx.rw(tables.member_digests); - auto v = tx.rw(tables.values); auto ma = tx.rw(tables.member_acks); - auto sig = tx.rw(tables.signatures); + auto sig = tx.ro(tables.signatures); - // The key to a CertDERs table must be a DER, for easy comparison against - // the DER peer cert retrieved from the connection auto member_cert_der = crypto::make_verifier(member_pub_info.cert)->cert_der(); + auto id = crypto::Sha256Hash(member_cert_der).hex_str(); - auto member_id = mc->get(member_cert_der); - if (member_id.has_value()) + auto member = m->get(id); + if (member.has_value()) { - throw std::logic_error(fmt::format( - "Member certificate already exists (member {})", member_id.value())); + throw std::logic_error(fmt::format("Member already exists: {}", id)); } - const auto id = get_next_id(v, ValueIds::NEXT_MEMBER_ID); m->put(id, MemberInfo(member_pub_info, MemberStatus::ACCEPTED)); - mc->put(member_cert_der, id); - - crypto::Sha256Hash member_cert_digest(member_pub_info.cert.contents()); - md->put(member_cert_digest.hex_str(), id); auto s = sig->get(0); if (!s) @@ -139,7 +129,7 @@ namespace ccf return id; } - void activate_member(MemberId member_id) + void activate_member(const MemberId& member_id) { auto members = tx.rw(tables.members); auto member = members->get(member_id); @@ -224,48 +214,34 @@ namespace ccf return member.value(); } - auto add_user(const ccf::UserInfo& user_info) + UserId add_user(const ccf::UserInfo& user_info) { auto u = tx.rw(tables.users); - auto uc = tx.rw(tables.user_certs); - auto ud = tx.rw(tables.user_digests); - auto v = tx.rw(tables.values); auto user_cert_der = crypto::make_verifier(user_info.cert)->cert_der(); + auto id = crypto::Sha256Hash(user_cert_der).hex_str(); // Cert should be unique - auto user_id = uc->get(user_cert_der); - if (user_id.has_value()) + auto user = u->get(id); + if (user.has_value()) { - throw std::logic_error(fmt::format( - "User certificate already exists (user {})", user_id.value())); + throw std::logic_error( + fmt::format("User certificate already exists: {}", id)); } - const auto id = get_next_id(v, ValueIds::NEXT_USER_ID); u->put(id, user_info); - uc->put(user_cert_der, id); - - crypto::Sha256Hash user_cert_digest(user_info.cert.contents()); - ud->put(user_cert_digest.hex_str(), id); return id; } - bool remove_user(UserId user_id) + bool remove_user(const UserId& user_id) { auto u = tx.rw(tables.users); - auto uc = tx.rw(tables.user_certs); - auto user_info = u->get(user_id); if (!user_info.has_value()) { return false; } - - auto pem = crypto::Pem(user_info.value().cert); - auto user_cert_der = crypto::make_verifier(pem)->cert_der(); - u->remove(user_id); - uc->remove(user_cert_der); return true; } diff --git a/src/node/jwt_key_auto_refresh.h b/src/node/jwt_key_auto_refresh.h index 39c0a16cce64..a8666fee2e2b 100644 --- a/src/node/jwt_key_auto_refresh.h +++ b/src/node/jwt_key_auto_refresh.h @@ -34,7 +34,7 @@ namespace ccf const std::shared_ptr& rpcsessions, const std::shared_ptr& rpc_map, const crypto::KeyPairPtr& node_sign_kp, - crypto::Pem node_cert) : + const crypto::Pem& node_cert) : refresh_interval_s(refresh_interval_s), network(network), consensus(consensus), @@ -115,10 +115,8 @@ namespace ccf http::headers::CONTENT_TYPE, http::headervalues::contenttype::JSON); request.set_body(&body); - const auto contents = node_cert.contents(); - crypto::Sha256Hash hash({contents.data(), contents.size()}); - const std::string key_id = fmt::format("{:02x}", fmt::join(hash.h, "")); - + auto node_cert_der = crypto::cert_pem_to_der(node_cert); + const auto key_id = crypto::Sha256Hash(node_cert_der).hex_str(); http::sign_request(request, node_sign_kp, key_id); auto packed = request.build_request(); diff --git a/src/node/members.h b/src/node/members.h index ea3b397988eb..10b505daa990 100644 --- a/src/node/members.h +++ b/src/node/members.h @@ -4,7 +4,7 @@ #include "client_signatures.h" #include "crypto/pem.h" #include "ds/hash.h" -#include "entities.h" +#include "entity_id.h" #include "node_signature.h" #include diff --git a/src/node/network_tables.h b/src/node/network_tables.h index 5fb61c615432..e64dadb95c1d 100644 --- a/src/node/network_tables.h +++ b/src/node/network_tables.h @@ -57,10 +57,7 @@ namespace ccf // // Governance tables // - // members, member_certs and member_digests tables should always be in sync Members members; - CertDERs member_certs; - CertDigests member_digests; Scripts gov_scripts; Modules modules; @@ -83,10 +80,7 @@ namespace ccf // // User tables // - // users, user_certs and user_digests tables should always be in sync Users users; - CertDERs user_certs; - CertDigests user_digests; ServicePrincipals service_principals; @@ -120,8 +114,6 @@ namespace ccf NetworkTables(const ConsensusType& consensus_type = ConsensusType::CFT) : tables(make_store(consensus_type)), members(Tables::MEMBERS), - member_certs(Tables::MEMBER_CERT_DERS), - member_digests(Tables::MEMBER_DIGESTS), gov_scripts(Tables::GOV_SCRIPTS), modules(Tables::MODULES), proposals(Tables::PROPOSALS), @@ -138,8 +130,6 @@ namespace ccf jwt_public_signing_keys(Tables::JWT_PUBLIC_SIGNING_KEYS), jwt_public_signing_key_issuer(Tables::JWT_PUBLIC_SIGNING_KEY_ISSUER), users(Tables::USERS), - user_certs(Tables::USER_CERT_DERS), - user_digests(Tables::USER_DIGESTS), service_principals(Tables::SERVICE_PRINCIPALS), nodes(Tables::NODES), app_scripts(Tables::APP_SCRIPTS), @@ -162,7 +152,6 @@ namespace ccf { return std::make_tuple( std::ref(members), - std::ref(member_certs), std::ref(gov_scripts), std::ref(modules), std::ref(proposals), @@ -176,7 +165,6 @@ namespace ccf std::ref(jwt_public_signing_keys), std::ref(jwt_public_signing_key_issuer), std::ref(users), - std::ref(user_certs), std::ref(service_principals), std::ref(nodes), std::ref(service), diff --git a/src/node/node_id.h b/src/node/node_id.h deleted file mode 100644 index 42a8c0202ec0..000000000000 --- a/src/node/node_id.h +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the Apache 2.0 License. -#pragma once - -#include "ds/json.h" - -#include -#include - -namespace ccf -{ - struct NodeId - { - // The underlying value type should be blit-serialisable so that it can be - // written to and read from the ring buffer - using Value = - std::string; // < hex-encoded hash of node's identity public key - Value id; - - NodeId() = default; - - NodeId(const Value& id_) : id(id_) {} - - void operator=(const NodeId& other) - { - id = other.id; - } - - void operator=(const Value& id_) - { - id = id_; - } - - bool operator==(const NodeId& other) const - { - return id == other.id; - } - - bool operator!=(const NodeId& other) const - { - return !(*this == other); - } - - bool operator<(const NodeId& other) const - { - return id < other.id; - } - - operator Value() const - { - return id; - } - - auto value() const - { - return id; - } - - size_t size() const - { - return id.size(); - } - - MSGPACK_DEFINE(id); - }; - - inline void to_json(nlohmann::json& j, const NodeId& node_id) - { - j = node_id.id; - } - - inline void from_json(const nlohmann::json& j, NodeId& node_id) - { - if (j.is_string()) - { - // We should check that the node ID is a valid hex-encoded sha-256 hash. - // However, the BFT variant of the consensus still uses monotic node IDs - // so this cannot be done just yet (see - // https://github.com/microsoft/CCF/issues/1852) - node_id = j.get(); - } - else - { - throw JsonParseError( - fmt::format("Unable to parse Node id from this JSON: {}", j.dump())); - } - } - - inline std::string schema_name(const NodeId&) - { - return "NodeId"; - } - - inline void fill_json_schema(nlohmann::json& schema, const NodeId&) - { - schema["type"] = "string"; - - // According to the spec, "format is an open value, so you can use any - // formats, even not those defined by the OpenAPI Specification" - // https://swagger.io/docs/specification/data-models/data-types/#format - schema["format"] = "hex"; - } - - template - void add_schema_components(T&, nlohmann::json& j, const NodeId&) - { - j["type"] = "string"; - j["pattern"] = "^[a-f0-9]{64}$"; - } -} - -namespace std -{ - static inline std::ostream& operator<<( - std::ostream& os, const ccf::NodeId& node_id) - { - os << node_id.id; - return os; - } - - template <> - struct hash - { - size_t operator()(const ccf::NodeId& node_id) const - { - return std::hash{}(node_id.id); - } - }; - -} - -// Only display the first node_id_truncation_max_char_count character when -// printing the node id to the node's log -static constexpr size_t node_id_truncation_max_char_count = 10; - -FMT_BEGIN_NAMESPACE -template <> -struct formatter -{ - template - auto parse(ParseContext& ctx) - { - return ctx.begin(); - } - - template - auto format(const ccf::NodeId& node_id, FormatContext& ctx) - -> decltype(ctx.out()) - { - return format_to( - ctx.out(), - "{}", - node_id.id.substr( - 0, std::min(node_id.id.size(), node_id_truncation_max_char_count))); - } -}; -FMT_END_NAMESPACE \ No newline at end of file diff --git a/src/node/node_state.h b/src/node/node_state.h index 312bb4c7589f..5d20eed04b48 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -368,7 +368,7 @@ namespace ccf // Pad node id string to avoid memory alignment issues on // node-to-node messages - self = fmt::format("{:#08}", 0); + self = NodeId(fmt::format("{:#08}", 0)); } setup_snapshotter(); @@ -397,7 +397,7 @@ namespace ccf reset_data(quote_info.endorsements); sm.advance(State::partOfNetwork); - LOG_INFO_FMT("Created new node {}", self.value()); + LOG_INFO_FMT("Created new node {}", self); return {node_cert, network.identity->cert}; } case StartType::Join: @@ -416,7 +416,7 @@ namespace ccf sm.advance(State::pending); } - LOG_INFO_FMT("Created join node {}", self.value()); + LOG_INFO_FMT("Created join node {}", self); return {node_cert, {}}; } case StartType::Recover: @@ -449,7 +449,7 @@ namespace ccf sm.advance(State::readingPublicLedger); - LOG_INFO_FMT("Created recovery node {}", self.value()); + LOG_INFO_FMT("Created recovery node {}", self); return {node_cert, network.identity->cert}; } default: @@ -831,7 +831,8 @@ namespace ccf if ( startup_snapshot_info && startup_snapshot_info->has_evidence && - last_sig->commit_seqno >= startup_snapshot_info->evidence_seqno) + static_cast(last_sig->commit_seqno) >= + startup_snapshot_info->evidence_seqno) { startup_snapshot_info->is_evidence_committed = true; } @@ -1497,9 +1498,8 @@ namespace ccf request.set_body(&body); - const auto contents = node_cert.contents(); - crypto::Sha256Hash hash({contents.data(), contents.size()}); - const std::string key_id = fmt::format("{:02x}", fmt::join(hash.h, "")); + auto node_cert_der = crypto::cert_pem_to_der(node_cert); + const auto key_id = crypto::Sha256Hash(node_cert_der).hex_str(); http::sign_request(request, node_sign_kp, key_id); diff --git a/src/node/nodes.h b/src/node/nodes.h index 94692b74817e..7a03098daaab 100644 --- a/src/node/nodes.h +++ b/src/node/nodes.h @@ -2,8 +2,8 @@ // Licensed under the Apache 2.0 License. #pragma once #include "entities.h" +#include "entity_id.h" #include "kv/map.h" -#include "node_id.h" #include "node_info_network.h" #include "quote_info.h" diff --git a/src/node/rpc/common_endpoint_registry.h b/src/node/rpc/common_endpoint_registry.h index 785dec2ba139..d32c49d8d3a9 100644 --- a/src/node/rpc/common_endpoint_registry.h +++ b/src/node/rpc/common_endpoint_registry.h @@ -57,16 +57,11 @@ namespace ccf */ class CommonEndpointRegistry : public BaseEndpointRegistry { - protected: - std::string certs_table_name; - public: CommonEndpointRegistry( const std::string& method_prefix_, - ccfapp::AbstractNodeContext& context_, - const std::string& certs_table_name_ = "") : - BaseEndpointRegistry(method_prefix_, context_), - certs_table_name(certs_table_name_) + ccfapp::AbstractNodeContext& context_) : + BaseEndpointRegistry(method_prefix_, context_) {} void init_handlers() override @@ -131,65 +126,6 @@ namespace ccf ccf::endpoints::ExecuteOutsideConsensus::Locally) .install(); - auto get_caller_id = [this](auto& args, nlohmann::json&& params) { - GetCallerId::Out out; - - if (!params.is_null()) - { - const GetCallerId::In in = params; - auto certs = args.tx.template ro(certs_table_name); - std::vector pem(in.cert.begin(), in.cert.end()); - std::vector der = crypto::make_verifier(pem)->cert_der(); - auto caller_id_opt = certs->get(der); - - if (!caller_id_opt.has_value()) - { - return make_error( - HTTP_STATUS_BAD_REQUEST, - ccf::errors::UnknownCertificate, - "Certificate not recognised."); - } - - out.caller_id = caller_id_opt.value(); - } - else if ( - auto user_cert_ident = - args.template try_get_caller()) - { - out.caller_id = user_cert_ident->user_id; - } - else if ( - auto member_cert_ident = - args.template try_get_caller()) - { - out.caller_id = member_cert_ident->member_id; - } - else if ( - auto user_sig_ident = - args.template try_get_caller()) - { - out.caller_id = user_cert_ident->user_id; - } - else if ( - auto member_sig_ident = - args.template try_get_caller()) - { - out.caller_id = member_cert_ident->member_id; - } - - return make_success(out); - }; - make_read_only_endpoint( - "caller_id", - HTTP_GET, - json_read_only_adapter(get_caller_id), - {user_cert_auth_policy, - user_signature_auth_policy, - member_cert_auth_policy, - member_signature_auth_policy}) - .set_auto_schema() - .install(); - auto get_code = [](auto& args, nlohmann::json&&) { GetCode::Out out; diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index a57842d16cdd..583799a00d8b 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -1042,7 +1042,7 @@ namespace ccf } // does the voter agree? - votes[std::to_string(vote.first)] = tsr.run( + votes[vote.first.value()] = tsr.run( tx, {vote.second, {}, // can't write @@ -1129,12 +1129,12 @@ namespace ccf return get_proposal_info(proposal_id, proposal); } - bool check_member_active(kv::ReadOnlyTx& tx, MemberId id) + bool check_member_active(kv::ReadOnlyTx& tx, const MemberId& id) { return check_member_status(tx, id, {MemberStatus::ACTIVE}); } - bool check_member_accepted(kv::ReadOnlyTx& tx, MemberId id) + bool check_member_accepted(kv::ReadOnlyTx& tx, const MemberId& id) { return check_member_status( tx, id, {MemberStatus::ACTIVE, MemberStatus::ACCEPTED}); @@ -1142,7 +1142,7 @@ namespace ccf bool check_member_status( kv::ReadOnlyTx& tx, - MemberId id, + const MemberId& id, std::initializer_list allowed) { auto member = tx.ro(this->network.members)->get(id); @@ -1161,7 +1161,7 @@ namespace ccf } void record_voting_history( - kv::Tx& tx, MemberId caller_id, const SignedReq& signed_request) + kv::Tx& tx, const MemberId& caller_id, const SignedReq& signed_request) { auto governance_history = tx.rw(network.governance_history); governance_history->put(caller_id, {signed_request}); @@ -1186,7 +1186,7 @@ namespace ccf MemberId& member_id, std::string& error) { - return get_path_param(params, "member_id", member_id, error); + return get_path_param(params, "member_id", member_id.value(), error); } NetworkState& network; @@ -1198,10 +1198,7 @@ namespace ccf NetworkState& network, ccfapp::AbstractNodeContext& context, ShareManager& share_manager) : - CommonEndpointRegistry( - get_actor_prefix(ActorsType::members), - context, - Tables::MEMBER_CERT_DERS), + CommonEndpointRegistry(get_actor_prefix(ActorsType::members), context), network(network), share_manager(share_manager), tsr(network) @@ -1212,7 +1209,8 @@ namespace ccf "public governance tables."; } - static MemberId get_caller_member_id(CommandEndpointContext& ctx) + static std::optional get_caller_member_id( + CommandEndpointContext& ctx) { if ( const auto* sig_ident = @@ -1228,7 +1226,7 @@ namespace ccf } LOG_FATAL_FMT("Request was not authenticated with a member auth policy"); - return INVALID_ID; + return std::nullopt; } void init_handlers() override @@ -1242,9 +1240,17 @@ namespace ccf auto read = [this](EndpointContext& ctx, nlohmann::json&& params) { const auto member_id = get_caller_member_id(ctx); + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Member is unknown."); + } + if (!check_member_status( ctx.tx, - member_id, + member_id.value(), {MemberStatus::ACTIVE, MemberStatus::ACCEPTED})) { return make_error( @@ -1285,7 +1291,14 @@ namespace ccf auto query = [this](EndpointContext& ctx, nlohmann::json&& params) { const auto member_id = get_caller_member_id(ctx); - if (!check_member_accepted(ctx.tx, member_id)) + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Member is unknown."); + } + if (!check_member_accepted(ctx.tx, member_id.value())) { return make_error( HTTP_STATUS_FORBIDDEN, @@ -1386,7 +1399,15 @@ namespace ccf auto get_proposal = [this](ReadOnlyEndpointContext& ctx, nlohmann::json&&) { const auto member_id = get_caller_member_id(ctx); - if (!check_member_active(ctx.tx, member_id)) + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Member is unknown."); + } + + if (!check_member_active(ctx.tx, member_id.value())) { return make_error( HTTP_STATUS_FORBIDDEN, @@ -1568,8 +1589,16 @@ namespace ccf .install(); auto get_vote = [this](ReadOnlyEndpointContext& ctx, nlohmann::json&&) { - const auto caller_member_id = get_caller_member_id(ctx); - if (!check_member_active(ctx.tx, caller_member_id)) + const auto member_id = get_caller_member_id(ctx); + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Member is unknown."); + } + + if (!check_member_active(ctx.tx, member_id.value())) { return make_error( HTTP_STATUS_FORBIDDEN, @@ -1719,22 +1748,31 @@ namespace ccf auto update_state_digest = [this](EndpointContext& ctx, nlohmann::json&&) { const auto member_id = get_caller_member_id(ctx); + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Caller is a not a valid member id"); + } + auto mas = ctx.tx.rw(this->network.member_acks); auto sig = ctx.tx.rw(this->network.signatures); - auto ma = mas->get(member_id); + auto ma = mas->get(member_id.value()); if (!ma) { return make_error( HTTP_STATUS_FORBIDDEN, ccf::errors::AuthorizationFailed, - fmt::format("No ACK record exists for caller {}.", member_id)); + fmt::format( + "No ACK record exists for caller {}.", member_id.value())); } auto s = sig->get(0); if (s) { ma->state_digest = s->root.hex_str(); - mas->put(member_id, ma.value()); + mas->put(member_id.value(), ma.value()); } nlohmann::json j; j["state_digest"] = ma->state_digest; @@ -1749,32 +1787,39 @@ namespace ccf .set_auto_schema() .install(); - auto get_encrypted_recovery_share = [this]( - EndpointContext& ctx, - nlohmann::json&&) { - const auto member_id = get_caller_member_id(ctx); - if (!check_member_active(ctx.tx, member_id)) - { - return make_error( - HTTP_STATUS_FORBIDDEN, - ccf::errors::AuthorizationFailed, - "Only active members are given recovery shares."); - } + auto get_encrypted_recovery_share = + [this](EndpointContext& ctx, nlohmann::json&&) { + const auto member_id = get_caller_member_id(ctx); + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Member is unknown."); + } + if (!check_member_active(ctx.tx, member_id.value())) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Only active members are given recovery shares."); + } - auto encrypted_share = - share_manager.get_encrypted_share(ctx.tx, member_id); + auto encrypted_share = + share_manager.get_encrypted_share(ctx.tx, member_id.value()); - if (!encrypted_share.has_value()) - { - return make_error( - HTTP_STATUS_NOT_FOUND, - ccf::errors::ResourceNotFound, - fmt::format("Recovery share not found for member {}.", member_id)); - } + if (!encrypted_share.has_value()) + { + return make_error( + HTTP_STATUS_NOT_FOUND, + ccf::errors::ResourceNotFound, + fmt::format( + "Recovery share not found for member {}.", member_id->value())); + } - return make_success( - GetRecoveryShare::Out{tls::b64_from_raw(encrypted_share.value())}); - }; + return make_success( + GetRecoveryShare::Out{tls::b64_from_raw(encrypted_share.value())}); + }; make_endpoint( "recovery_share", HTTP_GET, @@ -1788,7 +1833,14 @@ namespace ccf nlohmann::json&& params) { // Only active members can submit their shares for recovery const auto member_id = get_caller_member_id(ctx); - if (!check_member_active(ctx.tx, member_id)) + if (!member_id.has_value()) + { + return make_error( + HTTP_STATUS_FORBIDDEN, + ccf::errors::AuthorizationFailed, + "Member is unknown."); + } + if (!check_member_active(ctx.tx, member_id.value())) { return make_error( HTTP_STATUS_FORBIDDEN, @@ -1821,7 +1873,7 @@ namespace ccf try { submitted_shares_count = share_manager.submit_recovery_share( - ctx.tx, member_id, raw_recovery_share); + ctx.tx, member_id.value(), raw_recovery_share); } catch (const std::exception& e) { diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 6eef476a086a..4a74a8bb27ca 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -282,7 +282,7 @@ class RpcContextRecorder public: // session->caller_cert may be DER or PEM, we always convert to PEM crypto::Pem last_caller_cert; - CallerId last_caller_id = INVALID_ID; + std::optional last_caller_id = std::nullopt; void record_ctx(EndpointContext& ctx) { @@ -298,7 +298,7 @@ class RpcContextRecorder } else { - last_caller_id = INVALID_ID; + last_caller_id.reset(); } } }; @@ -408,9 +408,8 @@ http::Request create_signed_request( s.set_body(body); - const auto contents = caller_cert.contents(); - crypto::Sha256Hash hash({contents.data(), contents.size()}); - const std::string key_id = fmt::format("{:02x}", fmt::join(hash.h, "")); + auto caller_cert_der = crypto::cert_pem_to_der(caller_cert); + const auto key_id = crypto::Sha256Hash(caller_cert_der).hex_str(); http::sign_request(s, kp, key_id); @@ -461,11 +460,10 @@ auto member_session = make_shared( auto anonymous_session = make_shared( enclave::InvalidSessionId, anonymous_caller_der); -UserId user_id = INVALID_ID; -UserId invalid_user_id = INVALID_ID; +UserId user_id; -MemberId member_id = INVALID_ID; -MemberId invalid_member_id = INVALID_ID; +MemberId member_id; +MemberId invalid_member_id; void prepare_callers(NetworkState& network) { @@ -1338,7 +1336,7 @@ TEST_CASE("Nodefrontend forwarding" * doctest::test_suite("forwarding")) CHECK(response.status == HTTP_STATUS_OK); CHECK(node_frontend_primary.last_caller_cert == node_caller); - CHECK(node_frontend_primary.last_caller_id == INVALID_ID); + CHECK(!node_frontend_primary.last_caller_id.has_value()); } TEST_CASE("Userfrontend forwarding" * doctest::test_suite("forwarding")) @@ -1380,7 +1378,7 @@ TEST_CASE("Userfrontend forwarding" * doctest::test_suite("forwarding")) CHECK(response.status == HTTP_STATUS_OK); CHECK(user_frontend_primary.last_caller_cert == user_caller); - CHECK(user_frontend_primary.last_caller_id == 0); + CHECK(user_frontend_primary.last_caller_id.value() == user_id); } TEST_CASE("Memberfrontend forwarding" * doctest::test_suite("forwarding")) @@ -1426,7 +1424,7 @@ TEST_CASE("Memberfrontend forwarding" * doctest::test_suite("forwarding")) CHECK(response.status == HTTP_STATUS_OK); CHECK(member_frontend_primary.last_caller_cert == member_caller); - CHECK(member_frontend_primary.last_caller_id == 0); + CHECK(member_frontend_primary.last_caller_id.value() == member_id); } class TestConflictFrontend : public BaseTestFrontend diff --git a/src/node/rpc/test/frontend_test_infra.h b/src/node/rpc/test/frontend_test_infra.h index 207431dc42b5..8f71c4c0d9ad 100644 --- a/src/node/rpc/test/frontend_test_infra.h +++ b/src/node/rpc/test/frontend_test_infra.h @@ -120,8 +120,8 @@ std::vector create_signed_request( r.set_body(&body); const auto contents = caller.contents(); - crypto::Sha256Hash hash({contents.data(), contents.size()}); - const std::string key_id = fmt::format("{:02x}", fmt::join(hash.h, "")); + auto caller_der = crypto::cert_pem_to_der(caller); + const auto key_id = crypto::Sha256Hash(caller_der).hex_str(); http::sign_request(r, kp_, key_id); @@ -185,7 +185,7 @@ auto get_proposal( auto get_vote( MemberRpcFrontend& frontend, ProposalId proposal_id, - MemberId voter, + const MemberId& voter, const crypto::Pem& caller) { const auto getter = create_request( diff --git a/src/node/rpc/test/member_voting_test.cpp b/src/node/rpc/test/member_voting_test.cpp index f12ad42b179f..0fa3b488b2dc 100644 --- a/src/node/rpc/test/member_voting_test.cpp +++ b/src/node/rpc/test/member_voting_test.cpp @@ -288,7 +288,8 @@ DOCTEST_TEST_CASE("Reject duplicate vote") struct NewMember { - MemberId id; + size_t local_id; + MemberId service_id; crypto::KeyPairPtr kp = crypto::make_key_pair(); crypto::Pem cert; }; @@ -336,17 +337,19 @@ DOCTEST_TEST_CASE("Add new members until there are 7 then reject") auto i = 0ul; for (auto& new_member : new_members) { - new_member.id = initial_members + i++; + new_member.local_id = initial_members + i++; // new member certificate - auto cert_pem = - new_member.kp->self_sign(fmt::format("CN=new member{}", new_member.id)); + auto cert_pem = new_member.kp->self_sign( + fmt::format("CN=new member{}", new_member.local_id)); auto encryption_pub_key = dummy_enc_pubk; + auto cert_der = crypto::make_verifier(cert_pem)->cert_der(); + new_member.service_id = crypto::Sha256Hash(cert_der).hex_str(); new_member.cert = cert_pem; // check new_member id does not work before member is added const auto read_next_req = create_request( - read_params(ValueIds::NEXT_MEMBER_ID, Tables::VALUES), "read"); + read_params(new_member.service_id, Tables::MEMBER_ACKS), "read"); const auto r = frontend_process(frontend, read_next_req, new_member.cert); check_error(r, HTTP_STATUS_UNAUTHORIZED); @@ -416,14 +419,13 @@ DOCTEST_TEST_CASE("Add new members until there are 7 then reject") const auto r = frontend_process(frontend, vote, voter_a_cert); const auto result = parse_response_body(r); - if (new_member.id < max_members) + if (new_member.local_id < max_members) { // vote should succeed DOCTEST_CHECK(result.state == ProposalState::ACCEPTED); // check that member with the new new_member cert can make RPCs now - DOCTEST_CHECK( - parse_response_body(frontend_process( - frontend, read_next_req, new_member.cert)) == new_member.id + 1); + auto r = frontend_process(frontend, read_next_req, new_member.cert); + DOCTEST_CHECK(r.status == HTTP_STATUS_OK); // successful proposals are removed from the kv, so we can't confirm // their final state @@ -460,7 +462,7 @@ DOCTEST_TEST_CASE("Add new members until there are 7 then reject") { // (1) read ack entry const auto read_state_digest_req = create_request( - read_params(new_member->id, Tables::MEMBER_ACKS), "read"); + read_params(new_member->service_id, Tables::MEMBER_ACKS), "read"); const auto ack0 = parse_response_body( frontend_process(frontend, read_state_digest_req, new_member->cert)); DOCTEST_REQUIRE(std::all_of( @@ -509,8 +511,8 @@ DOCTEST_TEST_CASE("Add new members until there are 7 then reject") DOCTEST_CHECK(good_response.status == HTTP_STATUS_NO_CONTENT); // (6) read own member status - const auto read_status_req = - create_request(read_params(new_member->id, Tables::MEMBERS), "read"); + const auto read_status_req = create_request( + read_params(new_member->service_id, Tables::MEMBERS), "read"); const auto mi = parse_response_body( frontend_process(frontend, read_status_req, new_member->cert)); DOCTEST_CHECK(mi.status == MemberStatus::ACTIVE); @@ -740,15 +742,6 @@ ProposalInfo test_raw_writes( network, gen, context, share_manager, n_members, member_certs); frontend.open(); - // check values before - { - auto tx = network.tables->create_tx(); - auto next_member_id_r = - tx.rw(network.values)->get(ValueIds::NEXT_MEMBER_ID); - DOCTEST_CHECK(next_member_id_r); - DOCTEST_CHECK(*next_member_id_r == n_members); - } - // propose ProposalId proposal_id; { @@ -840,9 +833,8 @@ DOCTEST_TEST_CASE("Propose raw writes") {R"xxx( local tables, recovery_threshold = ... local p = Puts:new() - p:put("public:ccf.gov.service.config", 0, {recovery_threshold = recovery_threshold, consensus = "CFT"}) - return Calls:call("raw_puts", p) - )xxx"s, + p:put("public:ccf.gov.service.config", 0, {recovery_threshold = +recovery_threshold, consensus = "CFT"}) return Calls:call("raw_puts", p) )xxx"s, 4}, n_members, pro_votes, @@ -1083,6 +1075,7 @@ DOCTEST_TEST_CASE("Add and remove user via proposed calls") frontend.open(); ccf::Cert user_der; + ccf::UserId user_id; { DOCTEST_INFO("Add user"); @@ -1113,13 +1106,9 @@ DOCTEST_TEST_CASE("Add and remove user via proposed calls") DOCTEST_CHECK(r.state == ProposalState::ACCEPTED); auto tx1 = network.tables->create_tx(); - const auto uid = tx1.rw(network.values)->get(ValueIds::NEXT_USER_ID); - DOCTEST_CHECK(uid); - DOCTEST_CHECK(*uid == 1); user_der = crypto::make_verifier(user_cert)->cert_der(); - const auto uid1 = tx1.rw(network.user_certs)->get(user_der); - DOCTEST_CHECK(uid1); - DOCTEST_CHECK(*uid1 == 0); + user_id = crypto::Sha256Hash(user_der).hex_str(); + DOCTEST_REQUIRE(tx1.rw(network.users)->get(user_id).has_value()); } { @@ -1131,7 +1120,7 @@ DOCTEST_TEST_CASE("Add and remove user via proposed calls") )xxx"); const auto propose = create_signed_request( - Propose::In{proposal, 0}, "proposals", kp, member_cert); + Propose::In{proposal, user_id}, "proposals", kp, member_cert); auto r = parse_response_body( frontend_process(frontend, propose, member_cert)); @@ -1150,10 +1139,8 @@ DOCTEST_TEST_CASE("Add and remove user via proposed calls") DOCTEST_CHECK(r.state == ProposalState::ACCEPTED); auto tx1 = network.tables->create_tx(); - auto user = tx1.rw(network.users)->get(0); + auto user = tx1.rw(network.users)->get(user_id); DOCTEST_CHECK(!user.has_value()); - auto user_cert = tx1.rw(network.user_certs)->get(user_der); - DOCTEST_CHECK(!user_cert.has_value()); } } @@ -1183,7 +1170,7 @@ DOCTEST_TEST_CASE( gen.activate_member(operator_id); // Non-operating members - std::map members; + std::map members; for (size_t i = 1; i < 4; i++) { auto cert = get_cert(i, kp); @@ -1203,8 +1190,8 @@ DOCTEST_TEST_CASE( frontend.open(); ProposalId proposal_id; - size_t proposer_id = 1; - size_t voter_id = 2; + auto proposer_id = members.begin()->first; + auto voter_id = std::next(members.begin())->first; const ccf::Script vote_for("return true"); const ccf::Script vote_against("return false"); @@ -1322,7 +1309,7 @@ DOCTEST_TEST_CASE("Passing operator change" * doctest::test_suite("operator")) gen.activate_member(operator_id); // Non-operating members - std::map members; + std::map members; for (size_t i = 1; i < 4; i++) { auto cert = get_cert(i, kp); @@ -1424,7 +1411,10 @@ DOCTEST_TEST_CASE("Passing operator change" * doctest::test_suite("operator")) DOCTEST_INFO("New operator retires original operator"); Propose::In proposal; proposal.script = fmt::format( - R"xxx(return Calls:call("retire_member", {}))xxx", operator_id); + R"xxx( + local tables, member_id = ... + return Calls:call("retire_member", member_id))xxx"); + proposal.parameter = operator_id; const auto propose = create_signed_request( proposal, "proposals", new_operator_kp, new_operator_cert); @@ -1466,7 +1456,10 @@ DOCTEST_TEST_CASE("Passing operator change" * doctest::test_suite("operator")) Propose::In proposal; proposal.script = fmt::format( - R"xxx(return Calls:call("retire_member", {}))xxx", normal_member_id); + R"xxx( + local tables, member_id = ... + return Calls:call("retire_member", member_id))xxx"); + proposal.parameter = normal_member_id; const auto propose = create_signed_request( proposal, "proposals", new_operator_kp, new_operator_cert); @@ -1504,7 +1497,7 @@ DOCTEST_TEST_CASE( gen.activate_member(proposer_id); // Non-operating members - std::map members; + std::map members; for (size_t i = 1; i < 3; i++) { auto cert = get_cert(i, kp); @@ -1566,8 +1559,8 @@ DOCTEST_TEST_CASE( check_result_state(r, ProposalState::OPEN); } - size_t first_voter_id = 1; - size_t second_voter_id = 2; + MemberId first_voter_id = members.begin()->first; + MemberId second_voter_id = std::next(members.begin())->first; { DOCTEST_INFO("First member votes for proposal"); @@ -1678,16 +1671,13 @@ DOCTEST_TEST_CASE("User data") DOCTEST_INFO("user data can be set to an object"); Propose::In proposal; - proposal.script = fmt::format( + proposal.script = std::string( R"xxx( - proposed_user_data = {{ - name = "bob", - permissions = {{"read", "delete"}} - }} - return Calls:call("set_user_data", {{user_id = {}, user_data = - proposed_user_data}}) - )xxx", - user_id); + local tables, user_data = ... + return Calls:call("set_user_data", user_data) + )xxx"); + proposal.parameter["user_id"] = user_id; + proposal.parameter["user_data"] = user_data_object; const auto proposal_serialized = create_signed_request(proposal, "proposals", kp, member_cert); const auto propose_response = parse_response_body( @@ -1720,12 +1710,11 @@ DOCTEST_TEST_CASE("User data") DOCTEST_INFO("user data can be overwritten"); Propose::In proposal; proposal.script = std::string(R"xxx( - local tables, param = ... - return Calls:call("set_user_data", {user_id = param.id, user_data = - param.data}) + local tables, user_data = ... + return Calls:call("set_user_data", user_data) )xxx"); - proposal.parameter["id"] = user_id; - proposal.parameter["data"] = user_data_string; + proposal.parameter["user_id"] = user_id; + proposal.parameter["user_data"] = user_data_string; const auto proposal_serialized = create_signed_request(proposal, "proposals", kp, member_cert); const auto propose_response = parse_response_body( @@ -1762,12 +1751,12 @@ DOCTEST_TEST_CASE("Submit recovery shares") ShareManager share_manager(network); StubRecoverableNodeContext context(share_manager); MemberRpcFrontend frontend(network, context, share_manager); - std::map> members; + std::map> members; size_t members_count = 4; size_t recovery_threshold = 2; DOCTEST_REQUIRE(recovery_threshold <= members_count); - std::map> retrieved_shares; + std::map> retrieved_shares; DOCTEST_INFO("Setup state"); { @@ -1809,7 +1798,7 @@ DOCTEST_TEST_CASE("Submit recovery shares") DOCTEST_INFO("Submit share before the service is in correct state"); { - MemberId member_id = 0; + MemberId member_id = members.begin()->first; const auto submit_recovery_share = create_request( SubmitRecoveryShare::In{tls::b64_from_raw(retrieved_shares[member_id])}, "recovery_share"); @@ -1921,7 +1910,7 @@ DOCTEST_TEST_CASE("Number of active members with recovery shares limits") MemberRpcFrontend frontend(network, context, share_manager); frontend.open(); - std::map members; + std::map members; auto gen_tx = network.tables->create_tx(); GenesisGenerator gen(network, gen_tx); @@ -1944,11 +1933,12 @@ DOCTEST_TEST_CASE("Number of active members with recovery shares limits") DOCTEST_INFO("Activate members until reaching limit"); { + size_t member_count = 0; for (auto const& m : members) { auto resp = activate(frontend, kp, m.second); - if (m.first >= max_active_recovery_members) + if (member_count >= max_active_recovery_members) { DOCTEST_CHECK(resp.status == HTTP_STATUS_FORBIDDEN); } @@ -1956,6 +1946,7 @@ DOCTEST_TEST_CASE("Number of active members with recovery shares limits") { DOCTEST_CHECK(resp.status == HTTP_STATUS_NO_CONTENT); } + member_count++; } } @@ -1985,7 +1976,7 @@ DOCTEST_TEST_CASE("Open network sequence") ShareManager share_manager(network); StubNodeContext context; MemberRpcFrontend frontend(network, context, share_manager); - std::map>> members; + std::map>> members; size_t members_count = 4; size_t recovery_threshold = 100; diff --git a/src/node/rpc/user_frontend.h b/src/node/rpc/user_frontend.h index 9a9a8432f27e..7bdba66bba87 100644 --- a/src/node/rpc/user_frontend.h +++ b/src/node/rpc/user_frontend.h @@ -28,8 +28,7 @@ namespace ccf { public: UserEndpointRegistry(ccfapp::AbstractNodeContext& context) : - CommonEndpointRegistry( - get_actor_prefix(ActorsType::users), context, Tables::USER_CERT_DERS) + CommonEndpointRegistry(get_actor_prefix(ActorsType::users), context) {} }; diff --git a/src/node/share_manager.h b/src/node/share_manager.h index 856bfad78010..995792464f50 100644 --- a/src/node/share_manager.h +++ b/src/node/share_manager.h @@ -328,7 +328,7 @@ namespace ccf } std::optional get_encrypted_share( - kv::Tx& tx, MemberId member_id) + kv::Tx& tx, const MemberId& member_id) { auto recovery_shares_info = tx.rw(network.shares)->get(0); if (!recovery_shares_info.has_value()) @@ -454,11 +454,11 @@ namespace ccf { auto submitted_shares = tx.rw(network.submitted_shares); - std::vector submitted_share_ids = {}; + std::vector submitted_share_ids = {}; submitted_shares->foreach( [&submitted_share_ids]( - const MemberId member_id, const std::vector&) { + const MemberId& member_id, const std::vector&) { submitted_share_ids.push_back(member_id); return true; }); diff --git a/src/node/signatures.h b/src/node/signatures.h index 81faf2873914..6ff7942fd6dc 100644 --- a/src/node/signatures.h +++ b/src/node/signatures.h @@ -15,8 +15,8 @@ namespace ccf { kv::Consensus::SeqNo seqno = 0; kv::Consensus::View view = 0; - ObjectId commit_seqno = 0; - ObjectId commit_view = 0; + kv::Consensus::SeqNo commit_seqno = 0; + kv::Consensus::View commit_view = 0; crypto::Sha256Hash root; std::vector tree = {0}; @@ -60,5 +60,7 @@ namespace ccf DECLARE_JSON_TYPE_WITH_BASE(PrimarySignature, NodeSignature) DECLARE_JSON_REQUIRED_FIELDS( PrimarySignature, seqno, view, commit_seqno, commit_view, root, tree) - using Signatures = kv::Map; + + // Signatures are always stored at key `0` + using Signatures = kv::Map; } \ No newline at end of file diff --git a/src/node/test/historical_queries.cpp b/src/node/test/historical_queries.cpp index 707683564c46..9a2ab8304d31 100644 --- a/src/node/test/historical_queries.cpp +++ b/src/node/test/historical_queries.cpp @@ -124,8 +124,12 @@ TestState create_and_init_state(bool initialise_ledger_rekey = true) auto members = tx.rw(ccf::Tables::MEMBERS); ccf::MemberInfo mi; mi.status = ccf::MemberStatus::ACTIVE; + + auto kp = crypto::make_key_pair(); + mi.cert = kp->self_sign("CN=member"); mi.encryption_pub_key = crypto::make_rsa_key_pair()->public_key_pem(); - members->put(0, mi); + members->put( + crypto::Sha256Hash(crypto::cert_pem_to_der(mi.cert)).hex_str(), mi); REQUIRE(tx.commit() == kv::CommitResult::SUCCESS); } diff --git a/src/node/test/msgpack_serialization.cpp b/src/node/test/msgpack_serialization.cpp index 1b4b63735af5..90ee23729f9f 100644 --- a/src/node/test/msgpack_serialization.cpp +++ b/src/node/test/msgpack_serialization.cpp @@ -83,7 +83,7 @@ TEST_CASE("Proposal") INFO("Initial proposal"); Script s("return true"); nlohmann::json p("hello world"); - MemberId m(0); + auto m = MemberId("member"); Proposal proposal(s, p, m); const auto converted = msgpack_roundtrip(proposal); CHECK(proposal == converted); @@ -93,12 +93,13 @@ TEST_CASE("Proposal") INFO("Voted proposal"); Script s("return true"); nlohmann::json p("hello world"); - MemberId m(0); + auto m = MemberId("member"); Proposal proposal(s, p, m); - proposal.votes[1] = Script("return true"); - proposal.votes[2] = Script("return false"); - proposal.votes[3] = Script("return RoN"); - proposal.votes[4] = Script("Robert'); DROP TABLE Students;--"); + proposal.votes[MemberId("member1")] = Script("return true"); + proposal.votes[MemberId("member2")] = Script("return false"); + proposal.votes[MemberId("member3")] = Script("return RoN"); + proposal.votes[MemberId("member4")] = + Script("Robert'); DROP TABLE Students;--"); const auto converted = msgpack_roundtrip(proposal); CHECK(proposal == converted); } @@ -186,7 +187,7 @@ TEST_CASE("Signature") INFO("Simple sig"); PrimarySignature sig; sig.sig.push_back(0); - sig.node = "node"; + sig.node = NodeId("node"); sig.seqno = 1; sig.view = 2; sig.commit_seqno = 3; @@ -198,7 +199,7 @@ TEST_CASE("Signature") INFO("Rand sig"); PrimarySignature sig; fill_rand(sig.sig, 256); - sig.node = "node"; + sig.node = NodeId("node"); sig.seqno = rand(); sig.view = rand(); sig.commit_seqno = rand(); diff --git a/src/node/values.h b/src/node/values.h index 5afbd4af2e3e..c006057987b5 100644 --- a/src/node/values.h +++ b/src/node/values.h @@ -10,15 +10,14 @@ namespace ccf { using ValueId = uint8_t; using Value = uint64_t; + + // This table is only used to keep track of node IDs for the BFT variant of + // the consensus using Values = kv::Map; enum ValueIds : ValueId { - NEXT_MEMBER_ID = 0, - NEXT_USER_ID = 1, - NEXT_NODE_ID = 2, - NEXT_PROPOSAL_ID = 3, - NEXT_CODE_ID = 4, + NEXT_NODE_ID = 0, // not to be used END_ID }; diff --git a/src/node/view_change.h b/src/node/view_change.h index 3afc939c6350..463e448a753a 100644 --- a/src/node/view_change.h +++ b/src/node/view_change.h @@ -86,5 +86,6 @@ namespace ccf DECLARE_JSON_REQUIRED_FIELDS( ViewChangeConfirmation, view, seqno, signature, view_change_messages); - using NewViewsMap = kv::Map; + // Always recorded at key 0 + using NewViewsMap = kv::Map; } \ No newline at end of file diff --git a/src/perf_client/perf_client.h b/src/perf_client/perf_client.h index 56f693aac293..6246616da954 100644 --- a/src/perf_client/perf_client.h +++ b/src/perf_client/perf_client.h @@ -7,6 +7,7 @@ // CCF #include "clients/rpc_tls_client.h" +#include "crypto/verifier.h" #include "ds/cli_helper.h" #include "ds/files.h" #include "ds/logger.h" @@ -304,8 +305,8 @@ namespace client key = crypto::Pem(raw_key); - crypto::Sha256Hash hash({raw_cert.data(), raw_cert.size()}); - key_id = fmt::format("{:02x}", fmt::join(hash.h, "")); + auto cert_der = crypto::cert_pem_to_der(raw_cert); + key_id = crypto::Sha256Hash(cert_der).hex_str(); tls_cert = std::make_shared( std::make_shared(ca), raw_cert, key); diff --git a/src/runtime_config/default_whitelists.h b/src/runtime_config/default_whitelists.h index 40ba4725a213..770b58fe626e 100644 --- a/src/runtime_config/default_whitelists.h +++ b/src/runtime_config/default_whitelists.h @@ -10,10 +10,8 @@ namespace ccf static const std::map default_whitelists = { {MEMBER_CAN_READ, {Tables::MEMBERS, - Tables::MEMBER_CERT_DERS, Tables::MEMBER_ACKS, Tables::USERS, - Tables::USER_CERT_DERS, Tables::NODES, Tables::VALUES, Tables::SIGNATURES, @@ -33,7 +31,6 @@ namespace ccf {MEMBER_CAN_PROPOSE, {Tables::USERS, - Tables::USER_CERT_DERS, Tables::VALUES, Tables::WHITELISTS, Tables::GOV_SCRIPTS, diff --git a/src/runtime_config/operator_gov.lua b/src/runtime_config/operator_gov.lua index 8316519bc75d..f50b0095c5c6 100644 --- a/src/runtime_config/operator_gov.lua +++ b/src/runtime_config/operator_gov.lua @@ -70,7 +70,7 @@ return { for member, vote in pairs(votes) do if vote then - if not is_operator(tonumber(member)) then + if not is_operator(member) then member_votes = member_votes + 1 end end @@ -117,7 +117,7 @@ return { end -- operators proposing operator changes can pass them without a vote - if operator_change and is_operator(tonumber(proposer_id)) then + if operator_change and is_operator(proposer_id) then return PASSED end diff --git a/tests/ca_certs.py b/tests/ca_certs.py index 9baebf66bd54..44ff6325ec66 100644 --- a/tests/ca_certs.py +++ b/tests/ca_certs.py @@ -57,9 +57,7 @@ def test_cert_store(network, args): cert_pem_fp.flush() network.consortium.set_ca_cert_bundle(primary, cert_name, cert_pem_fp.name) - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.tls.ca_cert_bundles", "key": cert_name}, @@ -74,9 +72,7 @@ def test_cert_store(network, args): LOG.info("Member removes a ca cert") network.consortium.remove_ca_cert_bundle(primary, cert_name) - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.tls.ca_cert_bundles", "key": cert_name}, diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index a15e4439618e..a7e047565095 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -161,13 +161,13 @@ def test_cert_prefix(network, args): if args.package == "liblogging": primary, _ = network.find_primary() - for user_id in network.user_ids: - with primary.client(f"user{user_id}") as c: + for user in network.users: + with primary.client(user.local_id) as c: log_id = 101 msg = "This message will be prefixed" c.post("/app/log/private/prefix_cert", {"id": log_id, "msg": msg}) r = c.get(f"/app/log/private?id={log_id}") - assert f"CN=user{user_id}" in r.body.json()["msg"], r + assert f"CN={user.local_id}" in r.body.json()["msg"], r else: LOG.warning( @@ -184,7 +184,7 @@ def test_anonymous_caller(network, args): primary, _ = network.find_primary() # Create a new user but do not record its identity - network.create_user(5, args.participants_curve, record=False) + network.create_user("user5", args.participants_curve, record=False) log_id = 101 msg = "This message is anonymous" @@ -210,7 +210,10 @@ def test_anonymous_caller(network, args): @reqs.supports_methods("multi_auth") def test_multi_auth(network, args): primary, _ = network.find_primary() - with primary.client("user0") as c: + user = network.users[0] + member = network.consortium.members[0] + + with primary.client(user.local_id) as c: response = c.get("/app/api") supported_methods = response.body.json()["paths"] if "/multi_auth" in supported_methods.keys(): @@ -228,15 +231,15 @@ def require_new_response(r): require_new_response(r) LOG.info("Authenticate as a user, via TLS cert") - with primary.client("user0") as c: + with primary.client(user.local_id) as c: r = c.get("/app/multi_auth") require_new_response(r) LOG.info("Authenticate as same user, now with user data") network.consortium.set_user_data( - primary, 0, {"some": ["interesting", "data", 42]} + primary, user.service_id, {"some": ["interesting", "data", 42]} ) - with primary.client("user0") as c: + with primary.client(user.local_id) as c: r = c.get("/app/multi_auth") require_new_response(r) @@ -246,15 +249,15 @@ def require_new_response(r): require_new_response(r) LOG.info("Authenticate as a member, via TLS cert") - with primary.client("member0") as c: + with primary.client(member.local_id) as c: r = c.get("/app/multi_auth") require_new_response(r) LOG.info("Authenticate as same member, now with user data") network.consortium.set_member_data( - primary, 0, {"distinct": {"arbitrary": ["data"]}} + primary, member.service_id, {"distinct": {"arbitrary": ["data"]}} ) - with primary.client("member0") as c: + with primary.client(member.local_id) as c: r = c.get("/app/multi_auth") require_new_response(r) @@ -264,12 +267,12 @@ def require_new_response(r): require_new_response(r) LOG.info("Authenticate as a user, via HTTP signature") - with primary.client(None, "user0") as c: + with primary.client(None, user.local_id) as c: r = c.get("/app/multi_auth") require_new_response(r) LOG.info("Authenticate as a member, via HTTP signature") - with primary.client(None, "member0") as c: + with primary.client(None, member.local_id) as c: r = c.get("/app/multi_auth") require_new_response(r) @@ -278,7 +281,7 @@ def require_new_response(r): r = c.get("/app/multi_auth") require_new_response(r) - network.create_user(5, args.participants_curve, record=False) + network.create_user("user5", args.participants_curve, record=False) LOG.info("Authenticate as invalid user5 but sign as valid user3") with primary.client("user5", "user3") as c: @@ -617,31 +620,31 @@ def test_user_data_ACL(network, args): primary, _ = network.find_primary() proposing_member = network.consortium.get_any_active_member() - user_id = 0 + user = network.users[0] # Give isAdmin permissions to a single user proposal_body, careful_vote = ccf.proposal_generator.set_user_data( - user_id, + user.service_id, {"isAdmin": True}, ) proposal = proposing_member.propose(primary, proposal_body) network.consortium.vote_using_majority(primary, proposal, careful_vote) # Confirm that user can now use this endpoint - with primary.client(f"user{user_id}") as c: + with primary.client(user.local_id) as c: r = c.post("/app/log/private/admin_only", {"id": 42, "msg": "hello world"}) assert r.status_code == http.HTTPStatus.OK.value, r.status_code # Remove permission proposal_body, careful_vote = ccf.proposal_generator.set_user_data( - user_id, + user.service_id, {"isAdmin": False}, ) proposal = proposing_member.propose(primary, proposal_body) network.consortium.vote_using_majority(primary, proposal, careful_vote) # Confirm that user is now forbidden on this endpoint - with primary.client(f"user{user_id}") as c: + with primary.client(user.local_id) as c: r = c.post("/app/log/private/admin_only", {"id": 42, "msg": "hello world"}) assert r.status_code == http.HTTPStatus.FORBIDDEN.value, r.status_code diff --git a/tests/governance.py b/tests/governance.py index e5c30ebebcca..d38b4a562f0e 100644 --- a/tests/governance.py +++ b/tests/governance.py @@ -11,8 +11,6 @@ import infra.e2e_args import suite.test_requirements as reqs import infra.logging_app as app -import json -import urllib.parse from loguru import logger as LOG @@ -87,19 +85,19 @@ def test_user(network, args, verify=True): # Note: This test should not be chained in the test suite as it creates # a new user and uses its own LoggingTxs primary, _ = network.find_nodes() - new_user_id = 3 - network.create_user(new_user_id, args.participants_curve) + new_user_local_id = f"user{3}" + new_user = network.create_user(new_user_local_id, args.participants_curve) user_data = {"lifetime": "temporary"} - network.consortium.add_user(primary, new_user_id, user_data) - txs = app.LoggingTxs(user_id=new_user_id) + network.consortium.add_user(primary, new_user.local_id, user_data) + txs = app.LoggingTxs(user_id=new_user.local_id) txs.issue( network=network, number_txs=1, ) if verify: txs.verify() - network.consortium.remove_user(primary, new_user_id) - with primary.client(f"user{new_user_id}") as c: + network.consortium.remove_user(primary, new_user.service_id) + with primary.client(new_user_local_id) as c: r = c.get("/app/log/private") assert r.status_code == http.HTTPStatus.UNAUTHORIZED.value return network @@ -135,29 +133,16 @@ def member_info(mid): for member in network.get_members(): if member.member_data: assert ( - member_info(member.member_id)["member_data"] == member.member_data + member_info(member.service_id)["member_data"] == member.member_data ) md_count += 1 else: - assert "member_data" not in member_info(member.member_id) + assert "member_data" not in member_info(member.service_id) assert md_count == args.initial_operator_count return network -@reqs.description("Check caller_id") -def test_caller_id(network, args): - primary, _ = network.find_nodes() - with primary.client("user0") as uc: - with open(network.consortium.user_cert_path(1), "r") as ucert: - pem = ucert.read() - json_pem = json.dumps(pem) - r = uc.get(f"/app/caller_id?cert={urllib.parse.quote_plus(json_pem)}") - assert r.status_code == http.HTTPStatus.OK.value - assert r.body.json()["caller_id"] == 1 - return network - - @reqs.description("Check network/nodes endpoint") def test_node_ids(network, args): nodes = network.find_nodes() @@ -248,7 +233,6 @@ def run(args): network = test_quote(network, args) network = test_user(network, args) network = test_no_quote(network, args) - network = test_caller_id(network, args) network = test_service_principals(network, args) network = test_ack_state_digest_update(network, args) diff --git a/tests/governance_history.py b/tests/governance_history.py index bf9854057758..c1c28c591560 100644 --- a/tests/governance_history.py +++ b/tests/governance_history.py @@ -22,17 +22,23 @@ def count_governance_operations(ledger): for chunk in ledger: for tr in chunk: tables = tr.get_public_domain().get_tables() - if "public:ccf.internal.members.certs_der" in tables: - members_table = tables["public:ccf.internal.members.certs_der"] - for cert, member_id in members_table.items(): - cert_unpacked = ccf.ledger.extract_msgpacked_data(cert) - member_id_unpacked = ccf.ledger.extract_msgpacked_data(member_id) - members[member_id_unpacked] = cert_unpacked + if "public:ccf.gov.members.info" in tables: + members_table = tables["public:ccf.gov.members.info"] + for member_id, member_info in members_table.items(): + member_id_unpacked = str( + ccf.ledger.extract_msgpacked_data(member_id) + ) + member_info_unpacked = ccf.ledger.extract_msgpacked_data( + member_info + ) + members[member_id_unpacked] = member_info_unpacked[0][0][0] if "public:ccf.gov.history" in tables: governance_history_table = tables["public:ccf.gov.history"] for member_id, signed_request in governance_history_table.items(): - member_id_unpacked = ccf.ledger.extract_msgpacked_data(member_id) + member_id_unpacked = str( + ccf.ledger.extract_msgpacked_data(member_id) + ) signed_request_unpacked = ccf.ledger.extract_msgpacked_data( signed_request ) @@ -102,7 +108,7 @@ def run(args): proposals_issued += 1 with primary.client() as c: - response = network.consortium.get_member_by_id( + response = network.consortium.get_member_by_local_id( new_member_proposal.proposer_id ).withdraw(primary, new_member_proposal) infra.checker.Checker(c)(response) diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index fe0789ca3a6a..39d10d792e7c 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -24,7 +24,7 @@ def __init__( common_dir, key_generator, share_script, - member_ids=None, + members_info=None, curve=None, remote_node=None, authenticate_session=True, @@ -38,11 +38,11 @@ def __init__( self.authenticate_session = authenticate_session # If a list of member IDs is passed in, generate fresh member identities. # Otherwise, recover the state of the consortium from the state of CCF. - if member_ids is not None: + if members_info is not None: self.recovery_threshold = 0 - for m_id, has_share, m_data in member_ids: + for m_local_id, has_share, m_data in members_info: new_member = infra.member.Member( - m_id, + f"member{m_local_id}", curve, common_dir, share_script, @@ -55,15 +55,16 @@ def __init__( self.recovery_threshold += 1 self.members.append(new_member) else: + # TODO: Why do we still need this? with remote_node.client("member0") as mc: r = mc.post( "/gov/query", { "text": """tables = ... non_retired_members = {} - tables["public:ccf.gov.members.info"]:foreach(function(member_id, info) + tables["public:ccf.gov.members.info"]:foreach(function(service_id, info) if info["status"] ~= "RETIRED" then - table.insert(non_retired_members, {member_id, info}) + table.insert(non_retired_members, {service_id, info}) end end) return non_retired_members @@ -72,7 +73,7 @@ def __init__( ) for m_id, info in r.body.json(): new_member = infra.member.Member( - m_id, + f"member{m_id}", curve, self.common_dir, share_script, @@ -137,9 +138,9 @@ def generate_and_propose_new_member( ): # The Member returned by this function is in state ACCEPTED. The new Member # should ACK to become active. - new_member_id = len(self.members) + new_member_local_id = f"member{len(self.members)}" new_member = infra.member.Member( - new_member_id, + new_member_local_id, curve, self.common_dir, self.share_script, @@ -150,8 +151,8 @@ def generate_and_propose_new_member( proposal_body, careful_vote = self.make_proposal( "new_member", - os.path.join(self.common_dir, f"member{new_member_id}_cert.pem"), - os.path.join(self.common_dir, f"member{new_member_id}_enc_pubk.pem") + os.path.join(self.common_dir, f"{new_member_local_id}_cert.pem"), + os.path.join(self.common_dir, f"{new_member_local_id}_enc_pubk.pem") if recovery_member else None, member_data, @@ -207,9 +208,16 @@ def get_any_active_member(self, recovery_member=None): else: return random.choice(self.get_active_members()) - def get_member_by_id(self, member_id): + def get_member_by_local_id(self, local_id): return next( - (member for member in self.members if member.member_id == member_id), None + (member for member in self.members if member.local_id == local_id), + None, + ) + + def get_member_by_service_id(self, service_id): + return next( + (member for member in self.members if member.service_id == service_id), + None, ) def vote_using_majority( @@ -270,7 +278,7 @@ def get_proposals(self, remote_node): proposals.append( infra.proposal.Proposal( proposal_id=proposal_id, - proposer_id=int(attr["proposer"]), + proposer_id=attr["proposer"], state=infra.proposal.ProposalState(attr["state"]), ) ) @@ -314,7 +322,7 @@ def trust_node(self, remote_node, node_id, timeout=3): def retire_member(self, remote_node, member_to_retire): proposal_body, careful_vote = self.make_proposal( - "retire_member", member_to_retire.member_id + "retire_member", member_to_retire.service_id ) proposal = self.get_any_active_member().propose(remote_node, proposal_body) self.vote_using_majority(remote_node, proposal, careful_vote) @@ -344,7 +352,7 @@ def update_recovery_shares(self, remote_node): return self.vote_using_majority(remote_node, proposal, careful_vote) def user_cert_path(self, user_id): - return os.path.join(self.common_dir, f"user{user_id}_cert.pem") + return os.path.join(self.common_dir, f"{user_id}_cert.pem") def add_user(self, remote_node, user_id, user_data=None): proposal, careful_vote = self.make_proposal( @@ -375,10 +383,10 @@ def set_user_data(self, remote_node, user_id, user_data): proposal = self.get_any_active_member().propose(remote_node, proposal) self.vote_using_majority(remote_node, proposal, careful_vote) - def set_member_data(self, remote_node, member_id, member_data): + def set_member_data(self, remote_node, service_id, member_data): proposal, careful_vote = self.make_proposal( "set_member_data", - member_id, + service_id, member_data, ) proposal = self.get_any_active_member().propose(remote_node, proposal) diff --git a/tests/infra/crypto.py b/tests/infra/crypto.py index e80d8b71887a..184e258f6319 100644 --- a/tests/infra/crypto.py +++ b/tests/infra/crypto.py @@ -41,7 +41,7 @@ class CCFDigestType(IntEnum): def verify_request_sig(raw_cert, sig, req, request_body, md): - cert = x509.load_der_x509_certificate(raw_cert, backend=default_backend()) + cert = x509.load_pem_x509_certificate(raw_cert, backend=default_backend()) # Verify that the request digest matches the hash of the body h = hashes.Hash(hashes.SHA256(), backend=default_backend()) @@ -168,3 +168,8 @@ def compute_public_key_der_hash_hex_from_pem(pem: str): Encoding.DER, PublicFormat.SubjectPublicKeyInfo ) return hashlib.sha256(pub_key).hexdigest() + + +def compute_cert_der_hash_hex_from_pem(pem: str): + cert = load_pem_x509_certificate(pem.encode(), default_backend()) + return cert.fingerprint(hashes.SHA256()).hex() diff --git a/tests/infra/logging_app.py b/tests/infra/logging_app.py index bf10c43c427f..4661c9060eff 100644 --- a/tests/infra/logging_app.py +++ b/tests/infra/logging_app.py @@ -38,11 +38,11 @@ def sample_list(l, n): class LoggingTxs: - def __init__(self, user_id=0): + def __init__(self, user_id="user0"): self.pub = defaultdict(list) self.priv = defaultdict(list) self.idx = 0 - self.user = f"user{user_id}" + self.user = user_id self.network = None def get_last_tx(self, priv=True, idx=None): diff --git a/tests/infra/member.py b/tests/infra/member.py index 88e5d73208d2..1f4f3489e5cc 100644 --- a/tests/infra/member.py +++ b/tests/infra/member.py @@ -12,6 +12,8 @@ import json from typing import NamedTuple, Optional +from loguru import logger as LOG + class NoRecoveryShareFound(Exception): def __init__(self, response): @@ -34,7 +36,7 @@ class MemberInfo(NamedTuple): class Member: def __init__( self, - member_id, + local_id, curve, common_dir, share_script, @@ -44,7 +46,7 @@ def __init__( authenticate_session=True, ): self.common_dir = common_dir - self.member_id = member_id + self.local_id = local_id self.status_code = MemberStatus.ACCEPTED self.share_script = share_script self.member_data = member_data @@ -52,20 +54,17 @@ def __init__( self.authenticate_session = authenticate_session self.member_info = MemberInfo( - f"member{self.member_id}_cert.pem", - f"member{self.member_id}_enc_pubk.pem" if is_recovery_member else None, - f"member{self.member_id}_data.json" if member_data else None, + f"{self.local_id}_cert.pem", + f"{self.local_id}_enc_pubk.pem" if is_recovery_member else None, + f"{self.local_id}_data.json" if member_data else None, ) if key_generator is not None: - member = f"member{member_id}" - key_generator_args = [ "--name", - f"{member}", + self.local_id, "--curve", f"{curve.name}", - "--gen-enc-key", ] if is_recovery_member: @@ -83,7 +82,7 @@ def __init__( # If no key generator is passed in, the identity of the member # should have been created in advance (e.g. by a previous network) assert os.path.isfile( - os.path.join(self.common_dir, f"member{self.member_id}_privk.pem") + os.path.join(self.common_dir, f"{self.local_id}_privk.pem") ) assert os.path.isfile( os.path.join(self.common_dir, self.member_info.certificate_file) @@ -95,14 +94,21 @@ def __init__( ) as md: json.dump(member_data, md) + with open( + os.path.join(self.common_dir, self.member_info.certificate_file) + ) as c: + self.service_id = infra.crypto.compute_cert_der_hash_hex_from_pem(c.read()) + + LOG.info(f"Member {self.local_id} created: {self.service_id}") + def auth(self, write=False): if self.authenticate_session: if write: - return (f"member{self.member_id}", f"member{self.member_id}") + return (self.local_id, self.local_id) else: - return (f"member{self.member_id}", None) + return (self.local_id, None) else: - return (None, f"member{self.member_id}") + return (None, self.local_id) def is_active(self): return self.status_code == MemberStatus.ACTIVE @@ -118,7 +124,7 @@ def propose(self, remote_node, proposal): raise infra.proposal.ProposalNotCreated(r) return infra.proposal.Proposal( - proposer_id=self.member_id, + proposer_id=self.local_id, proposal_id=r.body.json()["proposal_id"], state=infra.proposal.ProposalState(r.body.json()["state"]), view=r.view, @@ -156,7 +162,7 @@ def ack(self, remote_node): def get_and_decrypt_recovery_share(self, remote_node): if not self.is_recovery_member: - raise ValueError(f"Member {self.member_id} does not have a recovery share") + raise ValueError(f"Member {self.local_id} does not have a recovery share") with remote_node.client(*self.auth()) as mc: r = mc.get("/gov/recovery_share") @@ -164,7 +170,7 @@ def get_and_decrypt_recovery_share(self, remote_node): raise NoRecoveryShareFound(r) with open( - os.path.join(self.common_dir, f"member{self.member_id}_enc_privk.pem"), + os.path.join(self.common_dir, f"{self.local_id}_enc_privk.pem"), "r", ) as priv_enc_key: return infra.crypto.unwrap_key_rsa_oaep( @@ -174,17 +180,17 @@ def get_and_decrypt_recovery_share(self, remote_node): def get_and_submit_recovery_share(self, remote_node): if not self.is_recovery_member: - raise ValueError(f"Member {self.member_id} does not have a recovery share") + raise ValueError(f"Member {self.local_id} does not have a recovery share") res = infra.proc.ccall( self.share_script, f"https://{remote_node.pubhost}:{remote_node.pubport}", "--member-enc-privk", - os.path.join(self.common_dir, f"member{self.member_id}_enc_privk.pem"), + os.path.join(self.common_dir, f"{self.local_id}_enc_privk.pem"), "--cert", - os.path.join(self.common_dir, f"member{self.member_id}_cert.pem"), + os.path.join(self.common_dir, f"{self.local_id}_cert.pem"), "--key", - os.path.join(self.common_dir, f"member{self.member_id}_privk.pem"), + os.path.join(self.common_dir, f"{self.local_id}_privk.pem"), "--cacert", os.path.join(self.common_dir, "networkcert.pem"), log_output=True, diff --git a/tests/infra/network.py b/tests/infra/network.py index 08af8b173b75..bfecbe913dfd 100644 --- a/tests/infra/network.py +++ b/tests/infra/network.py @@ -12,6 +12,7 @@ import infra.consortium from ccf.tx_status import TxStatus import random +from dataclasses import dataclass from math import ceil import http @@ -61,6 +62,12 @@ def get_common_folder_name(workspace, label): return os.path.join(workspace, f"{label}_{COMMON_FOLDER}") +@dataclass +class UserInfo: + local_id: int + service_id: str + + class Network: KEY_GEN = "keygenerator.sh" SHARE_SCRIPT = "submit_recovery_share.sh" @@ -116,7 +123,7 @@ def __init__( self.ignoring_shutdown_errors = False self.nodes = [] - self.user_ids = [] + self.users = [] self.hosts = hosts self.status = ServiceStatus.CLOSED self.binary_dir = binary_dir @@ -368,7 +375,9 @@ def start_and_join(self, args): args.participants_curve, authenticate_session=not args.disable_member_session_auth, ) - initial_users = list(range(max(0, args.initial_user_count))) + initial_users = [ + f"user{user_id}" for user_id in list(range(max(0, args.initial_user_count))) + ] self.create_users(initial_users, args.participants_curve) primary = self._start_all_nodes(args) @@ -621,22 +630,31 @@ def create_and_trust_node( return new_node - def create_user(self, user_id, curve, record=True): + def create_user(self, local_user_id, curve, record=True): infra.proc.ccall( self.key_generator, "--name", - f"user{user_id}", + local_user_id, "--curve", f"{curve.name}", path=self.common_dir, log_output=False, ).check_returncode() + + with open(os.path.join(self.common_dir, f"{local_user_id}_cert.pem")) as c: + service_user_id = infra.crypto.compute_cert_der_hash_hex_from_pem(c.read()) + new_user = UserInfo( + local_user_id, + service_user_id, + ) if record: - self.user_ids.append(user_id) + self.users.append(new_user) + + return new_user - def create_users(self, user_ids, curve): - for user_id in user_ids: - self.create_user(user_id, curve) + def create_users(self, local_user_ids, curve): + for local_user_id in local_user_ids: + self.create_user(local_user_id, curve) def get_members(self): return self.consortium.members @@ -849,7 +867,7 @@ def wait_for_commit_proof(self, node, seqno, timeout=3): current_commit_seqno = r.body.json()["seqno"] if current_commit_seqno >= seqno: with node.client( - f"member{self.consortium.get_any_active_member().member_id}" + self.consortium.get_any_active_member().local_id ) as nc: # Using update_state_digest here as a convenient write tx # that is app agnostic diff --git a/tests/js-modules/modules.py b/tests/js-modules/modules.py index 6f55a7e5259d..05b88b33dada 100644 --- a/tests/js-modules/modules.py +++ b/tests/js-modules/modules.py @@ -60,9 +60,7 @@ def test_module_set_and_remove(network, args): make_module_set_proposal(module_path, module_file_path, network) module_content = open(module_file_path, "r").read() - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post("/gov/read", {"table": "public:gov.modules", "key": module_path}) assert r.status_code == http.HTTPStatus.OK, r.status_code assert r.body.json()["js"] == module_content, r.body @@ -74,9 +72,7 @@ def test_module_set_and_remove(network, args): ) network.consortium.vote_using_majority(primary, proposal, careful_vote) - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post("/gov/read", {"table": "public:gov.modules", "key": module_path}) assert r.status_code == http.HTTPStatus.NOT_FOUND, r.status_code return network @@ -113,9 +109,7 @@ def test_app_bundle(network, args): network.consortium.deploy_js_app(primary, bundle_path) LOG.info("Verifying that modules and endpoints were added") - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post("/gov/read", {"table": "public:gov.modules", "key": "/math.js"}) assert r.status_code == http.HTTPStatus.OK, r.status_code @@ -146,9 +140,7 @@ def test_app_bundle(network, args): r = c.post("/app/compute", valid_body) assert r.status_code == http.HTTPStatus.NOT_FOUND, r.status_code - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post("/gov/read", {"table": "public:gov.modules", "key": "/math.js"}) assert r.status_code == http.HTTPStatus.NOT_FOUND, r.status_code diff --git a/tests/jwt_test.py b/tests/jwt_test.py index ad9fe271e6a0..9b308caf3ea6 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -80,9 +80,7 @@ def test_jwt_without_key_policy(network, args): network.consortium.set_jwt_public_signing_keys(primary, issuer, jwks_fp.name) LOG.info("Check if JWT signing key was stored correctly") - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.jwt.public_signing_keys", "key": kid} ) @@ -100,9 +98,7 @@ def test_jwt_without_key_policy(network, args): network.consortium.remove_jwt_issuer(primary, issuer) LOG.info("Check if JWT signing key was deleted") - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.jwt.public_signing_keys", "key": kid} ) @@ -115,9 +111,7 @@ def test_jwt_without_key_policy(network, args): network.consortium.set_jwt_issuer(primary, metadata_fp.name) LOG.info("Check if JWT signing key was stored correctly") - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.jwt.public_signing_keys", "key": kid} ) @@ -242,9 +236,7 @@ def test_jwt_with_sgx_key_filter(network, args): network.consortium.set_jwt_public_signing_keys(primary, issuer, jwks_fp.name) LOG.info("Check that only SGX cert was added") - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.jwt.public_signing_keys", "key": non_oe_kid}, @@ -316,9 +308,7 @@ def __exit__(self, exc_type, exc_value, traceback): def check_kv_jwt_key_matches(network, kid, cert_pem): primary, _ = network.find_nodes() - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/read", {"table": "public:ccf.gov.jwt.public_signing_keys", "key": kid}, @@ -339,9 +329,7 @@ def check_kv_jwt_key_matches(network, kid, cert_pem): def get_jwt_refresh_endpoint_metrics(network) -> dict: primary, _ = network.find_nodes() - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.get("/gov/api/metrics") m = next( v diff --git a/tests/memberclient.py b/tests/memberclient.py index 292de81e112f..2f32c46b4654 100644 --- a/tests/memberclient.py +++ b/tests/memberclient.py @@ -18,7 +18,7 @@ def test_missing_signature_header(network, args): node = network.find_node_by_role() member = network.consortium.get_any_active_member() - with node.client(f"member{member.member_id}") as mc: + with node.client(member.local_id) as mc: r = mc.post("/gov/proposals") assert r.status_code == http.HTTPStatus.UNAUTHORIZED, r.status_code www_auth = "www-authenticate" @@ -155,20 +155,20 @@ def test_governance(network, args): LOG.debug("Further vote requests fail as the proposal has already been accepted") params_error = http.HTTPStatus.BAD_REQUEST.value assert ( - network.consortium.get_member_by_id(0) + network.consortium.get_member_by_local_id("member0") .vote(node, new_member_proposal, careful_vote) .status_code == params_error ) assert ( - network.consortium.get_member_by_id(1) + network.consortium.get_member_by_local_id("member1") .vote(node, new_member_proposal, careful_vote) .status_code == params_error ) LOG.debug("Accepted proposal cannot be withdrawn") - response = network.consortium.get_member_by_id( + response = network.consortium.get_member_by_local_id( new_member_proposal.proposer_id ).withdraw(node, new_member_proposal) assert response.status_code == params_error @@ -204,7 +204,9 @@ def test_governance(network, args): proposal = new_member.propose(node, proposal_recovery_threshold) LOG.debug("Other members (non proposer) are unable to withdraw new proposal") - response = network.consortium.get_member_by_id(1).withdraw(node, proposal) + response = network.consortium.get_member_by_local_id("member1").withdraw( + node, proposal + ) assert response.status_code == http.HTTPStatus.FORBIDDEN.value LOG.debug("Proposer withdraws their proposal") diff --git a/tests/membership.py b/tests/membership.py index 50b09359c9ad..3a40bd98c6fc 100644 --- a/tests/membership.py +++ b/tests/membership.py @@ -136,7 +136,7 @@ def recovery_shares_scenario(args): # Members 0 and 1 are recovery members, member 2 isn't args.initial_member_count = 3 args.initial_recovery_member_count = 2 - non_recovery_member_id = 2 + non_recovery_member_id = "member2" # Recovery threshold is initially set to number of recovery members (2) with infra.network.network( @@ -155,11 +155,11 @@ def recovery_shares_scenario(args): LOG.info("Non-recovery member does not have a recovery share") primary, _ = network.find_primary() - with primary.client(f"member{non_recovery_member_id}") as mc: + with primary.client(non_recovery_member_id) as mc: r = mc.get("/gov/recovery_share") assert r.status_code == http.HTTPStatus.NOT_FOUND.value assert ( - f"Recovery share not found for member {non_recovery_member_id}" + f"Recovery share not found for member {network.consortium.get_member_by_local_id(non_recovery_member_id).service_id}" in r.body.json()["error"]["message"] ) @@ -174,7 +174,9 @@ def recovery_shares_scenario(args): # However, retiring a non-recovery member is allowed LOG.info("Retiring a non-recovery member is still possible") - member_to_retire = network.consortium.get_member_by_id(non_recovery_member_id) + member_to_retire = network.consortium.get_member_by_local_id( + non_recovery_member_id + ) test_retire_member(network, args, member_to_retire=member_to_retire) LOG.info("Adding one non-recovery member") diff --git a/tests/start_network.py b/tests/start_network.py index d42f3f4c0508..a929f2fa6270 100644 --- a/tests/start_network.py +++ b/tests/start_network.py @@ -54,10 +54,11 @@ def run(args): else: network.start_and_join(args) - primary, backups = network.find_nodes() + nodes = network.get_joined_nodes() + max_len = max([len(str(node.local_node_id)) for node in nodes]) # To be sure, confirm that the app frontend is open on each node - for node in [primary, *backups]: + for node in nodes: with node.client("user0") as c: if args.verbose: r = c.get("/app/commit") @@ -65,17 +66,14 @@ def run(args): r = c.get("/app/commit", log_capture=[]) assert r.status_code == http.HTTPStatus.OK, r.status_code - LOG.info("Started CCF network with the following nodes:") - LOG.info( - " Node [{}] = https://{}:{}".format( - primary.local_node_id, primary.pubhost, primary.pubport - ) - ) + def pad_node_id(nid): + return (f"{{:{max_len}d}}").format(nid) - for b in backups: + LOG.info("Started CCF network with the following nodes:") + for node in nodes: LOG.info( " Node [{}] = https://{}:{}".format( - b.local_node_id, b.pubhost, b.pubport + pad_node_id(node.local_node_id), node.pubhost, node.pubport ) ) diff --git a/tests/suite/test_requirements.py b/tests/suite/test_requirements.py index f6904bbd7c0c..a9a43ecdeaad 100644 --- a/tests/suite/test_requirements.py +++ b/tests/suite/test_requirements.py @@ -98,9 +98,7 @@ def check(network, args, *nargs, **kwargs): def can_kill_n_nodes(nodes_to_kill_count): def check(network, args, *nargs, **kwargs): primary, _ = network.find_primary() - with primary.client( - f"member{network.consortium.get_any_active_member().member_id}" - ) as c: + with primary.client(network.consortium.get_any_active_member().local_id) as c: r = c.post( "/gov/query", {