Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions attestation/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/fil-forge/ucantone/did"
"github.com/fil-forge/ucantone/ucan"
"github.com/fil-forge/ucantone/ucan/invocation"
"github.com/fil-forge/ucantone/validator"
"github.com/fil-forge/ucantone/ucan/token"
)

// Verifier is a ucan.Verifier for a DID whose signing is attested by an
Expand Down Expand Up @@ -54,7 +54,13 @@ func (v Verifier) Verify(msg []byte, sig []byte) bool {
return false
}

if validator.ValidateInvocation(v.ctx, inv) != nil {
// 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.
Comment on lines +57 to +62

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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! 😅

if !token.VerifySignature(inv, v.authorityVerifier) {
return false
}

Expand Down
74 changes: 74 additions & 0 deletions attestation/verifier_web_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
package attestation_test

import (
"context"
"fmt"
"testing"

"github.com/stretchr/testify/require"

"github.com/fil-forge/ucantone/did"
"github.com/fil-forge/ucantone/did/key"
"github.com/fil-forge/ucantone/ucan/command"
"github.com/fil-forge/ucantone/ucan/delegation"
"github.com/fil-forge/ucantone/validator"

"github.com/fil-forge/libforge/attestation"
"github.com/fil-forge/libforge/attestation/didmailto"
"github.com/fil-forge/libforge/identity"
"github.com/fil-forge/libforge/testutil"
)

// TestSigner_WebAuthority exercises an attestation whose authority is a did:web
// service (e.g. did:web:upload), resolved via its DID document — the real
// service flow. The existing TestSigner uses a did:key authority, which the
// default key.Resolver handles, so it never caught that the attestation verifier
// re-resolved the authority with the did:key-only default resolver. With a
// did:web authority that path fails ("signature mismatch"); this test guards the
// fix that verifies with the already-resolved authority verifier.
func TestSigner_WebAuthority(t *testing.T) {
authority, err := identity.New("", "did:web:example.com")
require.NoError(t, err)

doc, err := authority.DIDDocument()
require.NoError(t, err)

alice, err := did.Parse("did:mailto:example.com:alice")
require.NoError(t, err)

issuer := attestation.Attest(t.Context(), alice, authority)

del, err := delegation.Delegate(
issuer,
testutil.RandomDID(t),
issuer.DID(),
command.MustParse("/example/command"),
)
require.NoError(t, err)

encoded, err := delegation.Encode(del)
require.NoError(t, err)
decoded, err := delegation.Decode(encoded)
require.NoError(t, err)

// Serve the authority's generated did:web document.
webResolver := did.ResolverFunc(func(_ context.Context, d did.DID) (did.Document, error) {
if d == authority.DID() {
return doc, nil
}
return did.Document{}, fmt.Errorf("unexpected did %s", d)
})
resolver := did.ResolverMap{
"key": key.Resolver,
"web": webResolver,
"mailto": didmailto.NewResolver(authority.DID()),
}
factories := validator.DefaultFactories()
factories[attestation.Type] = attestation.NewVerifierFactory(resolver, factories)

err = validator.ValidateToken(t.Context(), decoded,
validator.WithDIDResolver(resolver),
validator.WithVerifierFactories(factories),
)
require.NoError(t, err)
}
Loading