From c6fe4e2521cc6e5ff9090279c14727797783dffa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:24:35 +0000 Subject: [PATCH 01/21] Initial plan From fc6c0fc0dd83962d75f21a0e88ecd5ddca3ce5cc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:33:52 +0000 Subject: [PATCH 02/21] Harden constitution validator inputs --- doc/build_apps/auth/jwt.rst | 4 +- samples/constitutions/default/actions.js | 149 +++++++++++++++++++---- samples/minimal_ccf/app/actions.js | 149 +++++++++++++++++++---- tests/ca_certs.py | 17 ++- tests/infra/jwt_issuer.py | 2 +- tests/jwt_test.py | 45 +++++++ 6 files changed, 315 insertions(+), 51 deletions(-) diff --git a/doc/build_apps/auth/jwt.rst b/doc/build_apps/auth/jwt.rst index f3bf72053c44..48dd1fcdd4c9 100644 --- a/doc/build_apps/auth/jwt.rst +++ b/doc/build_apps/auth/jwt.rst @@ -20,7 +20,7 @@ Before adding public token signing keys to a running CCF network, the IdP has to { "name": "set_jwt_issuer", "args": { - "issuer": "my_issuer", + "issuer": "https://my.issuer", "auto_refresh": false } } @@ -38,7 +38,7 @@ After this proposal is accepted, signing keys for an issuer can be updated with { "name": "set_jwt_public_signing_keys", "args": { - "issuer": "my_issuer", + "issuer": "https://my.issuer", "jwks": { "keys": [ { diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 667a044071ae..422ad2516675 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -95,6 +95,74 @@ function checkArrayBufferLength(value, min, max, field) { } } +function checkBase64Url(value, field) { + checkType(value, "string", field); + if (!/^[A-Za-z0-9_-]+$/.test(value) || value.length % 4 === 1) { + throw new Error(`${field} must be base64url encoded`); + } +} + +function base64UrlByteLength(value, field) { + checkBase64Url(value, field); + return Math.floor((value.length * 3) / 4); +} + +function splitX509CertBundle(value) { + const sep = "-----END CERTIFICATE-----"; + const items = value.split(sep); + if (items.length === 1) { + return []; + } + return items.slice(0, -1).map((p) => p + sep); +} + +function checkX509CACertBundle(value, field) { + checkX509CertBundle(value, field); + for (const [i, cert] of splitX509CertBundle(value).entries()) { + if (!ccf.crypto.isValidX509CertChain(cert, cert)) { + throw new Error( + `${field}[${i}] must be a valid CA certificate with a currently valid chain to itself`, + ); + } + } +} + +function checkRsaPublicKey(jwk, field) { + checkType(jwk.n, "string", `${field}.n`); + checkType(jwk.e, "string", `${field}.e`); + if (base64UrlByteLength(jwk.n, `${field}.n`) < 256) { + throw new Error(`${field}.n must be at least 2048 bits`); + } + base64UrlByteLength(jwk.e, `${field}.e`); + ccf.crypto.pubRsaJwkToPem({ + kty: "RSA", + kid: jwk.kid, + n: jwk.n, + e: jwk.e, + }); +} + +function checkEcPublicKey(jwk, field) { + checkType(jwk.x, "string", `${field}.x`); + checkType(jwk.y, "string", `${field}.y`); + checkType(jwk.crv, "string", `${field}.crv`); + checkEnum(jwk.crv, ["P-256", "P-384"], `${field}.crv`); + const coordinateLength = jwk.crv === "P-256" ? 32 : 48; + if (base64UrlByteLength(jwk.x, `${field}.x`) !== coordinateLength) { + throw new Error(`${field}.x must be ${coordinateLength} bytes`); + } + if (base64UrlByteLength(jwk.y, `${field}.y`) !== coordinateLength) { + throw new Error(`${field}.y must be ${coordinateLength} bytes`); + } + ccf.crypto.pubJwkToPem({ + kty: "EC", + kid: jwk.kid, + crv: jwk.crv, + x: jwk.x, + y: jwk.y, + }); +} + const cpuid_length_bytes = 4; function checkValidCpuid(value, field) { checkType(value, "string", field); @@ -174,26 +242,60 @@ function getActiveRecoveryMembersCount() { function checkJwks(value, field) { checkType(value, "object", field); checkType(value.keys, "array", `${field}.keys`); + const kids = new Set(); for (const [i, jwk] of value.keys.entries()) { + const keyField = `${field}.keys[${i}]`; checkType(jwk.kid, "string", `${field}.keys[${i}].kid`); + if (kids.has(jwk.kid)) { + throw new Error(`${field}.keys[${i}].kid must be unique`); + } + kids.add(jwk.kid); checkType(jwk.kty, "string", `${field}.keys[${i}].kty`); + checkEnum(jwk.kty, ["RSA", "EC"], `${field}.keys[${i}].kty`); + if (jwk.use !== undefined) { + checkType(jwk.use, "string", `${keyField}.use`); + checkEnum(jwk.use, ["sig"], `${keyField}.use`); + } + if (jwk.alg !== undefined) { + checkType(jwk.alg, "string", `${keyField}.alg`); + checkEnum( + jwk.alg, + jwk.kty === "RSA" ? ["RS256"] : ["ES256"], + `${keyField}.alg`, + ); + } if (jwk.x5c) { checkArrayLength(jwk.x5c, 1, null, `${field}.keys[${i}].x5c`); + let certBundle = ""; for (const [j, b64der] of jwk.x5c.entries()) { checkType(b64der, "string", `${field}.keys[${i}].x5c[${j}]`); + if (!/^[A-Za-z0-9+/]+={0,2}$/.test(b64der)) { + throw new Error(`${field}.keys[${i}].x5c[${j}] must be base64 encoded`); + } const pem = "-----BEGIN CERTIFICATE-----\n" + b64der + "\n-----END CERTIFICATE-----"; checkX509CertBundle(pem, `${field}.keys[${i}].x5c[${j}]`); + certBundle += pem; + } + const trustedRoot = + "-----BEGIN CERTIFICATE-----\n" + + jwk.x5c[jwk.x5c.length - 1] + + "\n-----END CERTIFICATE-----"; + if (!ccf.crypto.isValidX509CertChain(certBundle, trustedRoot)) { + throw new Error(`${field}.keys[${i}].x5c must chain to its root`); } } else if (jwk.n && jwk.e) { - checkType(jwk.n, "string", `${field}.keys[${i}].n`); - checkType(jwk.e, "string", `${field}.keys[${i}].e`); + if (jwk.kty !== "RSA") { + throw new Error(`${field}.keys[${i}].kty must be RSA for n/e keys`); + } + checkRsaPublicKey(jwk, keyField); } else if (jwk.x && jwk.y) { - checkType(jwk.x, "string", `${field}.keys[${i}].x`); - checkType(jwk.y, "string", `${field}.keys[${i}].y`); - checkType(jwk.crv, "string", `${field}.keys[${i}].crv`); + if (jwk.kty !== "EC") { + throw new Error(`${field}.keys[${i}].kty must be EC for x/y keys`); + } + checkEcPublicKey(jwk, keyField); } else { throw new Error( "JWK must contain either x5c, or n/e for RSA key type, or x/y/crv for EC key type", @@ -437,7 +539,12 @@ const actions = new Map([ "Cannot specify a recovery_role value when encryption_pub_key is not specified", ); } - // Also check that public encryption key is well formed, if it exists + if (args.encryption_pub_key !== null && args.encryption_pub_key !== undefined) { + checkRsaPublicKey( + ccf.crypto.pubRsaPemToJwk(args.encryption_pub_key), + "encryption_pub_key", + ); + } }, function (args) { @@ -928,7 +1035,7 @@ const actions = new Map([ new Action( function (args) { checkType(args.name, "string", "name"); - checkX509CertBundle(args.cert_bundle, "cert_bundle"); + checkX509CACertBundle(args.cert_bundle, "cert_bundle"); }, function (args) { const name = args.name; @@ -963,28 +1070,24 @@ const actions = new Map([ if (args.jwks) { checkJwks(args.jwks, "jwks"); } + let url; + try { + url = parseUrl(args.issuer); + } catch (e) { + throw new Error("issuer must be a URL"); + } + if (url.scheme != "https" || !url.authority) { + throw new Error("issuer must be a URL starting with https://"); + } + if (url.query || url.fragment) { + throw new Error("issuer must be a URL without query/fragment"); + } if (args.auto_refresh) { if (!args.ca_cert_bundle_name) { throw new Error( "ca_cert_bundle_name is missing but required if auto_refresh is true", ); } - let url; - try { - url = parseUrl(args.issuer); - } catch (e) { - throw new Error("issuer must be a URL if auto_refresh is true"); - } - if (url.scheme != "https") { - throw new Error( - "issuer must be a URL starting with https:// if auto_refresh is true", - ); - } - if (url.query || url.fragment) { - throw new Error( - "issuer must be a URL without query/fragment if auto_refresh is true", - ); - } } }, function (args) { diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 5531ace40060..defa8d1a874f 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -95,6 +95,74 @@ function checkArrayBufferLength(value, min, max, field) { } } +function checkBase64Url(value, field) { + checkType(value, "string", field); + if (!/^[A-Za-z0-9_-]+$/.test(value) || value.length % 4 === 1) { + throw new Error(`${field} must be base64url encoded`); + } +} + +function base64UrlByteLength(value, field) { + checkBase64Url(value, field); + return Math.floor((value.length * 3) / 4); +} + +function splitX509CertBundle(value) { + const sep = "-----END CERTIFICATE-----"; + const items = value.split(sep); + if (items.length === 1) { + return []; + } + return items.slice(0, -1).map((p) => p + sep); +} + +function checkX509CACertBundle(value, field) { + checkX509CertBundle(value, field); + for (const [i, cert] of splitX509CertBundle(value).entries()) { + if (!ccf.crypto.isValidX509CertChain(cert, cert)) { + throw new Error( + `${field}[${i}] must be a valid CA certificate with a currently valid chain to itself`, + ); + } + } +} + +function checkRsaPublicKey(jwk, field) { + checkType(jwk.n, "string", `${field}.n`); + checkType(jwk.e, "string", `${field}.e`); + if (base64UrlByteLength(jwk.n, `${field}.n`) < 256) { + throw new Error(`${field}.n must be at least 2048 bits`); + } + base64UrlByteLength(jwk.e, `${field}.e`); + ccf.crypto.pubRsaJwkToPem({ + kty: "RSA", + kid: jwk.kid, + n: jwk.n, + e: jwk.e, + }); +} + +function checkEcPublicKey(jwk, field) { + checkType(jwk.x, "string", `${field}.x`); + checkType(jwk.y, "string", `${field}.y`); + checkType(jwk.crv, "string", `${field}.crv`); + checkEnum(jwk.crv, ["P-256", "P-384"], `${field}.crv`); + const coordinateLength = jwk.crv === "P-256" ? 32 : 48; + if (base64UrlByteLength(jwk.x, `${field}.x`) !== coordinateLength) { + throw new Error(`${field}.x must be ${coordinateLength} bytes`); + } + if (base64UrlByteLength(jwk.y, `${field}.y`) !== coordinateLength) { + throw new Error(`${field}.y must be ${coordinateLength} bytes`); + } + ccf.crypto.pubJwkToPem({ + kty: "EC", + kid: jwk.kid, + crv: jwk.crv, + x: jwk.x, + y: jwk.y, + }); +} + function checkValidCpuid(value, field) { checkType(value, "string", field); if (value !== value.toLowerCase()) { @@ -154,26 +222,60 @@ function getActiveRecoveryMembersCount() { function checkJwks(value, field) { checkType(value, "object", field); checkType(value.keys, "array", `${field}.keys`); + const kids = new Set(); for (const [i, jwk] of value.keys.entries()) { + const keyField = `${field}.keys[${i}]`; checkType(jwk.kid, "string", `${field}.keys[${i}].kid`); + if (kids.has(jwk.kid)) { + throw new Error(`${field}.keys[${i}].kid must be unique`); + } + kids.add(jwk.kid); checkType(jwk.kty, "string", `${field}.keys[${i}].kty`); + checkEnum(jwk.kty, ["RSA", "EC"], `${field}.keys[${i}].kty`); + if (jwk.use !== undefined) { + checkType(jwk.use, "string", `${keyField}.use`); + checkEnum(jwk.use, ["sig"], `${keyField}.use`); + } + if (jwk.alg !== undefined) { + checkType(jwk.alg, "string", `${keyField}.alg`); + checkEnum( + jwk.alg, + jwk.kty === "RSA" ? ["RS256"] : ["ES256"], + `${keyField}.alg`, + ); + } if (jwk.x5c) { checkArrayLength(jwk.x5c, 1, null, `${field}.keys[${i}].x5c`); + let certBundle = ""; for (const [j, b64der] of jwk.x5c.entries()) { checkType(b64der, "string", `${field}.keys[${i}].x5c[${j}]`); + if (!/^[A-Za-z0-9+/]+={0,2}$/.test(b64der)) { + throw new Error(`${field}.keys[${i}].x5c[${j}] must be base64 encoded`); + } const pem = "-----BEGIN CERTIFICATE-----\n" + b64der + "\n-----END CERTIFICATE-----"; checkX509CertBundle(pem, `${field}.keys[${i}].x5c[${j}]`); + certBundle += pem; + } + const trustedRoot = + "-----BEGIN CERTIFICATE-----\n" + + jwk.x5c[jwk.x5c.length - 1] + + "\n-----END CERTIFICATE-----"; + if (!ccf.crypto.isValidX509CertChain(certBundle, trustedRoot)) { + throw new Error(`${field}.keys[${i}].x5c must chain to its root`); } } else if (jwk.n && jwk.e) { - checkType(jwk.n, "string", `${field}.keys[${i}].n`); - checkType(jwk.e, "string", `${field}.keys[${i}].e`); + if (jwk.kty !== "RSA") { + throw new Error(`${field}.keys[${i}].kty must be RSA for n/e keys`); + } + checkRsaPublicKey(jwk, keyField); } else if (jwk.x && jwk.y) { - checkType(jwk.x, "string", `${field}.keys[${i}].x`); - checkType(jwk.y, "string", `${field}.keys[${i}].y`); - checkType(jwk.crv, "string", `${field}.keys[${i}].crv`); + if (jwk.kty !== "EC") { + throw new Error(`${field}.keys[${i}].kty must be EC for x/y keys`); + } + checkEcPublicKey(jwk, keyField); } else { throw new Error( "JWK must contain either x5c, or n/e for RSA key type, or x/y/crv for EC key type", @@ -416,7 +518,12 @@ const actions = new Map([ "Cannot specify a recovery_role value when encryption_pub_key is not specified", ); } - // Also check that public encryption key is well formed, if it exists + if (args.encryption_pub_key !== null && args.encryption_pub_key !== undefined) { + checkRsaPublicKey( + ccf.crypto.pubRsaPemToJwk(args.encryption_pub_key), + "encryption_pub_key", + ); + } }, function (args) { @@ -907,7 +1014,7 @@ const actions = new Map([ new Action( function (args) { checkType(args.name, "string", "name"); - checkX509CertBundle(args.cert_bundle, "cert_bundle"); + checkX509CACertBundle(args.cert_bundle, "cert_bundle"); }, function (args) { const name = args.name; @@ -942,28 +1049,24 @@ const actions = new Map([ if (args.jwks) { checkJwks(args.jwks, "jwks"); } + let url; + try { + url = parseUrl(args.issuer); + } catch (e) { + throw new Error("issuer must be a URL"); + } + if (url.scheme != "https" || !url.authority) { + throw new Error("issuer must be a URL starting with https://"); + } + if (url.query || url.fragment) { + throw new Error("issuer must be a URL without query/fragment"); + } if (args.auto_refresh) { if (!args.ca_cert_bundle_name) { throw new Error( "ca_cert_bundle_name is missing but required if auto_refresh is true", ); } - let url; - try { - url = parseUrl(args.issuer); - } catch (e) { - throw new Error("issuer must be a URL if auto_refresh is true"); - } - if (url.scheme != "https") { - throw new Error( - "issuer must be a URL starting with https:// if auto_refresh is true", - ); - } - if (url.query || url.fragment) { - throw new Error( - "issuer must be a URL without query/fragment if auto_refresh is true", - ); - } } }, function (args) { diff --git a/tests/ca_certs.py b/tests/ca_certs.py index 9c2ff41775ad..a19d646357c6 100644 --- a/tests/ca_certs.py +++ b/tests/ca_certs.py @@ -45,11 +45,24 @@ def test_cert_store(network, args): else: assert False, "Proposal should not have been accepted" + LOG.info("Member makes a ca cert update proposal with a non-CA cert") + key_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) + non_ca_cert_pem = infra.crypto.generate_cert(key_priv_pem) + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as cert_pem_fp: + cert_pem_fp.write(non_ca_cert_pem) + cert_pem_fp.flush() + try: + network.consortium.set_ca_cert_bundle(primary, cert_name, cert_pem_fp.name) + except (infra.proposal.ProposalNotAccepted, infra.proposal.ProposalNotCreated): + pass + else: + assert False, "Proposal should not have accepted a non-CA certificate" + LOG.info("Member makes a ca cert update proposal with valid certs") key_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) - cert_pem = infra.crypto.generate_cert(key_priv_pem) + cert_pem = infra.crypto.generate_cert(key_priv_pem, ca=True) key2_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) - cert2_pem = infra.crypto.generate_cert(key2_priv_pem) + cert2_pem = infra.crypto.generate_cert(key2_priv_pem, ca=True) with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as cert_pem_fp: cert_pem_fp.write(cert_pem) cert_pem_fp.write(cert2_pem) diff --git a/tests/infra/jwt_issuer.py b/tests/infra/jwt_issuer.py index 93844b535e91..4cb89454eeb8 100644 --- a/tests/infra/jwt_issuer.py +++ b/tests/infra/jwt_issuer.py @@ -156,7 +156,7 @@ def _generate_auth_data(self, cn=None): else: raise ValueError(f"Unsupported algorithm: {self._alg}") - cert = infra.crypto.generate_cert(key_priv, cn=cn) + cert = infra.crypto.generate_cert(key_priv, cn=cn, ca=True) return (key_priv, key_pub), cert def __init__( diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 3acf789ea1b3..71ab287ec09b 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -36,6 +36,50 @@ def set_issuer_with_keys(network, primary, issuer, kids): ) +def assert_set_jwt_issuer_rejected(network, primary, metadata): + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as metadata_fp: + json.dump(metadata, metadata_fp) + metadata_fp.flush() + try: + network.consortium.set_jwt_issuer(primary, metadata_fp.name) + except infra.proposal.ProposalNotCreated as e: + assert e.response.status_code == 400, e.response.body.text() + assert ( + e.response.body.json()["error"]["code"] == "ProposalFailedToValidate" + ), e.response.body.text() + else: + assert False, "set_jwt_issuer should have failed to validate" + + +@reqs.description("JWT issuer and JWKS validation") +def test_jwt_issuer_and_jwks_validation(network, args): + primary, _ = network.find_nodes() + issuer = infra.jwt_issuer.JwtIssuer("https://example.issuer") + valid_key = issuer.create_jwks("kid1")["keys"][0] + + assert_set_jwt_issuer_rejected(network, primary, {"issuer": "example.issuer"}) + assert_set_jwt_issuer_rejected( + network, + primary, + {"issuer": issuer.name, "jwks": {"keys": [valid_key, valid_key]}}, + ) + assert_set_jwt_issuer_rejected( + network, + primary, + {"issuer": issuer.name, "jwks": {"keys": [{**valid_key, "kty": "oct"}]}}, + ) + assert_set_jwt_issuer_rejected( + network, + primary, + {"issuer": issuer.name, "jwks": {"keys": [{**valid_key, "alg": "HS256"}]}}, + ) + assert_set_jwt_issuer_rejected( + network, + primary, + {"issuer": issuer.name, "jwks": {"keys": [{**valid_key, "use": "enc"}]}}, + ) + + @reqs.description("Refresh JWT issuer") def test_refresh_jwt_issuer(network, args): assert network.jwt_issuer.server, "JWT server is not started" @@ -701,6 +745,7 @@ def run_auto(args): args.nodes, args.binary_dir, args.debug_nodes, pdb=args.pdb ) as network: network.start_and_open(args) + test_jwt_issuer_and_jwks_validation(network, args) test_jwt_mulitple_issuers_same_kids_different_pem(network, args) test_jwt_mulitple_issuers_same_kids_same_pem(network, args) test_jwt_same_issuer_constraint_overwritten(network, args) From 7c40d78542ca5b2c4771382934f48436d25740c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:34:34 +0000 Subject: [PATCH 03/21] Validate JWKS embedded public keys --- samples/constitutions/default/actions.js | 21 +++++++++++++++++++-- samples/minimal_ccf/app/actions.js | 21 +++++++++++++++++++-- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 422ad2516675..e646714d1958 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -270,7 +270,9 @@ function checkJwks(value, field) { for (const [j, b64der] of jwk.x5c.entries()) { checkType(b64der, "string", `${field}.keys[${i}].x5c[${j}]`); if (!/^[A-Za-z0-9+/]+={0,2}$/.test(b64der)) { - throw new Error(`${field}.keys[${i}].x5c[${j}] must be base64 encoded`); + throw new Error( + `${field}.keys[${i}].x5c[${j}] must be base64 encoded`, + ); } const pem = "-----BEGIN CERTIFICATE-----\n" + @@ -286,6 +288,18 @@ function checkJwks(value, field) { if (!ccf.crypto.isValidX509CertChain(certBundle, trustedRoot)) { throw new Error(`${field}.keys[${i}].x5c must chain to its root`); } + if (jwk.n !== undefined || jwk.e !== undefined) { + if (jwk.kty !== "RSA") { + throw new Error(`${field}.keys[${i}].kty must be RSA for n/e keys`); + } + checkRsaPublicKey(jwk, keyField); + } + if (jwk.x !== undefined || jwk.y !== undefined || jwk.crv !== undefined) { + if (jwk.kty !== "EC") { + throw new Error(`${field}.keys[${i}].kty must be EC for x/y keys`); + } + checkEcPublicKey(jwk, keyField); + } } else if (jwk.n && jwk.e) { if (jwk.kty !== "RSA") { throw new Error(`${field}.keys[${i}].kty must be RSA for n/e keys`); @@ -539,7 +553,10 @@ const actions = new Map([ "Cannot specify a recovery_role value when encryption_pub_key is not specified", ); } - if (args.encryption_pub_key !== null && args.encryption_pub_key !== undefined) { + if ( + args.encryption_pub_key !== null && + args.encryption_pub_key !== undefined + ) { checkRsaPublicKey( ccf.crypto.pubRsaPemToJwk(args.encryption_pub_key), "encryption_pub_key", diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index defa8d1a874f..92f46a1a66a9 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -250,7 +250,9 @@ function checkJwks(value, field) { for (const [j, b64der] of jwk.x5c.entries()) { checkType(b64der, "string", `${field}.keys[${i}].x5c[${j}]`); if (!/^[A-Za-z0-9+/]+={0,2}$/.test(b64der)) { - throw new Error(`${field}.keys[${i}].x5c[${j}] must be base64 encoded`); + throw new Error( + `${field}.keys[${i}].x5c[${j}] must be base64 encoded`, + ); } const pem = "-----BEGIN CERTIFICATE-----\n" + @@ -266,6 +268,18 @@ function checkJwks(value, field) { if (!ccf.crypto.isValidX509CertChain(certBundle, trustedRoot)) { throw new Error(`${field}.keys[${i}].x5c must chain to its root`); } + if (jwk.n !== undefined || jwk.e !== undefined) { + if (jwk.kty !== "RSA") { + throw new Error(`${field}.keys[${i}].kty must be RSA for n/e keys`); + } + checkRsaPublicKey(jwk, keyField); + } + if (jwk.x !== undefined || jwk.y !== undefined || jwk.crv !== undefined) { + if (jwk.kty !== "EC") { + throw new Error(`${field}.keys[${i}].kty must be EC for x/y keys`); + } + checkEcPublicKey(jwk, keyField); + } } else if (jwk.n && jwk.e) { if (jwk.kty !== "RSA") { throw new Error(`${field}.keys[${i}].kty must be RSA for n/e keys`); @@ -518,7 +532,10 @@ const actions = new Map([ "Cannot specify a recovery_role value when encryption_pub_key is not specified", ); } - if (args.encryption_pub_key !== null && args.encryption_pub_key !== undefined) { + if ( + args.encryption_pub_key !== null && + args.encryption_pub_key !== undefined + ) { checkRsaPublicKey( ccf.crypto.pubRsaPemToJwk(args.encryption_pub_key), "encryption_pub_key", From fe739aaf0d0bedb5a4c04fcd4735689264215700 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:37:09 +0000 Subject: [PATCH 04/21] Clarify base64url byte length calculation --- samples/constitutions/default/actions.js | 2 +- samples/minimal_ccf/app/actions.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index e646714d1958..d76203c6af3b 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -104,7 +104,7 @@ function checkBase64Url(value, field) { function base64UrlByteLength(value, field) { checkBase64Url(value, field); - return Math.floor((value.length * 3) / 4); + return Math.floor(value.length / 4) * 3 + [0, 0, 1, 2][value.length % 4]; } function splitX509CertBundle(value) { diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 92f46a1a66a9..f3506f86d7d4 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -104,7 +104,7 @@ function checkBase64Url(value, field) { function base64UrlByteLength(value, field) { checkBase64Url(value, field); - return Math.floor((value.length * 3) / 4); + return Math.floor(value.length / 4) * 3 + [0, 0, 1, 2][value.length % 4]; } function splitX509CertBundle(value) { From 0fc1214107cdb46c085c77bc8a803ba520c5a6b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 12:38:23 +0000 Subject: [PATCH 05/21] Address validation review feedback --- samples/constitutions/default/actions.js | 34 +++++++++++++++--------- samples/minimal_ccf/app/actions.js | 34 +++++++++++++++--------- tests/jwt_test.py | 6 +++++ 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index d76203c6af3b..4406db2cddb8 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -134,12 +134,16 @@ function checkRsaPublicKey(jwk, field) { throw new Error(`${field}.n must be at least 2048 bits`); } base64UrlByteLength(jwk.e, `${field}.e`); - ccf.crypto.pubRsaJwkToPem({ - kty: "RSA", - kid: jwk.kid, - n: jwk.n, - e: jwk.e, - }); + try { + ccf.crypto.pubRsaJwkToPem({ + kty: "RSA", + kid: jwk.kid, + n: jwk.n, + e: jwk.e, + }); + } catch (e) { + throw new Error(`${field} must be a valid RSA public key`); + } } function checkEcPublicKey(jwk, field) { @@ -154,13 +158,17 @@ function checkEcPublicKey(jwk, field) { if (base64UrlByteLength(jwk.y, `${field}.y`) !== coordinateLength) { throw new Error(`${field}.y must be ${coordinateLength} bytes`); } - ccf.crypto.pubJwkToPem({ - kty: "EC", - kid: jwk.kid, - crv: jwk.crv, - x: jwk.x, - y: jwk.y, - }); + try { + ccf.crypto.pubJwkToPem({ + kty: "EC", + kid: jwk.kid, + crv: jwk.crv, + x: jwk.x, + y: jwk.y, + }); + } catch (e) { + throw new Error(`${field} must be a valid EC public key`); + } } const cpuid_length_bytes = 4; diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index f3506f86d7d4..9d6c34c00d34 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -134,12 +134,16 @@ function checkRsaPublicKey(jwk, field) { throw new Error(`${field}.n must be at least 2048 bits`); } base64UrlByteLength(jwk.e, `${field}.e`); - ccf.crypto.pubRsaJwkToPem({ - kty: "RSA", - kid: jwk.kid, - n: jwk.n, - e: jwk.e, - }); + try { + ccf.crypto.pubRsaJwkToPem({ + kty: "RSA", + kid: jwk.kid, + n: jwk.n, + e: jwk.e, + }); + } catch (e) { + throw new Error(`${field} must be a valid RSA public key`); + } } function checkEcPublicKey(jwk, field) { @@ -154,13 +158,17 @@ function checkEcPublicKey(jwk, field) { if (base64UrlByteLength(jwk.y, `${field}.y`) !== coordinateLength) { throw new Error(`${field}.y must be ${coordinateLength} bytes`); } - ccf.crypto.pubJwkToPem({ - kty: "EC", - kid: jwk.kid, - crv: jwk.crv, - x: jwk.x, - y: jwk.y, - }); + try { + ccf.crypto.pubJwkToPem({ + kty: "EC", + kid: jwk.kid, + crv: jwk.crv, + x: jwk.x, + y: jwk.y, + }); + } catch (e) { + throw new Error(`${field} must be a valid EC public key`); + } } function checkValidCpuid(value, field) { diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 71ab287ec09b..16554df2529f 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -58,6 +58,12 @@ def test_jwt_issuer_and_jwks_validation(network, args): valid_key = issuer.create_jwks("kid1")["keys"][0] assert_set_jwt_issuer_rejected(network, primary, {"issuer": "example.issuer"}) + assert_set_jwt_issuer_rejected( + network, primary, {"issuer": "https://example.issuer?foo=bar"} + ) + assert_set_jwt_issuer_rejected( + network, primary, {"issuer": "https://example.issuer#fragment"} + ) assert_set_jwt_issuer_rejected( network, primary, From 0416dc7958d1c68239dfc9a215cb57274e6cf6b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 13:07:32 +0000 Subject: [PATCH 06/21] Add P-521 EC key validation support --- include/ccf/crypto/curve.h | 11 +++++++--- include/ccf/crypto/jwk.h | 6 ++++-- js/ccf-app/src/global.ts | 3 ++- js/ccf-app/test/polyfill.test.ts | 9 +++++++- samples/constitutions/default/actions.js | 5 +++-- samples/minimal_ccf/app/actions.js | 5 +++-- src/crypto/openssl/ec_public_key.cpp | 9 ++++++++ src/crypto/test/crypto.cpp | 27 +++++++++++++++++++++++- src/js/extensions/ccf/crypto.cpp | 7 +++++- tests/npm_tests.py | 2 +- 10 files changed, 70 insertions(+), 14 deletions(-) diff --git a/include/ccf/crypto/curve.h b/include/ccf/crypto/curve.h index 7bbf890d5d44..a04a42cc15bd 100644 --- a/include/ccf/crypto/curve.h +++ b/include/ccf/crypto/curve.h @@ -24,7 +24,9 @@ namespace ccf::crypto SECP256R1, /// The CURVE25519 curve CURVE25519, - X25519 + X25519, + /// The SECP521R1 curve + SECP521R1 }; DECLARE_JSON_ENUM( @@ -32,8 +34,9 @@ namespace ccf::crypto {{CurveID::NONE, "None"}, {CurveID::SECP384R1, "Secp384R1"}, {CurveID::SECP256R1, "Secp256R1"}, - {CurveID::CURVE25519, "Curve25519"}, - {CurveID::X25519, "X25519"}}); + {CurveID::CURVE25519, "Curve25519"}, + {CurveID::X25519, "X25519"}, + {CurveID::SECP521R1, "Secp521R1"}}); static constexpr CurveID service_identity_curve_choice = CurveID::SECP384R1; // SNIPPET_END: supported_curves @@ -53,6 +56,8 @@ namespace ccf::crypto return MDType::SHA384; case CurveID::SECP256R1: return MDType::SHA256; + case CurveID::SECP521R1: + return MDType::SHA512; default: { throw std::logic_error(fmt::format("Unhandled CurveId: {}", ec)); diff --git a/include/ccf/crypto/jwk.h b/include/ccf/crypto/jwk.h index 2f9fd2546818..bcbceee61e06 100644 --- a/include/ccf/crypto/jwk.h +++ b/include/ccf/crypto/jwk.h @@ -78,6 +78,8 @@ namespace ccf::crypto return JsonWebKeyECCurve::P384; case CurveID::SECP256R1: return JsonWebKeyECCurve::P256; + case CurveID::SECP521R1: + return JsonWebKeyECCurve::P521; default: throw std::logic_error(fmt::format("Unknown curve {}", curve_id)); } @@ -88,8 +90,7 @@ namespace ccf::crypto switch (jwk_curve) { case JsonWebKeyECCurve::P521: - throw std::logic_error( - fmt::format("Unsupported JWK curve {}", jwk_curve)); + return CurveID::SECP521R1; case JsonWebKeyECCurve::P384: return CurveID::SECP384R1; case JsonWebKeyECCurve::P256: @@ -116,6 +117,7 @@ namespace ccf::crypto case CurveID::NONE: case CurveID::SECP384R1: case CurveID::SECP256R1: + case CurveID::SECP521R1: throw std::logic_error(fmt::format("Invalid EdDSA curve {}", curve_id)); case CurveID::CURVE25519: return JsonWebKeyEdDSACurve::ED25519; diff --git a/js/ccf-app/src/global.ts b/js/ccf-app/src/global.ts index 9497cb9ee6d6..4df3aa1d80a9 100644 --- a/js/ccf-app/src/global.ts +++ b/js/ccf-app/src/global.ts @@ -386,7 +386,8 @@ export interface CCFCrypto { /** * Generate an ECDSA key pair. * - * @param curve The name of the curve, one of "secp256r1", "secp384r1". + * @param curve The name of the curve, one of "secp256r1", "secp384r1", + * "secp521r1". */ generateEcdsaKeyPair(curve: string): CryptoKeyPair; diff --git a/js/ccf-app/test/polyfill.test.ts b/js/ccf-app/test/polyfill.test.ts index ce63a5a6eb89..975811b757c5 100644 --- a/js/ccf-app/test/polyfill.test.ts +++ b/js/ccf-app/test/polyfill.test.ts @@ -97,6 +97,13 @@ describe("polyfill", function () { assert.isTrue(pair.privateKey.startsWith("-----BEGIN PRIVATE KEY-----")); }); }); + describe("generateEcdsaKeyPair/secp521r1", function () { + it("generates a random ECDSA P521R1 key pair", function () { + const pair = ccf.crypto.generateEcdsaKeyPair("secp521r1"); + assert.isTrue(pair.publicKey.startsWith("-----BEGIN PUBLIC KEY-----")); + assert.isTrue(pair.privateKey.startsWith("-----BEGIN PRIVATE KEY-----")); + }); + }); describe("generateEddsaKeyPair/Curve25519", function () { it("generates a random EdDSA Curve25519 key pair", function () { const pair = ccf.crypto.generateEddsaKeyPair("curve25519"); @@ -584,7 +591,7 @@ describe("polyfill", function () { describe("pemToJwk and jwkToPem", function () { it("EC", function () { const my_kid = "my_kid"; - const curves = ["secp256r1", "secp384r1"]; + const curves = ["secp256r1", "secp384r1", "secp521r1"]; for (const curve of curves) { const pair = ccf.crypto.generateEcdsaKeyPair(curve); { diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 4406db2cddb8..e99a4359525c 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -150,8 +150,9 @@ function checkEcPublicKey(jwk, field) { checkType(jwk.x, "string", `${field}.x`); checkType(jwk.y, "string", `${field}.y`); checkType(jwk.crv, "string", `${field}.crv`); - checkEnum(jwk.crv, ["P-256", "P-384"], `${field}.crv`); - const coordinateLength = jwk.crv === "P-256" ? 32 : 48; + checkEnum(jwk.crv, ["P-256", "P-384", "P-521"], `${field}.crv`); + const coordinateLengths = { "P-256": 32, "P-384": 48, "P-521": 66 }; + const coordinateLength = coordinateLengths[jwk.crv]; if (base64UrlByteLength(jwk.x, `${field}.x`) !== coordinateLength) { throw new Error(`${field}.x must be ${coordinateLength} bytes`); } diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 9d6c34c00d34..7a1ec803e2f8 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -150,8 +150,9 @@ function checkEcPublicKey(jwk, field) { checkType(jwk.x, "string", `${field}.x`); checkType(jwk.y, "string", `${field}.y`); checkType(jwk.crv, "string", `${field}.crv`); - checkEnum(jwk.crv, ["P-256", "P-384"], `${field}.crv`); - const coordinateLength = jwk.crv === "P-256" ? 32 : 48; + checkEnum(jwk.crv, ["P-256", "P-384", "P-521"], `${field}.crv`); + const coordinateLengths = { "P-256": 32, "P-384": 48, "P-521": 66 }; + const coordinateLength = coordinateLengths[jwk.crv]; if (base64UrlByteLength(jwk.x, `${field}.x`) !== coordinateLength) { throw new Error(`${field}.x must be ${coordinateLength} bytes`); } diff --git a/src/crypto/openssl/ec_public_key.cpp b/src/crypto/openssl/ec_public_key.cpp index e6c46930a573..ff372baa7bd8 100644 --- a/src/crypto/openssl/ec_public_key.cpp +++ b/src/crypto/openssl/ec_public_key.cpp @@ -127,6 +127,8 @@ namespace ccf::crypto return CurveID::SECP384R1; case NID_X9_62_prime256v1: return CurveID::SECP256R1; + case NID_secp521r1: + return CurveID::SECP521R1; default: throw std::runtime_error(fmt::format("Unknown OpenSSL curve {}", nid)); } @@ -151,6 +153,11 @@ namespace ccf::crypto return NID_X9_62_prime256v1; } + if (gname == SN_secp521r1) + { + return NID_secp521r1; + } + throw std::runtime_error(fmt::format("Unknown OpenSSL group {}", gname)); } @@ -164,6 +171,8 @@ namespace ccf::crypto return NID_secp384r1; case CurveID::SECP256R1: return NID_X9_62_prime256v1; + case CurveID::SECP521R1: + return NID_secp521r1; default: throw std::logic_error( fmt::format("unsupported OpenSSL CurveID {}", gid)); diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index dc0d81ec1224..5d2b04bd2ff7 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -1006,7 +1006,8 @@ TEST_CASE("PEM to JWK and back") INFO("EC"); { - auto curves = {CurveID::SECP384R1, CurveID::SECP256R1}; + auto curves = { + CurveID::SECP384R1, CurveID::SECP256R1, CurveID::SECP521R1}; for (auto const& curve : curves) { @@ -1252,6 +1253,30 @@ TEST_CASE("COSE algorithm validation") REQUIRE_THROWS_WITH( p384_pubkey->check_is_cose_compatible(-100), "secp384r1 key cannot be used with COSE algorithm -100"); + + // P-521 (secp521r1) requires COSE alg -36 + auto p521_kp = ccf::crypto::make_ec_key_pair(CurveID::SECP521R1); + auto p521_pubkey = std::dynamic_pointer_cast( + ccf::crypto::make_ec_public_key(p521_kp->public_key_pem())); + + // Correct algorithm should work + REQUIRE_NOTHROW(p521_pubkey->check_is_cose_compatible(-36)); + + // Wrong algorithms should throw + REQUIRE_THROWS_WITH( + p521_pubkey->check_is_cose_compatible(-7), + "secp521r1 key cannot be used with COSE algorithm -7"); + REQUIRE_THROWS_WITH( + p521_pubkey->check_is_cose_compatible(-35), + "secp521r1 key cannot be used with COSE algorithm -35"); + + // Unknown COSE algorithm for EC keys should throw + REQUIRE_THROWS_WITH( + p521_pubkey->check_is_cose_compatible(0), + "secp521r1 key cannot be used with COSE algorithm 0"); + REQUIRE_THROWS_WITH( + p521_pubkey->check_is_cose_compatible(-100), + "secp521r1 key cannot be used with COSE algorithm -100"); } INFO("RSA keys accept PS256, PS384, and PS512"); diff --git a/src/js/extensions/ccf/crypto.cpp b/src/js/extensions/ccf/crypto.cpp index c5f860405d6a..e3bf8a1b67bf 100644 --- a/src/js/extensions/ccf/crypto.cpp +++ b/src/js/extensions/ccf/crypto.cpp @@ -154,10 +154,15 @@ namespace ccf::js::extensions { cid = ccf::crypto::CurveID::SECP384R1; } + else if (curve == "secp521r1") + { + cid = ccf::crypto::CurveID::SECP521R1; + } else { return JS_ThrowRangeError( - ctx, "Unsupported curve id, supported: secp256r1, secp384r1"); + ctx, + "Unsupported curve id, supported: secp256r1, secp384r1, secp521r1"); } try diff --git a/tests/npm_tests.py b/tests/npm_tests.py index a8099de1c202..fe24dd371f15 100644 --- a/tests/npm_tests.py +++ b/tests/npm_tests.py @@ -44,7 +44,7 @@ def generate_and_verify_jwk(client): assert r.status_code != http.HTTPStatus.OK # Elliptic curve - curves = [ec.SECP256R1, ec.SECP384R1] + curves = [ec.SECP256R1, ec.SECP384R1, ec.SECP521R1] for curve in curves: priv_pem, pub_pem = infra.crypto.generate_ec_keypair(curve) # Private From 4434f1f2994dc1f764570d504d3bb620afd9c955 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 13:11:32 +0000 Subject: [PATCH 07/21] Add changelog entry for validation hardening --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44ed15883dcc..bbcaf5815905 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `ccf.ledger` `MERKLE` verification level now also verifies COSE-only ledgers (previously a silent no-op) (#7904). - Nodes started in recovery or join mode from a snapshot more recent than the latest ledger file now correctly resume writing from the snapshot boundary (#7901). +- Sample constitution validation now rejects weak or malformed JWT, CA bundle, and member encryption key inputs, and supports P-521 EC JWK validation (#7924). ## [7.0.3] From bdfdd14e98b39528d7e92f8bb68b959aa6d2280c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 5 Jun 2026 13:12:09 +0000 Subject: [PATCH 08/21] Clarify changelog validation scope --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbcaf5815905..91765f554420 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `ccf.ledger` `MERKLE` verification level now also verifies COSE-only ledgers (previously a silent no-op) (#7904). - Nodes started in recovery or join mode from a snapshot more recent than the latest ledger file now correctly resume writing from the snapshot boundary (#7901). -- Sample constitution validation now rejects weak or malformed JWT, CA bundle, and member encryption key inputs, and supports P-521 EC JWK validation (#7924). +- Default and minimal sample constitution validation now rejects weak or malformed JWT, CA bundle, and member encryption key inputs, and supports P-521 EC JWK validation (#7924). ## [7.0.3] From 6ada538f468d0be0128cf1721596548de9fb39fd Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 8 Jun 2026 17:36:10 +0000 Subject: [PATCH 09/21] Fix P-521 JWK serialisation and JWT issuer URL in tests - Use ceiling division when sizing EC coordinate/private-scalar buffers so curves whose bit-size is not a multiple of 8 (P-521 has 521 bits) get the full 66-byte buffer rather than a truncated 65-byte one. This fixes BN_bn2binpad failures and dropped leading-zero bytes when round-tripping P-521 keys through JWK. - Update test_jwt_auth_raw_key to use an https:// JWT issuer URL so it satisfies the hardened set_jwt_issuer constitution validator added in this PR. - Run clang-format on curve.h and crypto.cpp. --- include/ccf/crypto/curve.h | 6 +++--- src/crypto/openssl/ec_key_pair.cpp | 5 ++++- src/crypto/openssl/ec_public_key.cpp | 6 +++++- src/crypto/test/crypto.cpp | 3 +-- tests/js-custom-authorization/custom_authorization.py | 6 +++++- 5 files changed, 18 insertions(+), 8 deletions(-) diff --git a/include/ccf/crypto/curve.h b/include/ccf/crypto/curve.h index a04a42cc15bd..f78494740935 100644 --- a/include/ccf/crypto/curve.h +++ b/include/ccf/crypto/curve.h @@ -34,9 +34,9 @@ namespace ccf::crypto {{CurveID::NONE, "None"}, {CurveID::SECP384R1, "Secp384R1"}, {CurveID::SECP256R1, "Secp256R1"}, - {CurveID::CURVE25519, "Curve25519"}, - {CurveID::X25519, "X25519"}, - {CurveID::SECP521R1, "Secp521R1"}}); + {CurveID::CURVE25519, "Curve25519"}, + {CurveID::X25519, "X25519"}, + {CurveID::SECP521R1, "Secp521R1"}}); static constexpr CurveID service_identity_curve_choice = CurveID::SECP384R1; // SNIPPET_END: supported_curves diff --git a/src/crypto/openssl/ec_key_pair.cpp b/src/crypto/openssl/ec_key_pair.cpp index ef7e3541ff03..c2236cd3b306 100644 --- a/src/crypto/openssl/ec_key_pair.cpp +++ b/src/crypto/openssl/ec_key_pair.cpp @@ -511,7 +511,10 @@ namespace ccf::crypto // As per https://www.openssl.org/docs/man1.0.2/man3/BN_num_bytes.html, size // should not be calculated with BN_num_bytes(d)! - size_t size = EVP_PKEY_bits(key) / CHAR_BIT; + // Use ceiling division so curves whose bit-size is not a multiple of 8 + // (e.g. P-521 with 521 bits) get the full 66-byte buffer rather than a + // truncated 65-byte one. + size_t size = (EVP_PKEY_bits(key) + CHAR_BIT - 1) / CHAR_BIT; std::vector bytes(size); Unique_BIGNUM d; BIGNUM* bn_d = nullptr; diff --git a/src/crypto/openssl/ec_public_key.cpp b/src/crypto/openssl/ec_public_key.cpp index ff372baa7bd8..0c97462d957d 100644 --- a/src/crypto/openssl/ec_public_key.cpp +++ b/src/crypto/openssl/ec_public_key.cpp @@ -316,7 +316,11 @@ namespace ccf::crypto x.reset(bn_x); CHECK1(EVP_PKEY_get_bn_param(key, OSSL_PKEY_PARAM_EC_PUB_Y, &bn_y)); y.reset(bn_y); - int sz = EC_GROUP_get_degree(group) / CHAR_BIT; + // Use ceiling division so curves whose bit-size is not a multiple of 8 + // (e.g. P-521 with 521 bits) get the full 66-byte coordinate buffer + // rather than a truncated 65-byte one, which would cause BN_bn2binpad to + // fail or silently drop a leading zero byte. + int sz = (EC_GROUP_get_degree(group) + CHAR_BIT - 1) / CHAR_BIT; r.x.resize(sz); r.y.resize(sz); BN_bn2binpad(x, r.x.data(), sz); diff --git a/src/crypto/test/crypto.cpp b/src/crypto/test/crypto.cpp index 5d2b04bd2ff7..dce3f0ec72b0 100644 --- a/src/crypto/test/crypto.cpp +++ b/src/crypto/test/crypto.cpp @@ -1006,8 +1006,7 @@ TEST_CASE("PEM to JWK and back") INFO("EC"); { - auto curves = { - CurveID::SECP384R1, CurveID::SECP256R1, CurveID::SECP521R1}; + auto curves = {CurveID::SECP384R1, CurveID::SECP256R1, CurveID::SECP521R1}; for (auto const& curve : curves) { diff --git a/tests/js-custom-authorization/custom_authorization.py b/tests/js-custom-authorization/custom_authorization.py index 010e4441923d..b259f1989199 100644 --- a/tests/js-custom-authorization/custom_authorization.py +++ b/tests/js-custom-authorization/custom_authorization.py @@ -447,7 +447,11 @@ def test_jwt_auth_raw_key(network, args): primary, _ = network.find_nodes() for alg in [JwtAlg.RS256, JwtAlg.ES256]: - issuer = JwtIssuer("noautorefresh://issuer", alg=alg, auth_type=JwtAuthType.KEY) + issuer = JwtIssuer( + "https://noautorefresh.example/issuer", + alg=alg, + auth_type=JwtAuthType.KEY, + ) jwt_kid = "my_key_id" issuer.register(network, kid=jwt_kid) From 92b94a6cdd55cf3b62964e73344f16b26046bcd9 Mon Sep 17 00:00:00 2001 From: Amaury Chamayou Date: Mon, 8 Jun 2026 18:46:02 +0000 Subject: [PATCH 10/21] Use base64url (not base64) for JWK n/e/x/y in JwtIssuer RFC 7518 requires JWK numeric fields to be base64url-encoded. The new checkBase64Url validator in the hardened constitution rejects standard base64 (with + or /), which caused the authn e2e test to fail when registering a JwtIssuer raw key whose modulus happened to encode to a byte sequence containing those characters. --- tests/infra/jwt_issuer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/infra/jwt_issuer.py b/tests/infra/jwt_issuer.py index 4cb89454eeb8..0973dbd97087 100644 --- a/tests/infra/jwt_issuer.py +++ b/tests/infra/jwt_issuer.py @@ -141,7 +141,8 @@ def get_jwt_keys(args, node): def to_b64(number: int): as_bytes = number.to_bytes((number.bit_length() + 7) // 8, "big") - return base64.b64encode(as_bytes).decode("ascii") + # JWK requires base64url without padding (RFC 7518). + return base64.urlsafe_b64encode(as_bytes).rstrip(b"=").decode("ascii") class JwtIssuer: From 6e954a39292bbe64ef19b0f81e071cbee3103d8b Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 8 Jun 2026 19:17:46 +0000 Subject: [PATCH 11/21] Accept intermediate CA bundles and bind EC alg to crv Two correctness fixes to the new constitution validators: 1. checkX509CACertBundle previously required every cert in the bundle to be self-signed (chain-to-self), which rejected the common case of an intermediate CA signed by a root CA also in the bundle (typical for real-world IdPs). Validate each cert against the whole bundle instead, which still enforces 'all certs are CAs' (verify_certificate calls X509_check_ca on every trusted cert) while accepting intermediates via X509_V_FLAG_PARTIAL_CHAIN. 2. checkJwks's alg enum allowed any RSA key to claim RS256 and any EC key to claim ES256, even when the crv was P-384 or P-521. Per RFC 7518 section 3.4 each ES* alg binds to a specific curve, so gate the EC alg list on jwk.crv when present (ES256/P-256, ES384/P-384, ES512/P-521). When only x5c is supplied and crv is absent, accept any of the supported ES* algorithms. Add e2e coverage in tests/ca_certs.py (intermediate+root bundle accepted) and tests/jwt_test.py (RSA key with ES256 rejected; EC P-256 key with ES384/ES512/RS256 rejected). --- samples/constitutions/default/actions.js | 35 +++++++++++++++++++----- samples/minimal_ccf/app/actions.js | 35 +++++++++++++++++++----- tests/ca_certs.py | 20 ++++++++++++++ tests/jwt_test.py | 27 ++++++++++++++++++ 4 files changed, 103 insertions(+), 14 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index e99a4359525c..78cbaa62c475 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -118,10 +118,17 @@ function splitX509CertBundle(value) { function checkX509CACertBundle(value, field) { checkX509CertBundle(value, field); + // isValidX509CertChain(target, trusted) is backed by verify_certificate(), + // which (a) rejects the call outright if any cert in `trusted` is not a CA + // (via X509_check_ca), and (b) enables X509_V_FLAG_PARTIAL_CHAIN so that + // intermediate CAs can act as trust anchors. By passing the whole bundle as + // `trusted`, we therefore enforce that every cert in the bundle is a CA + // certificate, while still accepting bundles containing intermediates as + // well as roots. for (const [i, cert] of splitX509CertBundle(value).entries()) { - if (!ccf.crypto.isValidX509CertChain(cert, cert)) { + if (!ccf.crypto.isValidX509CertChain(cert, value)) { throw new Error( - `${field}[${i}] must be a valid CA certificate with a currently valid chain to itself`, + `${field}[${i}] must be a CA certificate with a currently valid chain to a root within the bundle`, ); } } @@ -267,11 +274,25 @@ function checkJwks(value, field) { } if (jwk.alg !== undefined) { checkType(jwk.alg, "string", `${keyField}.alg`); - checkEnum( - jwk.alg, - jwk.kty === "RSA" ? ["RS256"] : ["ES256"], - `${keyField}.alg`, - ); + let allowedAlg; + if (jwk.kty === "RSA") { + allowedAlg = ["RS256"]; + } else { + // Per RFC 7518 section 3.4, EC alg is determined by the curve. When + // only x5c is supplied, crv may not be present on the JWK; in that + // case allow any of the supported ES* algorithms and rely on the cert + // to bind alg to curve. + const ecAlgByCrv = { + "P-256": "ES256", + "P-384": "ES384", + "P-521": "ES512", + }; + allowedAlg = + jwk.crv && ecAlgByCrv[jwk.crv] + ? [ecAlgByCrv[jwk.crv]] + : Object.values(ecAlgByCrv); + } + checkEnum(jwk.alg, allowedAlg, `${keyField}.alg`); } if (jwk.x5c) { checkArrayLength(jwk.x5c, 1, null, `${field}.keys[${i}].x5c`); diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 7a1ec803e2f8..ecce1d280cd0 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -118,10 +118,17 @@ function splitX509CertBundle(value) { function checkX509CACertBundle(value, field) { checkX509CertBundle(value, field); + // isValidX509CertChain(target, trusted) is backed by verify_certificate(), + // which (a) rejects the call outright if any cert in `trusted` is not a CA + // (via X509_check_ca), and (b) enables X509_V_FLAG_PARTIAL_CHAIN so that + // intermediate CAs can act as trust anchors. By passing the whole bundle as + // `trusted`, we therefore enforce that every cert in the bundle is a CA + // certificate, while still accepting bundles containing intermediates as + // well as roots. for (const [i, cert] of splitX509CertBundle(value).entries()) { - if (!ccf.crypto.isValidX509CertChain(cert, cert)) { + if (!ccf.crypto.isValidX509CertChain(cert, value)) { throw new Error( - `${field}[${i}] must be a valid CA certificate with a currently valid chain to itself`, + `${field}[${i}] must be a CA certificate with a currently valid chain to a root within the bundle`, ); } } @@ -247,11 +254,25 @@ function checkJwks(value, field) { } if (jwk.alg !== undefined) { checkType(jwk.alg, "string", `${keyField}.alg`); - checkEnum( - jwk.alg, - jwk.kty === "RSA" ? ["RS256"] : ["ES256"], - `${keyField}.alg`, - ); + let allowedAlg; + if (jwk.kty === "RSA") { + allowedAlg = ["RS256"]; + } else { + // Per RFC 7518 section 3.4, EC alg is determined by the curve. When + // only x5c is supplied, crv may not be present on the JWK; in that + // case allow any of the supported ES* algorithms and rely on the cert + // to bind alg to curve. + const ecAlgByCrv = { + "P-256": "ES256", + "P-384": "ES384", + "P-521": "ES512", + }; + allowedAlg = + jwk.crv && ecAlgByCrv[jwk.crv] + ? [ecAlgByCrv[jwk.crv]] + : Object.values(ecAlgByCrv); + } + checkEnum(jwk.alg, allowedAlg, `${keyField}.alg`); } if (jwk.x5c) { checkArrayLength(jwk.x5c, 1, null, `${field}.keys[${i}].x5c`); diff --git a/tests/ca_certs.py b/tests/ca_certs.py index a19d646357c6..f5210e00cdb1 100644 --- a/tests/ca_certs.py +++ b/tests/ca_certs.py @@ -58,6 +58,26 @@ def test_cert_store(network, args): else: assert False, "Proposal should not have accepted a non-CA certificate" + LOG.info( + "Member makes a ca cert update proposal with an intermediate CA " + "signed by a root CA also in the bundle" + ) + root_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) + root_cert_pem = infra.crypto.generate_cert(root_priv_pem, cn="root", ca=True) + intermediate_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) + intermediate_cert_pem = infra.crypto.generate_cert( + intermediate_priv_pem, + cn="intermediate", + issuer_priv_key_pem=root_priv_pem, + issuer_cn="root", + ca=True, + ) + with tempfile.NamedTemporaryFile(prefix="ccf", mode="w+") as cert_pem_fp: + cert_pem_fp.write(intermediate_cert_pem) + cert_pem_fp.write(root_cert_pem) + cert_pem_fp.flush() + network.consortium.set_ca_cert_bundle(primary, cert_name, cert_pem_fp.name) + LOG.info("Member makes a ca cert update proposal with valid certs") key_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) cert_pem = infra.crypto.generate_cert(key_priv_pem, ca=True) diff --git a/tests/jwt_test.py b/tests/jwt_test.py index 16554df2529f..ca7d67f7769e 100644 --- a/tests/jwt_test.py +++ b/tests/jwt_test.py @@ -85,6 +85,33 @@ def test_jwt_issuer_and_jwks_validation(network, args): {"issuer": issuer.name, "jwks": {"keys": [{**valid_key, "use": "enc"}]}}, ) + # An RSA key (with n/e) tagged with an EC alg must be rejected even though + # ES256 is otherwise an accepted alg value. + assert_set_jwt_issuer_rejected( + network, + primary, + {"issuer": issuer.name, "jwks": {"keys": [{**valid_key, "alg": "ES256"}]}}, + ) + + # EC keys must have alg matching crv per RFC 7518 section 3.4: ES256 binds + # to P-256, ES384 to P-384, ES512 to P-521. An ES256 alg on a P-256 key + # should pass; any other alg on a P-256 key should be rejected. + ec_issuer = infra.jwt_issuer.JwtIssuer( + "https://example.issuer", + alg=infra.jwt_issuer.JwtAlg.ES256, + auth_type=infra.jwt_issuer.JwtAuthType.KEY, + ) + ec_key = ec_issuer.create_jwks("kid1")["keys"][0] + for wrong_alg in ("ES384", "ES512", "RS256"): + assert_set_jwt_issuer_rejected( + network, + primary, + { + "issuer": ec_issuer.name, + "jwks": {"keys": [{**ec_key, "alg": wrong_alg}]}, + }, + ) + @reqs.description("Refresh JWT issuer") def test_refresh_jwt_issuer(network, args): From 6bdf382d4575161133e0b29a430304318cc370a0 Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 8 Jun 2026 19:56:05 +0000 Subject: [PATCH 12/21] Clean up constitution validators and expand CHANGELOG entry - checkRsaPublicKey: call checkBase64Url explicitly on e (rather than discarding the byte-length result) and explain why a 256-byte n is a sufficient proxy for a >=2048-bit modulus per RFC 7518 section 6.3.1.1. - splitX509CertBundle: drop the dead items.length === 1 branch and document that leading whitespace on intermediate certs is tolerated by the OpenSSL PEM parser. - CHANGELOG: move the constitution-validator note from Fixed to Changed and enumerate the operator-visible rejections (https-only issuer, CA-only bundles, base64url-only n/e/x/y, kty/key-material binding, kid uniqueness, use=sig, RFC 7518 section 3.4 alg-vs-curve binding, RSA >=2048 bits, RFC 7518 section 6.2.1.2 EC coordinate length, P-521 support). --- CHANGELOG.md | 2 +- samples/constitutions/default/actions.js | 22 ++++++++++++++++------ samples/minimal_ccf/app/actions.js | 22 ++++++++++++++++------ 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dcd238db7c2..fb88f37cf4af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - JSON parsing can now reject inputs whose object/array nesting depth exceeds a certain value, defaulting to 64 levels and overridable per call site via `ccf::parse_json_safe`'s `max_depth` parameter (#7896). - `NetworkIdentitySubsystem` retries when walking the previous-identity endorsement chain are now bounded by a new optional `identity_history_fetch` node startup config (`max_attempts`, `retry_interval`). On exhaustion the subsystem transitions to a new terminal `FetchStatus::Partial`: the validated suffix of the chain is still served, but historical receipts for seqnos below it fail with an error. Both the chain-extension retries and the pre-bootstrap waits (for the node to join the network, for the service-create txid, and for the first endorsement entry) now poll at `retry_interval` (default `100ms`); the pre-bootstrap polling interval was previously hardcoded to `1s` (#7922). +- The default and minimal sample constitutions have stricter validators for JWT, CA, and member-key proposals (#7924). Operators upgrading should review their proposals: `set_jwt_issuer` now requires `issuer` to be an `https://` URL with no query or fragment (previously any string was accepted when `auto_refresh` was false); `set_ca_cert_bundle` rejects bundles containing non-CA certificates; `set_jwt_public_signing_keys` rejects JWKs whose `n`/`e`/`x`/`y` are not base64url-encoded, whose `kty` does not match the supplied key material, whose `kid` is duplicated within a key set, whose `use` is anything other than `"sig"`, or whose `alg` does not match the key type and curve per RFC 7518 section 3.4 (`RS256` for RSA, `ES256`/`ES384`/`ES512` bound to `P-256`/`P-384`/`P-521` respectively); RSA keys must be at least 2048 bits and EC coordinates must use the full zero-padded length for their curve as required by RFC 7518 section 6.2.1.2; `set_member` validates that `encryption_pub_key` is a well-formed RSA public key. P-521 is now accepted as an EC JWK curve. ### Deprecated @@ -34,7 +35,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `ccf.ledger` `MERKLE` verification level now also verifies COSE-only ledgers (previously a silent no-op) (#7904). - Nodes started in recovery or join mode from a snapshot more recent than the latest ledger file now correctly resume writing from the snapshot boundary (#7901). -- Default and minimal sample constitution validation now rejects weak or malformed JWT, CA bundle, and member encryption key inputs, and supports P-521 EC JWK validation (#7924). ## [7.0.3] diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 78cbaa62c475..edb916e72539 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -108,12 +108,16 @@ function base64UrlByteLength(value, field) { } function splitX509CertBundle(value) { + // Split on the END marker, drop the trailing post-marker remainder (empty + // for a well-formed bundle), and re-append the END marker to each cert. + // Note that intermediate certs may still carry a leading newline (or other + // separator whitespace) from the original concatenation; this is fine + // because the OpenSSL PEM parser is tolerant of leading whitespace. const sep = "-----END CERTIFICATE-----"; - const items = value.split(sep); - if (items.length === 1) { - return []; - } - return items.slice(0, -1).map((p) => p + sep); + return value + .split(sep) + .slice(0, -1) + .map((p) => p + sep); } function checkX509CACertBundle(value, field) { @@ -137,10 +141,16 @@ function checkX509CACertBundle(value, field) { function checkRsaPublicKey(jwk, field) { checkType(jwk.n, "string", `${field}.n`); checkType(jwk.e, "string", `${field}.e`); + checkBase64Url(jwk.e, `${field}.e`); + // RFC 7518 section 6.3.1.1 requires `n` to be the unsigned big-endian modulus with + // no leading zero octets. Under that encoding a 2048-bit modulus is exactly + // 256 octets, so anything shorter is conclusively below 2048 bits and the + // key is too weak. Encoded length is a sufficient (and cheap) proxy here; + // the precise bit length is not exposed to JS, but pubRsaJwkToPem below + // catches any structurally-invalid modulus. if (base64UrlByteLength(jwk.n, `${field}.n`) < 256) { throw new Error(`${field}.n must be at least 2048 bits`); } - base64UrlByteLength(jwk.e, `${field}.e`); try { ccf.crypto.pubRsaJwkToPem({ kty: "RSA", diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index ecce1d280cd0..761e0f194658 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -108,12 +108,16 @@ function base64UrlByteLength(value, field) { } function splitX509CertBundle(value) { + // Split on the END marker, drop the trailing post-marker remainder (empty + // for a well-formed bundle), and re-append the END marker to each cert. + // Note that intermediate certs may still carry a leading newline (or other + // separator whitespace) from the original concatenation; this is fine + // because the OpenSSL PEM parser is tolerant of leading whitespace. const sep = "-----END CERTIFICATE-----"; - const items = value.split(sep); - if (items.length === 1) { - return []; - } - return items.slice(0, -1).map((p) => p + sep); + return value + .split(sep) + .slice(0, -1) + .map((p) => p + sep); } function checkX509CACertBundle(value, field) { @@ -137,10 +141,16 @@ function checkX509CACertBundle(value, field) { function checkRsaPublicKey(jwk, field) { checkType(jwk.n, "string", `${field}.n`); checkType(jwk.e, "string", `${field}.e`); + checkBase64Url(jwk.e, `${field}.e`); + // RFC 7518 section 6.3.1.1 requires `n` to be the unsigned big-endian modulus with + // no leading zero octets. Under that encoding a 2048-bit modulus is exactly + // 256 octets, so anything shorter is conclusively below 2048 bits and the + // key is too weak. Encoded length is a sufficient (and cheap) proxy here; + // the precise bit length is not exposed to JS, but pubRsaJwkToPem below + // catches any structurally-invalid modulus. if (base64UrlByteLength(jwk.n, `${field}.n`) < 256) { throw new Error(`${field}.n must be at least 2048 bits`); } - base64UrlByteLength(jwk.e, `${field}.e`); try { ccf.crypto.pubRsaJwkToPem({ kty: "RSA", From f1b91aba668784fc56baf0c515144f40ac239851 Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 8 Jun 2026 20:07:44 +0000 Subject: [PATCH 13/21] Split CHANGELOG validator entry and fix base64url RFC citation - Break the single 14-line constitution-validator bullet into four focused bullets, one per affected proposal action, so each operator-visible behaviour change is independently scannable. - to_b64: correct the RFC citation. Base64url is defined in RFC 4648 section 5; RFC 7518 section 6 only references it for JWK numeric fields. --- CHANGELOG.md | 5 ++++- tests/infra/jwt_issuer.py | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb88f37cf4af..5c60b845a429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - JSON parsing can now reject inputs whose object/array nesting depth exceeds a certain value, defaulting to 64 levels and overridable per call site via `ccf::parse_json_safe`'s `max_depth` parameter (#7896). - `NetworkIdentitySubsystem` retries when walking the previous-identity endorsement chain are now bounded by a new optional `identity_history_fetch` node startup config (`max_attempts`, `retry_interval`). On exhaustion the subsystem transitions to a new terminal `FetchStatus::Partial`: the validated suffix of the chain is still served, but historical receipts for seqnos below it fail with an error. Both the chain-extension retries and the pre-bootstrap waits (for the node to join the network, for the service-create txid, and for the first endorsement entry) now poll at `retry_interval` (default `100ms`); the pre-bootstrap polling interval was previously hardcoded to `1s` (#7922). -- The default and minimal sample constitutions have stricter validators for JWT, CA, and member-key proposals (#7924). Operators upgrading should review their proposals: `set_jwt_issuer` now requires `issuer` to be an `https://` URL with no query or fragment (previously any string was accepted when `auto_refresh` was false); `set_ca_cert_bundle` rejects bundles containing non-CA certificates; `set_jwt_public_signing_keys` rejects JWKs whose `n`/`e`/`x`/`y` are not base64url-encoded, whose `kty` does not match the supplied key material, whose `kid` is duplicated within a key set, whose `use` is anything other than `"sig"`, or whose `alg` does not match the key type and curve per RFC 7518 section 3.4 (`RS256` for RSA, `ES256`/`ES384`/`ES512` bound to `P-256`/`P-384`/`P-521` respectively); RSA keys must be at least 2048 bits and EC coordinates must use the full zero-padded length for their curve as required by RFC 7518 section 6.2.1.2; `set_member` validates that `encryption_pub_key` is a well-formed RSA public key. P-521 is now accepted as an EC JWK curve. +- The default and minimal sample constitutions reject `set_jwt_issuer` proposals whose `issuer` is not an `https://` URL with no query or fragment. Previously, any string was accepted when `auto_refresh` was `false` (#7924). +- The default and minimal sample constitutions reject `set_ca_cert_bundle` proposals containing non-CA certificates (#7924). +- The default and minimal sample constitutions validate every JWK in `set_jwt_issuer` and `set_jwt_public_signing_keys` proposals: `n`/`e`/`x`/`y` must be base64url-encoded, `kty` must match the supplied key material, `kid` must be unique within a key set, `use` (if present) must be `"sig"`, and `alg` (if present) must match the key type and curve per RFC 7518 section 3.4 (`RS256` for RSA; `ES256`/`ES384`/`ES512` bound to `P-256`/`P-384`/`P-521`). RSA keys must be at least 2048 bits, and EC coordinates must use the full zero-padded length for their curve (RFC 7518 section 6.2.1.2). P-521 is now an accepted EC curve (#7924). +- The default and minimal sample constitutions validate that `set_member`'s `encryption_pub_key`, when present, is a well-formed RSA public key (#7924). ### Deprecated diff --git a/tests/infra/jwt_issuer.py b/tests/infra/jwt_issuer.py index 0973dbd97087..00611ff9be18 100644 --- a/tests/infra/jwt_issuer.py +++ b/tests/infra/jwt_issuer.py @@ -141,7 +141,8 @@ def get_jwt_keys(args, node): def to_b64(number: int): as_bytes = number.to_bytes((number.bit_length() + 7) // 8, "big") - # JWK requires base64url without padding (RFC 7518). + # JWK numeric fields use unpadded base64url (RFC 7518 section 6 referencing + # RFC 4648 section 5). return base64.urlsafe_b64encode(as_bytes).rstrip(b"=").decode("ascii") From d9b8c15cc8a57b4b45bd32ebf17a0f3c7b792daf Mon Sep 17 00:00:00 2001 From: Copilot Date: Mon, 8 Jun 2026 20:10:18 +0000 Subject: [PATCH 14/21] CHANGELOG: move #7924 entries to 7.0.5 The constitution-validator changes ship in 7.0.5, not 7.0.4. Move the four bullets into a new Changed section under 7.0.5. --- CHANGELOG.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c60b845a429..dd6383eb449c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. [7.0.5]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.5 +### Changed + +- The default and minimal sample constitutions reject `set_jwt_issuer` proposals whose `issuer` is not an `https://` URL with no query or fragment. Previously, any string was accepted when `auto_refresh` was `false` (#7924). +- The default and minimal sample constitutions reject `set_ca_cert_bundle` proposals containing non-CA certificates (#7924). +- The default and minimal sample constitutions validate every JWK in `set_jwt_issuer` and `set_jwt_public_signing_keys` proposals: `n`/`e`/`x`/`y` must be base64url-encoded, `kty` must match the supplied key material, `kid` must be unique within a key set, `use` (if present) must be `"sig"`, and `alg` (if present) must match the key type and curve per RFC 7518 section 3.4 (`RS256` for RSA; `ES256`/`ES384`/`ES512` bound to `P-256`/`P-384`/`P-521`). RSA keys must be at least 2048 bits, and EC coordinates must use the full zero-padded length for their curve (RFC 7518 section 6.2.1.2). P-521 is now an accepted EC curve (#7924). +- The default and minimal sample constitutions validate that `set_member`'s `encryption_pub_key`, when present, is a well-formed RSA public key (#7924). + ### Security - Host-created files (ledger chunks, snapshots, PID file, and node certificate/key files) are now created with restrictive permissions (`0600`) instead of relying on the process `umask`. Existing deployments will not see existing files affected; only newly created files will have these restricted permissions (#7916). @@ -25,10 +32,6 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - JSON parsing can now reject inputs whose object/array nesting depth exceeds a certain value, defaulting to 64 levels and overridable per call site via `ccf::parse_json_safe`'s `max_depth` parameter (#7896). - `NetworkIdentitySubsystem` retries when walking the previous-identity endorsement chain are now bounded by a new optional `identity_history_fetch` node startup config (`max_attempts`, `retry_interval`). On exhaustion the subsystem transitions to a new terminal `FetchStatus::Partial`: the validated suffix of the chain is still served, but historical receipts for seqnos below it fail with an error. Both the chain-extension retries and the pre-bootstrap waits (for the node to join the network, for the service-create txid, and for the first endorsement entry) now poll at `retry_interval` (default `100ms`); the pre-bootstrap polling interval was previously hardcoded to `1s` (#7922). -- The default and minimal sample constitutions reject `set_jwt_issuer` proposals whose `issuer` is not an `https://` URL with no query or fragment. Previously, any string was accepted when `auto_refresh` was `false` (#7924). -- The default and minimal sample constitutions reject `set_ca_cert_bundle` proposals containing non-CA certificates (#7924). -- The default and minimal sample constitutions validate every JWK in `set_jwt_issuer` and `set_jwt_public_signing_keys` proposals: `n`/`e`/`x`/`y` must be base64url-encoded, `kty` must match the supplied key material, `kid` must be unique within a key set, `use` (if present) must be `"sig"`, and `alg` (if present) must match the key type and curve per RFC 7518 section 3.4 (`RS256` for RSA; `ES256`/`ES384`/`ES512` bound to `P-256`/`P-384`/`P-521`). RSA keys must be at least 2048 bits, and EC coordinates must use the full zero-padded length for their curve (RFC 7518 section 6.2.1.2). P-521 is now an accepted EC curve (#7924). -- The default and minimal sample constitutions validate that `set_member`'s `encryption_pub_key`, when present, is a well-formed RSA public key (#7924). ### Deprecated From 6de1419cd65f4fb0424101c75bbb7a59953c8398 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:35:26 +0000 Subject: [PATCH 15/21] Restrict CA bundle to root (self-signed) CAs only; reject intermediate CAs --- CHANGELOG.md | 2 +- js/ccf-app/src/crypto.ts | 5 ++ js/ccf-app/src/global.ts | 6 +++ js/ccf-app/src/polyfill.ts | 32 +++++++++++ js/ccf-app/test/crypto.ts | 28 ++++++++++ js/ccf-app/test/polyfill.test.ts | 37 ++++++++++++- samples/constitutions/default/actions.js | 15 +++--- samples/minimal_ccf/app/actions.js | 15 +++--- src/js/extensions/ccf/crypto.cpp | 67 ++++++++++++++++++++++++ tests/ca_certs.py | 9 +++- 10 files changed, 194 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd6383eb449c..fb3ee355f06b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - The default and minimal sample constitutions reject `set_jwt_issuer` proposals whose `issuer` is not an `https://` URL with no query or fragment. Previously, any string was accepted when `auto_refresh` was `false` (#7924). -- The default and minimal sample constitutions reject `set_ca_cert_bundle` proposals containing non-CA certificates (#7924). +- The default and minimal sample constitutions reject `set_ca_cert_bundle` proposals containing non-CA certificates or intermediate CA certificates; every certificate in the bundle must be a self-signed (root) CA (#7924). - The default and minimal sample constitutions validate every JWK in `set_jwt_issuer` and `set_jwt_public_signing_keys` proposals: `n`/`e`/`x`/`y` must be base64url-encoded, `kty` must match the supplied key material, `kid` must be unique within a key set, `use` (if present) must be `"sig"`, and `alg` (if present) must match the key type and curve per RFC 7518 section 3.4 (`RS256` for RSA; `ES256`/`ES384`/`ES512` bound to `P-256`/`P-384`/`P-521`). RSA keys must be at least 2048 bits, and EC coordinates must use the full zero-padded length for their curve (RFC 7518 section 6.2.1.2). P-521 is now an accepted EC curve (#7924). - The default and minimal sample constitutions validate that `set_member`'s `encryption_pub_key`, when present, is a well-formed RSA public key (#7924). diff --git a/js/ccf-app/src/crypto.ts b/js/ccf-app/src/crypto.ts index 6eef6aa4b01f..caacc1c17806 100644 --- a/js/ccf-app/src/crypto.ts +++ b/js/ccf-app/src/crypto.ts @@ -72,6 +72,11 @@ export const isValidX509CertBundle = ccf.crypto.isValidX509CertBundle; */ export const isValidX509CertChain = ccf.crypto.isValidX509CertChain; +/** + * @inheritDoc global!CCFCrypto.isValidX509RootCACert + */ +export const isValidX509RootCACert = ccf.crypto.isValidX509RootCACert; + /** * @inheritDoc global!CCFCrypto.pubPemToJwk */ diff --git a/js/ccf-app/src/global.ts b/js/ccf-app/src/global.ts index 4df3aa1d80a9..0d2be6d8bcff 100644 --- a/js/ccf-app/src/global.ts +++ b/js/ccf-app/src/global.ts @@ -442,6 +442,12 @@ export interface CCFCrypto { */ isValidX509CertChain(chain: string, trusted: string): boolean; + /** + * Returns whether a single PEM-encoded X.509 certificate is a self-signed (root) CA. + * Returns false for intermediate CA certificates, non-CA certificates, and malformed PEM. + */ + isValidX509RootCACert(pem: string): boolean; + /** * Converts an elliptic curve public key as PEM to JSON Web Key (JWK) object. * diff --git a/js/ccf-app/src/polyfill.ts b/js/ccf-app/src/polyfill.ts index 36a51c22202b..65f65e764900 100644 --- a/js/ccf-app/src/polyfill.ts +++ b/js/ccf-app/src/polyfill.ts @@ -463,6 +463,34 @@ class CCFPolyfill implements CCF { return false; } }, + isValidX509RootCACert(pem: string): boolean { + if (!("X509Certificate" in jscrypto)) { + throw new Error( + "X509 validation unsupported, Node.js version too old (< 15.6.0)", + ); + } + try { + const sep = "-----END CERTIFICATE-----"; + const items = pem.split(sep); + // Expect exactly one certificate. + if (items.length !== 2 || items[0].trim() === "") { + return false; + } + const cert = new (jscrypto).X509Certificate(items[0] + sep); + // Must be a CA certificate. + if (!cert.ca) { + return false; + } + // Must be self-signed: the certificate's public key must verify its own signature. + if (!cert.verify(cert.publicKey)) { + return false; + } + return true; + } catch (e: any) { + console.error(`isValidX509RootCACert validation failed: ${e.message}`); + return false; + } + }, pubPemToJwk(pem: string, kid?: string): JsonWebKeyECPublic { const key = jscrypto.createPublicKey({ key: pem, @@ -665,6 +693,10 @@ class CCFPolyfill implements CCF { return this.crypto.isValidX509CertChain(chain, trusted); } + isValidX509RootCACert(pem: string): boolean { + return this.crypto.isValidX509RootCACert(pem); + } + enableUntrustedDateTime(enable: boolean): boolean { throw new Error("Not implemented"); } diff --git a/js/ccf-app/test/crypto.ts b/js/ccf-app/test/crypto.ts index 77ecafa481e2..843c43d9e6c7 100644 --- a/js/ccf-app/test/crypto.ts +++ b/js/ccf-app/test/crypto.ts @@ -27,6 +27,34 @@ export function generateSelfSignedCert() { }; } +export function generateSelfSignedCACert(): string { + const keys = crypto.generateKeyPairSync("rsa", { + modulusLength: 2048, + publicKeyEncoding: { + type: "spki", + format: "pem", + }, + privateKeyEncoding: { + type: "pkcs8", + format: "pem", + }, + }); + const cert = forge.pki.createCertificate(); + cert.publicKey = forge.pki.publicKeyFromPem(keys.publicKey); + cert.setExtensions([ + { + name: "basicConstraints", + cA: true, + critical: true, + }, + ]); + cert.sign( + forge.pki.privateKeyFromPem(keys.privateKey), + forge.md.sha256.create(), + ); + return forge.pki.certificateToPem(cert); +} + export function generateCertChain(len: number): string[] { const keyPairs = []; for (let i = 0; i < len; i++) { diff --git a/js/ccf-app/test/polyfill.test.ts b/js/ccf-app/test/polyfill.test.ts index ad8a5a719fdd..fe1a74b72a0e 100644 --- a/js/ccf-app/test/polyfill.test.ts +++ b/js/ccf-app/test/polyfill.test.ts @@ -9,7 +9,11 @@ import type { } from "../src/global.js"; import { ccf } from "../src/global.js"; import * as textcodec from "../src/textcodec.js"; -import { generateSelfSignedCert, generateCertChain } from "./crypto.js"; +import { + generateSelfSignedCert, + generateSelfSignedCACert, + generateCertChain, +} from "./crypto.js"; import { toArrayBuffer } from "../src/utils.js"; beforeEach(function () { @@ -786,4 +790,35 @@ describe("polyfill", function () { assert.isFalse(ccf.crypto.isValidX509CertChain(chain, trusted)); }); }); + describe("isValidX509RootCACert", function (this) { + const supported = "X509Certificate" in crypto; + it("returns true for a self-signed CA certificate", function () { + if (!supported) { + this.skip(); + } + const pem = generateSelfSignedCACert(); + assert.isTrue(ccf.crypto.isValidX509RootCACert(pem)); + }); + it("returns false for a non-CA self-signed certificate", function () { + if (!supported) { + this.skip(); + } + const pem = generateSelfSignedCert().cert; + assert.isFalse(ccf.crypto.isValidX509RootCACert(pem)); + }); + it("returns false for an intermediate CA certificate", function () { + if (!supported) { + this.skip(); + } + // generateCertChain returns [leaf, intermediate, root]; [1] is intermediate. + const pems = generateCertChain(3); + assert.isFalse(ccf.crypto.isValidX509RootCACert(pems[0])); + }); + it("returns false for malformed input", function () { + if (!supported) { + this.skip(); + } + assert.isFalse(ccf.crypto.isValidX509RootCACert("garbage")); + }); + }); }); diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index edb916e72539..c3f19946b0ea 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -122,17 +122,14 @@ function splitX509CertBundle(value) { function checkX509CACertBundle(value, field) { checkX509CertBundle(value, field); - // isValidX509CertChain(target, trusted) is backed by verify_certificate(), - // which (a) rejects the call outright if any cert in `trusted` is not a CA - // (via X509_check_ca), and (b) enables X509_V_FLAG_PARTIAL_CHAIN so that - // intermediate CAs can act as trust anchors. By passing the whole bundle as - // `trusted`, we therefore enforce that every cert in the bundle is a CA - // certificate, while still accepting bundles containing intermediates as - // well as roots. + // isValidX509RootCACert(pem) is backed by a C++ function that checks both + // X509_check_ca (CA:TRUE or self-signed x509v1) and EXFLAG_SS (self-signed). + // Every certificate in the bundle must be a root (self-signed) CA; intermediate + // CAs are rejected even when their signing root is also present in the bundle. for (const [i, cert] of splitX509CertBundle(value).entries()) { - if (!ccf.crypto.isValidX509CertChain(cert, value)) { + if (!ccf.crypto.isValidX509RootCACert(cert)) { throw new Error( - `${field}[${i}] must be a CA certificate with a currently valid chain to a root within the bundle`, + `${field}[${i}] must be a self-signed (root) CA certificate`, ); } } diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 761e0f194658..dae0d6716ea9 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -122,17 +122,14 @@ function splitX509CertBundle(value) { function checkX509CACertBundle(value, field) { checkX509CertBundle(value, field); - // isValidX509CertChain(target, trusted) is backed by verify_certificate(), - // which (a) rejects the call outright if any cert in `trusted` is not a CA - // (via X509_check_ca), and (b) enables X509_V_FLAG_PARTIAL_CHAIN so that - // intermediate CAs can act as trust anchors. By passing the whole bundle as - // `trusted`, we therefore enforce that every cert in the bundle is a CA - // certificate, while still accepting bundles containing intermediates as - // well as roots. + // isValidX509RootCACert(pem) is backed by a C++ function that checks both + // X509_check_ca (CA:TRUE or self-signed x509v1) and EXFLAG_SS (self-signed). + // Every certificate in the bundle must be a root (self-signed) CA; intermediate + // CAs are rejected even when their signing root is also present in the bundle. for (const [i, cert] of splitX509CertBundle(value).entries()) { - if (!ccf.crypto.isValidX509CertChain(cert, value)) { + if (!ccf.crypto.isValidX509RootCACert(cert)) { throw new Error( - `${field}[${i}] must be a CA certificate with a currently valid chain to a root within the bundle`, + `${field}[${i}] must be a self-signed (root) CA certificate`, ); } } diff --git a/src/js/extensions/ccf/crypto.cpp b/src/js/extensions/ccf/crypto.cpp index e3bf8a1b67bf..94b592300f91 100644 --- a/src/js/extensions/ccf/crypto.cpp +++ b/src/js/extensions/ccf/crypto.cpp @@ -397,6 +397,69 @@ namespace ccf::js::extensions return ccf::js::core::constants::True; } + JSValue js_is_valid_x509_root_ca_cert( + JSContext* ctx, JSValueConst, int argc, JSValueConst* argv) + { + // Returns true iff the argument is a single, self-signed CA certificate. + // Unlike isValidX509CertChain, this rejects intermediate CAs: a cert must + // be self-signed (EXFLAG_SS) as well as passing X509_check_ca. + if (argc != 1) + { + return JS_ThrowTypeError( + ctx, "Passed %d arguments, but expected 1", argc); + } + + js::core::Context& jsctx = + *reinterpret_cast(JS_GetContextOpaque(ctx)); + + auto pem_str = jsctx.to_str(argv[0]); + if (!pem_str) + { + return ccf::js::core::constants::Exception; + } + + try + { + auto certs = ccf::crypto::split_x509_cert_bundle(*pem_str); + if (certs.size() != 1) + { + throw std::runtime_error( + "expected exactly one certificate, got " + + std::to_string(certs.size())); + } + + auto verifier = ccf::crypto::make_unique_verifier(certs[0]); + + // Reject intermediate CAs: the cert must be self-signed. + if (!verifier->is_self_signed()) + { + return ccf::js::core::constants::False; + } + + // Confirm it is a CA by verifying it against itself; verify_certificate + // runs X509_check_ca on each trusted cert and rejects non-CA certs. + const ccf::crypto::Pem* pem_ptr = &certs[0]; + std::vector trusted = {pem_ptr}; + std::vector chain = {}; + if (!verifier->verify_certificate(trusted, chain)) + { + return ccf::js::core::constants::False; + } + } + catch (const std::runtime_error& e) + { + LOG_DEBUG_FMT("isValidX509RootCACert: {}", e.what()); + return ccf::js::core::constants::False; + } + catch (const std::logic_error& e) + { + return JS_ThrowInternalError( + ctx, "isValidX509RootCACert failed: %s", e.what()); + } + + return ccf::js::core::constants::True; + } + template JSValue js_pem_to_jwk( JSContext* ctx, JSValueConst, int argc, JSValueConst* argv) @@ -1219,6 +1282,10 @@ namespace ccf::js::extensions "isValidX509CertChain", ctx.new_c_function( js_is_valid_x509_cert_chain, "isValidX509CertChain", 2))); + JS_CHECK_OR_THROW(crypto.set( + "isValidX509RootCACert", + ctx.new_c_function( + js_is_valid_x509_root_ca_cert, "isValidX509RootCACert", 1))); auto ccf = ctx.get_or_create_global_property("ccf", ctx.new_obj()); JS_CHECK_OR_THROW(ccf.set("crypto", std::move(crypto))); diff --git a/tests/ca_certs.py b/tests/ca_certs.py index f5210e00cdb1..1e0670752877 100644 --- a/tests/ca_certs.py +++ b/tests/ca_certs.py @@ -60,7 +60,7 @@ def test_cert_store(network, args): LOG.info( "Member makes a ca cert update proposal with an intermediate CA " - "signed by a root CA also in the bundle" + "signed by a root CA also in the bundle -- must be rejected" ) root_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) root_cert_pem = infra.crypto.generate_cert(root_priv_pem, cn="root", ca=True) @@ -76,7 +76,12 @@ def test_cert_store(network, args): cert_pem_fp.write(intermediate_cert_pem) cert_pem_fp.write(root_cert_pem) cert_pem_fp.flush() - network.consortium.set_ca_cert_bundle(primary, cert_name, cert_pem_fp.name) + try: + network.consortium.set_ca_cert_bundle(primary, cert_name, cert_pem_fp.name) + except (infra.proposal.ProposalNotAccepted, infra.proposal.ProposalNotCreated): + pass + else: + assert False, "Proposal should not have accepted an intermediate CA certificate" LOG.info("Member makes a ca cert update proposal with valid certs") key_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) From 4c2f995a840b0db64e5e643651ec2bb6bc047d24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:38:49 +0000 Subject: [PATCH 16/21] Fix intermediate CA test: use a proper CA cert signed by a different key --- js/ccf-app/test/crypto.ts | 32 ++++++++++++++++++++++++++++++++ js/ccf-app/test/polyfill.test.ts | 7 ++++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/js/ccf-app/test/crypto.ts b/js/ccf-app/test/crypto.ts index 843c43d9e6c7..9bb4e29a5ea4 100644 --- a/js/ccf-app/test/crypto.ts +++ b/js/ccf-app/test/crypto.ts @@ -55,6 +55,38 @@ export function generateSelfSignedCACert(): string { return forge.pki.certificateToPem(cert); } +/** + * Returns a PEM-encoded intermediate CA certificate (CA:TRUE, signed by a + * freshly-generated root CA). The certificate is NOT self-signed. + */ +export function generateIntermediateCACert(): string { + const rootKeys = crypto.generateKeyPairSync("rsa", { + modulusLength: 2048, + publicKeyEncoding: { type: "spki", format: "pem" }, + privateKeyEncoding: { type: "pkcs8", format: "pem" }, + }); + const intermediateKeys = crypto.generateKeyPairSync("rsa", { + modulusLength: 2048, + publicKeyEncoding: { type: "spki", format: "pem" }, + privateKeyEncoding: { type: "pkcs8", format: "pem" }, + }); + const cert = forge.pki.createCertificate(); + cert.publicKey = forge.pki.publicKeyFromPem(intermediateKeys.publicKey); + cert.setExtensions([ + { + name: "basicConstraints", + cA: true, + critical: true, + }, + ]); + // Sign with the root key so the cert is NOT self-signed. + cert.sign( + forge.pki.privateKeyFromPem(rootKeys.privateKey), + forge.md.sha256.create(), + ); + return forge.pki.certificateToPem(cert); +} + export function generateCertChain(len: number): string[] { const keyPairs = []; for (let i = 0; i < len; i++) { diff --git a/js/ccf-app/test/polyfill.test.ts b/js/ccf-app/test/polyfill.test.ts index fe1a74b72a0e..05f57f1b3e44 100644 --- a/js/ccf-app/test/polyfill.test.ts +++ b/js/ccf-app/test/polyfill.test.ts @@ -12,6 +12,7 @@ import * as textcodec from "../src/textcodec.js"; import { generateSelfSignedCert, generateSelfSignedCACert, + generateIntermediateCACert, generateCertChain, } from "./crypto.js"; import { toArrayBuffer } from "../src/utils.js"; @@ -810,9 +811,9 @@ describe("polyfill", function () { if (!supported) { this.skip(); } - // generateCertChain returns [leaf, intermediate, root]; [1] is intermediate. - const pems = generateCertChain(3); - assert.isFalse(ccf.crypto.isValidX509RootCACert(pems[0])); + // An intermediate CA has CA:TRUE but is signed by a different key (not self-signed). + const pem = generateIntermediateCACert(); + assert.isFalse(ccf.crypto.isValidX509RootCACert(pem)); }); it("returns false for malformed input", function () { if (!supported) { From 9486d33b41b51ce99b2fab781129858766f8992c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:44:12 +0000 Subject: [PATCH 17/21] Harden splitX509CertBundle to validate PEM structure --- samples/constitutions/default/actions.js | 34 +++++++++++++++++------- samples/minimal_ccf/app/actions.js | 34 +++++++++++++++++------- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index c3f19946b0ea..17308c1c51cb 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -108,16 +108,30 @@ function base64UrlByteLength(value, field) { } function splitX509CertBundle(value) { - // Split on the END marker, drop the trailing post-marker remainder (empty - // for a well-formed bundle), and re-append the END marker to each cert. - // Note that intermediate certs may still carry a leading newline (or other - // separator whitespace) from the original concatenation; this is fine - // because the OpenSSL PEM parser is tolerant of leading whitespace. - const sep = "-----END CERTIFICATE-----"; - return value - .split(sep) - .slice(0, -1) - .map((p) => p + sep); + // Match complete PEM certificates with both BEGIN and END markers. + // This ensures we only extract valid PEM blocks and reject malformed input. + const pemPattern = + /-----BEGIN CERTIFICATE-----[\s\S]*?-----END CERTIFICATE-----/g; + const certs = value.match(pemPattern); + + if (!certs || certs.length === 0) { + throw new Error("No valid PEM certificates found in bundle"); + } + + // Verify the input contains only certificates and whitespace. + // Remove all matched certificates and ensure only whitespace remains. + let remaining = value; + for (const cert of certs) { + remaining = remaining.replace(cert, ""); + } + + if (remaining.trim() !== "") { + throw new Error( + "Certificate bundle contains invalid content between certificates", + ); + } + + return certs; } function checkX509CACertBundle(value, field) { diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index dae0d6716ea9..022c286cece5 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -108,16 +108,30 @@ function base64UrlByteLength(value, field) { } function splitX509CertBundle(value) { - // Split on the END marker, drop the trailing post-marker remainder (empty - // for a well-formed bundle), and re-append the END marker to each cert. - // Note that intermediate certs may still carry a leading newline (or other - // separator whitespace) from the original concatenation; this is fine - // because the OpenSSL PEM parser is tolerant of leading whitespace. - const sep = "-----END CERTIFICATE-----"; - return value - .split(sep) - .slice(0, -1) - .map((p) => p + sep); + // Match complete PEM certificates with both BEGIN and END markers. + // This ensures we only extract valid PEM blocks and reject malformed input. + const pemPattern = + /-----BEGIN CERTIFICATE-----[\s\S]*?-----END CERTIFICATE-----/g; + const certs = value.match(pemPattern); + + if (!certs || certs.length === 0) { + throw new Error("No valid PEM certificates found in bundle"); + } + + // Verify the input contains only certificates and whitespace. + // Remove all matched certificates and ensure only whitespace remains. + let remaining = value; + for (const cert of certs) { + remaining = remaining.replace(cert, ""); + } + + if (remaining.trim() !== "") { + throw new Error( + "Certificate bundle contains invalid content between certificates", + ); + } + + return certs; } function checkX509CACertBundle(value, field) { From 3dee01bd417dbd1513b99acd2f7ec0716e24a864 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:46:50 +0000 Subject: [PATCH 18/21] Fix duplicate cert handling: use replaceAll instead of replace --- samples/constitutions/default/actions.js | 2 +- samples/minimal_ccf/app/actions.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 17308c1c51cb..1d3cb8ad26c2 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -122,7 +122,7 @@ function splitX509CertBundle(value) { // Remove all matched certificates and ensure only whitespace remains. let remaining = value; for (const cert of certs) { - remaining = remaining.replace(cert, ""); + remaining = remaining.replaceAll(cert, ""); } if (remaining.trim() !== "") { diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index 022c286cece5..ddfc2829992a 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -122,7 +122,7 @@ function splitX509CertBundle(value) { // Remove all matched certificates and ensure only whitespace remains. let remaining = value; for (const cert of certs) { - remaining = remaining.replace(cert, ""); + remaining = remaining.replaceAll(cert, ""); } if (remaining.trim() !== "") { From 4b5efffc85733cf52de5f5830ae88ad22ed21239 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:49:31 +0000 Subject: [PATCH 19/21] Use single-pass regex replace for cleaner validation --- samples/constitutions/default/actions.js | 8 +++----- samples/minimal_ccf/app/actions.js | 8 +++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/samples/constitutions/default/actions.js b/samples/constitutions/default/actions.js index 1d3cb8ad26c2..20c9bd69bb0e 100644 --- a/samples/constitutions/default/actions.js +++ b/samples/constitutions/default/actions.js @@ -119,11 +119,9 @@ function splitX509CertBundle(value) { } // Verify the input contains only certificates and whitespace. - // Remove all matched certificates and ensure only whitespace remains. - let remaining = value; - for (const cert of certs) { - remaining = remaining.replaceAll(cert, ""); - } + // Use a single-pass approach: replace all matched certificates with empty string + // using the global regex, then check if only whitespace remains. + const remaining = value.replace(pemPattern, ""); if (remaining.trim() !== "") { throw new Error( diff --git a/samples/minimal_ccf/app/actions.js b/samples/minimal_ccf/app/actions.js index ddfc2829992a..d04e54c83494 100644 --- a/samples/minimal_ccf/app/actions.js +++ b/samples/minimal_ccf/app/actions.js @@ -119,11 +119,9 @@ function splitX509CertBundle(value) { } // Verify the input contains only certificates and whitespace. - // Remove all matched certificates and ensure only whitespace remains. - let remaining = value; - for (const cert of certs) { - remaining = remaining.replaceAll(cert, ""); - } + // Use a single-pass approach: replace all matched certificates with empty string + // using the global regex, then check if only whitespace remains. + const remaining = value.replace(pemPattern, ""); if (remaining.trim() !== "") { throw new Error( From 297781c01742121e398fdc4e684a58f61b4d6790 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Jun 2026 17:53:13 +0000 Subject: [PATCH 20/21] Apply Python formatting to tests/ca_certs.py Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- tests/ca_certs.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/ca_certs.py b/tests/ca_certs.py index 1e0670752877..caca8663f139 100644 --- a/tests/ca_certs.py +++ b/tests/ca_certs.py @@ -81,7 +81,9 @@ def test_cert_store(network, args): except (infra.proposal.ProposalNotAccepted, infra.proposal.ProposalNotCreated): pass else: - assert False, "Proposal should not have accepted an intermediate CA certificate" + assert ( + False + ), "Proposal should not have accepted an intermediate CA certificate" LOG.info("Member makes a ca cert update proposal with valid certs") key_priv_pem, _ = infra.crypto.generate_rsa_keypair(2048) From 13249ddf6c396fe54a7ca125a49052e1cb07e909 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Jun 2026 19:13:09 +0000 Subject: [PATCH 21/21] Fix clang-tidy readability-container-data-pointer warning in crypto.cpp Co-authored-by: achamayou <4016369+achamayou@users.noreply.github.com> --- src/js/extensions/ccf/crypto.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/extensions/ccf/crypto.cpp b/src/js/extensions/ccf/crypto.cpp index 94b592300f91..02eeb642f1ca 100644 --- a/src/js/extensions/ccf/crypto.cpp +++ b/src/js/extensions/ccf/crypto.cpp @@ -438,7 +438,7 @@ namespace ccf::js::extensions // Confirm it is a CA by verifying it against itself; verify_certificate // runs X509_check_ca on each trusted cert and rejects non-CA certs. - const ccf::crypto::Pem* pem_ptr = &certs[0]; + const ccf::crypto::Pem* pem_ptr = certs.data(); std::vector trusted = {pem_ptr}; std::vector chain = {}; if (!verifier->verify_certificate(trusted, chain))