Skip to content

fix(ceremony): derive the PKCE verifier the way ceremony-common does - #24

Merged
xgreenx merged 1 commit into
mainfrom
fix/pkce-verifier-follows-the-spec
Sep 7, 2026
Merged

xgreenx merged 1 commit into
mainfrom
fix/pkce-verifier-follows-the-spec

Conversation

@xgreenx

@xgreenx xgreenx commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

The contracts and the merged ceremony-common §7 built two different PKCE
verifiers, so a prover written from the specification produced a
code_verifier TlsNotaryVerifierBase would never recompute, and every X
and GitHub claim reverted with CodeVerifierMismatch before it could reach a
Consumer.

Two differences, not one:

preimage nonce
contracts (de229ac) keccak256("libid.identity.pkce") || digest || pkceNonce a second field beside authorizationNonce
ceremony-common §7 digest || authorizationNonce the one the digest already commits

This moves the contracts to the specification, which is what
libid#20 merged.

What changed

  • CeremonyAuthorization.PKCE_DOMAIN is gone; verifierHash is
    SHA256(authorizationDigest || authorizationNonce), exactly 64 bytes.
  • TlsNotaryProof.pkceNonce is gone. The verifier is recomputed under
    p.authorizationNonce, so a caller has no second value left to move —
    changing the nonce moves the digest too (REQ-COMMON-12).
  • CeremonyAuthorization.t.sol pins the merged §7 vector, transcribed from
    the specification: verifierHash = 0xe6d7810e…c8e65a, code_verifier = 5teBDl6cz4U77aFweV5PbMhBJ_lEFv6LLNKzqnDI5lo, code_challenge = c8HLMaJOzc8OUoRYc7AocL5ioAkXVtAOmoGxoSY60IQ.
  • test_rejectsAForgedPkceNonce becomes
    test_rejectsAVerifierDerivedUnderAnotherNonce: it forges the revealed
    verifier rather than the removed field, which is the same property once
    there is one nonce.
  • Two prose sites that cited libid.identity.pkce as the one literal
    ceremony-common fixes — it now fixes none — and the Google
    layout-confusion test, whose old comment named pkceNonce as the word that
    trips the decoder. It is now word five, the identity session's offset, read
    as a publicInputs length of 64.

No platformCeremonyVersion bump

REQ-COMMON-12 bumps the version for a change to a published construction.
Here the boundary is not the artifact: REQ-PLAT-01A puts the proof statement
of a Platform Ceremony Version outside any Platform Verifier artifact
("chain artifacts implement the versioned ceremony boundary; they do not
define it"), and TEST-PLAT-17 still fixes the launch pairs at
("google", 1), ("x", 1) and ("github", 1). Nothing is deployed under
the old construction, so x/v1 is what §7 says and this artifact was simply
not conforming to it. Bumping would open an x/v2 the same merged text does
not list.

Verification

  • forge build, forge fmt --check, forge test — 404 passed, 0 failed.
  • The §7 vector was reproduced independently of both this library and the
    specification's own text before it was written into the test.
  • Solidity only: the payload struct reaches no Rust or TypeScript binding,
    and CeremonyProofVerifier — the one ceremony ABI the TS package generates
    — takes bytes, so its ABI is unchanged.

The contracts and the merged specification disagreed about section 7 in
two places at once, so a prover built from the specification produced a
verifier this code would never recompute, and no X or GitHub claim could
be made at all.

  here   SHA256(keccak256("libid.identity.pkce") || digest || pkceNonce)
  §7     SHA256(digest || authorizationNonce)

The domain constant is gone, and the second nonce with it. The verifier
is derived under the same `authorizationNonce` the digest already
commits, which is what REQ-COMMON-12 names, and `TlsNotaryProof` loses
its `pkceNonce` field: a caller has no second value left to move, since
changing the nonce moves the digest too.

The specification is the side that defines the boundary. REQ-PLAT-01A
puts the proof statement of a Platform Ceremony Version outside any
verifier artifact, and TEST-PLAT-17 still fixes the launch pairs at
("google", 1), ("x", 1) and ("github", 1). Nothing is deployed under the
old construction, so this makes the artifact conform to `x/v1` and
`github/v1` rather than opening a v2.

The section 7 vector in CeremonyAuthorization.t.sol is the merged one,
transcribed from the specification rather than produced by this library.
The X test that forged the second nonce now forges the revealed verifier
instead, which is the same property once there is one nonce.

Assisted-by: Claude Opus 5
Signed-off-by: xgreenx <xgreenx9999@gmail.com>
@xgreenx
xgreenx merged commit 856b57a into main Sep 7, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants