fix(attestation): verify attested signatures from did:web authorities#43
Open
frrist wants to merge 1 commit into
Open
fix(attestation): verify attested signatures from did:web authorities#43frrist wants to merge 1 commit into
frrist wants to merge 1 commit into
Conversation
The attestation Verifier re-validated the attestation invocation with validator.ValidateInvocation(v.ctx, inv) and no options. ValidateInvocation reads its DID resolver from options, defaulting to key.Resolver (did:key only) — so it could not resolve a did:web authority (e.g. did:web:upload), and attested-signature verification failed with "signature mismatch". did:key authorities (and the existing TestSigner) resolved fine, which is why this slipped through. Verify the attestation invocation's signature with the authority verifier the Verifier already holds (v.authorityVerifier) instead of re-resolving the authority. Add TestSigner_WebAuthority covering a did:web authority resolved via its DID document; without this change it fails with the same "signature mismatch" seen in the wild (e.g. `guppy login`). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes attested-signature verification when the attesting authority is a
did:webservice (e.g.did:web:upload). This is the bug behind thepost–Attested-Signatures
guppy loginfailure, where claiming the accountdelegation failed with:
Root cause
attestation/verifier.go'sVerifier.Verifyre-validated the inner attestationinvocation via:
ValidateInvocationtakes its DID resolver from options, and with none itdefaults to
key.Resolver— did:key only. So for adid:webauthority itcan't resolve the authority's document,
verifyTokenSignaturefails, and theattestation is rejected as a signature mismatch.
did:key authorities resolve fine under the default, so the existing
TestSigner(which uses a did:key authority) passed and the gap shipped. TheVerifieralready holds the resolvedauthorityVerifier, butVerifyneverused it.
Fix
Verify the attestation invocation's signature directly with the
already-resolved
authorityVerifier, instead of re-resolving the authority:Testing
TestSigner_WebAuthorityexercises adid:webauthority resolved via itsgenerated DID document — the path the existing did:key test never covered.
Without this fix it fails with the same
did:mailto:…#did:web:…: signature mismatcherror seen in the running stack; with it, it passes.libforgetest suite passes.Confirmed end-to-end in smelt
Verified against the full forge stack (smelt), not just unit tests:
main,guppy loginfails with the exactsignature mismatchabove.sprue(upload) —/access/claimhasaudience
did:web:upload, so sprue validates the attested account-delegationproof, and the
InvalidSignatureis returned in sprue's receipt (guppy onlyrelays it). Rebuilding guppy alone against this fix left login failing
identically; rebuilding sprue against it is what fixed it.
spruerebuilt against this branch and deployed to smelt,guppy loginsucceeds:
Successfully logged in as test@example.com!(exit 0).Rollout — all services must update after merge
This is a verification-side fix in a shared library, so merging it is not
enough on its own: every service that validates attested signatures must bump
its
libforgedependency and rebuild to pick it up — confirmed for sprue /upload (login path), and likewise needed for delegator, indexer, and
piri for their attested flows. Services that only sign or serve their
own did:web document are unaffected, but bumping uniformly is simplest.
Notes
identity.DIDDocument()emits a malformed verification-method id(
#%23key-0instead of#key-0, fromdoc.Fragment("#key-0")). It's cosmeticfor verification (matching is by VM
Material, notid) and is not fixedhere; worth a follow-up for spec-correct did:web documents.
🤖 Generated with Claude Code