Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove signed HTTP request support #5137

Merged
merged 52 commits into from
Apr 19, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
86d4d4e
Disable member_signature_auth_policy
achamayou Mar 23, 2023
8fb4199
Switch auth
achamayou Mar 23, 2023
4ca5473
Merge branch 'main' into disable_signed_request_support
achamayou Mar 23, 2023
cc678c7
Avoid replay
achamayou Mar 23, 2023
6424433
Merge branch 'main' into disable_signed_request_support
achamayou Mar 24, 2023
2c1e44d
Fix schema test
achamayou Mar 24, 2023
ab70610
Merge branch 'main' into disable_signed_request_support
achamayou Mar 24, 2023
f3dc0b0
fmt
achamayou Mar 24, 2023
717dfab
Fix membership tests
achamayou Mar 24, 2023
025d2da
fmt
achamayou Mar 24, 2023
d3744c3
Fix governance tests
achamayou Mar 24, 2023
2b90f11
Remove un-necessary additional client
achamayou Mar 24, 2023
677c409
Merge branch 'main' into disable_signed_request_support
achamayou Mar 31, 2023
362ee98
Update documentation
achamayou Mar 31, 2023
b73bbe4
disable_support_for_http_sig_in_curl_client
achamayou Mar 31, 2023
0f8550e
Merge branch 'main' into disable_signed_request_support
achamayou Apr 12, 2023
d4cbcb6
Merge branch 'main' into disable_signed_request_support
achamayou Apr 12, 2023
35919bd
Merge branch 'main' into disable_signed_request_support
achamayou Apr 13, 2023
23a83e1
tmp
achamayou Apr 13, 2023
85047f9
Merge branch 'disable_signed_request_support' of https://github.com/a…
achamayou Apr 13, 2023
a243484
Remove tests
achamayou Apr 13, 2023
96c8c4b
Merge branch 'main' into disable_signed_request_support
achamayou Apr 13, 2023
1ce1383
Stop unnecessarily signing the first request with the node key
achamayou Apr 13, 2023
cfaf9fb
Merge branch 'main' into disable_signed_request_support
achamayou Apr 13, 2023
381c8cc
More removal
achamayou Apr 13, 2023
261999d
Merge branch 'main' into disable_signed_request_support
achamayou Apr 13, 2023
d686706
.
achamayou Apr 13, 2023
8acc82d
Merge branch 'disable_signed_request_support' of https://github.com/a…
achamayou Apr 14, 2023
028e61d
.
achamayou Apr 14, 2023
5a37d12
.
achamayou Apr 14, 2023
da1814d
Merge branch 'main' into disable_signed_request_support
achamayou Apr 14, 2023
d4fa6ad
.
achamayou Apr 14, 2023
3a2165e
.
achamayou Apr 14, 2023
e0096ee
Add warning
achamayou Apr 14, 2023
a97b853
.
achamayou Apr 14, 2023
a49bb6c
Update documentation and changelog
achamayou Apr 14, 2023
8f37a2e
Update documentation and changelog
achamayou Apr 14, 2023
d5ab6ae
.
achamayou Apr 14, 2023
1ca5469
Merge branch 'main' into disable_signed_request_support
achamayou Apr 17, 2023
9006d31
Sleep faster
achamayou Apr 17, 2023
7cf4eb7
clock
achamayou Apr 17, 2023
ac6a25e
fmt
achamayou Apr 17, 2023
45c89b0
Merge branch 'main' into disable_signed_request_support
achamayou Apr 17, 2023
6cbdeb0
daily
achamayou Apr 17, 2023
6f6431b
.
achamayou Apr 18, 2023
40ebf64
compat
achamayou Apr 18, 2023
97a9032
.
achamayou Apr 18, 2023
8075b2b
.
achamayou Apr 18, 2023
6007ed0
.
achamayou Apr 18, 2023
14d37d1
.
achamayou Apr 19, 2023
1d7fe30
Merge branch 'main' into disable_signed_request_support
achamayou Apr 19, 2023
099a49d
Merge branch 'main' into disable_signed_request_support
achamayou Apr 19, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 1 addition & 27 deletions doc/schemas/gov_openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -1246,11 +1246,6 @@
"description": "Request payload must be a COSE Sign1 document, with expected protected headers. Signer must be a member identity registered with this service.",
"scheme": "cose_sign1",
"type": "http"
},
"member_signature": {
"description": "Request must be signed according to the HTTP Signature scheme. The signer must be a member identity registered with this service.",
"scheme": "signature",
"type": "http"
}
},
"x-ccf-forwarding": {
Expand All @@ -1271,7 +1266,7 @@
"info": {
"description": "This API is used to submit and query proposals which affect CCF's public governance tables.",
"title": "CCF Governance API",
"version": "2.24.0"
"version": "2.25.0"
},
"openapi": "3.0.0",
"paths": {
Expand All @@ -1296,9 +1291,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down Expand Up @@ -1327,9 +1319,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down Expand Up @@ -2232,9 +2221,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down Expand Up @@ -2349,9 +2335,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down Expand Up @@ -2431,9 +2414,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down Expand Up @@ -2496,9 +2476,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down Expand Up @@ -2535,9 +2512,6 @@
}
},
"security": [
{
"member_signature": []
},
{
"member_cose_sign1": []
}
Expand Down
11 changes: 4 additions & 7 deletions src/node/rpc/member_frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -447,9 +447,9 @@ namespace ccf
cose_recent_proposals->put(key, proposal_id);
// Only keep the most recent window_size proposals, to avoid
// unbounded memory usage
if (replay_keys.size() >= window_size)
if (replay_keys.size() >= (window_size - 1) /* We just added one */)
achamayou marked this conversation as resolved.
Show resolved Hide resolved
{
for (size_t i = 0; i < (replay_keys.size() - window_size); i++)
for (size_t i = 0; i < (replay_keys.size() - (window_size - 1)); i++)
{
cose_recent_proposals->remove(replay_keys[i]);
}
Expand Down Expand Up @@ -587,7 +587,7 @@ namespace ccf
openapi_info.description =
"This API is used to submit and query proposals which affect CCF's "
"public governance tables.";
openapi_info.document_version = "2.24.0";
openapi_info.document_version = "2.25.0";
}

static std::optional<MemberId> get_caller_member_id(
Expand Down Expand Up @@ -661,16 +661,13 @@ namespace ccf

AuthnPolicies member_sig_only_policies(const std::string& gov_msg_type)
{
return {
member_signature_auth_policy,
std::make_shared<MemberCOSESign1AuthnPolicy>(gov_msg_type)};
return {std::make_shared<MemberCOSESign1AuthnPolicy>(gov_msg_type)};
}

AuthnPolicies member_cert_or_sig_policies(const std::string& gov_msg_type)
{
return {
member_cert_auth_policy,
member_signature_auth_policy,
std::make_shared<MemberCOSESign1AuthnPolicy>(gov_msg_type)};
}

Expand Down
91 changes: 13 additions & 78 deletions tests/governance.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import infra.logging_app as app
import json
import jinja2
import requests
import infra.crypto
from datetime import datetime
import governance_js
Expand Down Expand Up @@ -358,45 +357,6 @@ def test_ack_state_digest_update(network, args):
return network


@reqs.description("Test invalid client signatures")
def test_invalid_client_signature(network, args):
primary, _ = network.find_primary()

def post_proposal_request_raw(node, headers=None, expected_error_msg=None):
r = requests.post(
f"https://{node.get_public_rpc_host()}:{node.get_public_rpc_port()}/gov/proposals",
headers=headers,
verify=os.path.join(node.common_dir, "service_cert.pem"),
timeout=3,
).json()
assert r["error"]["code"] == "InvalidAuthenticationInfo"
assert (
expected_error_msg in r["error"]["details"][0]["message"]
), f"Expected error message '{expected_error_msg}' not in '{r['error']['details'][0]['message']}'"

# Verify that _some_ HTTP signature parsing errors are communicated back to the client
post_proposal_request_raw(
primary,
headers=None,
expected_error_msg="Missing signature",
)
post_proposal_request_raw(
primary,
headers={"Authorization": "invalid"},
expected_error_msg="'authorization' header only contains one field",
)
post_proposal_request_raw(
primary,
headers={"Authorization": "invalid invalid"},
expected_error_msg="'authorization' scheme for signature should be 'Signature",
)
post_proposal_request_raw(
primary,
headers={"Authorization": "Signature invalid"},
expected_error_msg="Error verifying HTTP 'digest' header: Missing 'digest' header",
)


@reqs.description("Renew certificates of all nodes, one by one")
def test_each_node_cert_renewal(network, args):
primary, _ = network.find_primary()
Expand Down Expand Up @@ -632,7 +592,6 @@ def gov(args):
test_no_quote(network, args)
test_node_data(network, args)
test_ack_state_digest_update(network, args)
test_invalid_client_signature(network, args)
test_each_node_cert_renewal(network, args)
test_binding_proposal_to_service_identity(network, args)
test_all_nodes_cert_renewal(network, args)
Expand Down Expand Up @@ -676,23 +635,25 @@ def js_gov(args):
governance_js.test_proposal_withdrawal(network, args)
governance_js.test_ballot_storage(network, args)
governance_js.test_pure_proposals(network, args)
if args.authenticate_session == "COSE":
governance_js.test_proposal_replay_protection(network, args)
governance_js.test_cose_msg_type_validation(network, args)
# This test sends proposals identical in content to those sent by
# test_read_write_restrictions, so if it run too soon before or after, it
# risks signing them in the same second and hitting the replay protection.
governance_js.test_set_constitution(network, args)
governance_js.test_proposals_with_votes(network, args)
governance_js.test_vote_failure_reporting(network, args)
governance_js.test_operator_proposals_and_votes(network, args)
governance_js.test_operator_provisioner_proposals_and_votes(network, args)
governance_js.test_apply(network, args)
# See above for why this test needs to be run sufficiently later
# than test_set_constitution
governance_js.test_read_write_restrictions(network, args)


def gov_replay(args):
with infra.network.network(
args.nodes, args.binary_dir, args.debug_nodes, args.perf_nodes, pdb=args.pdb
) as network:
network.start_and_open(args)
network.consortium.set_authenticate_session(args.authenticate_session)
governance_js.test_proposal_replay_protection(network, args)
governance_js.test_cose_msg_type_validation(network, args)


if __name__ == "__main__":

def add(parser):
Expand Down Expand Up @@ -722,36 +683,18 @@ def add(parser):
authenticate_session="COSE",
)

cr.add(
"session_auth",
gov,
package="samples/apps/logging/liblogging",
nodes=infra.e2e_args.max_nodes(cr.args, f=0),
initial_user_count=3,
authenticate_session=True,
)

cr.add(
"session_noauth",
gov,
package="samples/apps/logging/liblogging",
nodes=infra.e2e_args.max_nodes(cr.args, f=0),
initial_user_count=3,
authenticate_session=False,
)

cr.add(
"js",
js_gov,
package="samples/apps/logging/liblogging",
nodes=infra.e2e_args.max_nodes(cr.args, f=0),
initial_user_count=3,
authenticate_session=True,
authenticate_session="COSE",
)

cr.add(
"js_cose",
js_gov,
"replay",
gov_replay,
package="samples/apps/logging/liblogging",
nodes=infra.e2e_args.max_nodes(cr.args, f=0),
initial_user_count=3,
Expand All @@ -763,14 +706,6 @@ def add(parser):
governance_history.run,
package="samples/apps/logging/liblogging",
nodes=infra.e2e_args.max_nodes(cr.args, f=0),
authenticate_session=False,
)

cr.add(
"cose_history",
governance_history.run,
package="samples/apps/logging/liblogging",
nodes=infra.e2e_args.max_nodes(cr.args, f=0),
authenticate_session="COSE",
)

Expand Down
Loading