fix: handle non-ASCII challenge nonces without TypeError

[imran-siddique]

What this changes

A signed record carrying a non-ASCII nonce such as nønce made verify_record raise TypeError when a challenge was supplied. Compare UTF-8 bytes with compare_digest so matching strings verify and mismatches raise the documented ValueError. No Unicode normalization is applied.

Eight regression cases cover ASCII control cases, matching non-ASCII and supplementary-plane strings, mismatches on either side, and visually similar strings with different encodings. Six failed before the fix. Reported by @chernistry in #381.

Closes #381.

Type of change

Implementation bug fix. No schema, wire-format, or normative text changes.

Spec section

Implements the existing challenge-nonce matching rule in TRACE v0.2 section 3.2.2.

Validation

• All 165 signing tests pass: the eight Unicode cases, five invalid configuration cases, and a test that requires the nonce verdict to use compare_digest. The five configuration cases failed before the follow-up fix. The comparison test catches plain string comparison, plain byte comparison, and calling compare_digest while ignoring its result.
• Ruff, mypy (13 source files), house-style and diff checks pass.
• Full Windows Python 3.12 suite: 1,975 passed, 6 skipped, 5 generator-fixture failures. The same five failures reproduce with the production fix removed (15 other generator tests pass). Hosted Linux checks passed on 267dbb1; checks for the review follow-up at 7897d82 are pending.
• Local setup used the pinned development requirements plus Windows-only colorama 0.4.6 and an editable install of this checkout.

Checklist

• DCO sign-off on all commits
• CHANGELOG.md updated
• Breaking-change markers and compatibility statement: not applicable.

[imran-siddique]

@lywinged, could you review #383 and approve it if the change looks right? It fixes the non-ASCII nonce crash from #381. Matching and mismatching Unicode cases are covered, and CI is green.

Your review of #376 covered the related findings. These separate fixes are also ready for review:

• fix: preserve record verification when revocation bundles are malformed #386: malformed revocation bundles return an unverified revocation result without rejecting an otherwise valid record.
• fix: reject malformed embedded IPv4 URI literals #387: reject leading zeros in IPv4 octets inside IPv6 URI literals.
• fix: apply ECMA regex boundaries in Trust Record validation #388: apply the published schema's regex rules consistently in Python.
• fix: distinguish missing and malformed signatures #390: distinguish an absent signature from a present value of the wrong type.

Please start with #383 and review whichever others you have time for. Each PR includes its reproducer and test results. Please flag anything that needs changing before approval. #387 and #388 touch the same URI checker, so both checks need to survive when we combine them.

[lywinged]

Checked at 267dbb1 against main at ccbd5b9. Every figure I can recompute here reproduces:
159 in tests/test_sign.py, the eight new cases, six of them failing with the three-line
production change reverted and the tests kept, mypy's 13 files, the 15 other generator tests,
and §3.2.2 carrying the rule. ruff check src tests scripts is clean. The Windows figures I
cannot reach, and your own note already settles what they mean: the five reproduce with the fix
removed, which is a controlled experiment on the machine where they happen. What I can add is
narrow, so take it as narrow: all 20 tests in tests/test_generators_reproduce_fixtures.py pass
on this Linux checkout, 20 of 20, no skips.

The fix is right. Three things, and the first is a line of code.

expected_nonce is verifier configuration and nothing checks its type

>>> verify_record(record, jwk, expected_nonce=b"a-nonce-value")
AttributeError: 'bytes' object has no attribute 'encode'     # at sign.py:800

Five shapes, bytes, int, list, bool, dict, all AttributeError on the head. On main
all five are TypeError. So this is not a regression and neither is in the documented contract.
But TypeError is what Python raises for a caller who passed the wrong type, and an
AttributeError leaking out of .encode reads as the verifier breaking rather than as the
caller being told. The house answer is three lines above your change:

Both bounds are verifier configuration, not record data, and a malformed one is checked
before it is used -- see _check_seconds.

max_age_seconds and max_future_skew_seconds get that treatment and expected_nonce does not.
Not folded into the condition you already have, though: _check_seconds' docstring is explicit
that the point is a caller seeing "an error naming the cause" rather than a uniform refusal, and
raise ValueError("record runtime.nonce does not match expected_nonce") is the uniform refusal.
A separate line above it, in its shape:

if not isinstance(expected_nonce, str):
    raise ValueError("expected_nonce must be a string")

All five shapes then give that message, and your 159 stay green.

Record-side inputs are fine: schema validation at 694 refuses a missing runtime, a non-dict
runtime and a non-string nonce before the compare runs, so the isinstance(actual_nonce, str)
guard is now belt and braces. I could not find a record that reaches a wrong verdict.

The eight cases pin the verdict, not the reason compare_digest is there

Both of these are green on all eight:

actual_nonce.encode("utf-8") != expected_nonce.encode("utf-8")
actual_nonce != expected_nonce

Your matrix does what it was written for, and the ("\u00e9", "e\u0301") case, one glyph spelled two ways, earns its place:
normalising both sides to NFC turns that one red and nothing else. This is a separate property,
not a gap in what you claimed. Worth closing while you are here because a constant-time
comparison is unobservable in a verdict, so what can be asserted is that the verdict comes from
it. Watch the first draft of that test, which I wrote and then broke on purpose: patching
compare_digest to record its arguments passes against a version that calls it and then decides
with != anyway. Returning False from the patch is what closes that, and it has to be narrowed
to bytes operands or the thumbprint compare at 771 intercepts it first.

import hmac  # tests/test_sign.py does not import it yet

def test_the_nonce_verdict_is_decided_by_compare_digest(monkeypatch):
    seen = []
    real = hmac.compare_digest

    def spy(a, b):
        seen.append((type(a).__name__, type(b).__name__))
        return False if isinstance(a, bytes) else real(a, b)

    monkeypatch.setattr(hmac, "compare_digest", spy)
    key = generate_key()
    record = _fresh_record()
    record["runtime"]["nonce"] = "n\u00f8nce"
    signed = sign_record(record, key)
    with pytest.raises(ValueError, match="record runtime.nonce does not match"):
        verify_record(signed, key_to_jwk(key), expected_nonce="n\u00f8nce")
    assert ("bytes", "bytes") in seen

Passes on your fix, fails on both lines above and on the discard-the-result version.
compare_digest is imported inside verify_record, so the patch goes on hmac and not on the
module attribute. There are seven call sites in the package and nothing under tests/ asserts
any of them. #381 could not have reached the other six, for two reasons rather than one: five
compare _digest or jwk_thumbprint output, which is ASCII by construction, and
intent_bridge.py:316 compares _jcs output, which is bytes and so has no ASCII requirement
at all. Either way the nonce-only scope is right.

Two smaller ones

A record whose runtime.nonce is a lone surrogate, with expected_nonce set, raises
UnicodeEncodeError at 800, inside the compare that opens at 799. It stays in contract, because
that subclasses ValueError, and it is an improvement on main's TypeError for the same
input. Nothing about the ordering changed: canonicalization is after the compare on both sides,
which is why the same record still reaches CanonicalizationError when expected_nonce is
unset. One line only because it is the last member of #381's family.

#381 offered a second remedy, a base64url pattern on runtime.nonce. Declining it is right,
since it would reject records main accepts, but that leaves a description saying base64url with
nothing enforcing it, which is another instance of the family on #247 you are already tracking.
Worth a sentence there rather than here.

One question on the description: "ASCII controls" reads as a class of string next to the other
three, and there is no character in category Cc in the matrix. If it means control cases, fine.

Approving.

[imran-siddique]

@lywinged I added both changes in 7897d82. Invalid non-string expected_nonce settings now raise ValueError("expected_nonce must be a string"). The new test requires the nonce verdict to use compare_digest; it fails with plain string comparison, plain byte comparison, and a call that ignores the comparison result.

All 165 signing tests pass. Ruff, mypy and style checks pass. The full Windows run has 1,975 passing tests, 6 skips and the same five previously reproduced generator-fixture failures. CI is running on the new commit.

I also clarified "ASCII control cases" in the description. The nonce format question remains separate from this fix; no base64url restriction was added.

Could you confirm the follow-up looks right before we merge?

[lywinged]

Checked at 7897d82. Both changes are right.

expected_nonce now raises before the comparison, and all five shapes I named reach it. The
comparison test does what you say: I reverted the production line to plain string comparison, to
plain byte comparison, and to a version that calls compare_digest and then decides with !=
anyway, and each one fails exactly that test. 165 pass here, CI is green on the head.

One thing that arrived while this sat: #386 merged, so the CHANGELOG.md conflict between the two
is now a conflict with main. git merge-tree against 74eafa0 reports that one file and nothing
else.

Approving.