From f43638ae3e72017321162a29c47735820e419607 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 23 Aug 2024 09:52:00 +0000 Subject: [PATCH 1/3] Remove JWT key filter and policy support --- .snpcc_canary | 2 +- CHANGELOG.md | 8 ++ include/ccf/service/tables/jwt.h | 32 +---- samples/constitutions/default/actions.js | 16 --- src/node/gov/handlers/service_state.h | 5 - src/node/rpc/jwt_management.h | 105 ++--------------- tests/infra/consortium.py | 2 - tests/jwt_test.py | 144 ----------------------- 8 files changed, 24 insertions(+), 290 deletions(-) diff --git a/.snpcc_canary b/.snpcc_canary index a2af0bef65a3..a90204f06158 100644 --- a/.snpcc_canary +++ b/.snpcc_canary @@ -3,4 +3,4 @@ O \ o | / /-xXx--//-----x=x--/-xXx--/---x---->>>--/ ... -/\/\(-_-) +/\/\d(-_-)b diff --git a/CHANGELOG.md b/CHANGELOG.md index 437388f956f6..2a1fa103c8d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). +## [6.0.0-dev0] + +[6.0.0-dev0]: https://github.com/microsoft/CCF/releases/tag/6.0.0-dev0 + +### Removed + +- SGX Platform support. + ## [5.0.4] [5.0.4]: https://github.com/microsoft/CCF/releases/tag/ccf-5.0.4 diff --git a/include/ccf/service/tables/jwt.h b/include/ccf/service/tables/jwt.h index f661b742971e..23ebe5268499 100644 --- a/include/ccf/service/tables/jwt.h +++ b/include/ccf/service/tables/jwt.h @@ -12,37 +12,17 @@ namespace ccf { - struct JwtIssuerKeyPolicy - { - /** OE claim name -> hex-encoded claim value - See openenclave/attestation/verifier.h */ - std::optional> sgx_claims; - - bool operator!=(const JwtIssuerKeyPolicy& rhs) const - { - return rhs.sgx_claims != sgx_claims; - } - }; - - DECLARE_JSON_TYPE(JwtIssuerKeyPolicy); - DECLARE_JSON_REQUIRED_FIELDS(JwtIssuerKeyPolicy, sgx_claims); - enum class JwtIssuerKeyFilter { - All, - SGX + All }; - DECLARE_JSON_ENUM( - JwtIssuerKeyFilter, - {{JwtIssuerKeyFilter::All, "all"}, {JwtIssuerKeyFilter::SGX, "sgx"}}); + DECLARE_JSON_ENUM(JwtIssuerKeyFilter, {{JwtIssuerKeyFilter::All, "all"}}); struct JwtIssuerMetadata { - /// JWT issuer key filter - JwtIssuerKeyFilter key_filter; - /// Optional Key Policy - std::optional key_policy; + /// JWT issuer key filter, kept for compatibility with existing ledgers + JwtIssuerKeyFilter key_filter = JwtIssuerKeyFilter::All; /// Optional CA bundle name used for authentication when auto-refreshing std::optional ca_cert_bundle_name; /// Whether to auto-refresh keys from the issuer @@ -50,9 +30,9 @@ namespace ccf }; DECLARE_JSON_TYPE_WITH_OPTIONAL_FIELDS(JwtIssuerMetadata); - DECLARE_JSON_REQUIRED_FIELDS(JwtIssuerMetadata, key_filter); + DECLARE_JSON_REQUIRED_FIELDS(JwtIssuerMetadata); DECLARE_JSON_OPTIONAL_FIELDS( - JwtIssuerMetadata, key_policy, ca_cert_bundle_name, auto_refresh); + JwtIssuerMetadata, key_filter, ca_cert_bundle_name, auto_refresh); using JwtIssuer = std::string; using JwtKeyId = std::string; diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index fa380f3c9364..654ecc065326 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -884,22 +884,6 @@ const actions = new Map([ checkType(args.issuer, "string", "issuer"); checkType(args.auto_refresh, "boolean?", "auto_refresh"); checkType(args.ca_cert_bundle_name, "string?", "ca_cert_bundle_name"); - checkEnum(args.key_filter, ["all", "sgx"], "key_filter"); - checkType(args.key_policy, "object?", "key_policy"); - if (args.key_policy) { - checkType( - args.key_policy.sgx_claims, - "object?", - "key_policy.sgx_claims", - ); - if (args.key_policy.sgx_claims) { - for (const [name, value] of Object.entries( - args.key_policy.sgx_claims, - )) { - checkType(value, "string", `key_policy.sgx_claims["${name}"]`); - } - } - } checkType(args.jwks, "object?", "jwks"); if (args.jwks) { checkJwks(args.jwks, "jwks"); diff --git a/src/node/gov/handlers/service_state.h b/src/node/gov/handlers/service_state.h index 1ce0e0c6e287..1667ff8256fd 100644 --- a/src/node/gov/handlers/service_state.h +++ b/src/node/gov/handlers/service_state.h @@ -549,11 +549,6 @@ namespace ccf::gov::endpoints jwt_issuer["keyFilter"] = metadata.key_filter; jwt_issuer["autoRefresh"] = metadata.auto_refresh; - if (metadata.key_policy.has_value()) - { - jwt_issuer["keyPolicy"] = metadata.key_policy.value(); - } - if (metadata.ca_cert_bundle_name.has_value()) { jwt_issuer["caCertBundleName"] = diff --git a/src/node/rpc/jwt_management.h b/src/node/rpc/jwt_management.h index 1af1cb4c8b52..af7c011ac8ef 100644 --- a/src/node/rpc/jwt_management.h +++ b/src/node/rpc/jwt_management.h @@ -9,17 +9,8 @@ #include "ccf/tx.h" #include "http/http_jwt.h" -#ifdef SGX_ATTESTATION_VERIFICATION -# include -#endif - #include #include -#if defined(INSIDE_ENCLAVE) && !defined(VIRTUAL_ENCLAVE) -# include -#elif defined(SGX_ATTESTATION_VERIFICATION) -# include -#endif namespace ccf { @@ -107,22 +98,6 @@ namespace ccf }); } -#ifdef SGX_ATTESTATION_VERIFICATION - static oe_result_t oe_verify_attestation_certificate_with_evidence_cb( - oe_claim_t* claims, size_t claims_length, void* arg) - { - auto claims_map = (std::map>*)arg; - for (size_t i = 0; i < claims_length; i++) - { - std::string claim_name(claims[i].name); - std::vector claim_value( - claims[i].value, claims[i].value + claims[i].value_size); - claims_map->emplace(std::move(claim_name), std::move(claim_value)); - } - return OE_OK; - } -#endif - static bool set_jwt_public_signing_keys( ccf::kv::Tx& tx, const std::string& log_prefix, @@ -171,83 +146,21 @@ namespace ccf return false; } - std::map> claims; - bool has_key_policy_sgx_claims = issuer_metadata.key_policy.has_value() && - issuer_metadata.key_policy.value().sgx_claims.has_value() && - !issuer_metadata.key_policy.value().sgx_claims.value().empty(); - if ( - issuer_metadata.key_filter == JwtIssuerKeyFilter::SGX || - has_key_policy_sgx_claims) + try { -#ifdef SGX_ATTESTATION_VERIFICATION - oe_verify_attestation_certificate_with_evidence( - der.data(), - der.size(), - oe_verify_attestation_certificate_with_evidence_cb, - &claims); -#else - LOG_FAIL_FMT("{}: SGX claims not supported", log_prefix); - return false; -#endif + ccf::crypto::make_unique_verifier( + (std::vector)der); // throws on error } - - if ( - issuer_metadata.key_filter == JwtIssuerKeyFilter::SGX && claims.empty()) + catch (std::invalid_argument& exc) { - LOG_INFO_FMT( - "{}: Skipping JWT signing key with kid {} (not OE " - "attested)", + LOG_FAIL_FMT( + "{}: JWKS kid {} has an invalid X.509 certificate: {}", log_prefix, - kid); - continue; + kid, + exc.what()); + return false; } - if (has_key_policy_sgx_claims) - { - for (auto& [claim_name, expected_claim_val_hex] : - issuer_metadata.key_policy.value().sgx_claims.value()) - { - if (claims.find(claim_name) == claims.end()) - { - LOG_FAIL_FMT( - "{}: JWKS kid {} is missing the {} SGX claim", - log_prefix, - kid, - claim_name); - return false; - } - auto& actual_claim_val = claims[claim_name]; - auto actual_claim_val_hex = ds::to_hex(actual_claim_val); - if (expected_claim_val_hex != actual_claim_val_hex) - { - LOG_FAIL_FMT( - "{}: JWKS kid {} has a mismatching {} SGX claim: {} != {}", - log_prefix, - kid, - claim_name, - expected_claim_val_hex, - actual_claim_val_hex); - return false; - } - } - } - else - { - try - { - ccf::crypto::make_unique_verifier( - (std::vector)der); // throws on error - } - catch (std::invalid_argument& exc) - { - LOG_FAIL_FMT( - "{}: JWKS kid {} has an invalid X.509 certificate: {}", - log_prefix, - kid, - exc.what()); - return false; - } - } LOG_INFO_FMT("{}: Storing JWT signing key with kid {}", log_prefix, kid); new_keys.emplace(kid, der); diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index 741c39201ecd..41cdaef7d7d8 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -611,8 +611,6 @@ def set_jwt_issuer(self, remote_node, json_path): obj = slurp_json(json_path) args = { "issuer": obj["issuer"], - "key_filter": obj.get("key_filter", "all"), - "key_policy": obj.get("key_policy"), "ca_cert_bundle_name": obj.get("ca_cert_bundle_name"), "auto_refresh": obj.get("auto_refresh", False), "jwks": obj.get("jwks"), diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 83771dca1904..ef0e861fd3f6 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -320,147 +320,6 @@ def make_attested_cert(network, args): return pem -@reqs.description("JWT with SGX key policy") -def test_jwt_with_sgx_key_policy(network, args): - primary, _ = network.find_nodes() - oe_cert_path = make_attested_cert(network, args) - - with open(oe_cert_path, encoding="utf-8") as f: - oe_cert_pem = f.read() - - kid = "my_kid_with_policy" - issuer = infra.jwt_issuer.JwtIssuer( - "https://example.issuer_with_policy", oe_cert_pem - ) - - oesign = os.path.join(args.oe_binary, "oesign") - oeutil_enc = os.path.join(args.oe_binary, "oeutil_enc.signed") - sc = infra.proc.ccall( - oesign, - "dump", - "-e", - oeutil_enc, - ) - sc.check_returncode() - lines = sc.stdout.decode().split() - for line in lines: - if line.startswith("mrsigner="): - mrsigner = line.strip().split("=")[1] - break - else: - assert False, f"Could not find mrsigner in {lines}" - - matching_key_policy = { - "sgx_claims": { - "signer_id": mrsigner, - "attributes": "0300000000000000", - } - } - - mismatching_key_policy = { - "sgx_claims": { - "signer_id": "da9ad7331448980aa28890ce73e433638377f179ab4456b2fe237193193a8d0a", - "attributes": "0300000000000000", - } - } - - LOG.info("Add JWT issuer with SGX key policy") - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: - json.dump( - {"issuer": issuer.name, "key_policy": matching_key_policy}, metadata_fp - ) - metadata_fp.flush() - network.consortium.set_jwt_issuer(primary, metadata_fp.name) - - LOG.info("Try to add a non-OE-attested cert") - non_oe_issuer_name = "non_oe_issuer" - non_oe_issuer = infra.jwt_issuer.JwtIssuer(non_oe_issuer_name) - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as jwks_fp: - json.dump(non_oe_issuer.create_jwks(kid), jwks_fp) - jwks_fp.flush() - try: - network.consortium.set_jwt_public_signing_keys( - primary, non_oe_issuer_name, jwks_fp.name - ) - except infra.proposal.ProposalNotAccepted: - pass - else: - assert False, "Proposal should not have been created" - - LOG.info("Add an OE-attested cert with matching claims") - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as jwks_fp: - json.dump(issuer.create_jwks(kid), jwks_fp) - jwks_fp.flush() - network.consortium.set_jwt_public_signing_keys( - primary, issuer.name, jwks_fp.name - ) - - LOG.info("Update JWT issuer with mismatching SGX key policy") - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: - json.dump( - { - "issuer": issuer.name, - "key_policy": mismatching_key_policy, - }, - metadata_fp, - ) - metadata_fp.flush() - network.consortium.set_jwt_issuer(primary, metadata_fp.name) - - LOG.info("Try to add an OE-attested cert with mismatching claims") - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as jwks_fp: - json.dump(non_oe_issuer.create_jwks(kid), jwks_fp) - jwks_fp.flush() - try: - network.consortium.set_jwt_public_signing_keys( - primary, non_oe_issuer_name, jwks_fp.name - ) - except infra.proposal.ProposalNotAccepted: - pass - else: - assert False, "Proposal should not have been created" - - return network - - -@reqs.description("JWT with SGX key filter") -def test_jwt_with_sgx_key_filter(network, args): - primary, _ = network.find_nodes() - - oe_cert_path = make_attested_cert(network, args) - with open(oe_cert_path, encoding="utf-8") as f: - oe_cert_pem = f.read() - - oe_issuer = infra.jwt_issuer.JwtIssuer("https://example.oe_issuer", oe_cert_pem) - non_oe_issuer = infra.jwt_issuer.JwtIssuer("https://example.non_oe_issuer") - - oe_kid = "oe_kid" - non_oe_kid = "non_oe_kid" - - LOG.info("Add JWT issuer with SGX key filter") - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: - json.dump({"issuer": oe_issuer.name, "key_filter": "sgx"}, metadata_fp) - metadata_fp.flush() - network.consortium.set_jwt_issuer(primary, metadata_fp.name) - - LOG.info("Add multiple certs (1 SGX, 1 non-SGX)") - with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as jwks_fp: - oe_jwks = oe_issuer.create_jwks(oe_kid) - non_oe_jwks = non_oe_issuer.create_jwks(non_oe_kid) - jwks = {"keys": non_oe_jwks["keys"] + oe_jwks["keys"]} - json.dump(jwks, jwks_fp) - jwks_fp.flush() - network.consortium.set_jwt_public_signing_keys( - primary, oe_issuer.name, jwks_fp.name - ) - - stored_jwt_signing_keys = get_jwt_keys(args, primary) - assert non_oe_kid not in stored_jwt_signing_keys, stored_jwt_signing_keys - assert oe_kid in stored_jwt_signing_keys, stored_jwt_signing_keys - - return network - - def check_kv_jwt_key_matches(args, network, kid, cert_pem): primary, _ = network.find_nodes() latest_jwt_signing_keys = get_jwt_keys(args, primary) @@ -796,9 +655,6 @@ def run_auto(args): test_jwt_issuer_domain_match(network, args) test_jwt_endpoint(network, args) test_jwt_without_key_policy(network, args) - if args.enclave_platform == "sgx": - test_jwt_with_sgx_key_policy(network, args) - test_jwt_with_sgx_key_filter(network, args) test_jwt_key_auto_refresh(network, args) # Check that auto refresh also works on backups From ce3e4aec2c83c68f9f1fea098fb758763d870acd Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 23 Aug 2024 14:28:46 +0000 Subject: [PATCH 2/3] schema --- doc/build_apps/auth/jwt.rst | 42 ---------------------------------- doc/schemas/gov_openapi.json | 22 ++---------------- src/node/rpc/member_frontend.h | 2 +- 3 files changed, 3 insertions(+), 63 deletions(-) diff --git a/doc/build_apps/auth/jwt.rst b/doc/build_apps/auth/jwt.rst index ffa9c8be491c..5ce24244b050 100644 --- a/doc/build_apps/auth/jwt.rst +++ b/doc/build_apps/auth/jwt.rst @@ -137,48 +137,6 @@ Token signing keys are stored in the ``public:ccf.gov.jwt.public_signing_keys`` If an application uses multiple token issuers, then the ``public:ccf.gov.jwt.public_signing_key_issuer`` kv map which maps key IDs to issuers can be used to determine the issuer that a key belongs to. -Advanced issuer configuration ------------------------------ - -CCF has special support for IdPs that issue tokens within SGX enclaves, for example MAA (`Microsoft Azure Attestation `_). -The goal is to validate that a token has indeed been issued from an SGX enclave that has certain properties. -CCF supports the approach taken by MAA where the token signing key and certificate are generated inside the enclave and the certificate embeds evidence from the enclave platform in an X.509 extension (see Open Enclave's `oe_get_attestation_certificate_with_evidence() `_ for details). -In this model it is sufficient to validate the evidence of the signing certificates when storing them in CCF. -After the signing certificates have been stored, token validation follows the same methods as described in earlier sections. - -CCF validates embedded SGX evidence if a key policy is given in the issuer metadata: - -.. code-block:: json - - { - "actions": [ - { - "name": "set_jwt_issuer", - "args": { - "issuer": "https://shareduks.uks.attest.azure.net", - "key_filter": "sgx", - "key_policy": { - "sgx_claims": { - "signer_id": "5e5410aaf99a32e32df2a97d579e65f8310f274816ec4f34cedeeb1be410a526", - "attributes": "0300000000000000" - } - }, - "auto_refresh": false - } - } - ] - } - -All claims contained in ``key_policy.sgx_claims`` must be identical to the ones embedded in the certificate. -Any attempt to add a certificate with mismatching claims in a ``set_jwt_public_signing_keys`` proposal for that issuer would result in failure. - -.. note:: - - See Open Enclave's `oe_verify_evidence() `_ for a list of available claim names and their meaning. Note that all claim values must be given hex-encoded. - -Some IdPs, like MAA, advertise a mix of SGX and non-SGX signing certificates. -In this case, ``key_filter`` must be set to ``sgx`` such that only those certificates are stored which contain SGX evidence. - Extracting JWT metrics ---------------------- diff --git a/doc/schemas/gov_openapi.json b/doc/schemas/gov_openapi.json index f679b85398b9..fdcc5e6df07d 100644 --- a/doc/schemas/gov_openapi.json +++ b/doc/schemas/gov_openapi.json @@ -305,22 +305,10 @@ }, "JwtIssuerKeyFilter": { "enum": [ - "all", - "sgx" + "all" ], "type": "string" }, - "JwtIssuerKeyPolicy": { - "properties": { - "sgx_claims": { - "$ref": "#/components/schemas/string_to_string" - } - }, - "required": [ - "sgx_claims" - ], - "type": "object" - }, "JwtIssuerMetadata": { "properties": { "auto_refresh": { @@ -331,14 +319,8 @@ }, "key_filter": { "$ref": "#/components/schemas/JwtIssuerKeyFilter" - }, - "key_policy": { - "$ref": "#/components/schemas/JwtIssuerKeyPolicy" } }, - "required": [ - "key_filter" - ], "type": "object" }, "KeyIdInfo": { @@ -1351,7 +1333,7 @@ "info": { "description": "This API is used to submit and query proposals which affect CCF's public governance tables.", "title": "CCF Governance API", - "version": "4.3.0" + "version": "4.4.0" }, "openapi": "3.0.0", "paths": { diff --git a/src/node/rpc/member_frontend.h b/src/node/rpc/member_frontend.h index fd97f0c36dde..6087e4aff6e1 100644 --- a/src/node/rpc/member_frontend.h +++ b/src/node/rpc/member_frontend.h @@ -599,7 +599,7 @@ namespace ccf openapi_info.description = "This API is used to submit and query proposals which affect CCF's " "public governance tables."; - openapi_info.document_version = "4.3.0"; + openapi_info.document_version = "4.4.0"; } static std::optional get_caller_member_id( From 3d6c3fe929400aee7ee5343e581b37cab9d77c6a Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Fri, 23 Aug 2024 14:46:18 +0000 Subject: [PATCH 3/3] compat --- CHANGELOG.md | 4 ++++ src/node/gov/handlers/service_state.h | 1 - tests/infra/consortium.py | 3 +++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a1fa103c8d8..01676efe7c59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. [6.0.0-dev0]: https://github.com/microsoft/CCF/releases/tag/6.0.0-dev0 +### Changed + +- The `set_jwt_issuer` governance action has been updated, and no longer accepts `key_filter` or `key_policy` arguments (#6450). + ### Removed - SGX Platform support. diff --git a/src/node/gov/handlers/service_state.h b/src/node/gov/handlers/service_state.h index 1667ff8256fd..c941e14f960a 100644 --- a/src/node/gov/handlers/service_state.h +++ b/src/node/gov/handlers/service_state.h @@ -546,7 +546,6 @@ namespace ccf::gov::endpoints const ccf::JwtIssuerMetadata& metadata) { auto jwt_issuer = nlohmann::json::object(); - jwt_issuer["keyFilter"] = metadata.key_filter; jwt_issuer["autoRefresh"] = metadata.auto_refresh; if (metadata.ca_cert_bundle_name.has_value()) diff --git a/tests/infra/consortium.py b/tests/infra/consortium.py index 41cdaef7d7d8..759be70bea64 100644 --- a/tests/infra/consortium.py +++ b/tests/infra/consortium.py @@ -610,6 +610,9 @@ def refresh_js_app_bytecode_cache(self, remote_node): def set_jwt_issuer(self, remote_node, json_path): obj = slurp_json(json_path) args = { + # Key filter is no longer used, but kept for compatibility with + # lts_compatibility tests. + "key_filter": "all", "issuer": obj["issuer"], "ca_cert_bundle_name": obj.get("ca_cert_bundle_name"), "auto_refresh": obj.get("auto_refresh", False),