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, "")