fix(attestation): verify attested signatures from did:web authorities#43
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>
| // Verify the attestation invocation was actually signed by the authority, | ||
| // using the already-resolved authority verifier directly. Previously this | ||
| // re-validated via validator.ValidateInvocation with no options, which | ||
| // defaults to a did:key-only resolver and therefore fails to resolve a | ||
| // did:web authority — the cause of "signature mismatch" for did:web services | ||
| // (e.g. did:web:upload) while did:key authorities (and unit tests) passed. |
There was a problem hiding this comment.
This comment isn't quite right. This didn't re-validate the (outer) token, it validated the signature, as an invocation, for the first time.
Technically, only verifying the signature leaves out a few ways the invocation could be invalid, like expiration, but we're not supposed to use any of those ways for these invocations. I think ideally this eventually change from using invocations to using assertions (or maybe they'll be called "attestations", which would certainly be appropriate), which would be a different kind of token and would specifically not have expiration dates and other things that aren't relevant.
More to the point, v.authorityVerifier isn't used for anything yet, so I'm pretty sure this is exactly why I added it, I just failed to do exactly this part. So good catch, thank you! 😅
Bump ucantone 7985ec0 -> a8f24fe (adds acceptance of the DID Core v1 `@context`, https://www.w3.org/ns/did/v1, alongside v1.1) and libforge eb26d871 -> 2b55dbcf (fil-forge/libforge#43, attested did:web signatures). Without the ucantone bump, piri rejects DID documents whose `@context` is the older v1 form with: publishing location commitment: unable to resolve "did:web:delegator": parsing DID document JSON: @context must list ".../did/v1.1" first This blocks /blob/accept (and therefore every upload) against services that still serve a v1-context did:web document (e.g. the delegator), which is what the smelt e2e suite hit once the attested-signature login fix (libforge#43) was in place. Dependency-only change: builds, vets, and the fx app graph validation (pkg/fx/app) pass unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017yUxrDH8gWAA4ufwzuq657
Bump ucantone 7985ec0 -> a8f24fe (accepts the DID Core v1 `@context` alongside v1.1) and libforge eb26d871 -> 2b55dbcf (fil-forge/libforge#43, attested did:web signatures). Part of the cross-service rollout for the attested-signatures / UCAN Principal Clarification migration: any service that resolves did:web documents needs the ucantone bump to accept v1-context DID documents, and the libforge#43 verifier fix for attested did:web signatures. Dependency-only change: builds and vets clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017yUxrDH8gWAA4ufwzuq657
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