From 7adcaeebf815b631c4974fb714c77030036262a6 Mon Sep 17 00:00:00 2001 From: Mudwood Labs Date: Thu, 3 Sep 2026 01:24:48 -0700 Subject: [PATCH] verify a creator signature against the bytes the creator signed verify_creator_signature re-derived the signed form by calling to_cbor_dict() on the DECODED metadata. Decoding is deliberately lossy: _cbor_str drops a wrong-typed field to "" rather than raising, because Photonic mints `loc` as an INTEGER on mainnet and refusing those tokens surfaced as "metadata: NONE". So the verifier compared the signature against bytes the creator never signed, and reported an HONEST, correctly-signed token as a forgery. Measured, with a verifying baseline and a matched control - the first attempt at this proof had a harness whose baseline ALSO failed, which would have attributed the mismatch to the wrong cause: in-memory, pyrxd-signed -> (True, '') TEXT loc, honestly signed -> (True, '') INTEGER loc, honestly signed -> (False, 'signature mismatch') <-- the bug `loc` is one instance; the defect is the class. ANY field the decoder normalises, now or the next time it learns a leniency, silently becomes a forgery verdict - so the failure gets WORSE as the decoder gets more forgiving, which is the opposite of how leniency should behave. Fixed at that level: the decoded object carries the exact bytes it was decoded from, and verification rebuilds the unsigned form from those, leaving every field with the type and value it had on chain. The re-encoded path remains for in-memory metadata, where the object IS the original. WHAT NOT TO TRADE IT FOR. A signature valid over on-chain bytes does not mean the creator signed what is being DISPLAYED. The old behaviour had that property by construction, at the cost of the false negative above, and dropping it silently would swap a visible defect for an invisible one. So when the decode normalised anything, the verdict says so explicitly rather than returning a bare "valid". Detected by comparing the two forms, not by tracking each field as it is normalised - a hand-kept list of lossy fields goes stale the first time the decoder learns a new leniency, which is how this class of bug arrives. source_cbor is compare=False: two tokens with identical fields are the same token regardless of which arrived over a wire, so existing equality assertions hold. REACHABILITY. verify_creator_signature has no caller inside pyrxd - it is exported public API, so its tests ARE its production entry point. Every case goes through decode_payload rather than building metadata by hand, which is the only way the defect is expressible at all. Verified by planting both halves: restoring the decoded-object derivation fails 8 of the new tests, and removing the lossy-decode disclosure fails 1. Every rescued case is paired with a forgery that must still be refused, including a forgery on the lossy path - the fix must not open a bypass for exactly the records it saved. Co-Authored-By: Claude Opus 5 (1M context) --- src/pyrxd/glyph/creator.py | 65 ++++++++- src/pyrxd/glyph/payload.py | 1 + src/pyrxd/glyph/types.py | 13 ++ ...tor_signature_verifies_the_signed_bytes.py | 125 ++++++++++++++++++ 4 files changed, 200 insertions(+), 4 deletions(-) create mode 100644 tests/test_creator_signature_verifies_the_signed_bytes.py diff --git a/src/pyrxd/glyph/creator.py b/src/pyrxd/glyph/creator.py index 630c2776..6fc4dcce 100644 --- a/src/pyrxd/glyph/creator.py +++ b/src/pyrxd/glyph/creator.py @@ -38,12 +38,51 @@ def _signing_message(commit_hash: bytes) -> bytes: def _cbor_for_signing(metadata: GlyphMetadata, pubkey_hex: str, algo: str) -> bytes: - """CBOR-encode metadata with creator.sig = "" (unsigned canonical form).""" + """CBOR-encode metadata with creator.sig = "" (unsigned canonical form). + + Used to SIGN, and to verify metadata built in memory. To verify metadata that came + off a chain, prefer :func:`_cbor_for_verifying`, which does not route the bytes + through this object's fields. + """ d = metadata.to_cbor_dict() d["creator"] = {"pubkey": pubkey_hex, "sig": "", "algo": algo} return cbor2.dumps(d) +def _cbor_for_verifying(metadata: GlyphMetadata, pubkey_hex: str, algo: str) -> bytes: + """The unsigned form, rebuilt from the ORIGINAL bytes when we still have them. + + DECODING IS LOSSY AND VERIFICATION MUST NOT BE. ``_cbor_str`` drops a wrong-typed + field to ``""`` rather than raising, which is correct for display — Photonic mints + ``loc`` as an INTEGER on mainnet, and refusing those tokens showed the user + "metadata: NONE". But re-encoding the decoded object and checking the creator's + signature over THAT compares against bytes the creator never signed, so an honest, + correctly-signed token comes back "signature mismatch". Verified: with an integer + ``loc``, a token signed by a real key and decoded by pyrxd was reported forged, + while the identical construction with a text ``loc`` verified. + + ``loc`` is one instance; the defect is the class. ANY field the decoder normalises, + now or later, silently becomes a forgery verdict — the failure gets worse as the + decoder gets more forgiving, which is the opposite of how leniency should behave. + + Falls back to the re-encoded form when there are no source bytes, which is the + in-memory case (a caller who just built and signed metadata), where the object IS + the original. + """ + if metadata.source_cbor is None: + return _cbor_for_signing(metadata, pubkey_hex, algo) + try: + d = cbor2.loads(metadata.source_cbor) + except Exception: # pragma: no cover - decode_payload already parsed these bytes + return _cbor_for_signing(metadata, pubkey_hex, algo) + if not isinstance(d, dict): # pragma: no cover - likewise + return _cbor_for_signing(metadata, pubkey_hex, algo) + # Blank the signature the same way the signer did, leaving every OTHER field with + # the type and value it had on chain. + d["creator"] = {"pubkey": pubkey_hex, "sig": "", "algo": algo} + return cbor2.dumps(d) + + def sign_metadata( metadata: GlyphMetadata, private_key: PrivateKey, @@ -122,8 +161,9 @@ def verify_creator_signature(metadata: GlyphMetadata) -> tuple[bool, str]: except Exception as e: return False, f"invalid creator.pubkey: {e}" - # Reconstruct canonical CBOR with sig="" to get the same commit hash - cbor_bytes = _cbor_for_signing(metadata, creator.pubkey, creator.algo) + # Reconstruct canonical CBOR with sig="" to get the same commit hash. From the + # ORIGINAL bytes where we have them — see `_cbor_for_verifying`. + cbor_bytes = _cbor_for_verifying(metadata, creator.pubkey, creator.algo) message = _signing_message(_commit_hash(cbor_bytes)) try: @@ -131,4 +171,21 @@ def verify_creator_signature(metadata: GlyphMetadata) -> tuple[bool, str]: except Exception as e: return False, f"signature verification error: {e}" - return (True, "") if valid else (False, "signature mismatch") + if not valid: + return False, "signature mismatch" + + # A VALID signature over the on-chain bytes does not by itself mean the creator + # signed the metadata being DISPLAYED. Verifying against the source bytes fixed a + # false-forgery verdict, and the thing not to trade it for is a silent gap in the + # other direction: if the decoder normalised a field, the object a caller renders + # is not the object that was signed, and "VERIFIED" would be overclaiming. + # + # Detected by comparison rather than by tracking each field as it is normalised — + # a hand-kept list of lossy fields would go stale the first time the decoder + # learns a new leniency, which is exactly how this class of bug arrives. + if metadata.source_cbor is not None and cbor_bytes != _cbor_for_signing(metadata, creator.pubkey, creator.algo): + return True, ( + "signature is valid over the on-chain bytes, but the decoder normalised at " + "least one field — the metadata shown is NOT byte-identical to what was signed" + ) + return True, "" diff --git a/src/pyrxd/glyph/payload.py b/src/pyrxd/glyph/payload.py index b8c5652f..ef4fd160 100644 --- a/src/pyrxd/glyph/payload.py +++ b/src/pyrxd/glyph/payload.py @@ -241,6 +241,7 @@ def decode_payload(cbor_bytes: bytes) -> GlyphMetadata: _log.warning("decode_payload: malformed 'crypto.timelock' field ignored: %s", e) return GlyphMetadata( + source_cbor=cbor_bytes, protocol=d["p"], timelock=timelock, container_refs=_decode_rel_refs(d.get("in"), "in"), diff --git a/src/pyrxd/glyph/types.py b/src/pyrxd/glyph/types.py index 6a9d548b..55667209 100644 --- a/src/pyrxd/glyph/types.py +++ b/src/pyrxd/glyph/types.py @@ -345,6 +345,19 @@ class GlyphMetadata: # format are mint-side concerns, and surfacing per-recipient key material through the # inspect path is not something to do incidentally. timelock: TimelockSpec | None = None + # The EXACT CBOR these fields were decoded from, when they came off a chain. + # + # Carried because decoding is LOSSY and creator-signature verification is not + # allowed to be. `_cbor_str` drops a wrong-typed field to "" rather than raising, + # which is right for display — Photonic mints `loc` as an INTEGER on mainnet and + # refusing those tokens showed the user "metadata: NONE". But re-encoding the + # decoded object and checking a signature over THAT reports an honest, + # correctly-signed token as a forgery, because the bytes the creator signed are + # not the bytes we rebuilt. Verification reads these instead. + # + # compare=False: two tokens with identical fields are the same token regardless of + # which one arrived over a wire, and every existing equality assertion stays true. + source_cbor: bytes | None = field(default=None, compare=False, repr=False) # CBOR ``in`` — the CONTAINER(s) this token is a member of. Membership points # CHILD -> PARENT and lives here, in the envelope, because it cannot live in # the locking script: an output may not carry a ref that a *sibling* output diff --git a/tests/test_creator_signature_verifies_the_signed_bytes.py b/tests/test_creator_signature_verifies_the_signed_bytes.py new file mode 100644 index 00000000..d2c80b2a --- /dev/null +++ b/tests/test_creator_signature_verifies_the_signed_bytes.py @@ -0,0 +1,125 @@ +"""A creator signature must be checked against the bytes the creator SIGNED. + +`verify_creator_signature` re-derived the signed form by calling `to_cbor_dict()` on +the DECODED metadata. Decoding is deliberately lossy — `_cbor_str` drops a +wrong-typed field to "" rather than raising, because Photonic mints `loc` as an +INTEGER on mainnet and refusing those tokens showed the user "metadata: NONE". So +the verifier compared the signature against bytes the creator never signed, and +reported an HONEST, correctly-signed token as a forgery. + +`loc` is one instance; the defect is the class. Any field the decoder normalises — +now, or the next time it learns a leniency — silently becomes a forgery verdict. The +failure therefore gets WORSE as the decoder gets more forgiving, which is the +opposite of how leniency should behave. + +The fix carries the original bytes on the decoded object and verifies against those. +The thing not to trade it for is a gap the other way: a signature valid over on-chain +bytes does not mean the creator signed what is being DISPLAYED. When the decode was +lossy, the verdict says so. + +`verify_creator_signature` has no caller inside pyrxd — it is exported public API, so +these tests ARE its production entry point, and every case below goes through +`decode_payload` rather than constructing metadata by hand. +""" + +from __future__ import annotations + +import hashlib +import os + +import cbor2 +import pytest + +from pyrxd.glyph.creator import _CREATOR_PREFIX, sign_metadata, verify_creator_signature +from pyrxd.glyph.payload import decode_payload +from pyrxd.glyph.types import GlyphMetadata +from pyrxd.hash import hash256 +from pyrxd.keys import PrivateKey + + +@pytest.fixture +def key() -> PrivateKey: + return PrivateKey(os.urandom(32)) # never a hand-written key + + +@pytest.fixture +def signed(key: PrivateKey) -> GlyphMetadata: + return sign_metadata(GlyphMetadata(protocol=[2], name="Test", ticker="TST", loc="ipfs://x"), key) + + +def _mint(key: PrivateKey, signed: GlyphMetadata, *, loc, tamper: str | None = None) -> GlyphMetadata: + """Mint as a THIRD-PARTY writer would, then read it back the way pyrxd does. + + The signature is made over the bytes as published, so `tamper` models a publisher + who signed one thing and put another on chain — not a corrupted signature. + """ + pubkey = key.public_key().serialize(compressed=True).hex() + d = signed.to_cbor_dict() + d["loc"] = loc + d["creator"] = {"pubkey": pubkey, "sig": "", "algo": "ecdsa-secp256k1"} + message = hashlib.sha256(_CREATOR_PREFIX + hash256(cbor2.dumps(d))).digest() + d["creator"]["sig"] = key.sign(message, hasher=None).hex() + if tamper is not None: + d["name"] = tamper + return decode_payload(cbor2.dumps(d)) + + +class TestAnHonestTokenIsNotCalledForged: + def test_an_integer_loc_still_verifies(self, key: PrivateKey, signed: GlyphMetadata) -> None: + """The shape measured on live mainnet. This returned "signature mismatch".""" + ok, _ = verify_creator_signature(_mint(key, signed, loc=0)) + assert ok + + @pytest.mark.parametrize("loc", [0, 42, b"bytes", ["a"], {"k": "v"}, True], ids=repr) + def test_ANY_normalised_type_still_verifies(self, key: PrivateKey, signed: GlyphMetadata, loc) -> None: + """Fixing only the integer case would leave the class intact — the decoder + drops every non-string, and each one was its own false forgery.""" + ok, _ = verify_creator_signature(_mint(key, signed, loc=loc)) + assert ok + + def test_the_control_still_verifies(self, key: PrivateKey, signed: GlyphMetadata) -> None: + """Same construction, an ordinary text loc. If this ever fails the tests above + prove nothing: they would be passing for a reason unrelated to the fix.""" + ok, reason = verify_creator_signature(_mint(key, signed, loc="ipfs://x")) + assert ok and reason == "" + + def test_in_memory_metadata_with_no_source_bytes_still_verifies(self, signed: GlyphMetadata) -> None: + """The path a caller takes right after signing, where the object IS the original.""" + assert signed.source_cbor is None + assert verify_creator_signature(signed) == (True, "") + + +class TestAForgeryIsStillRefused: + """The other half. A verifier that accepts everything passes every test above.""" + + def test_a_field_changed_after_signing_is_refused(self, key: PrivateKey, signed: GlyphMetadata) -> None: + ok, reason = verify_creator_signature(_mint(key, signed, loc="ipfs://x", tamper="Evil")) + assert not ok and "mismatch" in reason + + def test_tampering_is_refused_even_on_the_lossy_path(self, key: PrivateKey, signed: GlyphMetadata) -> None: + """The fix must not have opened a bypass for exactly the records it rescued.""" + ok, _ = verify_creator_signature(_mint(key, signed, loc=0, tamper="Evil")) + assert not ok + + def test_a_signature_from_another_key_is_refused(self, key: PrivateKey, signed: GlyphMetadata) -> None: + record = _mint(key, signed, loc=0) + other = PrivateKey(os.urandom(32)).public_key().serialize(compressed=True).hex() + ok, _ = verify_creator_signature( + GlyphMetadata( + protocol=record.protocol, + source_cbor=record.source_cbor, + creator=type(record.creator)(pubkey=other, sig=record.creator.sig, algo=record.creator.algo), + ) + ) + assert not ok + + +class TestALossyDecodeIsDisclosed: + """VERIFIED must not quietly mean "signed bytes you are not being shown".""" + + def test_a_lossy_decode_says_so(self, key: PrivateKey, signed: GlyphMetadata) -> None: + ok, reason = verify_creator_signature(_mint(key, signed, loc=0)) + assert ok and "NOT byte-identical" in reason + + def test_a_lossless_decode_says_nothing(self, key: PrivateKey, signed: GlyphMetadata) -> None: + assert verify_creator_signature(_mint(key, signed, loc="ipfs://x")) == (True, "")