diff --git a/spec/server-provenance-v1.md b/spec/server-provenance-v1.md index 5c7fade0..e97e9cc4 100644 --- a/spec/server-provenance-v1.md +++ b/spec/server-provenance-v1.md @@ -99,11 +99,11 @@ claim about the deployment rather than about the code, and is only as good as th |---|---|---| | `format` | yes | This document's identifier and version | | `kind` | yes | §1 | -| `issued_at` | yes | Unix seconds when this record was signed | +| `issued_at` | yes | Non-negative integer: Unix seconds when this record was signed | | `identity` | yes | §2; `artifact`, `endpoint`, or both | | `publisher` | yes | DID or SPIFFE URI of the signer. Not a display name: a name is not resolvable and a verifier cannot check one | | `tool_catalog.hash` | yes | Digest over the canonical tool list (§4) | -| `tool_catalog.tool_count` | yes | Number of tools. Redundant with the hash by design: a consumer that sees a plausible count and an unexpected hash learns more than one that sees only a mismatch | +| `tool_catalog.tool_count` | yes | Non-negative integer: number of tools. Redundant with the hash by design: a consumer that sees a plausible count and an unexpected hash learns more than one that sees only a mismatch | | `attestation` | when `kind` is `tee-attested` | The evidence, in the shape TRACE v0.2 §3.1 `runtime` uses. `null` otherwise | | `cnf.jwk` | yes | Public key the signature verifies under | | `signature` | yes | Over the RFC 8785 (JCS) canonical form with `signature` absent | diff --git a/src/agentrust_trace/provenance.py b/src/agentrust_trace/provenance.py index 97659ed1..e666802f 100644 --- a/src/agentrust_trace/provenance.py +++ b/src/agentrust_trace/provenance.py @@ -26,13 +26,13 @@ from agentrust_trace.models import RuntimeInfo from agentrust_trace.revocation import NO_CHECK, RevocationCheck from agentrust_trace.sign import ( - JCS_SAFE_INTEGER, RevocationStore, UnanchorableValue, _b64url_decode, _canonical_bytes, _check_not_revoked, _check_seconds, + _integer_value, _pubkey_from_jwk, anchor_bytes, jwk_thumbprint, @@ -85,10 +85,19 @@ def _nonempty_string(value: Any, field: str) -> str: def _tool_count(catalog: dict[str, Any]) -> int: - """Return the required catalog count as a JSON integer.""" - value = catalog.get("tool_count") - if not isinstance(value, int) or isinstance(value, bool) or value < 0: - raise ProvenanceError("tool_catalog.tool_count must be a non-negative integer") + """Return the required catalog count as a JSON integer, decided by value.""" + try: + value = _integer_value(catalog.get("tool_count"), "tool_catalog.tool_count") + except ValueError as exc: + raise ProvenanceError( + "tool_catalog.tool_count must be a non-negative integer within the JCS " + f"safe-integer range: {exc}" + ) from exc + if value < 0: + raise ProvenanceError( + "tool_catalog.tool_count must be a non-negative integer within the JCS " + f"safe-integer range: it is {value}" + ) return value @@ -223,7 +232,7 @@ def _check_structure( endpoint: dict[str, Any] | None, attestation: dict[str, Any] | None, issued_at: Any, -) -> None: +) -> int: """The structural rules both the producer and the consumer must apply. Shared rather than duplicated because these rules were enforced only in @@ -300,31 +309,27 @@ def _check_structure( f"kind={kind!r} carries attestation evidence. Evidence that is present but " "not claimed invites a consumer to read it as an attestation that was made." ) - # Type-based on purpose. Section 3.2.2's "What counts as an integer" does two - # things. It decides by value which numbers are integers for the range, and that - # half reaches this record through `_canonical_bytes`: a whole number past the - # safe-integer range is refused at signing and at verification wherever it sits - # and however it is written. It does not say which members of this record are - # typed `integer`; that is for spec/server-provenance-v1.md and -v2.md. Whether - # ``issued_at`` and ``tool_count`` take a whole number written as a float is a - # separate question from #247, and until it is settled a float is refused here - # as it always was. - # bool is an int subclass, and True would otherwise pass as a timestamp. The upper - # bound is the same JCS safe-integer limit #219 applies to every other signed integer - # in this package: above it there is no portable canonical form, so the producer would - # accept a timestamp it cannot sign and the caller would meet `rfc8785`'s - # `IntegerDomainError` instead of the class this module documents. - if ( - not isinstance(issued_at, int) - or isinstance(issued_at, bool) - or issued_at < 0 - or issued_at > JCS_SAFE_INTEGER - ): + # Decided by value (spec/trace-v0.2.md §3.2.2, spec/server-provenance-v1.md §3, #485): + # RFC 8785 writes 1760000000, 1760000000.0 and 1.76e9 as the same canonical bytes, so a + # verifier deciding by Python type refuses a spelling the signature covers. + # `_integer_value` keeps the bool exclusion (bool is an int subclass in Python, so True + # would otherwise pass as a timestamp) and the JCS safe-integer bound (#219, #320), + # above which there is no portable canonical form. + try: + timestamp = _integer_value(issued_at, "issued_at") + except ValueError as exc: + raise ProvenanceError( + "issued_at must be a non-negative integer Unix timestamp within the JCS " + f"safe-integer range: {exc}. A record with no usable issue time cannot be " + "aged, so a consumer has no way to reject a stale one." + ) from exc + if timestamp < 0: raise ProvenanceError( "issued_at must be a non-negative integer Unix timestamp within the JCS " - "safe-integer range. A record with no usable issue time cannot be aged, so " - "a consumer has no way to reject a stale one." + f"safe-integer range: it is {timestamp}. A record with no usable issue time " + "cannot be aged, so a consumer has no way to reject a stale one." ) + return timestamp def build_record( @@ -359,16 +364,15 @@ def build_record( ) # An explicitly supplied value reaches _check_structure untouched: coercing first # defeats the guard there, whose whole subject is what the caller actually passed - # (#320). int() on a bool, a float, or a numeric string yields something the - # isinstance test then accepts, and int() on anything else raises a class this + # (#320). int() on a bool, a non-whole float, or a numeric string yields something + # an integer test then accepts, and int() on anything else raises a class this # module does not document. - stamped_at = issued_at if issued_at is not None else int(time.time()) - _check_structure( + stamped_at = _check_structure( kind=kind, artifact=artifact, endpoint=endpoint, attestation=attestation, - issued_at=stamped_at, + issued_at=issued_at if issued_at is not None else int(time.time()), ) identity: dict[str, Any] = {} @@ -491,7 +495,7 @@ def verify_record( identity = _as_object(record.get("identity"), "identity") if not identity.get("artifact") and not identity.get("endpoint"): raise ProvenanceError("identity carries neither an artifact nor an endpoint") - _check_structure( + issued_at = _check_structure( kind=str(record.get("kind")), artifact=identity.get("artifact"), endpoint=identity.get("endpoint"), @@ -503,17 +507,17 @@ def verify_record( raise ProvenanceError("tool_catalog.hash is not a sha256: digest") _tool_count(catalog) - # Freshness. `issued_at` has been required and type-checked since the format + # Freshness. `issued_at` has been required and checked since the format # existed, with an error message explaining that a record with no issue time # cannot be aged; this is the step that reads it. `_check_structure` above has - # already established it is a non-negative int. + # already established it is a non-negative integer value. _check_seconds("max_future_skew_seconds", max_future_skew_seconds, exc=ProvenanceError) _check_seconds( "max_age_seconds", max_age_seconds, optional=True, exc=ProvenanceError ) _check_seconds("now", now, optional=True, exc=ProvenanceError) verification_time = time.time() if now is None else now - age = verification_time - int(record["issued_at"]) + age = verification_time - issued_at if age < -max_future_skew_seconds: raise ProvenanceError( f"record is dated {int(-age)}s in the future, exceeds " diff --git a/tests/test_integer_by_value.py b/tests/test_integer_by_value.py index 853d8e8e..098d1cec 100644 --- a/tests/test_integer_by_value.py +++ b/tests/test_integer_by_value.py @@ -51,7 +51,13 @@ verify_successor_artifact, ) from agentrust_trace.models import TrustRecord -from agentrust_trace.provenance import FORMAT, FORMAT_V2, ProvenanceError, build_record +from agentrust_trace.provenance import ( + FORMAT, + FORMAT_V2, + ProvenanceError, + build_record, + check_tool_catalog, +) from agentrust_trace.provenance import sign_record as sign_provenance_record from agentrust_trace.provenance import verify_record as verify_provenance_record from agentrust_trace.revocation import bundle_digest @@ -400,9 +406,10 @@ def test_each_signing_function_refuses_a_whole_float_past_the_range_in_its_own_e # ---- the provenance record ------------------------------------------------------ # # spec/server-provenance-v1.md signs "following TRACE v0.2 section 3.2 exactly, -# including the canonicalization", and -v2.md keeps v1's signatures. The range of -# section 3.2.2 therefore reaches the record; which of its members are integers is -# its own specification's to say. +# including the canonicalization", types `issued_at` and `tool_catalog.tool_count` +# as non-negative integers (#485), and -v2.md inherits both. Section 3.2.2's range +# and integer-by-value rules therefore govern the record and both of its integer +# members. def _provenance_record(fmt: str, **over: Any) -> dict[str, Any]: @@ -454,22 +461,126 @@ def test_a_provenance_record_with_a_whole_float_inside_the_range_signs_and_verif verify_provenance_record(signed, key_to_jwk(key)) -def test_the_provenance_record_keeps_its_own_typing_of_its_integer_members() -> None: - """Section 3.2.2 carries the range to this record and retypes none of its members. - Whether `issued_at` and `tool_catalog.tool_count` written as whole floats are the - integers they denote is for the provenance specification to say; until it does, - the reference refuses the float as it always did.""" - with pytest.raises(ProvenanceError, match="issued_at must be a non-negative integer"): - _provenance_record(FORMAT, issued_at=1760000000.0) - assert _provenance_record(FORMAT, issued_at=1760000000)["issued_at"] == 1760000000 +@pytest.mark.parametrize("fmt", [FORMAT, FORMAT_V2], ids=["v1", "v2"]) +@pytest.mark.parametrize( + ("issued_at_spelling", "count_spelling"), + [("1760000000.0", "1.0"), ("1.76e9", "1e0")], +) +def test_provenance_integer_members_written_as_whole_floats_are_the_values_signed( + fmt: str, issued_at_spelling: str, count_spelling: str +) -> None: + """Both `issued_at` and `tool_catalog.tool_count` are typed integer in + `spec/server-provenance-v1.md` (and inherited by v2), so section 3.2.2 decides + them by value (#485).""" + tools = [{"name": "search", "description": "search the docs", + "input_schema": {"type": "object"}}] + issued_at_float = json.loads(issued_at_spelling) + count_float = json.loads(count_spelling) + assert type(issued_at_float) is float and type(count_float) is float + + built = _provenance_record(fmt, issued_at=issued_at_float) + assert built["issued_at"] == 1760000000 + assert type(built["issued_at"]) is int + key = Ed25519PrivateKey.generate() - record = _provenance_record(FORMAT) - count = record["tool_catalog"]["tool_count"] - assert type(count) is int - respelled = {**record, "tool_catalog": {**record["tool_catalog"], "tool_count": float(count)}} - with pytest.raises(ProvenanceError, match="tool_count"): - verify_provenance_record(sign_provenance_record(respelled, key), key_to_jwk(key)) - verify_provenance_record(sign_provenance_record(record, key), key_to_jwk(key)) + baseline = sign_provenance_record(_provenance_record(fmt, issued_at=1760000000), key) + respelled_unsigned = { + **_provenance_record(fmt, issued_at=1760000000), + "issued_at": issued_at_float, + "tool_catalog": { + **baseline["tool_catalog"], + "tool_count": count_float, + }, + } + respelled_signed = sign_provenance_record(respelled_unsigned, key) + assert respelled_signed["signature"] == baseline["signature"] + + verify_provenance_record( + respelled_signed, + key_to_jwk(key), + now=1760000100, + max_age_seconds=300, + required_format=fmt, + ) + check_tool_catalog(respelled_signed, tools, required_format=fmt) + + +@pytest.mark.parametrize("fmt", [FORMAT, FORMAT_V2], ids=["v1", "v2"]) +@pytest.mark.parametrize( + ("value", "reason"), + [ + (1760000000.5, "issued_at is not an integer value: 1760000000.5 is not a whole number"), + (17600000005e-1, "issued_at is not an integer value: 1760000000.5 is not a whole number"), + ("1760000000", "issued_at is not an integer value: it is a str, not a JSON number"), + (True, "issued_at is not an integer value: it is a bool, not a JSON number"), + (False, "issued_at is not an integer value: it is a bool, not a JSON number"), + (-1, "it is -1"), + (-1.0, "it is -1"), + (9007199254740992, "issued_at is 9007199254740992, outside the safe-integer range"), + (9007199254740992.0, "issued_at is 9007199254740992, outside the safe-integer range"), + (1e21, "issued_at is 1000000000000000000000, outside the safe-integer range"), + ], +) +def test_a_provenance_issued_at_that_is_not_a_non_negative_integer_is_refused_with_its_reason( + fmt: str, value: Any, reason: str +) -> None: + pattern = re.escape( + f"issued_at must be a non-negative integer Unix timestamp within the JCS " + f"safe-integer range: {reason}" + ) + with pytest.raises(ProvenanceError, match=pattern): + _provenance_record(fmt, issued_at=value) + + key = Ed25519PrivateKey.generate() + signed = sign_provenance_record(_provenance_record(fmt, issued_at=1760000000), key) + with pytest.raises(ProvenanceError, match=pattern): + verify_provenance_record({**signed, "issued_at": value}, key_to_jwk(key)) + + +@pytest.mark.parametrize("fmt", [FORMAT, FORMAT_V2], ids=["v1", "v2"]) +@pytest.mark.parametrize( + ("value", "reason"), + [ + (1.5, "tool_catalog.tool_count is not an integer value: 1.5 is not a whole number"), + (15e-1, "tool_catalog.tool_count is not an integer value: 1.5 is not a whole number"), + ("1", "tool_catalog.tool_count is not an integer value: it is a str, not a JSON number"), + (True, "tool_catalog.tool_count is not an integer value: it is a bool, not a JSON number"), + (False, "tool_catalog.tool_count is not an integer value: it is a bool, not a JSON number"), + (-1, "it is -1"), + (-1.0, "it is -1"), + ( + 9007199254740992, + "tool_catalog.tool_count is 9007199254740992, outside the safe-integer range", + ), + ( + 9007199254740992.0, + "tool_catalog.tool_count is 9007199254740992, outside the safe-integer range", + ), + ( + 1e21, + "tool_catalog.tool_count is 1000000000000000000000, outside the safe-integer range", + ), + ], +) +def test_a_provenance_tool_count_that_is_not_a_non_negative_integer_is_refused_with_its_reason( + fmt: str, value: Any, reason: str +) -> None: + tools = [{"name": "search", "description": "search the docs", + "input_schema": {"type": "object"}}] + pattern = re.escape( + f"tool_catalog.tool_count must be a non-negative integer within the JCS " + f"safe-integer range: {reason}" + ) + key = Ed25519PrivateKey.generate() + signed = sign_provenance_record(_provenance_record(fmt, issued_at=1760000000), key) + tampered = { + **signed, + "tool_catalog": {**signed["tool_catalog"], "tool_count": value}, + } + with pytest.raises(ProvenanceError, match=pattern): + verify_provenance_record(tampered, key_to_jwk(key)) + with pytest.raises(ProvenanceError, match=pattern): + check_tool_catalog(tampered, tools, required_format=fmt) # ---- the bridge ----------------------------------------------------------------- diff --git a/tests/test_provenance.py b/tests/test_provenance.py index 20dacc8d..ad16b9bc 100644 --- a/tests/test_provenance.py +++ b/tests/test_provenance.py @@ -864,6 +864,45 @@ def test_a_valid_issued_at_is_still_carried_through_unchanged() -> None: assert _record(issued_at=1_754_000_000)["issued_at"] == 1_754_000_000 +@pytest.mark.parametrize( + ("accepted_spelling", "refused_spelling"), + [ + (1760000000.0, 1760000000.5), + (1.76e9, 17600000005e-1), + (float(JCS_SAFE_INTEGER), float(JCS_SAFE_INTEGER + 1)), + ], +) +def test_issued_at_is_decided_by_value_in_pairs( + accepted_spelling: float, refused_spelling: float +) -> None: + """#485: `issued_at` is typed integer in server-provenance-v1 and decided by value + per TRACE v0.2 §3.2.2, keeping the bool and range guards.""" + key = generate_key() + built = _record(issued_at=accepted_spelling) + assert built["issued_at"] == int(accepted_spelling) + assert type(built["issued_at"]) is int + + signed = sign_record(built, key) + respelled = {**signed, "issued_at": accepted_spelling} + assert type(respelled["issued_at"]) is float + verify_record( + respelled, + key_to_jwk(key), + now=int(accepted_spelling), + max_age_seconds=300, + ) + + with pytest.raises(ProvenanceError, match="issued_at"): + _record(issued_at=refused_spelling) + with pytest.raises(ProvenanceError, match="issued_at"): + verify_record( + {**signed, "issued_at": refused_spelling}, + key_to_jwk(key), + now=1760000000, + max_future_skew_seconds=10**19, + ) + + def test_an_omitted_issued_at_is_still_stamped_with_the_current_time() -> None: before = int(time.time()) stamped = _record()["issued_at"] diff --git a/tests/test_provenance_tool_count.py b/tests/test_provenance_tool_count.py index a5ce5f14..40daf5a0 100644 --- a/tests/test_provenance_tool_count.py +++ b/tests/test_provenance_tool_count.py @@ -10,7 +10,7 @@ sign_record, verify_record, ) -from agentrust_trace.sign import generate_key, key_to_jwk +from agentrust_trace.sign import JCS_SAFE_INTEGER, generate_key, key_to_jwk TOOLS = [ { @@ -46,20 +46,59 @@ def _signed_with_count(count: object = _MISSING): return sign_record(record, key), key_to_jwk(key) -@pytest.mark.parametrize("bad_count", [_MISSING, -1, True, "2", None]) +@pytest.mark.parametrize("bad_count", [_MISSING, -1, -1.0, 2.5, 25e-1, True, False, "2", None]) def test_verify_record_refuses_malformed_or_missing_tool_count(bad_count) -> None: record, trusted = _signed_with_count(bad_count) with pytest.raises(ProvenanceError, match="tool_catalog.tool_count must be"): verify_record(record, trusted) -@pytest.mark.parametrize("bad_count", [2.0, "2", True, -1, None]) +@pytest.mark.parametrize("bad_count", [JCS_SAFE_INTEGER + 1, float(JCS_SAFE_INTEGER + 1), 1e21]) +def test_verify_record_refuses_out_of_range_tool_count_in_both_spellings(bad_count) -> None: + record, trusted = _signed_with_count(len(TOOLS)) + record["tool_catalog"]["tool_count"] = bad_count + with pytest.raises( + ProvenanceError, + match=r"tool_catalog\.tool_count must be.*outside the safe-integer range", + ): + verify_record(record, trusted) + + +@pytest.mark.parametrize( + "bad_count", + [ + 2.5, + 25e-1, + "2", + True, + False, + -1, + -1.0, + None, + JCS_SAFE_INTEGER + 1, + float(JCS_SAFE_INTEGER + 1), + 1e21, + ], +) def test_check_tool_catalog_alone_refuses_malformed_count(bad_count) -> None: - record, _ = _signed_with_count(bad_count) + record, _ = _signed_with_count(len(TOOLS)) + record["tool_catalog"]["tool_count"] = bad_count with pytest.raises(ProvenanceError, match="tool_catalog.tool_count must be"): check_tool_catalog(record, TOOLS) +@pytest.mark.parametrize("whole_float", [2.0, 2e0]) +def test_whole_float_tool_count_verifies_and_matches_live_catalog(whole_float: float) -> None: + baseline, trusted = _signed_with_count(len(TOOLS)) + respelled = { + **baseline, + "tool_catalog": {**baseline["tool_catalog"], "tool_count": whole_float}, + } + assert type(respelled["tool_catalog"]["tool_count"]) is float + verify_record(respelled, trusted) + check_tool_catalog(respelled, TOOLS) + + def test_wrong_positive_count_is_detected_when_live_catalog_is_checked() -> None: record, trusted = _signed_with_count(999)