From a1fff516e4cf3e82aab30565d717ffb739e53f67 Mon Sep 17 00:00:00 2001 From: Mohammed Zoheb Shaik Date: Wed, 19 Aug 2026 22:01:11 +0400 Subject: [PATCH] fix(catalog): make the approval signing input the JCS it claims to be canonical_json is documented as RFC 8785 compatible and serialized with ensure_ascii=True, which is the one thing JCS does not do. Any record carrying a non-ASCII principal_id, issuer, role, catalog_id, or policy_id was signed over different bytes than a conforming producer signs, so a record produced by any other implementation failed with an invalid-signature error that points nowhere near the encoding: cmcp : b'{"principal_id":"jos\u00e9"}' jcs : b'{"principal_id":"jos\xc3\xa9"}' sort_keys was the second divergence, ordering members by code point where JCS orders by UTF-16 code unit. The two disagree for any key outside the BMP, since a surrogate pair leads with 0xD800 and sorts below a BMP character above 0xE000. Members are now ordered on their UTF-16BE bytes, which is the same comparison since every code unit occupies two bytes, and the output is UTF-8. Values JCS cannot pin down are refused rather than serialized into a signing input two implementations would read differently: * floating point numbers, whose JCS serialization is the ECMAScript number algorithm and is not implemented here * integers beyond 2**53 - 1, which JCS treats as IEEE 754 doubles * non-string object keys, which have no canonical order * unpaired surrogates, which no longer escape as UnicodeEncodeError Each raises CatalogApprovalError, matching how the rest of the module reports malformed input. Approval records carry none of these, so refusing them only closes a door. ASCII-only records serialize to the same bytes as before, pinned by test, so nothing that verifies today stops verifying. #517 asks for JCS reuse precisely so a record can cross implementations, and nothing consumes these records at runtime yet, so the encoding is still free to correct. Verified by reverting canonical_json alone: 11 of the 13 new tests fail. The two that survive are the ASCII byte-stability pin and the largest exact integer, both of which are meant to be invariant. Not addressed: audit/trace_claim.py, cmcp_verify/embodied_action.py, and catalog/loader.py each carry their own copy of the sort_keys idiom with the same two divergences. Only this one is described as RFC 8785 in its docstring, and only this one signs reviewer identities, so the others are left alone here. Signed-off-by: Mohammed Zoheb Shaik --- CHANGELOG.md | 6 ++ docs/spec/catalog-approval-provenance.md | 9 +++ src/cmcp_runtime/catalog/approval.py | 44 +++++++++++- tests/unit/test_catalog_canonical_json.py | 86 +++++++++++++++++++++++ 4 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 tests/unit/test_catalog_canonical_json.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f31794a..00adb4d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **The catalog-approval signing input was not the JCS it claims to be (#517).** `canonical_json` is documented as RFC 8785 compatible and serialized with `ensure_ascii=True`, which is the one thing JCS does not do: it emits an ASCII escape where the standard emits UTF-8. Any record carrying a non-ASCII `principal_id`, `issuer`, `role`, `catalog_id`, or `policy_id` was signed over different bytes than a conforming producer signs, so a record produced anywhere but here failed with an invalid-signature error that points nowhere near the encoding. `sort_keys` was the second divergence, ordering members by code point where JCS orders by UTF-16 code unit. The two disagree for any key outside the BMP, since a surrogate pair leads with `0xD800` and sorts below a BMP character above `0xE000`. + + Members are now ordered on their UTF-16BE bytes and the output is UTF-8. Values JCS cannot pin down are refused rather than serialized into a signing input that two implementations would read differently: floating point numbers, integers beyond `2**53 - 1`, non-string keys, and unpaired surrogates, each as `CatalogApprovalError` rather than as an escaping `UnicodeEncodeError` or `TypeError`. Approval records carry none of those, so refusing them only closes a door. + + ASCII-only records serialize to the same bytes as before, pinned by test, so nothing that verifies today stops verifying. #517 asks for JCS reuse precisely so a record can cross implementations, and nothing consumes these records at runtime yet, so the encoding is still free to correct. + - **cMCP could not verify a v0.2 Agent Manifest at all (agent-manifest#315, phase 4 of agent-manifest#243).** The pin moved to `agent-manifest>=0.11`, which carries the COSE verifier, and nothing here ever presented a v0.2 manifest to it. It could not have worked: `load_agent_manifest` read JSON and `_verify_with_sdk` passed a dict, and from v0.2 the COSE_Sign1 structure **is** the signature (ADR-0011), so a v0.2 document handed over as a dict has nothing to appraise. The SDK correctly reported a missing signature, and an operator would have read that as a malformed manifest rather than a manifest supplied in the wrong form. `load_agent_manifest_document()` now returns the decoded document alongside the envelope bytes it arrived in, and the envelope is what reaches `verify_manifest` when there is one. The file is sniffed rather than switched on its extension: a COSE envelope is CBOR and never parses as JSON, so trying JSON first is unambiguous and an operator does not have to name the file correctly for the gateway to read it. A v0.2 payload supplied as bare JSON is now named as such. `load_agent_manifest()` keeps its dict-returning signature for callers that only read identity fields. diff --git a/docs/spec/catalog-approval-provenance.md b/docs/spec/catalog-approval-provenance.md index 86fe59c5..e9bada70 100644 --- a/docs/spec/catalog-approval-provenance.md +++ b/docs/spec/catalog-approval-provenance.md @@ -15,6 +15,15 @@ bootstrap trust. It rejects unknown fields, revoked or expired keys, invalid signatures, duplicate principals or roles when the policy requires distinctness, and records whose `new_catalog_hash` differs from the runtime catalog hash. +The signing input is RFC 8785 (JCS): UTF-8 output, object members ordered by +their UTF-16 code units, and no escaping beyond what ECMAScript `JSON.stringify` +performs. A record whose identities are not ASCII therefore signs the same bytes +here as under any other JCS implementation. Where JCS cannot pin a value down, +cMCP refuses it rather than emit bytes another implementation would read +differently: floating point numbers, integers outside the exact range of an IEEE +754 double, non-string object keys, and unpaired surrogates are rejected as +malformed. Approval records carry none of them. + The record chain is not a freshness oracle. A verifier must obtain the expected previous-record checkpoint from an external pin or transparency receipt. A valid chain presented from an old checkpoint remains an old, valid chain rather diff --git a/src/cmcp_runtime/catalog/approval.py b/src/cmcp_runtime/catalog/approval.py index 1351151f..0e399f2e 100644 --- a/src/cmcp_runtime/catalog/approval.py +++ b/src/cmcp_runtime/catalog/approval.py @@ -17,6 +17,9 @@ PROFILE = "tag:agentrust-io.com,2026:cmcp-catalog-approval-v1" +# RFC 8785 numbers are IEEE 754 doubles, so integers stay exact only to 2**53 - 1. +_MAX_EXACT_INT = 2**53 - 1 + class CatalogApprovalError(ValueError): """The detached approval record is malformed or cannot be trusted.""" @@ -34,9 +37,46 @@ class TrustedReviewer: role: str | None = None +def _utf16_order(key: Any) -> bytes: + """Sort key placing object members in RFC 8785 order. + + Section 3.2.3 orders members by their UTF-16 code units, which is not the + code point order `sort_keys` applies. Comparing UTF-16BE bytes is the same + comparison, since every code unit occupies two bytes. + """ + if not isinstance(key, str): + raise CatalogApprovalError("canonical JSON object keys must be strings") + try: + return key.encode("utf-16-be") + except UnicodeEncodeError as exc: + raise CatalogApprovalError("canonical JSON cannot encode an unpaired surrogate") from exc + + +def _canonical_members(value: Any) -> Any: + """Rebuild containers in canonical order, refusing what JCS cannot pin down.""" + if isinstance(value, dict): + return {k: _canonical_members(v) for k, v in sorted(value.items(), key=lambda kv: _utf16_order(kv[0]))} + if isinstance(value, list): + return [_canonical_members(item) for item in value] + if isinstance(value, float): + raise CatalogApprovalError("canonical JSON does not accept floating point numbers") + if isinstance(value, int) and not isinstance(value, bool) and abs(value) > _MAX_EXACT_INT: + raise CatalogApprovalError("integer is outside the range RFC 8785 serializes exactly") + return value + + def canonical_json(value: Any) -> bytes: - """Return the RFC 8785-compatible JSON form used by cMCP records.""" - return json.dumps(value, sort_keys=True, separators=(",", ":"), ensure_ascii=True).encode() + """Return the RFC 8785 (JCS) form used by cMCP records. + + JCS emits UTF-8 and escapes only what ECMAScript `JSON.stringify` escapes, so + `ensure_ascii` would put ASCII escapes where an interoperating producer puts + UTF-8 bytes, giving two different signing inputs for one record. + """ + text = json.dumps(_canonical_members(value), ensure_ascii=False, allow_nan=False, separators=(",", ":")) + try: + return text.encode("utf-8") + except UnicodeEncodeError as exc: + raise CatalogApprovalError("canonical JSON cannot encode an unpaired surrogate") from exc def digest_json(value: Any) -> str: diff --git a/tests/unit/test_catalog_canonical_json.py b/tests/unit/test_catalog_canonical_json.py new file mode 100644 index 00000000..bed566ad --- /dev/null +++ b/tests/unit/test_catalog_canonical_json.py @@ -0,0 +1,86 @@ +"""RFC 8785 conformance for the catalog-approval signing input (#517).""" + +from __future__ import annotations + +import pytest +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +from cmcp_runtime.catalog.approval import ( + PROFILE, + CatalogApprovalError, + TrustedReviewer, + canonical_json, + digest_json, + sign_approval, + verify_catalog_change, +) + + +def test_non_ascii_is_emitted_as_utf8() -> None: + """JCS escapes only what JSON.stringify escapes, so no ASCII escapes appear.""" + assert canonical_json({"principal_id": "josé"}) == '{"principal_id":"josé"}'.encode() + + +def test_ascii_records_keep_their_existing_bytes() -> None: + """The signing input for an ASCII record must not move, or nothing already signed still verifies.""" + assert canonical_json({"b": 1, "a": "x", "n": None, "t": True}) == b'{"a":"x","b":1,"n":null,"t":true}' + assert digest_json({"added": ["ehr.read"], "removed": []}) == ( + "sha256:5abec4d8f1edbfc37f5fb61594363c2ac9ebb1ecaf2007203462eb41016e5678" + ) + + +def test_members_are_ordered_by_utf16_code_units() -> None: + """Section 3.2.3 orders by UTF-16 code units, so a non-BMP key sorts below U+FFFF.""" + assert canonical_json({"\uffff": 1, "\U00010000": 2}) == '{"\U00010000":2,"\uffff":1}'.encode() + assert canonical_json({"b": 1, "A": 2, "a": 3}) == b'{"A":2,"a":3,"b":1}' + + +def test_nested_containers_are_canonicalized_throughout() -> None: + assert canonical_json({"outer": [{"b": 1, "a": "é"}]}) == '{"outer":[{"a":"é","b":1}]}'.encode() + + +@pytest.mark.parametrize( + "value, message", + [ + ({"x": 1.5}, "floating point"), + ({"x": float("nan")}, "floating point"), + ({"x": 2**53}, "outside the range"), + ({"x": -(2**53)}, "outside the range"), + ({1: "a"}, "keys must be strings"), + ({"x": "\ud800"}, "unpaired surrogate"), + ({"\ud800": "x"}, "unpaired surrogate"), + ], +) +def test_values_jcs_cannot_pin_down_are_refused(value: object, message: str) -> None: + """Refusing beats emitting bytes an interoperating implementation would disagree with.""" + with pytest.raises(CatalogApprovalError, match=message): + canonical_json(value) + + +def test_the_largest_exact_integer_is_accepted() -> None: + assert canonical_json({"x": 2**53 - 1}) == b'{"x":9007199254740991}' + + +def test_a_non_ascii_reviewer_identity_signs_and_verifies() -> None: + key = Ed25519PrivateKey.generate() + policy = {"policy_id": "politique-des-catalogues", "threshold": 1, "distinct_principals": True, "distinct_roles": False} + record = { + "profile": PROFILE, "catalog_id": "passerelle-prod", "sequence": 2, + "previous_record_hash": "sha256:" + "1" * 64, + "previous_catalog_hash": "sha256:" + "2" * 64, + "new_catalog_hash": "sha256:" + "3" * 64, + "change_set_digest": digest_json({"added": ["dossier.lecture"], "removed": []}), + "approval_policy": {**policy, "policy_hash": digest_json(policy)}, + "automated_checks_digest": digest_json({"ci": "réussi"}), + "approvals": [], + } + record["approvals"] = [ + sign_approval(record, {"principal_id": "josé", "issuer": "idp", "key_id": "k1", "role": "sécurité", "approved_at": 100, "expires_at": 200}, key) + ] + assert b"\\u" not in canonical_json(record) + result = verify_catalog_change( + record, + {"k1": TrustedReviewer("josé", "idp", key.public_key(), "sécurité")}, + runtime_catalog_hash=record["new_catalog_hash"], now=150, + ) + assert result["verified"]