From 03826ff5032fa789c674a68f4c49c83d4d7d8078 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Fri, 13 Mar 2020 09:40:49 +0000 Subject: [PATCH 01/20] WIP --- src/apps/logging/logging.cpp | 13 +++++++++++++ src/apps/logging/logging_schema.h | 8 ++++++++ src/node/rpc/handlerregistry.h | 5 +++++ 3 files changed, 26 insertions(+) diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 7180d956f3f3..3d45faf5e45b 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -26,6 +26,9 @@ namespace ccfapp static constexpr auto LOG_GET_PUBLIC = "LOG_get_pub"; static constexpr auto LOG_RECORD_PREFIX_CERT = "LOG_record_prefix_cert"; + + static constexpr auto LOG_SELF_REGISTER_WITH_QUOTE = + "LOG_self_register_with_quote"; }; // SNIPPET: table_definition @@ -195,6 +198,10 @@ namespace ccfapp }; // SNIPPET_END: log_record_prefix_cert + auto self_register_with_quote = [this](RequestArgs& args) { + return make_error(HTTP_STATUS_BAD_REQUEST, "Not yet implemented"); + }; + install_with_auto_schema( Procs::LOG_RECORD, json_adapter(record), Write); // SNIPPET: install_get @@ -215,6 +222,12 @@ namespace ccfapp get_public_result_schema); install(Procs::LOG_RECORD_PREFIX_CERT, log_record_prefix_cert, Write); + install_with_auto_schema( + Procs::LOG_SELF_REGISTER_WITH_QUOTE, + json_adapter(self_register_with_quote), + Write, + false, // does not require client signature + false); // does not require valid caller nwt.signatures.set_global_hook([this, ¬ifier]( kv::Version version, diff --git a/src/apps/logging/logging_schema.h b/src/apps/logging/logging_schema.h index 58a39a1b948d..81cd15272882 100644 --- a/src/apps/logging/logging_schema.h +++ b/src/apps/logging/logging_schema.h @@ -30,6 +30,14 @@ namespace ccf }; }; + struct SelfRegister + { + struct In + { + std::vector quote; + }; + }; + DECLARE_JSON_TYPE(LoggingRecord::In); DECLARE_JSON_REQUIRED_FIELDS(LoggingRecord::In, id, msg); diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 129d38b8fb22..adc48b9df5bc 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -38,6 +38,7 @@ namespace ccf nlohmann::json params_schema; nlohmann::json result_schema; bool require_client_signature = false; + bool require_valid_caller = true; bool execute_locally = false; }; @@ -72,6 +73,7 @@ namespace ccf * @param params_schema JSON schema for params object in requests * @param result_schema JSON schema for result object in responses * @param require_client_signature If true, client request must be signed + * @param require_valid_caller If true, caller must be a known user * @param execute_locally If true, request is executed without consensus * (PBFT only) */ @@ -89,6 +91,7 @@ namespace ccf params_schema, result_schema, require_client_signature, + require_valid_caller, execute_locally}; } @@ -113,6 +116,7 @@ namespace ccf F&& f, ReadWrite rw, bool require_client_signature = false, + bool require_valid_caller = true, bool execute_locally = false) { auto params_schema = nlohmann::json::object(); @@ -134,6 +138,7 @@ namespace ccf params_schema, result_schema, require_client_signature, + require_valid_caller, execute_locally); } From da617b924f13a5cbe42051d5cd6535c4f68e35d3 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Mon, 16 Mar 2020 12:45:05 +0000 Subject: [PATCH 02/20] WIP2 --- src/node/rpc/frontend.h | 71 +++++++++++++++++++++-------------- src/node/rpc/memberfrontend.h | 8 ++-- tests/governance_history.py | 7 ++++ 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 85da5d7b04cd..5773d00d2184 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -236,29 +236,29 @@ namespace ccf return ctx->serialise_response(); } - const auto signed_request = ctx->get_signed_request(); - if (signed_request.has_value()) - { - if ( - !ctx->is_create_request && - !verify_client_signature( - ctx->session->caller_cert, - caller_id.value(), - signed_request.value())) - { - set_response_unauthorized(ctx); - return ctx->serialise_response(); - } - - // Client signature is only recorded on the primary - if ( - consensus == nullptr || consensus->is_primary() || - ctx->is_create_request) - { - record_client_signature( - tx, caller_id.value(), signed_request.value()); - } - } + // const auto signed_request = ctx->get_signed_request(); + // if (signed_request.has_value()) + // { + // if ( + // !ctx->is_create_request && + // !verify_client_signature( + // ctx->session->caller_cert, + // caller_id.value(), + // signed_request.value())) + // { + // set_response_unauthorized(ctx); + // return ctx->serialise_response(); + // } + + // // Client signature is only recorded on the primary + // if ( + // consensus == nullptr || consensus->is_primary() || + // ctx->is_create_request) + // { + // record_client_signature( + // tx, caller_id.value(), signed_request.value()); + // } + // } if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { @@ -454,13 +454,26 @@ namespace ccf return ctx->serialise_response(); } - if ( - handler->require_client_signature && - !ctx->get_signed_request().has_value()) + if (handler->require_client_signature) { - set_response_unauthorized( - ctx, fmt::format("'{}' RPC must be signed", method)); - return ctx->serialise_response(); + const auto signed_request = ctx->get_signed_request(); + if (!signed_request.has_value()) + { + set_response_unauthorized( + ctx, fmt::format("'{}' RPC must be signed by client", method)); + return ctx->serialise_response(); + } + else + { + if ( + !ctx->is_create_request && + !verify_client_signature( + ctx->session->caller_cert, caller_id, signed_request.value())) + { + set_response_unauthorized(ctx); + return ctx->serialise_response(); + } + } } update_history(); diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index 38c3ca690553..61d2bd44ae61 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -429,7 +429,7 @@ namespace ccf return false; } - void record_voting_history( + void record_governance_history( Store::Tx& tx, CallerId caller_id, const SignedReq& signed_request) { auto governance_history = tx.get_view(network.governance_history); @@ -525,7 +525,7 @@ namespace ccf proposal.votes[args.caller_id] = in.ballot; proposals->put(proposal_id, proposal); - record_voting_history( + record_governance_history( args.tx, args.caller_id, args.rpc_ctx->get_signed_request().value()); return make_success( @@ -577,7 +577,7 @@ namespace ccf proposal->state = ProposalState::WITHDRAWN; proposals->put(proposal_id, proposal.value()); - record_voting_history( + record_governance_history( args.tx, args.caller_id, args.rpc_ctx->get_signed_request().value()); return make_success(get_proposal_info(proposal_id, proposal.value())); @@ -622,7 +622,7 @@ namespace ccf proposal->votes[args.caller_id] = vote.ballot; proposals->put(vote.id, proposal.value()); - record_voting_history( + record_governance_history( args.tx, args.caller_id, args.rpc_ctx->get_signed_request().value()); return make_success( diff --git a/tests/governance_history.py b/tests/governance_history.py index ce5182b52569..63a28586feb4 100644 --- a/tests/governance_history.py +++ b/tests/governance_history.py @@ -37,6 +37,13 @@ def count_governance_operations(ledger): req = signed_request[0][1] request_body = signed_request[0][2] digest = signed_request[0][3] + LOG.warning(f"cert: {cert}") + LOG.warning(f"sig: {sig}") + LOG.warning(f"req: {req}") + LOG.warning(f"body: {request_body}") + LOG.warning(f"digest: {digest}") + + infra.crypto.verify_request_sig(cert, sig, req, request_body, digest) if "members/propose" in req.decode(): verified_proposals += 1 From 0673984c1884e8b928d35ece671b0303b05cc1db Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Mon, 16 Mar 2020 14:04:14 +0000 Subject: [PATCH 03/20] Revert "WIP2" This reverts commit da617b924f13a5cbe42051d5cd6535c4f68e35d3. --- src/node/rpc/frontend.h | 71 ++++++++++++++--------------------- src/node/rpc/memberfrontend.h | 8 ++-- tests/governance_history.py | 7 ---- 3 files changed, 33 insertions(+), 53 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 5773d00d2184..85da5d7b04cd 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -236,29 +236,29 @@ namespace ccf return ctx->serialise_response(); } - // const auto signed_request = ctx->get_signed_request(); - // if (signed_request.has_value()) - // { - // if ( - // !ctx->is_create_request && - // !verify_client_signature( - // ctx->session->caller_cert, - // caller_id.value(), - // signed_request.value())) - // { - // set_response_unauthorized(ctx); - // return ctx->serialise_response(); - // } - - // // Client signature is only recorded on the primary - // if ( - // consensus == nullptr || consensus->is_primary() || - // ctx->is_create_request) - // { - // record_client_signature( - // tx, caller_id.value(), signed_request.value()); - // } - // } + const auto signed_request = ctx->get_signed_request(); + if (signed_request.has_value()) + { + if ( + !ctx->is_create_request && + !verify_client_signature( + ctx->session->caller_cert, + caller_id.value(), + signed_request.value())) + { + set_response_unauthorized(ctx); + return ctx->serialise_response(); + } + + // Client signature is only recorded on the primary + if ( + consensus == nullptr || consensus->is_primary() || + ctx->is_create_request) + { + record_client_signature( + tx, caller_id.value(), signed_request.value()); + } + } if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { @@ -454,26 +454,13 @@ namespace ccf return ctx->serialise_response(); } - if (handler->require_client_signature) + if ( + handler->require_client_signature && + !ctx->get_signed_request().has_value()) { - const auto signed_request = ctx->get_signed_request(); - if (!signed_request.has_value()) - { - set_response_unauthorized( - ctx, fmt::format("'{}' RPC must be signed by client", method)); - return ctx->serialise_response(); - } - else - { - if ( - !ctx->is_create_request && - !verify_client_signature( - ctx->session->caller_cert, caller_id, signed_request.value())) - { - set_response_unauthorized(ctx); - return ctx->serialise_response(); - } - } + set_response_unauthorized( + ctx, fmt::format("'{}' RPC must be signed", method)); + return ctx->serialise_response(); } update_history(); diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index 61d2bd44ae61..38c3ca690553 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -429,7 +429,7 @@ namespace ccf return false; } - void record_governance_history( + void record_voting_history( Store::Tx& tx, CallerId caller_id, const SignedReq& signed_request) { auto governance_history = tx.get_view(network.governance_history); @@ -525,7 +525,7 @@ namespace ccf proposal.votes[args.caller_id] = in.ballot; proposals->put(proposal_id, proposal); - record_governance_history( + record_voting_history( args.tx, args.caller_id, args.rpc_ctx->get_signed_request().value()); return make_success( @@ -577,7 +577,7 @@ namespace ccf proposal->state = ProposalState::WITHDRAWN; proposals->put(proposal_id, proposal.value()); - record_governance_history( + record_voting_history( args.tx, args.caller_id, args.rpc_ctx->get_signed_request().value()); return make_success(get_proposal_info(proposal_id, proposal.value())); @@ -622,7 +622,7 @@ namespace ccf proposal->votes[args.caller_id] = vote.ballot; proposals->put(vote.id, proposal.value()); - record_governance_history( + record_voting_history( args.tx, args.caller_id, args.rpc_ctx->get_signed_request().value()); return make_success( diff --git a/tests/governance_history.py b/tests/governance_history.py index 63a28586feb4..ce5182b52569 100644 --- a/tests/governance_history.py +++ b/tests/governance_history.py @@ -37,13 +37,6 @@ def count_governance_operations(ledger): req = signed_request[0][1] request_body = signed_request[0][2] digest = signed_request[0][3] - LOG.warning(f"cert: {cert}") - LOG.warning(f"sig: {sig}") - LOG.warning(f"req: {req}") - LOG.warning(f"body: {request_body}") - LOG.warning(f"digest: {digest}") - - infra.crypto.verify_request_sig(cert, sig, req, request_body, digest) if "members/propose" in req.decode(): verified_proposals += 1 From 506f5d8a4938d6ffa067d957e08e02eaf1a9bb47 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Mon, 16 Mar 2020 17:08:41 +0000 Subject: [PATCH 04/20] Signature works --- src/node/rpc/commonhandlerregistry.h | 1 + src/node/rpc/frontend.h | 151 +++++++++++++++++---------- src/node/rpc/handlerregistry.h | 7 +- src/node/rpc/memberfrontend.h | 2 +- src/node/rpc/test/frontend_test.cpp | 132 ++++++++++++++--------- 5 files changed, 181 insertions(+), 112 deletions(-) diff --git a/src/node/rpc/commonhandlerregistry.h b/src/node/rpc/commonhandlerregistry.h index f1bce715f19b..ceedee6599a0 100644 --- a/src/node/rpc/commonhandlerregistry.h +++ b/src/node/rpc/commonhandlerregistry.h @@ -244,6 +244,7 @@ namespace ccf json_adapter(get_metrics), Read, false, // does not require client signature + true, // requires valid caller true); // executed locally install_with_auto_schema( GeneralProcs::MK_SIGN, json_adapter(make_signature), Write); diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 85da5d7b04cd..49909ad31f8d 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -218,51 +218,51 @@ namespace ccf Store::Tx tx; - // Retrieve id of caller + // // Retrieve id of caller std::optional caller_id; - if (ctx->is_create_request) - { - caller_id = INVALID_ID; - } - else - { - caller_id = handlers.valid_caller(tx, ctx->session->caller_cert); - } - - if (!caller_id.has_value()) - { - ctx->set_response_status(HTTP_STATUS_FORBIDDEN); - ctx->set_response_body(invalid_caller_error_message()); - return ctx->serialise_response(); - } - - const auto signed_request = ctx->get_signed_request(); - if (signed_request.has_value()) - { - if ( - !ctx->is_create_request && - !verify_client_signature( - ctx->session->caller_cert, - caller_id.value(), - signed_request.value())) - { - set_response_unauthorized(ctx); - return ctx->serialise_response(); - } - - // Client signature is only recorded on the primary - if ( - consensus == nullptr || consensus->is_primary() || - ctx->is_create_request) - { - record_client_signature( - tx, caller_id.value(), signed_request.value()); - } - } + // if (ctx->is_create_request) + // { + // caller_id = INVALID_ID; + // } + // else + // { + // caller_id = handlers.valid_caller(tx, ctx->session->caller_cert); + // } + + // if (!caller_id.has_value()) + // { + // ctx->set_response_status(HTTP_STATUS_FORBIDDEN); + // ctx->set_response_body(invalid_caller_error_message()); + // return ctx->serialise_response(); + // } + + // const auto signed_request = ctx->get_signed_request(); + // if (signed_request.has_value()) + // { + // if ( + // !ctx->is_create_request && + // !verify_client_signature( + // ctx->session->caller_cert, + // caller_id.value(), + // signed_request.value())) + // { + // set_response_unauthorized(ctx); + // return ctx->serialise_response(); + // } + + // // Client signature is only recorded on the primary + // if ( + // consensus == nullptr || consensus->is_primary() || + // ctx->is_create_request) + // { + // record_client_signature( + // tx, caller_id.value(), signed_request.value()); + // } + // } if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { - auto rep = process_if_local_node_rpc(ctx, tx, caller_id.value()); + auto rep = process_if_local_node_rpc(ctx, tx); if (rep.has_value()) { return rep.value(); @@ -302,7 +302,7 @@ namespace ccf } else { - auto rep = process_command(ctx, tx, caller_id.value()); + auto rep = process_command(ctx, tx); // If necessary, forward the RPC to the current primary if (!rep.has_value()) @@ -426,16 +426,14 @@ namespace ccf } std::optional process_if_local_node_rpc( - std::shared_ptr ctx, - Store::Tx& tx, - CallerId caller_id) + std::shared_ptr ctx, Store::Tx& tx) { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); auto handler = handlers.find_handler(local_method); if (handler != nullptr && handler->execute_locally) { - return process_command(ctx, tx, caller_id); + return process_command(ctx, tx); } return std::nullopt; } @@ -443,7 +441,7 @@ namespace ccf std::optional> process_command( std::shared_ptr ctx, Store::Tx& tx, - CallerId caller_id) + CallerId caller_id_ = 0) { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); @@ -454,19 +452,58 @@ namespace ccf return ctx->serialise_response(); } - if ( - handler->require_client_signature && - !ctx->get_signed_request().has_value()) + bool is_primary = (consensus == nullptr) || consensus->is_primary() || + ctx->is_create_request; + + std::optional caller_id; + if (handler->require_valid_caller) { - set_response_unauthorized( - ctx, fmt::format("'{}' RPC must be signed", method)); - return ctx->serialise_response(); + caller_id = handlers.valid_caller(tx, ctx->session->caller_cert); + + if (!caller_id.has_value()) + { + ctx->set_response_status(HTTP_STATUS_FORBIDDEN); + ctx->set_response_body(invalid_caller_error_message()); + return ctx->serialise_response(); + } + } + else + { + caller_id = INVALID_ID; } - update_history(); + if (handler->require_client_signature) + { + const auto signed_request = ctx->get_signed_request(); + if (signed_request.has_value()) + { + if ( + !ctx->is_create_request && + !verify_client_signature( + ctx->session->caller_cert, + caller_id.value(), + signed_request.value())) + { + set_response_unauthorized(ctx); + return ctx->serialise_response(); + } - bool is_primary = (consensus == nullptr) || consensus->is_primary() || - ctx->is_create_request; + // Client signature is only recorded on the primary + if (is_primary) + { + record_client_signature( + tx, caller_id.value(), signed_request.value()); + } + } + else + { + set_response_unauthorized( + ctx, fmt::format("'{}' RPC must be signed", method)); + return ctx->serialise_response(); + } + } + + update_history(); if (!is_primary && consensus->type() == ConsensusType::RAFT) { @@ -506,7 +543,7 @@ namespace ccf } auto func = handler->func; - auto args = RequestArgs{ctx, tx, caller_id}; + auto args = RequestArgs{ctx, tx, caller_id.value()}; tx_count++; diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index adc48b9df5bc..2be28ceadda8 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -84,6 +84,7 @@ namespace ccf const nlohmann::json& params_schema = nlohmann::json::object(), const nlohmann::json& result_schema = nlohmann::json::object(), bool require_client_signature = false, + bool require_valid_caller = true, bool execute_locally = false) { handlers[method] = {f, @@ -99,7 +100,8 @@ namespace ccf const std::string& method, HandleFunction f, ReadWrite rw, - bool require_client_signature) + bool require_client_signature, + bool require_valid_caller = true) { install( method, @@ -107,7 +109,8 @@ namespace ccf rw, nlohmann::json::object(), nlohmann::json::object(), - require_client_signature); + require_client_signature, + require_valid_caller); } template diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index 38c3ca690553..15877f0ca01f 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -878,7 +878,7 @@ namespace ccf LOG_INFO_FMT("Created service"); return make_success(true); }; - install(MemberProcs::CREATE, json_adapter(create), Write); + install(MemberProcs::CREATE, json_adapter(create), Write, true, false); } }; diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 481c4024b59d..1275eaf2250c 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -54,6 +54,16 @@ class TestUserFrontend : public SimpleUserRpcFrontend empty_function_signed, HandlerRegistry::Read, true); + + auto empty_function_no_auth = [this](RequestArgs& args) { + args.rpc_ctx->set_response_status(HTTP_STATUS_OK); + }; + install( + "empty_function_no_auth", + empty_function_no_auth, + HandlerRegistry::Read, + false, + true); } }; @@ -67,7 +77,7 @@ class TestReqNotStoredFrontend : public SimpleUserRpcFrontend auto empty_function = [this](RequestArgs& args) { args.rpc_ctx->set_response_status(HTTP_STATUS_OK); }; - install("empty_function", empty_function, HandlerRegistry::Read); + install("empty_function", empty_function, HandlerRegistry::Read, true); disable_request_storing(); } }; @@ -443,12 +453,10 @@ TEST_CASE("process_command") REQUIRE(response.status == HTTP_STATUS_OK); } -TEST_CASE("process") +TEST_CASE("process with signatures") { prepare_callers(); TestUserFrontend frontend(*network.tables); - const auto simple_call = create_simple_request(); - const auto [signed_call, signed_req] = create_signed_request(simple_call); SUBCASE("missing rpc") { @@ -462,41 +470,88 @@ TEST_CASE("process") REQUIRE(response.status == HTTP_STATUS_NOT_FOUND); } - SUBCASE("without signature") + SUBCASE("handler does not require signature") { - const auto serialized_call = simple_call.build_request(); - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); + const auto simple_call = create_simple_request(); + const auto [signed_call, signed_req] = create_signed_request(simple_call); + const auto serialized_simple_call = simple_call.build_request(); + const auto serialized_signed_call = signed_call.build_request(); + + auto simple_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_simple_call); + auto signed_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_signed_call); - const auto serialized_response = frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - REQUIRE(response.status == HTTP_STATUS_OK); + INFO("Unsigned RPC"); + { + const auto serialized_response = frontend.process(simple_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_OK); - auto signed_resp = get_signed_req(user_id); - CHECK(!signed_resp.has_value()); + auto signed_resp = get_signed_req(user_id); + CHECK(!signed_resp.has_value()); + } + + INFO("Signed RPC"); + { + const auto serialized_response = frontend.process(signed_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_OK); + + auto signed_resp = get_signed_req(user_id); + CHECK(!signed_resp.has_value()); + } } - SUBCASE("with signature") + SUBCASE("handler requires signature") { - const auto serialized_call = signed_call.build_request(); - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); + const auto simple_call = create_simple_request("empty_function_signed"); + const auto [signed_call, signed_req] = create_signed_request(simple_call); + const auto serialized_simple_call = simple_call.build_request(); + const auto serialized_signed_call = signed_call.build_request(); + + auto simple_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_simple_call); + auto signed_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_signed_call); - const auto serialized_response = frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - REQUIRE(response.status == HTTP_STATUS_OK); + INFO("Unsigned RPC"); + { + const auto serialized_response = frontend.process(simple_rpc_ctx).value(); + auto response = parse_response(serialized_response); - auto signed_resp = get_signed_req(user_id); - REQUIRE(signed_resp.has_value()); - auto value = signed_resp.value(); - CHECK(value == signed_req); + CHECK(response.status == HTTP_STATUS_UNAUTHORIZED); + const std::string error_msg(response.body.begin(), response.body.end()); + CHECK(error_msg.find("RPC must be signed") != std::string::npos); + + auto signed_resp = get_signed_req(user_id); + CHECK(!signed_resp.has_value()); + } + + INFO("Signed RPC"); + { + const auto serialized_response = frontend.process(signed_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_OK); + + auto signed_resp = get_signed_req(user_id); + REQUIRE(signed_resp.has_value()); + auto value = signed_resp.value(); + CHECK(value == signed_req); + } } SUBCASE("request with signature but do not store") { TestReqNotStoredFrontend frontend_nostore(*network.tables); - const auto serialized_call = signed_call.build_request(); - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); + const auto simple_call = create_simple_request("empty_function"); + const auto [signed_call, signed_req] = create_signed_request(simple_call); + const auto serialized_signed_call = signed_call.build_request(); + auto signed_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_signed_call); - const auto serialized_response = frontend_nostore.process(rpc_ctx).value(); + const auto serialized_response = + frontend_nostore.process(signed_rpc_ctx).value(); const auto response = parse_response(serialized_response); REQUIRE(response.status == HTTP_STATUS_OK); @@ -506,33 +561,6 @@ TEST_CASE("process") CHECK(value.req.empty()); CHECK(value.sig == signed_req.sig); } - - SUBCASE("request without signature on sign-only handler") - { - const auto unsigned_call = create_simple_request("empty_function_signed"); - const auto serialized_call = unsigned_call.build_request(); - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); - - const auto serialized_response = frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - - CHECK(response.status == HTTP_STATUS_UNAUTHORIZED); - const std::string error_msg(response.body.begin(), response.body.end()); - CHECK(error_msg.find("RPC must be signed") != std::string::npos); - } - - SUBCASE("request with signature on sign-only handler") - { - const auto unsigned_call = create_simple_request("empty_function_signed"); - const auto [signed_call, signed_req] = create_signed_request(unsigned_call); - const auto serialized_call = signed_call.build_request(); - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); - - const auto serialized_response = frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - - CHECK(response.status == HTTP_STATUS_OK); - } } TEST_CASE("MinimalHandleFunction") From 957d0a7b019a3c45f0e0ec3db1d0bc1e3ab2fa72 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 17 Mar 2020 10:42:24 +0000 Subject: [PATCH 05/20] Caller works with unit test --- src/node/rpc/frontend.h | 47 ++----------------- src/node/rpc/test/frontend_test.cpp | 72 +++++++++++++++++++++++++++-- 2 files changed, 74 insertions(+), 45 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 49909ad31f8d..74c16d9c8906 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -218,50 +218,10 @@ namespace ccf Store::Tx tx; - // // Retrieve id of caller - std::optional caller_id; - // if (ctx->is_create_request) - // { - // caller_id = INVALID_ID; - // } - // else - // { - // caller_id = handlers.valid_caller(tx, ctx->session->caller_cert); - // } - - // if (!caller_id.has_value()) - // { - // ctx->set_response_status(HTTP_STATUS_FORBIDDEN); - // ctx->set_response_body(invalid_caller_error_message()); - // return ctx->serialise_response(); - // } - - // const auto signed_request = ctx->get_signed_request(); - // if (signed_request.has_value()) - // { - // if ( - // !ctx->is_create_request && - // !verify_client_signature( - // ctx->session->caller_cert, - // caller_id.value(), - // signed_request.value())) - // { - // set_response_unauthorized(ctx); - // return ctx->serialise_response(); - // } - - // // Client signature is only recorded on the primary - // if ( - // consensus == nullptr || consensus->is_primary() || - // ctx->is_create_request) - // { - // record_client_signature( - // tx, caller_id.value(), signed_request.value()); - // } - // } - if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { + // TODO: Fix this + std::optional caller_id; auto rep = process_if_local_node_rpc(ctx, tx); if (rep.has_value()) { @@ -304,6 +264,9 @@ namespace ccf { auto rep = process_command(ctx, tx); + // TODO: Fix this + std::optional caller_id; + // If necessary, forward the RPC to the current primary if (!rep.has_value()) { diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 1275eaf2250c..3698636c1954 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -62,8 +62,8 @@ class TestUserFrontend : public SimpleUserRpcFrontend "empty_function_no_auth", empty_function_no_auth, HandlerRegistry::Read, - false, - true); + false, // does not require signature + false); // does not require valid caller } }; @@ -358,7 +358,7 @@ void prepare_callers() GenesisGenerator g(network, tx); g.init_values(); user_id = g.add_user(user_caller); - invalid_user_id = g.add_user(invalid_caller); + // invalid_user_id = g.add_user(invalid_caller); nos_id = g.add_user(nos_caller); member_id = g.add_member(member_caller, dummy_key_share); invalid_member_id = g.add_member(invalid_caller, dummy_key_share); @@ -498,6 +498,7 @@ TEST_CASE("process with signatures") auto response = parse_response(serialized_response); REQUIRE(response.status == HTTP_STATUS_OK); + // Even though the RPC is signed, the client signature is not recorded auto signed_resp = get_signed_req(user_id); CHECK(!signed_resp.has_value()); } @@ -563,6 +564,71 @@ TEST_CASE("process with signatures") } } +TEST_CASE("process with caller") +{ + prepare_callers(); + TestUserFrontend frontend(*network.tables); + + SUBCASE("handler does not require valid caller") + { + const auto simple_call = create_simple_request("empty_function_no_auth"); + const auto serialized_simple_call = simple_call.build_request(); + auto authenticated_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_simple_call); + auto invalid_rpc_ctx = + enclave::make_rpc_context(invalid_session, serialized_simple_call); + + INFO("Valid authentication"); + { + const auto serialized_response = + frontend.process(authenticated_rpc_ctx).value(); + auto response = parse_response(serialized_response); + + // Even though the RPC does not require authenticated caller, an + // authenticated RPC succeeds + REQUIRE(response.status == HTTP_STATUS_OK); + } + + INFO("Invalid authentication"); + { + const auto serialized_response = + frontend.process(invalid_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_OK); + } + } + + SUBCASE("handler requires valid caller") + { + const auto simple_call = create_simple_request("empty_function"); + const auto serialized_simple_call = simple_call.build_request(); + auto authenticated_rpc_ctx = + enclave::make_rpc_context(user_session, serialized_simple_call); + auto invalid_rpc_ctx = + enclave::make_rpc_context(invalid_session, serialized_simple_call); + + INFO("Valid authentication"); + { + const auto serialized_response = + frontend.process(authenticated_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_OK); + } + + INFO("Invalid authentication"); + { + const auto serialized_response = + frontend.process(invalid_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_FORBIDDEN); + const std::string error_msg(response.body.begin(), response.body.end()); + CHECK( + error_msg.find("Could not find matching user certificate") != + std::string::npos); + } + } +} + TEST_CASE("MinimalHandleFunction") { prepare_callers(); From 817629f56a89a98f3185d8655003e8b4ca7e02f3 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 17 Mar 2020 11:41:13 +0000 Subject: [PATCH 06/20] Store signed requests by default --- src/node/rpc/frontend.h | 49 ++++++++++++++--------------- src/node/rpc/test/frontend_test.cpp | 5 +-- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 74c16d9c8906..fa24d4546c75 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -404,7 +404,7 @@ namespace ccf std::optional> process_command( std::shared_ptr ctx, Store::Tx& tx, - CallerId caller_id_ = 0) + CallerId caller_id_ = 0) // TODO: Remove? { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); @@ -435,35 +435,34 @@ namespace ccf caller_id = INVALID_ID; } - if (handler->require_client_signature) + const auto signed_request = ctx->get_signed_request(); + if (handler->require_client_signature && !signed_request.has_value()) { - const auto signed_request = ctx->get_signed_request(); - if (signed_request.has_value()) - { - if ( - !ctx->is_create_request && - !verify_client_signature( - ctx->session->caller_cert, - caller_id.value(), - signed_request.value())) - { - set_response_unauthorized(ctx); - return ctx->serialise_response(); - } + set_response_unauthorized( + ctx, fmt::format("'{}' RPC must be signed", method)); + return ctx->serialise_response(); + } - // Client signature is only recorded on the primary - if (is_primary) - { - record_client_signature( - tx, caller_id.value(), signed_request.value()); - } - } - else + // By default, signed requests are verified and recorded, even on handlers + // that do not require client signatures + if (signed_request.has_value()) + { + if ( + !ctx->is_create_request && + !verify_client_signature( + ctx->session->caller_cert, + caller_id.value(), + signed_request.value())) { - set_response_unauthorized( - ctx, fmt::format("'{}' RPC must be signed", method)); + set_response_unauthorized(ctx); return ctx->serialise_response(); } + + if (is_primary) + { + record_client_signature( + tx, caller_id.value(), signed_request.value()); + } } update_history(); diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 3698636c1954..c1bb85e451cd 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -498,9 +498,10 @@ TEST_CASE("process with signatures") auto response = parse_response(serialized_response); REQUIRE(response.status == HTTP_STATUS_OK); - // Even though the RPC is signed, the client signature is not recorded auto signed_resp = get_signed_req(user_id); - CHECK(!signed_resp.has_value()); + REQUIRE(signed_resp.has_value()); + auto value = signed_resp.value(); + CHECK(value == signed_req); } } From faf7b272114753329626938e9a83cbab610046ec Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 17 Mar 2020 14:49:59 +0000 Subject: [PATCH 07/20] More test coverage and handling --- src/node/rpc/frontend.h | 54 +++++++++++++++++++---------- src/node/rpc/handlerregistry.h | 10 +++++- src/node/rpc/test/frontend_test.cpp | 36 ++++++++++++++++--- 3 files changed, 76 insertions(+), 24 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index fa24d4546c75..64b79a734401 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -218,11 +218,18 @@ namespace ccf Store::Tx tx; + auto caller_id = handlers.get_caller_id(tx, ctx->session->caller_cert); + if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { - // TODO: Fix this - std::optional caller_id; - auto rep = process_if_local_node_rpc(ctx, tx); + if (!caller_id.has_value()) + { + ctx->set_response_status(HTTP_STATUS_FORBIDDEN); + ctx->set_response_body(invalid_caller_error_message()); + return ctx->serialise_response(); + } + + auto rep = process_if_local_node_rpc(ctx, tx, caller_id); if (rep.has_value()) { return rep.value(); @@ -262,10 +269,7 @@ namespace ccf } else { - auto rep = process_command(ctx, tx); - - // TODO: Fix this - std::optional caller_id; + auto rep = process_command(ctx, tx, caller_id); // If necessary, forward the RPC to the current primary if (!rep.has_value()) @@ -389,14 +393,16 @@ namespace ccf } std::optional process_if_local_node_rpc( - std::shared_ptr ctx, Store::Tx& tx) + std::shared_ptr ctx, + Store::Tx& tx, + std::optional caller_id) { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); auto handler = handlers.find_handler(local_method); if (handler != nullptr && handler->execute_locally) { - return process_command(ctx, tx); + return process_command(ctx, tx, caller_id); } return std::nullopt; } @@ -404,7 +410,7 @@ namespace ccf std::optional> process_command( std::shared_ptr ctx, Store::Tx& tx, - CallerId caller_id_ = 0) // TODO: Remove? + std::optional caller_id) { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); @@ -418,21 +424,31 @@ namespace ccf bool is_primary = (consensus == nullptr) || consensus->is_primary() || ctx->is_create_request; - std::optional caller_id; - if (handler->require_valid_caller) + // If the frontend has a caller table (e.g. userfrontend): + // - if the caller is unknown (caller_id = {}) + // ==> if the handler requires auth, fails + // ==> if the handler does not require auth, continue with caller_id = + // INVALID_ID + // + // If the frontend has no caller table (e.g. nodefrontend): + // - if the caller is anonymous (caller_id = INVALID_ID) + // ==> if the handler requires auth, impossible + // ==> if the handler does not require auth, continue + + if (!caller_id.has_value()) { - caller_id = handlers.valid_caller(tx, ctx->session->caller_cert); - - if (!caller_id.has_value()) + if (handler->require_valid_caller) { ctx->set_response_status(HTTP_STATUS_FORBIDDEN); ctx->set_response_body(invalid_caller_error_message()); return ctx->serialise_response(); } - } - else - { - caller_id = INVALID_ID; + else + { + // Even though the caller is unknown, proceed with an invalid caller + // id as the handler does not require a valid caller + caller_id = INVALID_ID; + } } const auto signed_request = ctx->get_signed_request(); diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 2be28ceadda8..31122e6e476f 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -87,6 +87,14 @@ namespace ccf bool require_valid_caller = true, bool execute_locally = false) { + if (require_valid_caller && certs == nullptr) + { + throw std::logic_error(fmt::format( + "{} handler requires valid caller but registry does not have " + "certificates table", + method)); + } + handlers[method] = {f, rw, params_schema, @@ -198,7 +206,7 @@ namespace ccf virtual void tick(std::chrono::milliseconds elapsed, size_t tx_count) {} - virtual std::optional valid_caller( + virtual std::optional get_caller_id( Store::Tx& tx, const std::vector& caller) { if (certs == nullptr) diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index c1bb85e451cd..1a7fab02ec88 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -321,6 +321,8 @@ auto kp_other = tls::make_key_pair(); auto invalid_caller = kp_other -> self_sign("CN=name"); auto invalid_caller_der = tls::make_verifier(invalid_caller) -> der_cert_data(); +auto anonymous_caller_der = std::vector(); + std::vector dummy_key_share = {1, 2, 3}; auto user_session = make_shared( @@ -331,6 +333,8 @@ auto invalid_session = make_shared( enclave::InvalidSessionId, invalid_caller_der); auto member_session = make_shared( enclave::InvalidSessionId, member_caller_der); +auto anonymous_session = make_shared( + enclave::InvalidSessionId, anonymous_caller_der); UserId user_id = INVALID_ID; UserId invalid_user_id = INVALID_ID; @@ -578,8 +582,10 @@ TEST_CASE("process with caller") enclave::make_rpc_context(user_session, serialized_simple_call); auto invalid_rpc_ctx = enclave::make_rpc_context(invalid_session, serialized_simple_call); + auto anonymous_rpc_ctx = + enclave::make_rpc_context(anonymous_session, serialized_simple_call); - INFO("Valid authentication"); + INFO("Valid authentication"); // (2) { const auto serialized_response = frontend.process(authenticated_rpc_ctx).value(); @@ -590,13 +596,21 @@ TEST_CASE("process with caller") REQUIRE(response.status == HTTP_STATUS_OK); } - INFO("Invalid authentication"); + INFO("Invalid authentication"); // (4) { const auto serialized_response = frontend.process(invalid_rpc_ctx).value(); auto response = parse_response(serialized_response); REQUIRE(response.status == HTTP_STATUS_OK); } + + INFO("Anonymous caller"); // (10) + { + const auto serialized_response = + frontend.process(anonymous_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_OK); + } } SUBCASE("handler requires valid caller") @@ -607,8 +621,10 @@ TEST_CASE("process with caller") enclave::make_rpc_context(user_session, serialized_simple_call); auto invalid_rpc_ctx = enclave::make_rpc_context(invalid_session, serialized_simple_call); + auto anonymous_rpc_ctx = + enclave::make_rpc_context(anonymous_session, serialized_simple_call); - INFO("Valid authentication"); + INFO("Valid authentication"); // (1) { const auto serialized_response = frontend.process(authenticated_rpc_ctx).value(); @@ -616,7 +632,7 @@ TEST_CASE("process with caller") REQUIRE(response.status == HTTP_STATUS_OK); } - INFO("Invalid authentication"); + INFO("Invalid authentication"); // (3) { const auto serialized_response = frontend.process(invalid_rpc_ctx).value(); @@ -627,6 +643,18 @@ TEST_CASE("process with caller") error_msg.find("Could not find matching user certificate") != std::string::npos); } + + INFO("Anonymous caller"); // (9) + { + const auto serialized_response = + frontend.process(anonymous_rpc_ctx).value(); + auto response = parse_response(serialized_response); + REQUIRE(response.status == HTTP_STATUS_FORBIDDEN); + const std::string error_msg(response.body.begin(), response.body.end()); + CHECK( + error_msg.find("Could not find matching user certificate") != + std::string::npos); + } } } From 9ffdc593008a30eab3e4af6b8e4c644bda052d3c Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 17 Mar 2020 16:25:09 +0000 Subject: [PATCH 08/20] Cleaning up --- src/apps/logging/logging.cpp | 10 --- src/apps/logging/logging_schema.h | 8 -- src/node/rpc/commonhandlerregistry.h | 2 +- src/node/rpc/frontend.h | 13 +-- src/node/rpc/handlerregistry.h | 29 +++--- src/node/rpc/memberfrontend.h | 2 +- src/node/rpc/test/frontend_test.cpp | 129 ++++++++++++--------------- 7 files changed, 77 insertions(+), 116 deletions(-) diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 3d45faf5e45b..7ed37ef853e5 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -198,10 +198,6 @@ namespace ccfapp }; // SNIPPET_END: log_record_prefix_cert - auto self_register_with_quote = [this](RequestArgs& args) { - return make_error(HTTP_STATUS_BAD_REQUEST, "Not yet implemented"); - }; - install_with_auto_schema( Procs::LOG_RECORD, json_adapter(record), Write); // SNIPPET: install_get @@ -222,12 +218,6 @@ namespace ccfapp get_public_result_schema); install(Procs::LOG_RECORD_PREFIX_CERT, log_record_prefix_cert, Write); - install_with_auto_schema( - Procs::LOG_SELF_REGISTER_WITH_QUOTE, - json_adapter(self_register_with_quote), - Write, - false, // does not require client signature - false); // does not require valid caller nwt.signatures.set_global_hook([this, ¬ifier]( kv::Version version, diff --git a/src/apps/logging/logging_schema.h b/src/apps/logging/logging_schema.h index 81cd15272882..58a39a1b948d 100644 --- a/src/apps/logging/logging_schema.h +++ b/src/apps/logging/logging_schema.h @@ -30,14 +30,6 @@ namespace ccf }; }; - struct SelfRegister - { - struct In - { - std::vector quote; - }; - }; - DECLARE_JSON_TYPE(LoggingRecord::In); DECLARE_JSON_REQUIRED_FIELDS(LoggingRecord::In, id, msg); diff --git a/src/node/rpc/commonhandlerregistry.h b/src/node/rpc/commonhandlerregistry.h index ceedee6599a0..14e0b72b3ed9 100644 --- a/src/node/rpc/commonhandlerregistry.h +++ b/src/node/rpc/commonhandlerregistry.h @@ -244,7 +244,7 @@ namespace ccf json_adapter(get_metrics), Read, false, // does not require client signature - true, // requires valid caller + false, // requires valid caller true); // executed locally install_with_auto_schema( GeneralProcs::MK_SIGN, json_adapter(make_signature), Write); diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 64b79a734401..75717ce240cb 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -424,20 +424,9 @@ namespace ccf bool is_primary = (consensus == nullptr) || consensus->is_primary() || ctx->is_create_request; - // If the frontend has a caller table (e.g. userfrontend): - // - if the caller is unknown (caller_id = {}) - // ==> if the handler requires auth, fails - // ==> if the handler does not require auth, continue with caller_id = - // INVALID_ID - // - // If the frontend has no caller table (e.g. nodefrontend): - // - if the caller is anonymous (caller_id = INVALID_ID) - // ==> if the handler requires auth, impossible - // ==> if the handler does not require auth, continue - if (!caller_id.has_value()) { - if (handler->require_valid_caller) + if (!handler->disable_caller_auth) { ctx->set_response_status(HTTP_STATUS_FORBIDDEN); ctx->set_response_body(invalid_caller_error_message()); diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 31122e6e476f..17c9ff1ea56d 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -38,7 +38,7 @@ namespace ccf nlohmann::json params_schema; nlohmann::json result_schema; bool require_client_signature = false; - bool require_valid_caller = true; + bool disable_caller_auth = false; bool execute_locally = false; }; @@ -73,7 +73,7 @@ namespace ccf * @param params_schema JSON schema for params object in requests * @param result_schema JSON schema for result object in responses * @param require_client_signature If true, client request must be signed - * @param require_valid_caller If true, caller must be a known user + * @param disable_caller_auth If true, any caller can execute this handler * @param execute_locally If true, request is executed without consensus * (PBFT only) */ @@ -84,15 +84,18 @@ namespace ccf const nlohmann::json& params_schema = nlohmann::json::object(), const nlohmann::json& result_schema = nlohmann::json::object(), bool require_client_signature = false, - bool require_valid_caller = true, + bool disable_caller_auth = false, bool execute_locally = false) { - if (require_valid_caller && certs == nullptr) + if (disable_caller_auth && certs == nullptr) { - throw std::logic_error(fmt::format( - "{} handler requires valid caller but registry does not have " - "certificates table", - method)); + std::cout << "Install failed: " << method << std::endl; + LOG_FAIL_FMT( + "Failed to disable caller auth on {} handler as its registry does " + "not have certificate table. Defaulting to not requiring valid " + "caller.", + method); + disable_caller_auth = false; } handlers[method] = {f, @@ -100,7 +103,7 @@ namespace ccf params_schema, result_schema, require_client_signature, - require_valid_caller, + disable_caller_auth, execute_locally}; } @@ -109,7 +112,7 @@ namespace ccf HandleFunction f, ReadWrite rw, bool require_client_signature, - bool require_valid_caller = true) + bool disable_caller_auth = false) { install( method, @@ -118,7 +121,7 @@ namespace ccf nlohmann::json::object(), nlohmann::json::object(), require_client_signature, - require_valid_caller); + disable_caller_auth); } template @@ -127,7 +130,7 @@ namespace ccf F&& f, ReadWrite rw, bool require_client_signature = false, - bool require_valid_caller = true, + bool disable_caller_auth = false, bool execute_locally = false) { auto params_schema = nlohmann::json::object(); @@ -149,7 +152,7 @@ namespace ccf params_schema, result_schema, require_client_signature, - require_valid_caller, + disable_caller_auth, execute_locally); } diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index 15877f0ca01f..f1d96aefb5ca 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -878,7 +878,7 @@ namespace ccf LOG_INFO_FMT("Created service"); return make_success(true); }; - install(MemberProcs::CREATE, json_adapter(create), Write, true, false); + install(MemberProcs::CREATE, json_adapter(create), Write, true, true); } }; diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 1a7fab02ec88..e005d47b1094 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -63,7 +63,7 @@ class TestUserFrontend : public SimpleUserRpcFrontend empty_function_no_auth, HandlerRegistry::Read, false, // does not require signature - false); // does not require valid caller + true); // disable valid caller } }; @@ -149,6 +149,9 @@ class TestNoCertsFrontend : public RpcFrontend args.rpc_ctx->set_response_status(HTTP_STATUS_OK); }; handlers.install("empty_function", empty_function, HandlerRegistry::Read); + // false, // no client signature + // false // does not require valid caller (since no certs) + // ); } }; @@ -658,6 +661,60 @@ TEST_CASE("process with caller") } } +TEST_CASE("No certs table") +{ + prepare_callers(); + TestNoCertsFrontend frontend(*network.tables); + auto simple_call = create_simple_request(); + std::vector serialized_call = simple_call.build_request(); + + INFO("Authenticated caller"); // (6) + { + auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); + std::vector serialized_response = + frontend.process(rpc_ctx).value(); + auto response = parse_response(serialized_response); + CHECK(response.status == HTTP_STATUS_OK); + } + + INFO("Anonymous caller"); // (12) + { + auto rpc_ctx = + enclave::make_rpc_context(anonymous_session, serialized_call); + std::vector serialized_response = + frontend.process(rpc_ctx).value(); + auto response = parse_response(serialized_response); + CHECK(response.status == HTTP_STATUS_OK); + } +} + +TEST_CASE("Member caller") +{ + prepare_callers(); + auto simple_call = create_simple_request(); + std::vector serialized_call = simple_call.build_request(); + TestMemberFrontend frontend(network, stub_node); + + SUBCASE("valid caller") + { + auto member_rpc_ctx = + enclave::make_rpc_context(member_session, serialized_call); + std::vector serialized_response = + frontend.process(member_rpc_ctx).value(); + auto response = parse_response(serialized_response); + CHECK(response.status == HTTP_STATUS_OK); + } + + SUBCASE("invalid caller") + { + auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); + std::vector serialized_response = + frontend.process(rpc_ctx).value(); + auto response = parse_response(serialized_response); + CHECK(response.status == HTTP_STATUS_FORBIDDEN); + } +} + TEST_CASE("MinimalHandleFunction") { prepare_callers(); @@ -759,76 +816,6 @@ TEST_CASE("MinimalHandleFunction") } } -// callers - -TEST_CASE("User caller") -{ - prepare_callers(); - auto simple_call = create_simple_request(); - std::vector serialized_call = simple_call.build_request(); - TestUserFrontend frontend(*network.tables); - - SUBCASE("valid caller") - { - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); - std::vector serialized_response = - frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - CHECK(response.status == HTTP_STATUS_OK); - } - - SUBCASE("invalid caller") - { - auto member_rpc_ctx = - enclave::make_rpc_context(member_session, serialized_call); - std::vector serialized_response = - frontend.process(member_rpc_ctx).value(); - auto response = parse_response(serialized_response); - CHECK(response.status == HTTP_STATUS_FORBIDDEN); - } -} - -TEST_CASE("Member caller") -{ - prepare_callers(); - auto simple_call = create_simple_request(); - std::vector serialized_call = simple_call.build_request(); - TestMemberFrontend frontend(network, stub_node); - - SUBCASE("valid caller") - { - auto member_rpc_ctx = - enclave::make_rpc_context(member_session, serialized_call); - std::vector serialized_response = - frontend.process(member_rpc_ctx).value(); - auto response = parse_response(serialized_response); - CHECK(response.status == HTTP_STATUS_OK); - } - - SUBCASE("invalid caller") - { - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); - std::vector serialized_response = - frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - CHECK(response.status == HTTP_STATUS_FORBIDDEN); - } -} - -TEST_CASE("No certs table") -{ - prepare_callers(); - - auto simple_call = create_simple_request(); - std::vector serialized_call = simple_call.build_request(); - TestNoCertsFrontend frontend(*network.tables); - - auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); - std::vector serialized_response = frontend.process(rpc_ctx).value(); - auto response = parse_response(serialized_response); - CHECK(response.status == HTTP_STATUS_OK); -} - TEST_CASE("Signed read requests can be executed on backup") { prepare_callers(); From 3180d51e0a718c9f4d0241d49ec6f65f63b493da Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 17 Mar 2020 16:57:42 +0000 Subject: [PATCH 09/20] e2e test on logging app --- src/apps/logging/logging.cpp | 46 ++++++++++++++++++++++-- src/node/rpc/handlerregistry.h | 1 - src/node/rpc/test/frontend_test.cpp | 4 --- tests/e2e_logging.py | 54 +++++++++++++++++++++++------ 4 files changed, 87 insertions(+), 18 deletions(-) diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 7ed37ef853e5..d98ab06cada3 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -26,9 +26,7 @@ namespace ccfapp static constexpr auto LOG_GET_PUBLIC = "LOG_get_pub"; static constexpr auto LOG_RECORD_PREFIX_CERT = "LOG_record_prefix_cert"; - - static constexpr auto LOG_SELF_REGISTER_WITH_QUOTE = - "LOG_self_register_with_quote"; + static constexpr auto LOG_RECORD_ANONYMOUS_CALLER = "LOG_record_anonymous"; }; // SNIPPET: table_definition @@ -198,6 +196,42 @@ namespace ccfapp }; // SNIPPET_END: log_record_prefix_cert + auto log_record_anonymous = [this](RequestArgs& args) { + const auto& cert_data = args.rpc_ctx->session->caller_cert; + + const auto body_j = + nlohmann::json::parse(args.rpc_ctx->get_request_body()); + + if (args.caller_id != INVALID_ID) + { + args.rpc_ctx->set_response_status(HTTP_STATUS_BAD_REQUEST); + args.rpc_ctx->set_response_header( + http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT); + args.rpc_ctx->set_response_body( + "Only anonymous callers can record anonymous messages"); + return; + } + + const auto in = body_j.get(); + if (in.msg.empty()) + { + args.rpc_ctx->set_response_status(HTTP_STATUS_BAD_REQUEST); + args.rpc_ctx->set_response_header( + http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT); + args.rpc_ctx->set_response_body("Cannot record an empty log message"); + return; + } + + const auto log_line = fmt::format("Anonymous: {}", in.msg); + auto view = args.tx.get_view(records); + view->put(in.id, log_line); + + args.rpc_ctx->set_response_status(HTTP_STATUS_OK); + args.rpc_ctx->set_response_header( + http::headers::CONTENT_TYPE, http::headervalues::contenttype::JSON); + args.rpc_ctx->set_response_body(nlohmann::json(true).dump()); + }; + install_with_auto_schema( Procs::LOG_RECORD, json_adapter(record), Write); // SNIPPET: install_get @@ -218,6 +252,12 @@ namespace ccfapp get_public_result_schema); install(Procs::LOG_RECORD_PREFIX_CERT, log_record_prefix_cert, Write); + install( + Procs::LOG_RECORD_ANONYMOUS_CALLER, + log_record_anonymous, + Write, + false, + true); nwt.signatures.set_global_hook([this, ¬ifier]( kv::Version version, diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 17c9ff1ea56d..aa3e7d9082fa 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -89,7 +89,6 @@ namespace ccf { if (disable_caller_auth && certs == nullptr) { - std::cout << "Install failed: " << method << std::endl; LOG_FAIL_FMT( "Failed to disable caller auth on {} handler as its registry does " "not have certificate table. Defaulting to not requiring valid " diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index e005d47b1094..c392c940f41f 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -149,9 +149,6 @@ class TestNoCertsFrontend : public RpcFrontend args.rpc_ctx->set_response_status(HTTP_STATUS_OK); }; handlers.install("empty_function", empty_function, HandlerRegistry::Read); - // false, // no client signature - // false // does not require valid caller (since no certs) - // ); } }; @@ -365,7 +362,6 @@ void prepare_callers() GenesisGenerator g(network, tx); g.init_values(); user_id = g.add_user(user_caller); - // invalid_user_id = g.add_user(invalid_caller); nos_id = g.add_user(nos_caller); member_id = g.add_member(member_caller, dummy_key_share); invalid_member_id = g.add_member(invalid_caller, dummy_key_share); diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index 348d595721c5..fdb8173adfea 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -73,6 +73,39 @@ def test_cert_prefix(network, args): return network +@reqs.description("Write as anonymous caller") +@reqs.supports_methods("LOG_record_prefix_cert", "LOG_get") +def test_anonymous_caller(network, args): + if args.package == "liblogging": + primary, _ = network.find_primary() + + # Create a new user but do not record its identity in CCF + network.create_users([4], args.default_curve) + + log_id = 101 + msg = "This message is anonymous" + with primary.user_client(user_id=4) as c: + r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) + assert r.result == True + r = c.rpc("LOG_get", {"id": log_id}) + assert ( + r.error is not None + ), "Anonymous user is not authorised to call LOG_get" + + with primary.user_client(user_id=0) as c: + r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) + assert ( + r.error is not None + ), "Only anonymous users are authorised to call LOG_record_anonymous" + r = c.rpc("LOG_get", {"id": log_id}) + assert r.result is not None + assert f"Anonymous: {msg}" in r.result["msg"] + else: + LOG.warning("Skipping test_cert_prefix as application is not C++") + + return network + + @reqs.description("Testing forwarding on member and node frontends") @reqs.supports_methods("mkSign") @reqs.at_least_n_nodes(2) @@ -150,16 +183,17 @@ def run(args): hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb, ) as network: network.start_and_join(args) - network = test( - network, - args, - notifications_queue, - verify=args.package is not "libjsgeneric", - ) - network = test_large_messages(network, args) - network = test_forwarding_frontends(network, args) - network = test_update_lua(network, args) - network = test_cert_prefix(network, args) + # network = test( + # network, + # args, + # notifications_queue, + # verify=args.package is not "libjsgeneric", + # ) + # network = test_large_messages(network, args) + # network = test_forwarding_frontends(network, args) + # network = test_update_lua(network, args) + # network = test_cert_prefix(network, args) + network = test_anonymous_caller(network, args) if __name__ == "__main__": From 7e8b499172662fb23ec89fd035a6e568a9c0535d Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Tue, 17 Mar 2020 17:23:05 +0000 Subject: [PATCH 10/20] We need a test for forwarding --- src/node/rpc/frontend.h | 4 ++-- src/node/rpc/handlerregistry.h | 20 ++++++++++---------- tests/e2e_logging.py | 18 +++++++++--------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 75717ce240cb..bccb3e702c13 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -426,7 +426,7 @@ namespace ccf if (!caller_id.has_value()) { - if (!handler->disable_caller_auth) + if (!handler->caller_auth_disabled) { ctx->set_response_status(HTTP_STATUS_FORBIDDEN); ctx->set_response_body(invalid_caller_error_message()); @@ -434,7 +434,7 @@ namespace ccf } else { - // Even though the caller is unknown, proceed with an invalid caller + // Even though the caller is unknown, proceed with the invalid caller // id as the handler does not require a valid caller caller_id = INVALID_ID; } diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index aa3e7d9082fa..d32a7b37a6a8 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -38,7 +38,7 @@ namespace ccf nlohmann::json params_schema; nlohmann::json result_schema; bool require_client_signature = false; - bool disable_caller_auth = false; + bool caller_auth_disabled = false; bool execute_locally = false; }; @@ -73,7 +73,7 @@ namespace ccf * @param params_schema JSON schema for params object in requests * @param result_schema JSON schema for result object in responses * @param require_client_signature If true, client request must be signed - * @param disable_caller_auth If true, any caller can execute this handler + * @param caller_auth_disabled If true, any caller can execute this handler * @param execute_locally If true, request is executed without consensus * (PBFT only) */ @@ -84,17 +84,17 @@ namespace ccf const nlohmann::json& params_schema = nlohmann::json::object(), const nlohmann::json& result_schema = nlohmann::json::object(), bool require_client_signature = false, - bool disable_caller_auth = false, + bool caller_auth_disabled = false, bool execute_locally = false) { - if (disable_caller_auth && certs == nullptr) + if (caller_auth_disabled && certs == nullptr) { LOG_FAIL_FMT( "Failed to disable caller auth on {} handler as its registry does " "not have certificate table. Defaulting to not requiring valid " "caller.", method); - disable_caller_auth = false; + caller_auth_disabled = false; } handlers[method] = {f, @@ -102,7 +102,7 @@ namespace ccf params_schema, result_schema, require_client_signature, - disable_caller_auth, + caller_auth_disabled, execute_locally}; } @@ -111,7 +111,7 @@ namespace ccf HandleFunction f, ReadWrite rw, bool require_client_signature, - bool disable_caller_auth = false) + bool caller_auth_disabled = false) { install( method, @@ -120,7 +120,7 @@ namespace ccf nlohmann::json::object(), nlohmann::json::object(), require_client_signature, - disable_caller_auth); + caller_auth_disabled); } template @@ -129,7 +129,7 @@ namespace ccf F&& f, ReadWrite rw, bool require_client_signature = false, - bool disable_caller_auth = false, + bool caller_auth_disabled = false, bool execute_locally = false) { auto params_schema = nlohmann::json::object(); @@ -151,7 +151,7 @@ namespace ccf params_schema, result_schema, require_client_signature, - disable_caller_auth, + caller_auth_disabled, execute_locally); } diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index fdb8173adfea..8e03896301b8 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -77,7 +77,7 @@ def test_cert_prefix(network, args): @reqs.supports_methods("LOG_record_prefix_cert", "LOG_get") def test_anonymous_caller(network, args): if args.package == "liblogging": - primary, _ = network.find_primary() + other, primary = network.find_primary_and_any_backup() # Create a new user but do not record its identity in CCF network.create_users([4], args.default_curve) @@ -92,14 +92,14 @@ def test_anonymous_caller(network, args): r.error is not None ), "Anonymous user is not authorised to call LOG_get" - with primary.user_client(user_id=0) as c: - r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) - assert ( - r.error is not None - ), "Only anonymous users are authorised to call LOG_record_anonymous" - r = c.rpc("LOG_get", {"id": log_id}) - assert r.result is not None - assert f"Anonymous: {msg}" in r.result["msg"] + # with primary.user_client(user_id=0) as c: + # r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) + # assert ( + # r.error is not None + # ), "Only anonymous users are authorised to call LOG_record_anonymous" + # r = c.rpc("LOG_get", {"id": log_id}) + # assert r.result is not None + # assert f"Anonymous: {msg}" in r.result["msg"] else: LOG.warning("Skipping test_cert_prefix as application is not C++") From 8edc1c149c8509e912551c88effae8d6a4195387 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 12:30:42 +0000 Subject: [PATCH 11/20] Forwarding works --- src/enclave/rpccontext.h | 2 +- src/node/rpc/frontend.h | 70 +++++++++++++---------------- src/node/rpc/handlerregistry.h | 21 +++++---- src/node/rpc/test/frontend_test.cpp | 4 +- tests/e2e_logging.py | 2 +- 5 files changed, 47 insertions(+), 52 deletions(-) diff --git a/src/enclave/rpccontext.h b/src/enclave/rpccontext.h index 696dfcc5fb88..ff0ecfb44b3f 100644 --- a/src/enclave/rpccontext.h +++ b/src/enclave/rpccontext.h @@ -16,7 +16,7 @@ namespace enclave { size_t client_session_id = InvalidSessionId; std::vector caller_cert = {}; - bool is_forwarded = false; + bool forward = false; // // Only set in the case of a forwarded RPC diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 3e7dc6d58498..74c8363d0016 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -222,12 +222,12 @@ namespace ccf if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { - if (!caller_id.has_value()) - { - ctx->set_response_status(HTTP_STATUS_FORBIDDEN); - ctx->set_response_body(invalid_caller_error_message()); - return ctx->serialise_response(); - } + // if (!caller_id.has_value()) + // { + // ctx->set_response_status(HTTP_STATUS_FORBIDDEN); + // ctx->set_response_body(invalid_caller_error_message()); + // return ctx->serialise_response(); + // } auto rep = process_if_local_node_rpc(ctx, tx, caller_id); if (rep.has_value()) @@ -237,14 +237,13 @@ namespace ccf kv::TxHistory::RequestID reqid; update_history(); - reqid = {caller_id.value(), - ctx->session->client_session_id, - ctx->get_request_index()}; + reqid = { + caller_id, ctx->session->client_session_id, ctx->get_request_index()}; if (history) { if (!history->add_request( reqid, - caller_id.value(), + caller_id, ctx->session->caller_cert, ctx->get_serialised_request())) { @@ -281,7 +280,7 @@ namespace ccf if ( primary_id != NoNode && cmd_forwarder && cmd_forwarder->forward_command( - ctx, primary_id, caller_id.value(), get_cert_to_forward(ctx))) + ctx, primary_id, caller_id, get_cert_to_forward(ctx))) { // Indicate that the RPC has been forwarded to primary LOG_DEBUG_FMT("RPC forwarded to primary {}", primary_id); @@ -370,13 +369,7 @@ namespace ccf Store::Tx tx; - if (!lookup_forwarded_caller_cert(ctx, tx)) - { - ctx->set_response_status(HTTP_STATUS_FORBIDDEN); - ctx->set_response_body(invalid_caller_error_message()); - return ctx->serialise_response(); - } - + // TODO: This should go // Store client signature. It is assumed that the forwarder node has // already verified the client signature. const auto signed_request = ctx->get_signed_request(); @@ -400,7 +393,7 @@ namespace ccf std::optional process_if_local_node_rpc( std::shared_ptr ctx, Store::Tx& tx, - std::optional caller_id) + CallerId caller_id) { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); @@ -415,7 +408,7 @@ namespace ccf std::optional> process_command( std::shared_ptr ctx, Store::Tx& tx, - std::optional caller_id) + CallerId caller_id) { const auto method = ctx->get_method(); const auto local_method = method.substr(method.find_first_not_of('/')); @@ -429,20 +422,20 @@ namespace ccf bool is_primary = (consensus == nullptr) || consensus->is_primary() || ctx->is_create_request; - if (!caller_id.has_value()) + if (!handler->caller_auth_disabled && handlers.has_certs()) { - if (!handler->caller_auth_disabled) + // Only if handler requires auth. + // If a request is forwarded, check that the caller is known. Otherwise, + // only check that the caller id is valid. + if ( + (ctx->session->fwd.has_value() && + !lookup_forwarded_caller_cert(ctx, tx)) || + caller_id == INVALID_ID) { ctx->set_response_status(HTTP_STATUS_FORBIDDEN); ctx->set_response_body(invalid_caller_error_message()); return ctx->serialise_response(); } - else - { - // Even though the caller is unknown, proceed with the invalid caller - // id as the handler does not require a valid caller - caller_id = INVALID_ID; - } } const auto signed_request = ctx->get_signed_request(); @@ -453,16 +446,14 @@ namespace ccf return ctx->serialise_response(); } - // By default, signed requests are verified and recorded, even on handlers - // that do not require client signatures + // By default, signed requests are verified and recorded, even on + // handlers that do not require client signatures if (signed_request.has_value()) { if ( !ctx->is_create_request && !verify_client_signature( - ctx->session->caller_cert, - caller_id.value(), - signed_request.value())) + ctx->session->caller_cert, caller_id, signed_request.value())) { set_response_unauthorized(ctx); return ctx->serialise_response(); @@ -470,8 +461,7 @@ namespace ccf if (is_primary) { - record_client_signature( - tx, caller_id.value(), signed_request.value()); + record_client_signature(tx, caller_id, signed_request.value()); } } @@ -483,7 +473,7 @@ namespace ccf { case HandlerRegistry::Read: { - if (ctx->session->is_forwarded) + if (ctx->session->forward) { return forward_or_redirect_json(ctx); } @@ -492,7 +482,7 @@ namespace ccf case HandlerRegistry::Write: { - ctx->session->is_forwarded = true; + ctx->session->forward = true; return forward_or_redirect_json(ctx); } @@ -502,10 +492,10 @@ namespace ccf ctx->get_request_header(http::headers::CCF_READ_ONLY); if (!read_only_it.has_value() || (read_only_it.value() != "true")) { - ctx->session->is_forwarded = true; + ctx->session->forward = true; return forward_or_redirect_json(ctx); } - else if (ctx->session->is_forwarded) + else if (ctx->session->forward) { return forward_or_redirect_json(ctx); } @@ -515,7 +505,7 @@ namespace ccf } auto func = handler->func; - auto args = RequestArgs{ctx, tx, caller_id.value()}; + auto args = RequestArgs{ctx, tx, caller_id}; tx_count++; diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 4d684a1a9124..07db4a68cbc9 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -210,23 +210,28 @@ namespace ccf virtual void tick(std::chrono::milliseconds elapsed, size_t tx_count) {} - virtual std::optional get_caller_id( + bool has_certs() + { + return certs != nullptr; + } + + virtual CallerId get_caller_id( Store::Tx& tx, const std::vector& caller) { - if (certs == nullptr) + if (certs == nullptr || caller.empty()) { return INVALID_ID; } - if (caller.empty()) - { - return {}; - } - auto certs_view = tx.get_view(*certs); auto caller_id = certs_view->get(caller); - return caller_id; + if (!caller_id.has_value()) + { + return INVALID_ID; + } + + return caller_id.value(); } void set_consensus(kv::Consensus* c) diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index b26b51281f2d..ae42f4542533 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -860,7 +860,7 @@ TEST_CASE("Forwarding" * doctest::test_suite("forwarding")) } user_frontend_backup.set_cmd_forwarder(backup_forwarder); - backup_ctx->session->is_forwarded = false; + backup_ctx->session->forward = false; { INFO("Read command is not forwarded to primary"); @@ -971,7 +971,7 @@ TEST_CASE("Forwarding" * doctest::test_suite("forwarding")) // On a session that was previously forwarded, and is now primary, // commands should still succeed - ctx->session->is_forwarded = true; + ctx->session->forward = true; { INFO("Write command primary on a forwarded session succeeds"); REQUIRE(channel_stub->is_empty()); diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index 1dd447198dbd..10c559262106 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -76,7 +76,7 @@ def test_cert_prefix(network, args): @reqs.supports_methods("LOG_record_prefix_cert", "LOG_get") def test_anonymous_caller(network, args): if args.package == "liblogging": - primary, _ = network.find_primary_and_any_backup() + other, primary = network.find_primary_and_any_backup() # Create a new user but do not record its identity in CCF network.create_users([4], args.default_curve) From 618b49a7424fce3c3bf2ec888fe56fc59842ae75 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 14:04:52 +0000 Subject: [PATCH 12/20] Cleanup before PR --- src/node/rpc/frontend.h | 27 +++++-------------- src/node/rpc/handlerregistry.h | 2 +- src/node/rpc/test/frontend_test.cpp | 16 +++++------ tests/e2e_logging.py | 42 ++++++++++++++--------------- tests/infra/ccf.py | 22 ++++++++------- 5 files changed, 48 insertions(+), 61 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 74c8363d0016..8b83fcb59e5e 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -222,13 +222,6 @@ namespace ccf if (consensus != nullptr && consensus->type() == ConsensusType::PBFT) { - // if (!caller_id.has_value()) - // { - // ctx->set_response_status(HTTP_STATUS_FORBIDDEN); - // ctx->set_response_body(invalid_caller_error_message()); - // return ctx->serialise_response(); - // } - auto rep = process_if_local_node_rpc(ctx, tx, caller_id); if (rep.has_value()) { @@ -369,16 +362,6 @@ namespace ccf Store::Tx tx; - // TODO: This should go - // Store client signature. It is assumed that the forwarder node has - // already verified the client signature. - const auto signed_request = ctx->get_signed_request(); - if (signed_request.has_value()) - { - record_client_signature( - tx, ctx->session->fwd->caller_id, signed_request.value()); - } - auto rep = process_command(ctx, tx, ctx->session->fwd->caller_id); if (!rep.has_value()) { @@ -419,9 +402,6 @@ namespace ccf return ctx->serialise_response(); } - bool is_primary = (consensus == nullptr) || consensus->is_primary() || - ctx->is_create_request; - if (!handler->caller_auth_disabled && handlers.has_certs()) { // Only if handler requires auth. @@ -438,6 +418,9 @@ namespace ccf } } + bool is_primary = (consensus == nullptr) || consensus->is_primary() || + ctx->is_create_request; + const auto signed_request = ctx->get_signed_request(); if (handler->require_client_signature && !signed_request.has_value()) { @@ -450,8 +433,10 @@ namespace ccf // handlers that do not require client signatures if (signed_request.has_value()) { + // For forwarded requests, skip verification as it is assumed that the + // verification was done by the forwarder node. if ( - !ctx->is_create_request && + (!ctx->is_create_request && !ctx->session->fwd.has_value()) && !verify_client_signature( ctx->session->caller_cert, caller_id, signed_request.value())) { diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 07db4a68cbc9..1485f4ae3847 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -93,7 +93,7 @@ namespace ccf return *this; } - // If true, caller is not authenticated + // If true, caller does not need to be authenticated bool caller_auth_disabled = false; Handler& set_caller_auth_disabled(bool v) diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index ae42f4542533..41dd532b34eb 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -579,7 +579,7 @@ TEST_CASE("process with caller") auto anonymous_rpc_ctx = enclave::make_rpc_context(anonymous_session, serialized_simple_call); - INFO("Valid authentication"); // (2) + INFO("Valid authentication"); { const auto serialized_response = frontend.process(authenticated_rpc_ctx).value(); @@ -590,7 +590,7 @@ TEST_CASE("process with caller") REQUIRE(response.status == HTTP_STATUS_OK); } - INFO("Invalid authentication"); // (4) + INFO("Invalid authentication"); { const auto serialized_response = frontend.process(invalid_rpc_ctx).value(); @@ -598,7 +598,7 @@ TEST_CASE("process with caller") REQUIRE(response.status == HTTP_STATUS_OK); } - INFO("Anonymous caller"); // (10) + INFO("Anonymous caller"); { const auto serialized_response = frontend.process(anonymous_rpc_ctx).value(); @@ -618,7 +618,7 @@ TEST_CASE("process with caller") auto anonymous_rpc_ctx = enclave::make_rpc_context(anonymous_session, serialized_simple_call); - INFO("Valid authentication"); // (1) + INFO("Valid authentication"); { const auto serialized_response = frontend.process(authenticated_rpc_ctx).value(); @@ -626,7 +626,7 @@ TEST_CASE("process with caller") REQUIRE(response.status == HTTP_STATUS_OK); } - INFO("Invalid authentication"); // (3) + INFO("Invalid authentication"); { const auto serialized_response = frontend.process(invalid_rpc_ctx).value(); @@ -638,7 +638,7 @@ TEST_CASE("process with caller") std::string::npos); } - INFO("Anonymous caller"); // (9) + INFO("Anonymous caller"); { const auto serialized_response = frontend.process(anonymous_rpc_ctx).value(); @@ -659,7 +659,7 @@ TEST_CASE("No certs table") auto simple_call = create_simple_request(); std::vector serialized_call = simple_call.build_request(); - INFO("Authenticated caller"); // (6) + INFO("Authenticated caller"); { auto rpc_ctx = enclave::make_rpc_context(user_session, serialized_call); std::vector serialized_response = @@ -668,7 +668,7 @@ TEST_CASE("No certs table") CHECK(response.status == HTTP_STATUS_OK); } - INFO("Anonymous caller"); // (12) + INFO("Anonymous caller"); { auto rpc_ctx = enclave::make_rpc_context(anonymous_session, serialized_call); diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index 10c559262106..1281f4e9f440 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -76,10 +76,10 @@ def test_cert_prefix(network, args): @reqs.supports_methods("LOG_record_prefix_cert", "LOG_get") def test_anonymous_caller(network, args): if args.package == "liblogging": - other, primary = network.find_primary_and_any_backup() + primary, _ = network.find_primary() - # Create a new user but do not record its identity in CCF - network.create_users([4], args.default_curve) + # Create a new user but do not record its identity + network.create_user(4, args.default_curve) log_id = 101 msg = "This message is anonymous" @@ -91,14 +91,14 @@ def test_anonymous_caller(network, args): r.error is not None ), "Anonymous user is not authorised to call LOG_get" - # with primary.user_client(user_id=0) as c: - # r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) - # assert ( - # r.error is not None - # ), "Only anonymous users are authorised to call LOG_record_anonymous" - # r = c.rpc("LOG_get", {"id": log_id}) - # assert r.result is not None - # assert f"Anonymous: {msg}" in r.result["msg"] + with primary.user_client(user_id=0) as c: + r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) + assert ( + r.error is not None + ), "Only anonymous users are authorised to call LOG_record_anonymous" + r = c.rpc("LOG_get", {"id": log_id}) + assert r.result is not None + assert f"Anonymous: {msg}" in r.result["msg"] else: LOG.warning("Skipping test_cert_prefix as application is not C++") @@ -182,16 +182,16 @@ def run(args): hosts, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb, ) as network: network.start_and_join(args) - # network = test( - # network, - # args, - # notifications_queue, - # verify=args.package is not "libjsgeneric", - # ) - # network = test_large_messages(network, args) - # network = test_forwarding_frontends(network, args) - # network = test_update_lua(network, args) - # network = test_cert_prefix(network, args) + network = test( + network, + args, + notifications_queue, + verify=args.package is not "libjsgeneric", + ) + network = test_large_messages(network, args) + network = test_forwarding_frontends(network, args) + network = test_update_lua(network, args) + network = test_cert_prefix(network, args) network = test_anonymous_caller(network, args) diff --git a/tests/infra/ccf.py b/tests/infra/ccf.py index 3d941181a4f6..7c0cd8986a98 100644 --- a/tests/infra/ccf.py +++ b/tests/infra/ccf.py @@ -350,16 +350,18 @@ def create_and_trust_node(self, lib_name, host, args, target_node=None): return new_node - def create_users(self, users, curve): - users = ["user{}".format(u) for u in users] - for u in users: - infra.proc.ccall( - self.key_generator, - f"--name={u}", - f"--curve={curve.name}", - path=self.common_dir, - log_output=False, - ).check_returncode() + def create_user(self, user_id, curve): + infra.proc.ccall( + self.key_generator, + f"--name=user{user_id}", + f"--curve={curve.name}", + path=self.common_dir, + log_output=False, + ).check_returncode() + + def create_users(self, user_ids, curve): + for user_id in user_ids: + self.create_user(user_id, curve) def get_members(self): return self.consortium.members From 8cbfd349dbc95399c894f82d6ceb146c067e4387 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 14:22:15 +0000 Subject: [PATCH 13/20] Log message --- src/node/rpc/handlerregistry.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 1485f4ae3847..5b8c86d754be 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -100,9 +100,9 @@ namespace ccf { if (v && registry->certs == nullptr) { - LOG_FAIL_FMT( - "Failed to disable caller auth on {} handler as its registry does " - "not have certificate table (no effect).", + LOG_INFO_FMT( + "Disabling caller auth on {} handler has no effect since its " + "registry does not have certificates table", method); return *this; } From 3e21a2576bb117ef2a77088bea63dbbc983d37d9 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 15:51:55 +0000 Subject: [PATCH 14/20] Fix for sig perf test with PBFT --- src/node/rpc/frontend.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 8b83fcb59e5e..64c4b1040b38 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -433,10 +433,12 @@ namespace ccf // handlers that do not require client signatures if (signed_request.has_value()) { - // For forwarded requests, skip verification as it is assumed that the - // verification was done by the forwarder node. + // For forwarded requests (Raft only), skip verification as it is + // assumed that the verification was done by the forwarder node. if ( - (!ctx->is_create_request && !ctx->session->fwd.has_value()) && + (!ctx->is_create_request && + (!(consensus->type() == ConsensusType::RAFT) || + !ctx->session->fwd.has_value())) && !verify_client_signature( ctx->session->caller_cert, caller_id, signed_request.value())) { From 19d544b4c275044da5aada98faca99f095162054 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 16:13:36 +0000 Subject: [PATCH 15/20] Update consensus --- src/node/rpc/frontend.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 64c4b1040b38..21d26f1cc4c8 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -360,6 +360,8 @@ namespace ccf "Processing forwarded command with unitialised forwarded context"); } + update_consensus(); + Store::Tx tx; auto rep = process_command(ctx, tx, ctx->session->fwd->caller_id); From 38818d5f04da9317301ed3acb1e9f1def57c11e0 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 17:49:47 +0000 Subject: [PATCH 16/20] Fill null consensus --- src/node/rpc/frontend.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index 21d26f1cc4c8..be63fd5f81d9 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -435,11 +435,12 @@ namespace ccf // handlers that do not require client signatures if (signed_request.has_value()) { - // For forwarded requests (Raft only), skip verification as it is + // For forwarded requests (raft only), skip verification as it is // assumed that the verification was done by the forwarder node. if ( (!ctx->is_create_request && - (!(consensus->type() == ConsensusType::RAFT) || + (!(consensus != nullptr && + consensus->type() == ConsensusType::RAFT) || !ctx->session->fwd.has_value())) && !verify_client_signature( ctx->session->caller_cert, caller_id, signed_request.value())) From a11b69ba9c962f7a63f4b3d462fb20b990056486 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 17:51:33 +0000 Subject: [PATCH 17/20] Caller auth function rename --- src/apps/logging/logging.cpp | 2 +- src/node/rpc/handlerregistry.h | 2 +- src/node/rpc/memberfrontend.h | 2 +- src/node/rpc/test/frontend_test.cpp | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 12f2f98678df..feedc51cee7a 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -249,7 +249,7 @@ namespace ccfapp install(Procs::LOG_RECORD_PREFIX_CERT, log_record_prefix_cert, Write); install(Procs::LOG_RECORD_ANONYMOUS_CALLER, log_record_anonymous, Write) - .set_caller_auth_disabled(true); + .set_caller_auth(true); nwt.signatures.set_global_hook([this, ¬ifier]( kv::Version version, diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 5b8c86d754be..1cc098307f12 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -96,7 +96,7 @@ namespace ccf // If true, caller does not need to be authenticated bool caller_auth_disabled = false; - Handler& set_caller_auth_disabled(bool v) + Handler& set_caller_auth(bool v) { if (v && registry->certs == nullptr) { diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index 08a962bef6c3..ecc425e2376a 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -886,7 +886,7 @@ namespace ccf return make_success(true); }; install(MemberProcs::CREATE, json_adapter(create), Write) - .set_caller_auth_disabled(true); + .set_caller_auth(true); } }; diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 41dd532b34eb..2ca97eaf57a4 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -58,7 +58,7 @@ class TestUserFrontend : public SimpleUserRpcFrontend }; install( "empty_function_no_auth", empty_function_no_auth, HandlerRegistry::Read) - .set_caller_auth_disabled(true); + .set_caller_auth(true); } }; From 93eb8876d6ed19cfb31fcfd33e35f020859087a1 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Wed, 18 Mar 2020 18:38:16 +0000 Subject: [PATCH 18/20] Fix logging app --- src/apps/logging/logging.cpp | 63 +++++++++++++++------------------- src/node/rpc/handlerregistry.h | 2 +- 2 files changed, 28 insertions(+), 37 deletions(-) diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index feedc51cee7a..229b83f2cf9e 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -196,41 +196,28 @@ namespace ccfapp }; // SNIPPET_END: log_record_prefix_cert - auto log_record_anonymous = [this](RequestArgs& args) { - const auto& cert_data = args.rpc_ctx->session->caller_cert; - - const auto body_j = - nlohmann::json::parse(args.rpc_ctx->get_request_body()); - - if (args.caller_id != INVALID_ID) - { - args.rpc_ctx->set_response_status(HTTP_STATUS_BAD_REQUEST); - args.rpc_ctx->set_response_header( - http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT); - args.rpc_ctx->set_response_body( - "Only anonymous callers can record anonymous messages"); - return; - } - - const auto in = body_j.get(); - if (in.msg.empty()) - { - args.rpc_ctx->set_response_status(HTTP_STATUS_BAD_REQUEST); - args.rpc_ctx->set_response_header( - http::headers::CONTENT_TYPE, http::headervalues::contenttype::TEXT); - args.rpc_ctx->set_response_body("Cannot record an empty log message"); - return; - } - - const auto log_line = fmt::format("Anonymous: {}", in.msg); - auto view = args.tx.get_view(records); - view->put(in.id, log_line); - - args.rpc_ctx->set_response_status(HTTP_STATUS_OK); - args.rpc_ctx->set_response_header( - http::headers::CONTENT_TYPE, http::headervalues::contenttype::JSON); - args.rpc_ctx->set_response_body(nlohmann::json(true).dump()); - }; + auto log_record_anonymous = + [this](Store::Tx& tx, CallerId caller_id, nlohmann::json&& params) { + if (caller_id != INVALID_ID) + { + return make_error( + HTTP_STATUS_BAD_REQUEST, + "Only anonymous callers can record anonymous messages"); + } + + const auto in = params.get(); + + if (in.msg.empty()) + { + return make_error( + HTTP_STATUS_BAD_REQUEST, "Cannot record an empty log message"); + } + + const auto log_line = fmt::format("Anonymous: {}", in.msg); + auto view = tx.get_view(records); + view->put(in.id, log_line); + return make_success(true); + }; install(Procs::LOG_RECORD, json_adapter(record), Write) .set_auto_schema(); @@ -248,7 +235,11 @@ namespace ccfapp .set_result_schema(get_public_result_schema); install(Procs::LOG_RECORD_PREFIX_CERT, log_record_prefix_cert, Write); - install(Procs::LOG_RECORD_ANONYMOUS_CALLER, log_record_anonymous, Write) + install( + Procs::LOG_RECORD_ANONYMOUS_CALLER, + json_adapter(log_record_anonymous), + Write) + .set_auto_schema() .set_caller_auth(true); nwt.signatures.set_global_hook([this, ¬ifier]( diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index 1cc098307f12..d41f21aacd5c 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -98,7 +98,7 @@ namespace ccf Handler& set_caller_auth(bool v) { - if (v && registry->certs == nullptr) + if (v && registry != nullptr && !registry->has_certs()) { LOG_INFO_FMT( "Disabling caller auth on {} handler has no effect since its " From 68c363f956f433fae0d60549192f90eebfd940f3 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 19 Mar 2020 09:28:23 +0000 Subject: [PATCH 19/20] Add schema + tweaks frontend --- .../schemas/LOG_record_anonymous_params.json | 19 ++++++++++++ .../schemas/LOG_record_anonymous_result.json | 5 ++++ sphinx/source/users/rpc_api.rst | 1 + src/apps/logging/logging.cpp | 9 +----- src/enclave/rpccontext.h | 7 +++-- src/node/rpc/forwarder.h | 2 +- src/node/rpc/frontend.h | 30 ++++++++++--------- src/node/rpc/memberfrontend.h | 2 +- src/node/rpc/test/frontend_test.cpp | 4 +-- src/node/rpc/userfrontend.h | 2 +- tests/e2e_logging.py | 2 +- 11 files changed, 52 insertions(+), 31 deletions(-) create mode 100644 sphinx/source/schemas/LOG_record_anonymous_params.json create mode 100644 sphinx/source/schemas/LOG_record_anonymous_result.json diff --git a/sphinx/source/schemas/LOG_record_anonymous_params.json b/sphinx/source/schemas/LOG_record_anonymous_params.json new file mode 100644 index 000000000000..b8faf8fafbf5 --- /dev/null +++ b/sphinx/source/schemas/LOG_record_anonymous_params.json @@ -0,0 +1,19 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "properties": { + "id": { + "maximum": 18446744073709551615, + "minimum": 0, + "type": "number" + }, + "msg": { + "type": "string" + } + }, + "required": [ + "id", + "msg" + ], + "title": "LOG_record_anonymous/params", + "type": "object" +} \ No newline at end of file diff --git a/sphinx/source/schemas/LOG_record_anonymous_result.json b/sphinx/source/schemas/LOG_record_anonymous_result.json new file mode 100644 index 000000000000..214e41846f4c --- /dev/null +++ b/sphinx/source/schemas/LOG_record_anonymous_result.json @@ -0,0 +1,5 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "LOG_record_anonymous/result", + "type": "boolean" +} \ No newline at end of file diff --git a/sphinx/source/users/rpc_api.rst b/sphinx/source/users/rpc_api.rst index c1e8c787e643..5c27fe159bad 100644 --- a/sphinx/source/users/rpc_api.rst +++ b/sphinx/source/users/rpc_api.rst @@ -13,6 +13,7 @@ The API can also be retrieved from a running service using the `listMethods`_ an "LOG_get", "LOG_get_pub", "LOG_record", + "LOG_record_anonymous", "LOG_record_prefix_cert", "LOG_record_pub", "getCommit", diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 229b83f2cf9e..0e61ab641c51 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -197,14 +197,7 @@ namespace ccfapp // SNIPPET_END: log_record_prefix_cert auto log_record_anonymous = - [this](Store::Tx& tx, CallerId caller_id, nlohmann::json&& params) { - if (caller_id != INVALID_ID) - { - return make_error( - HTTP_STATUS_BAD_REQUEST, - "Only anonymous callers can record anonymous messages"); - } - + [this](Store::Tx& tx, nlohmann::json&& params) { const auto in = params.get(); if (in.msg.empty()) diff --git a/src/enclave/rpccontext.h b/src/enclave/rpccontext.h index ff0ecfb44b3f..90c23de1e947 100644 --- a/src/enclave/rpccontext.h +++ b/src/enclave/rpccontext.h @@ -16,7 +16,7 @@ namespace enclave { size_t client_session_id = InvalidSessionId; std::vector caller_cert = {}; - bool forward = false; + bool is_forwarding = false; // // Only set in the case of a forwarded RPC @@ -32,7 +32,7 @@ namespace enclave caller_id(caller_id_) {} }; - std::optional fwd = std::nullopt; + std::optional original_caller = std::nullopt; // Constructor used for non-forwarded RPC SessionContext( @@ -46,7 +46,8 @@ namespace enclave size_t fwd_session_id_, ccf::CallerId caller_id_, const std::vector& caller_cert_ = {}) : - fwd(std::make_optional(fwd_session_id_, caller_id_)), + original_caller( + std::make_optional(fwd_session_id_, caller_id_)), caller_cert(caller_cert_) {} }; diff --git a/src/node/rpc/forwarder.h b/src/node/rpc/forwarder.h index 86638cfc1f73..95e1c85fb215 100644 --- a/src/node/rpc/forwarder.h +++ b/src/node/rpc/forwarder.h @@ -208,7 +208,7 @@ namespace ccf } if (!send_forwarded_response( - ctx->session->fwd->client_session_id, + ctx->session->original_caller->client_session_id, from_node, fwd_handler->process_forwarded(ctx))) { diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index be63fd5f81d9..ab5638a25a32 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -73,10 +73,10 @@ namespace ccf handlers.set_history(history); } - std::optional forward_or_redirect_json( + std::optional> forward_or_redirect_json( std::shared_ptr ctx) { - if (cmd_forwarder && !ctx->session->fwd.has_value()) + if (cmd_forwarder && !ctx->session->original_caller.has_value()) { return std::nullopt; } @@ -225,7 +225,7 @@ namespace ccf auto rep = process_if_local_node_rpc(ctx, tx, caller_id); if (rep.has_value()) { - return rep.value(); + return rep; } kv::TxHistory::RequestID reqid; @@ -319,13 +319,14 @@ namespace ccf auto req_view = tx.get_view(*pbft_requests_map); req_view->put( 0, - {ctx->session->fwd.value().caller_id, + {ctx->session->original_caller.value().caller_id, ctx->session->caller_cert, ctx->get_serialised_request(), ctx->pbft_raw}); } - auto rep = process_command(ctx, tx, ctx->session->fwd->caller_id); + auto rep = + process_command(ctx, tx, ctx->session->original_caller->caller_id); version = tx.get_version(); @@ -354,7 +355,7 @@ namespace ccf std::vector process_forwarded( std::shared_ptr ctx) override { - if (!ctx->session->fwd.has_value()) + if (!ctx->session->original_caller.has_value()) { throw std::logic_error( "Processing forwarded command with unitialised forwarded context"); @@ -364,7 +365,8 @@ namespace ccf Store::Tx tx; - auto rep = process_command(ctx, tx, ctx->session->fwd->caller_id); + auto rep = + process_command(ctx, tx, ctx->session->original_caller->caller_id); if (!rep.has_value()) { // This should never be called when process_command is called with a @@ -375,7 +377,7 @@ namespace ccf return rep.value(); } - std::optional process_if_local_node_rpc( + std::optional> process_if_local_node_rpc( std::shared_ptr ctx, Store::Tx& tx, CallerId caller_id) @@ -410,7 +412,7 @@ namespace ccf // If a request is forwarded, check that the caller is known. Otherwise, // only check that the caller id is valid. if ( - (ctx->session->fwd.has_value() && + (ctx->session->original_caller.has_value() && !lookup_forwarded_caller_cert(ctx, tx)) || caller_id == INVALID_ID) { @@ -441,7 +443,7 @@ namespace ccf (!ctx->is_create_request && (!(consensus != nullptr && consensus->type() == ConsensusType::RAFT) || - !ctx->session->fwd.has_value())) && + !ctx->session->original_caller.has_value())) && !verify_client_signature( ctx->session->caller_cert, caller_id, signed_request.value())) { @@ -463,7 +465,7 @@ namespace ccf { case HandlerRegistry::Read: { - if (ctx->session->forward) + if (ctx->session->is_forwarding) { return forward_or_redirect_json(ctx); } @@ -472,7 +474,7 @@ namespace ccf case HandlerRegistry::Write: { - ctx->session->forward = true; + ctx->session->is_forwarding = true; return forward_or_redirect_json(ctx); } @@ -482,10 +484,10 @@ namespace ccf ctx->get_request_header(http::headers::CCF_READ_ONLY); if (!read_only_it.has_value() || (read_only_it.value() != "true")) { - ctx->session->forward = true; + ctx->session->is_forwarding = true; return forward_or_redirect_json(ctx); } - else if (ctx->session->forward) + else if (ctx->session->is_forwarding) { return forward_or_redirect_json(ctx); } diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index ecc425e2376a..c9071bf1ed2a 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -921,7 +921,7 @@ namespace ccf { // Lookup the caller member's certificate from the forwarded caller id auto members_view = tx.get_view(*members); - auto caller = members_view->get(ctx->session->fwd->caller_id); + auto caller = members_view->get(ctx->session->original_caller->caller_id); if (!caller.has_value()) { return false; diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index 2ca97eaf57a4..a41df09fbf7a 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -860,7 +860,7 @@ TEST_CASE("Forwarding" * doctest::test_suite("forwarding")) } user_frontend_backup.set_cmd_forwarder(backup_forwarder); - backup_ctx->session->forward = false; + backup_ctx->session->is_forwarding = false; { INFO("Read command is not forwarded to primary"); @@ -971,7 +971,7 @@ TEST_CASE("Forwarding" * doctest::test_suite("forwarding")) // On a session that was previously forwarded, and is now primary, // commands should still succeed - ctx->session->forward = true; + ctx->session->is_forwarding = true; { INFO("Write command primary on a forwarded session succeeds"); REQUIRE(channel_stub->is_empty()); diff --git a/src/node/rpc/userfrontend.h b/src/node/rpc/userfrontend.h index fca21922fccb..819485f71a78 100644 --- a/src/node/rpc/userfrontend.h +++ b/src/node/rpc/userfrontend.h @@ -44,7 +44,7 @@ namespace ccf { // Lookup the calling user's certificate from the forwarded caller id auto users_view = tx.get_view(*users); - auto caller = users_view->get(ctx->session->fwd->caller_id); + auto caller = users_view->get(ctx->session->original_caller->caller_id); if (!caller.has_value()) { return false; diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index 1281f4e9f440..6934fab1064b 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -98,7 +98,7 @@ def test_anonymous_caller(network, args): ), "Only anonymous users are authorised to call LOG_record_anonymous" r = c.rpc("LOG_get", {"id": log_id}) assert r.result is not None - assert f"Anonymous: {msg}" in r.result["msg"] + assert msg in r.result["msg"] else: LOG.warning("Skipping test_cert_prefix as application is not C++") From c9ae5fb9980fae5c9ffb59f787e02f95ab4d1b38 Mon Sep 17 00:00:00 2001 From: Julien Maffre Date: Thu, 19 Mar 2020 09:56:34 +0000 Subject: [PATCH 20/20] Rename new flag to require_client_identity --- src/apps/logging/logging.cpp | 2 +- src/node/rpc/frontend.h | 4 ++-- src/node/rpc/handlerregistry.h | 14 +++++++------- src/node/rpc/memberfrontend.h | 2 +- src/node/rpc/test/frontend_test.cpp | 2 +- tests/e2e_logging.py | 4 ---- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/apps/logging/logging.cpp b/src/apps/logging/logging.cpp index 0e61ab641c51..1fb8d113a7a1 100644 --- a/src/apps/logging/logging.cpp +++ b/src/apps/logging/logging.cpp @@ -233,7 +233,7 @@ namespace ccfapp json_adapter(log_record_anonymous), Write) .set_auto_schema() - .set_caller_auth(true); + .set_require_client_identity(false); nwt.signatures.set_global_hook([this, ¬ifier]( kv::Version version, diff --git a/src/node/rpc/frontend.h b/src/node/rpc/frontend.h index ab5638a25a32..125198f0b472 100644 --- a/src/node/rpc/frontend.h +++ b/src/node/rpc/frontend.h @@ -406,9 +406,9 @@ namespace ccf return ctx->serialise_response(); } - if (!handler->caller_auth_disabled && handlers.has_certs()) + if (handler->require_client_identity && handlers.has_certs()) { - // Only if handler requires auth. + // Only if handler requires client identity. // If a request is forwarded, check that the caller is known. Otherwise, // only check that the caller id is valid. if ( diff --git a/src/node/rpc/handlerregistry.h b/src/node/rpc/handlerregistry.h index d41f21aacd5c..1464c525481f 100644 --- a/src/node/rpc/handlerregistry.h +++ b/src/node/rpc/handlerregistry.h @@ -93,21 +93,21 @@ namespace ccf return *this; } - // If true, caller does not need to be authenticated - bool caller_auth_disabled = false; + // If true, client must be known in certs table + bool require_client_identity = true; - Handler& set_caller_auth(bool v) + Handler& set_require_client_identity(bool v) { - if (v && registry != nullptr && !registry->has_certs()) + if (!v && registry != nullptr && !registry->has_certs()) { LOG_INFO_FMT( - "Disabling caller auth on {} handler has no effect since its " - "registry does not have certificates table", + "Disabling client identity requirement on {} handler has no effect " + "since its registry does not have certificates table", method); return *this; } - caller_auth_disabled = v; + require_client_identity = v; return *this; } diff --git a/src/node/rpc/memberfrontend.h b/src/node/rpc/memberfrontend.h index c9071bf1ed2a..51d273fa37b1 100644 --- a/src/node/rpc/memberfrontend.h +++ b/src/node/rpc/memberfrontend.h @@ -886,7 +886,7 @@ namespace ccf return make_success(true); }; install(MemberProcs::CREATE, json_adapter(create), Write) - .set_caller_auth(true); + .set_require_client_identity(false); } }; diff --git a/src/node/rpc/test/frontend_test.cpp b/src/node/rpc/test/frontend_test.cpp index a41df09fbf7a..86c2fc670eba 100644 --- a/src/node/rpc/test/frontend_test.cpp +++ b/src/node/rpc/test/frontend_test.cpp @@ -58,7 +58,7 @@ class TestUserFrontend : public SimpleUserRpcFrontend }; install( "empty_function_no_auth", empty_function_no_auth, HandlerRegistry::Read) - .set_caller_auth(true); + .set_require_client_identity(false); } }; diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py index 6934fab1064b..fc7788964762 100644 --- a/tests/e2e_logging.py +++ b/tests/e2e_logging.py @@ -92,10 +92,6 @@ def test_anonymous_caller(network, args): ), "Anonymous user is not authorised to call LOG_get" with primary.user_client(user_id=0) as c: - r = c.rpc("LOG_record_anonymous", {"id": log_id, "msg": msg}) - assert ( - r.error is not None - ), "Only anonymous users are authorised to call LOG_record_anonymous" r = c.rpc("LOG_get", {"id": log_id}) assert r.result is not None assert msg in r.result["msg"]