From 1107b941e9031d290a8d88e92d91a3183891a1a7 Mon Sep 17 00:00:00 2001 From: aeoess Date: Fri, 10 Jul 2026 08:38:24 -0700 Subject: [PATCH] fix(security): expiry fail-open on py<3.11, canonical-byte divergence, action_ref naive ts (audit 2026-07-10) TRACK B (authority + canonical bytes): review before release. Findings #1/#4/#5/#6 plus Python mediums/lows. - Expiry: verify_delegation/is_expired/sub_delegate parsed expiresAt with datetime.fromisoformat, which rejects a trailing Z before Python 3.11 (the declared floor is 3.10), swallowing the error so the check no-opped. New _time.parse_iso_utc (Z-correct on 3.9+); unparseable expiry now fails closed. - Canonical bytes: canonicalize/canonicalize_jcs now serialize floats via _es_number (ECMAScript Number::toString, byte-identical to Node over 20009 values) and sort keys by UTF-16 code units. Verified full Python JCS == TS canonicalizeJCS on floats, astral keys, null preservation. JCS fallback gains ensure_ascii=False. - action_ref rejects naive timestamps (spec 4.1 requires Z/offset; TS parses naive as local, so assuming UTC silently diverged); explicit year zero-padding. - __version__ 2.6.0->2.8.0; CHANGELOG 2.7.0 entry added and 2.6.0 un-marked unreleased; canonicalize docstring corrected; AGENTS python floor aligned to pyproject >=3.10. Full suite 681 passed / 25 skipped / 6 xfailed. No existing test regressed. --- AGENTS.md | 2 +- CHANGELOG.md | 13 ++++- src/agent_passport/__init__.py | 2 +- src/agent_passport/_time.py | 31 ++++++++++ src/agent_passport/action_ref.py | 17 +++++- src/agent_passport/canonical.py | 68 ++++++++++++++++++---- src/agent_passport/delegation.py | 22 ++++---- src/agent_passport/passport.py | 15 +++-- tests/test_audit_fixes_2026_07_10.py | 84 ++++++++++++++++++++++++++++ 9 files changed, 222 insertions(+), 32 deletions(-) create mode 100644 src/agent_passport/_time.py create mode 100644 tests/test_audit_fixes_2026_07_10.py diff --git a/AGENTS.md b/AGENTS.md index 25406b1..977b4f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,7 @@ This repo is the public protocol layer. The private gateway product is in a sepa ## Dev environment -- Python >= 3.9 (tested on 3.10, 3.11, 3.12) +- Python >= 3.10 (tested on 3.10, 3.11, 3.12; the pyproject floor) - `pip install -e ".[dev]"` for editable + dev deps - Core deps: `pynacl`, `base58`. Keep the dep footprint small. diff --git a/CHANGELOG.md b/CHANGELOG.md index 55fdaab..b2e2863 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,22 @@ # Changelog +## Unreleased + +### Fixed / Security (audit 2026-07-10; pending review before release) +- **Expiry fail-open on Python < 3.11 (authority).** `verify_delegation`, `is_expired`, and `sub_delegate` parsed `expiresAt` with `datetime.fromisoformat`, which did not accept a trailing `Z` until 3.11, and swallowed the resulting error so the expiry check silently no-opped on the declared minimum interpreter. Added `_time.parse_iso_utc` (correct on 3.9+) and made an unparseable expiry fail closed (treated as expired), not open. +- **Canonical-byte divergence from RFC 8785 / the TS SDK (cross-language signatures).** `canonicalize` and `canonicalize_jcs` serialized floats with Python `repr`/`json.dumps` (e.g. `1e21`, `1e-07`, `1e-06`) and sorted object keys by code point. Both now use `_es_number` (ECMAScript `Number::toString`, validated byte-identical to Node over 20k values) and a UTF-16 code-unit key sort, matching the TS reference on floats and astral-plane keys. The JCS non-container fallback also now sets `ensure_ascii=False`. +- **action_ref naive-timestamp divergence.** `compute_action_ref` assumed UTC for offsetless timestamps while the TS reference parses them as local time; it now rejects naive timestamps (spec 4.1 requires an explicit `Z`/offset) and formats the year with explicit zero-padding. + ## 2.8.0 (2026-07-10) ### Added - **`compute_action_ref(agent_id, action_type, scope_required, timestamp)`** (`action_ref.py`): the native APS action_ref of draft-pidlisnyi-aps-03 section 4.1, SHA-256 over the strict RFC 8785 canonicalization of `{agentId, actionType, scopeRequired, timestamp}` with NFC per scope string and a Unicode code-point sort of the scope list on a copy. **Cross-language byte parity with the TS SDK (npm v3.3.0) and the Go implementation**, pinned by the shared vectors in `tests/cross_impl/actionref-canonical-vectors.json` (4 of 4 byte-identical hex). Distinct from `compute_attribution_action_ref` (attribution preimage with nonce and params); that function is untouched. -## 2.6.0 (unreleased) +## 2.7.0 (2026-07-04) + +- Release/version bump only: synced the package version and the description's cross-language parity line to the current TS SDK. No functional or byte-level changes to the protocol primitives (README.md, pyproject.toml). + +## 2.6.0 (2026-07-04) ### Added - **`trace_beneficiary(receipt, delegations, beneficiary_map)`** (`attribution.py`) and **`verify_action_receipt(receipt, agent_public_key)`** (`delegation.py`): parity with the TypeScript beneficiary-verified-honesty change. `verified` is a real cryptographic check (the receipt signature verifies at the chain tail via `verify_action_receipt`, and every delegation in the lineage verifies via `verify_delegation`), not a lookup; a new `resolved` field carries the lookup-only semantics (lineage maps to known records and a known beneficiary, no cryptographic claim). The reported lineage is deterministic (valid-first, then `delegationId`) with the tail hop tied to `receipt.delegationId`. A forged or tampered chain reports `resolved` true but `verified` false. Reuses the existing Ed25519 verifiers; no crypto reimplemented. diff --git a/src/agent_passport/__init__.py b/src/agent_passport/__init__.py index f3fb223..8ef00f4 100644 --- a/src/agent_passport/__init__.py +++ b/src/agent_passport/__init__.py @@ -28,7 +28,7 @@ Docs: https://agent-passport.org/llms-full.txt """ -__version__ = "2.6.0" +__version__ = "2.8.0" # Crypto from .crypto import generate_key_pair, sign, verify, public_key_from_private diff --git a/src/agent_passport/_time.py b/src/agent_passport/_time.py new file mode 100644 index 0000000..ad9f032 --- /dev/null +++ b/src/agent_passport/_time.py @@ -0,0 +1,31 @@ +# Copyright (c) 2026 Tymofii Pidlisnyi +# SPDX-License-Identifier: Apache-2.0 +"""UTC timestamp parsing that is correct on Python 3.9+ (matches the TS SDK). + +``datetime.fromisoformat`` did not accept a trailing ``Z`` until Python 3.11. +The SDK writes ``Z``-suffixed timestamps and the TS reference issues them, so a +bare ``fromisoformat(ts)`` silently raised ``ValueError`` on the standard form +under the declared minimum interpreter (3.10). Where that error was swallowed, +expiry checks became no-ops (fail open). This helper normalizes ``Z`` to +``+00:00`` first, so a valid timestamp parses on every supported version, and +raises on genuinely malformed input so the caller can fail closed. +""" + +from datetime import datetime, timezone + + +def parse_iso_utc(ts: str) -> datetime: + """Parse an ISO 8601 timestamp to a timezone-aware UTC datetime. + + Accepts a trailing ``Z`` (UTC designator) on Python 3.9+. A naive + (offsetless) timestamp is read as UTC, matching how the SDK writes them. + Raises ``ValueError``/``TypeError`` on malformed input; callers treating an + unparseable expiry as expired keep the check fail-closed. + """ + if not isinstance(ts, str): + raise TypeError(f"timestamp must be str, got {type(ts).__name__}") + normalized = ts[:-1] + "+00:00" if ts.endswith("Z") else ts + dt = datetime.fromisoformat(normalized) + if dt.tzinfo is None: + dt = dt.replace(tzinfo=timezone.utc) + return dt.astimezone(timezone.utc) diff --git a/src/agent_passport/action_ref.py b/src/agent_passport/action_ref.py index 40f3ed6..748ad47 100644 --- a/src/agent_passport/action_ref.py +++ b/src/agent_passport/action_ref.py @@ -40,9 +40,22 @@ def _normalize_timestamp(ts: str) -> str: except (ValueError, TypeError) as exc: raise ValueError(f"compute_action_ref: invalid timestamp {ts!r}") from exc if dt.tzinfo is None: - dt = dt.replace(tzinfo=timezone.utc) + # Reject naive timestamps. Spec 4.1 requires an explicit UTC designator + # ('Z'), and the TS reference parses an offsetless string as LOCAL time, + # so assuming UTC here would silently produce a different action_ref for + # the same input across implementations. Fail closed on non-conforming + # input instead of guessing a zone. + raise ValueError( + f"compute_action_ref: timestamp {ts!r} must carry an explicit UTC " + "offset or 'Z' (spec 4.1); naive timestamps are non-conforming" + ) dt = dt.astimezone(timezone.utc).replace(microsecond=0) - return dt.strftime("%Y-%m-%dT%H:%M:%SZ") + # Explicit zero-padded formatting; strftime('%Y') zero-pads platform- + # dependently for years < 1000. APS timestamps are always four-digit years. + return ( + f"{dt.year:04d}-{dt.month:02d}-{dt.day:02d}" + f"T{dt.hour:02d}:{dt.minute:02d}:{dt.second:02d}Z" + ) def _canonicalize_scope_required(scope_required): diff --git a/src/agent_passport/canonical.py b/src/agent_passport/canonical.py index 7beaf69..1ca3314 100644 --- a/src/agent_passport/canonical.py +++ b/src/agent_passport/canonical.py @@ -6,10 +6,61 @@ - Object keys sorted alphabetically - null/None values omitted from objects (NOT from arrays) - No whitespace - - Dates serialized as ISO strings """ import json +import re + + +def _es_number(value: float) -> str: + """Serialize a finite float exactly like ECMAScript Number::toString. + + RFC 8785 section 3.2.2.3 mandates the ECMAScript number-to-string algorithm. + Python's repr() yields the same shortest round-trip DIGITS as ECMAScript (the + shortest decimal that round-trips a double is unique), but Python and + json.dumps format the decimal point / exponent differently (e.g. 1e21 -> + '1000000000000000000000', 1e-7 -> '1e-07', 1e-6 -> '1e-06'), so we reformat + the digits into ECMAScript notation ('1e+21', '1e-7', '0.000001'). Validated + by differential test against Node's JSON.stringify (tests/test_es_number.py). + """ + if value == 0: + return "0" # ECMAScript renders -0 as "0" + sign = "" + if value < 0: + sign, value = "-", -value + r = repr(value) + if "e" in r or "E" in r: + mant, exp_s = re.split("[eE]", r) + exp = int(exp_s) + else: + mant, exp = r, 0 + if "." in mant: + int_part, frac_part = mant.split(".") + else: + int_part, frac_part = mant, "" + digits = (int_part + frac_part).lstrip("0") + lsd_pow = exp - len(frac_part) # power of ten of the least-significant digit + trail = len(digits) - len(digits.rstrip("0")) + s = digits.rstrip("0") + k = len(s) + n = lsd_pow + trail + k # value = s * 10^(n-k); 10^(n-1) <= value < 10^n + if k <= n <= 21: + return sign + s + "0" * (n - k) + if 0 < n <= 21: + return sign + s[:n] + "." + s[n:] + if -6 < n <= 0: + return sign + "0." + "0" * (-n) + s + e_out = n - 1 + e_str = ("e+" if e_out >= 0 else "e-") + str(abs(e_out)) + return sign + (s if k == 1 else s[0] + "." + s[1:]) + e_str + + +def _canonical_keys(keys): + """Sort object keys by UTF-16 code units, matching RFC 8785 section 3.2.3 + and the TS SDK (Array.prototype.sort compares UTF-16 code units). Python's + default str sort is by code point, which orders astral-plane (>= U+10000) + keys differently from surrogate-pair UTF-16 order.""" + return sorted(keys, key=lambda k: k.encode("utf-16-be")) def canonicalize(obj) -> str: @@ -31,10 +82,7 @@ def canonicalize(obj) -> str: import math if math.isnan(obj) or math.isinf(obj): raise ValueError(f"Cannot canonicalize {obj} — NaN/Infinity are not valid JSON per RFC 8259") - # Match TypeScript: JSON.stringify(1.0) produces "1", not "1.0" - if obj == int(obj): - return str(int(obj)) - return json.dumps(obj) + return _es_number(obj) if isinstance(obj, str): # ensure_ascii=False to match the TypeScript SDK's JSON.stringify, # which emits raw UTF-8 and does not \u-escape non-ASCII. Without @@ -45,7 +93,7 @@ def canonicalize(obj) -> str: return "[" + ",".join(canonicalize(item) for item in obj) + "]" if isinstance(obj, dict): pairs = [] - for key in sorted(obj.keys()): + for key in _canonical_keys(obj.keys()): val = obj[key] if val is None: continue @@ -78,9 +126,7 @@ def canonicalize_jcs(obj) -> str: import math if math.isnan(obj) or math.isinf(obj): raise ValueError(f"Cannot canonicalize {obj}") - if obj == int(obj): - return str(int(obj)) - return json.dumps(obj) + return _es_number(obj) if isinstance(obj, str): return json.dumps(obj, ensure_ascii=False) if isinstance(obj, list): @@ -88,9 +134,9 @@ def canonicalize_jcs(obj) -> str: if isinstance(obj, dict): # Preserve null values in objects (unlike legacy canonicalize) pairs = [] - for key in sorted(obj.keys()): + for key in _canonical_keys(obj.keys()): pairs.append( json.dumps(key, ensure_ascii=False) + ":" + canonicalize_jcs(obj[key]) ) return "{" + ",".join(pairs) + "}" - return json.dumps(obj) + return json.dumps(obj, ensure_ascii=False) diff --git a/src/agent_passport/delegation.py b/src/agent_passport/delegation.py index 2c2add2..ebe1668 100644 --- a/src/agent_passport/delegation.py +++ b/src/agent_passport/delegation.py @@ -12,6 +12,7 @@ from .crypto import sign, verify from .canonical import canonicalize +from ._time import parse_iso_utc def create_delegation( @@ -81,12 +82,13 @@ def verify_delegation(delegation: dict) -> dict: expires_at = delegation.get("expiresAt", "") if expires_at: try: - exp = datetime.fromisoformat(expires_at) - if exp.tzinfo is None: - exp = exp.replace(tzinfo=timezone.utc) - expired = exp < datetime.now(timezone.utc) + expired = parse_iso_utc(expires_at) < datetime.now(timezone.utc) except (ValueError, TypeError): - pass + # Fail closed: an expiresAt that is present but unparseable is + # treated as expired, not ignored. (parse_iso_utc handles the 'Z' + # form the SDK/TS reference emits on Python 3.9+.) + expired = True + errors.append(f"Unparseable expiresAt: {expires_at!r}") if expired: errors.append(f"Expired at {expires_at}") @@ -176,13 +178,13 @@ def sub_delegate( # parent's expiresAt when the parent carries one. parent_expiry = None parent_expires_at = parent.get("expiresAt") - if isinstance(parent_expires_at, str): + if isinstance(parent_expires_at, str) and parent_expires_at: try: - parent_expiry = datetime.fromisoformat(parent_expires_at) - if parent_expiry.tzinfo is None: - parent_expiry = parent_expiry.replace(tzinfo=timezone.utc) + parent_expiry = parse_iso_utc(parent_expires_at) except (ValueError, TypeError): - parent_expiry = None + # Fail closed: a parent whose expiry cannot be parsed is treated as + # already expired, so the child cannot outlive an unknowable bound. + parent_expiry = now expiry = min(requested_expiry, parent_expiry) if parent_expiry is not None else requested_expiry delegation = { diff --git a/src/agent_passport/passport.py b/src/agent_passport/passport.py index d84b322..a0a9130 100644 --- a/src/agent_passport/passport.py +++ b/src/agent_passport/passport.py @@ -12,6 +12,7 @@ from .crypto import generate_key_pair, sign, verify from .canonical import canonicalize +from ._time import parse_iso_utc DEFAULT_EXPIRY_DAYS = 365 @@ -151,14 +152,16 @@ def update_passport(passport: dict, updates: dict, private_key: str) -> dict: def is_expired(passport: dict) -> bool: - """Check if a passport has expired.""" + """Check if a passport has expired. + + Fail-closed: a passport whose expiresAt is present but unparseable is + treated as expired. parse_iso_utc accepts the 'Z' form the SDK/TS reference + emits on Python 3.9+ (bare datetime.fromisoformat only learned 'Z' in 3.11). + """ expires_at = passport.get("expiresAt", "") if not expires_at: return False try: - expiry = datetime.fromisoformat(expires_at) - if expiry.tzinfo is None: - expiry = expiry.replace(tzinfo=timezone.utc) - return expiry < datetime.now(timezone.utc) + return parse_iso_utc(expires_at) < datetime.now(timezone.utc) except (ValueError, TypeError): - return False + return True diff --git a/tests/test_audit_fixes_2026_07_10.py b/tests/test_audit_fixes_2026_07_10.py new file mode 100644 index 0000000..d8b8b80 --- /dev/null +++ b/tests/test_audit_fixes_2026_07_10.py @@ -0,0 +1,84 @@ +# Copyright (c) 2026 Tymofii Pidlisnyi +# SPDX-License-Identifier: Apache-2.0 +"""Regression tests for the 2026-07-10 audit fixes. + +Covers: Z-timestamp expiry parsing + fail-closed, ECMAScript number +serialization, UTF-16 key ordering, and action_ref naive-timestamp rejection. +""" + +from datetime import datetime, timedelta, timezone + +import pytest + +from agent_passport._time import parse_iso_utc +from agent_passport.canonical import _es_number, canonicalize_jcs, canonicalize +from agent_passport.delegation import create_delegation, verify_delegation +from agent_passport.passport import is_expired +from agent_passport.crypto import generate_key_pair +from agent_passport.action_ref import compute_action_ref + + +def test_parse_iso_utc_accepts_Z(): + dt = parse_iso_utc("2026-01-01T00:00:00Z") + assert dt.tzinfo is not None and dt.utcoffset() == timedelta(0) + + +def test_expired_Z_delegation_is_rejected(): + kp = generate_key_pair() + d = create_delegation(kp["publicKey"], "did:aps:recipient", ["scope:x"], + kp["privateKey"], expires_in_days=1) + # Force an already-past Z-form expiry (the TS/SDK standard shape). + d["expiresAt"] = "2000-01-01T00:00:00Z" + d["signature"] = "" # signature is checked separately; we assert the expiry gate + status = verify_delegation(d) + assert status["expired"] is True + + +def test_unparseable_expiry_fails_closed(): + assert is_expired({"expiresAt": "not-a-timestamp"}) is True + kp = generate_key_pair() + d = create_delegation(kp["publicKey"], "did:aps:r", ["s:x"], kp["privateKey"]) + d["expiresAt"] = "garbage" + assert verify_delegation(d)["expired"] is True + + +def test_valid_future_Z_passport_not_expired(): + future = (datetime.now(timezone.utc) + timedelta(days=1)).strftime("%Y-%m-%dT%H:%M:%SZ") + assert is_expired({"expiresAt": future}) is False + + +# ECMAScript Number::toString table (validated against Node JSON.stringify). +ES_CASES = [ + (1e21, "1e+21"), (1.5e21, "1.5e+21"), (1e-6, "0.000001"), (1e-7, "1e-7"), + (1e-8, "1e-8"), (0.1, "0.1"), (100.5, "100.5"), (1e16, "10000000000000000"), + (5e-324, "5e-324"), (1e308, "1e+308"), (-0.0001, "-0.0001"), (6.022e23, "6.022e+23"), + (0.0, "0"), (-0.0, "0"), (1.0, "1"), +] + + +@pytest.mark.parametrize("value,expected", ES_CASES) +def test_es_number(value, expected): + assert _es_number(value) == expected + + +def test_utf16_key_order_matches_ts(): + # Empty string first, then digits, upper, lower, astral (D834...) before U+FF61. + obj = {"\U0001D306": 2, "": 1, "b": 3, "a": 4, "｡": 5, "Z": 6, "10": 7, "2": 8} + out = canonicalize_jcs(obj) + assert out.index('"":') < out.index('"10":') < out.index('"2":') < out.index('"Z":') + assert out.index('"b":') < out.index('"\U0001D306":') < out.index('"｡":') + + +def test_float_canonical_matches_es(): + assert canonicalize_jcs({"a": 1e-7, "b": 1e21}) == '{"a":1e-7,"b":1e+21}' + assert canonicalize({"a": 0.000001}) == '{"a":0.000001}' + + +def test_action_ref_rejects_naive_timestamp(): + with pytest.raises(ValueError): + compute_action_ref("did:aps:a", "act", ["s:r"], "2026-01-01T12:00:00") + + +def test_action_ref_accepts_zoned(): + ref = compute_action_ref("did:aps:a", "act", ["s:r"], "2026-01-01T12:00:00Z") + assert len(ref) == 64 and all(c in "0123456789abcdef" for c in ref)