Skip to content

Commit

Permalink
change JAR validate to use OpenID configuration of client_id
Browse files Browse the repository at this point in the history
  • Loading branch information
@woutslakhorst
woutslakhorst committed Sep 2, 2024
1 parent fb5718a commit c4bf2bc
Show file tree
Hide file tree
Showing 13 changed files with 151 additions and 173 deletions.
30 changes: 18 additions & 12 deletions auth/api/iam/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
"net/http"
"net/url"
"strings"
"sync/atomic"
"time"

"github.com/lestrrat-go/jwx/v2/jwt"
Expand Down Expand Up @@ -101,8 +102,8 @@ type Wrapper struct {
vdr vdr.VDR
jwtSigner nutsCrypto.JWTSigner
keyResolver resolver.KeyResolver
jar JAR
subjectManager didsubject.SubjectManager
_jar atomic.Value
}

func New(
Expand All @@ -114,6 +115,7 @@ func New(
if err != nil {
panic(err)
}
keyResolver := resolver.DIDKeyResolver{Resolver: vdrInstance.Resolver()}
return &Wrapper{
auth: authInstance,
policyBackend: policyBackend,
Expand All @@ -123,14 +125,18 @@ func New(
subjectManager: subjectManager,
jsonldManager: jsonldManager,
jwtSigner: jwtSigner,
keyResolver: resolver.DIDKeyResolver{Resolver: vdrInstance.Resolver()},
jar: &jar{
auth: authInstance,
jwtSigner: jwtSigner,
keyResolver: resolver.DIDKeyResolver{Resolver: vdrInstance.Resolver()},
resolver: vdrInstance.Resolver(),
},
keyResolver: keyResolver,
}
}

func (r Wrapper) jar() JAR {
// todo sync
current := r._jar.Load().(JAR)
if current == nil {
current = NewJAR(r.auth, r.jwtSigner, r.keyResolver, r.auth.IAMClient())
r._jar.Store(current)
}
return current
}

func (r Wrapper) Routes(router core.EchoRouter) {
Expand Down Expand Up @@ -457,7 +463,7 @@ func (r Wrapper) HandleAuthorizeRequest(ctx context.Context, request HandleAutho
// The caller must ensure ownDID is actually owned by this node.
func (r Wrapper) handleAuthorizeRequest(ctx context.Context, subject string, ownMetadata oauth.AuthorizationServerMetadata, request url.URL) (HandleAuthorizeRequestResponseObject, error) {
// parse and validate as JAR (RFC9101, JWT Authorization Request)
requestObject, err := r.jar.Parse(ctx, ownMetadata, request.Query())
requestObject, err := r.jar().Parse(ctx, ownMetadata, request.Query())
if err != nil {
// already an oauth.OAuth2Error
return nil, err
Expand Down Expand Up @@ -523,7 +529,7 @@ func (r Wrapper) RequestJWTByGet(ctx context.Context, request RequestJWTByGetReq
}

// TODO: supported signature types should be checked
token, err := r.jar.Sign(ctx, ro.Claims)
token, err := r.jar().Sign(ctx, ro.Claims)
if err != nil {
return nil, oauth.OAuth2Error{
Code: oauth.ServerError,
Expand Down Expand Up @@ -577,7 +583,7 @@ func (r Wrapper) RequestJWTByPost(ctx context.Context, request RequestJWTByPostR
}

// TODO: supported signature types should be checked
token, err := r.jar.Sign(ctx, ro.Claims)
token, err := r.jar().Sign(ctx, ro.Claims)
if err != nil {
return nil, oauth.OAuth2Error{
Code: oauth.ServerError,
Expand Down Expand Up @@ -896,7 +902,7 @@ func (r Wrapper) createAuthorizationRequest(ctx context.Context, subject string,

// request_uri
requestURIID := nutsCrypto.GenerateNonce()
requestObj := r.jar.Create(*clientDID, clientID.String(), audience, modifier)
requestObj := r.jar().Create(*clientDID, clientID.String(), audience, modifier)
if err := r.authzRequestObjectStore().Put(requestURIID, requestObj); err != nil {
return nil, err
}
Expand Down
23 changes: 12 additions & 11 deletions auth/api/iam/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1406,6 +1406,17 @@ func newCustomTestClient(t testing.TB, publicURL *url.URL, authEndpointEnabled b
subjectManager.EXPECT().Exists(gomock.Any(), verifierSubject).Return(true, nil).AnyTimes()
subjectManager.EXPECT().Exists(gomock.Any(), unknownSubjectID).Return(false, nil).AnyTimes()

client := &Wrapper{
auth: authnServices,
vdr: mockVDR,
subjectManager: subjectManager,
vcr: mockVCR,
storageEngine: storageEngine,
policyBackend: policyInstance,
keyResolver: keyResolver,
jwtSigner: jwtSigner,
}
client._jar.Store(mockJAR)
return &testCtx{
ctrl: ctrl,
authnServices: authnServices,
Expand All @@ -1423,16 +1434,6 @@ func newCustomTestClient(t testing.TB, publicURL *url.URL, authEndpointEnabled b
keyResolver: keyResolver,
jwtSigner: jwtSigner,
jar: mockJAR,
client: &Wrapper{
auth: authnServices,
vdr: mockVDR,
subjectManager: subjectManager,
vcr: mockVCR,
storageEngine: storageEngine,
policyBackend: policyInstance,
keyResolver: keyResolver,
jwtSigner: jwtSigner,
jar: mockJAR,
},
client: client,
}
}
66 changes: 45 additions & 21 deletions auth/api/iam/jar.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,19 @@
package iam

import (
"bytes"
"context"
"crypto"
"net/url"
"slices"

"errors"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/lestrrat-go/jwx/v2/jwt"
"github.com/nuts-foundation/go-did/did"
"github.com/nuts-foundation/nuts-node/auth"
"github.com/nuts-foundation/nuts-node/auth/client/iam"
"github.com/nuts-foundation/nuts-node/auth/oauth"
cryptoNuts "github.com/nuts-foundation/nuts-node/crypto"
"github.com/nuts-foundation/nuts-node/vdr/resolver"
"net/url"
)

// requestObjectModifier is a function that modifies the Claims/params of an unsigned or signed (JWT) OAuth2 request
Expand All @@ -47,7 +49,7 @@ type jar struct {
auth auth.AuthenticationServices
jwtSigner cryptoNuts.JWTSigner
keyResolver resolver.KeyResolver
resolver resolver.DIDResolver
client iam.Client
}

type JAR interface {
Expand All @@ -69,6 +71,15 @@ type JAR interface {
Parse(ctx context.Context, ownMetadata oauth.AuthorizationServerMetadata, q url.Values) (oauthParameters, error)
}

func NewJAR(auth auth.AuthenticationServices, jwtSigner cryptoNuts.JWTSigner, keyResolver resolver.KeyResolver, client iam.Client) JAR {
return jar{
auth: auth,
jwtSigner: jwtSigner,
keyResolver: keyResolver,
client: client,
}
}

func (j jar) Create(client did.DID, clientID string, audience string, modifier requestObjectModifier) jarRequest {
return createJarRequest(client, clientID, audience, modifier)
}
Expand Down Expand Up @@ -147,10 +158,13 @@ func (j jar) Parse(ctx context.Context, ownMetadata oauth.AuthorizationServerMet
// the client_id must match the signer of the JWT.
func (j jar) validate(ctx context.Context, rawToken string, clientId string) (oauthParameters, error) {
var signerKid string
var publicKey crypto.PublicKey
// Parse and validate the JWT
token, err := cryptoNuts.ParseJWT(rawToken, func(kid string) (crypto.PublicKey, error) {
var err error
signerKid = kid
return j.keyResolver.ResolveKeyByID(kid, nil, resolver.AssertionMethod)
publicKey, err = j.keyResolver.ResolveKeyByID(kid, nil, resolver.AssertionMethod)
return publicKey, err
}, jwt.WithValidate(true))
if err != nil {
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "request signature validation failed", InternalError: err}
Expand All @@ -165,26 +179,36 @@ func (j jar) validate(ctx context.Context, rawToken string, clientId string) (oa
if clientId != params.get(oauth.ClientIDParam) {
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "invalid client_id claim in signed authorization request"}
}
// check if the signer of the JWT is the client
signer, err := did.ParseDIDURL(signerKid)
configuration, err := j.client.OpenIDConfiguration(ctx, clientId)
if err != nil {
// very unlikely since the key has already been resolved
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "invalid signer", InternalError: err}
return nil, oauth.OAuth2Error{Code: oauth.ServerError, Description: "failed to retrieve OpenID configuration", InternalError: err}
}

key, exists := configuration.JWKs.LookupKeyID(signerKid)
if !exists {
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "client_id does not own signer key"}
}
if err := compareThumbprint(key, publicKey); err != nil {
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "key mismatch between OpenID configuration and signer key", InternalError: err}
}
// resolve DID and check if clientId is in AlsoKnownAs
doc, _, err := j.resolver.Resolve(signer.DID, nil)
return params, nil
}

func compareThumbprint(configurationKey jwk.Key, publicKey crypto.PublicKey) error {
thumbprintLeft, err := configurationKey.Thumbprint(crypto.SHA256)
if err != nil {
if resolver.IsFunctionalResolveError(err) {
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "invalid signer", InternalError: err}
}
return nil, oauth.OAuth2Error{Code: oauth.ServerError, Description: "server error", InternalError: err}
return err
}
akaAsString := make([]string, len(doc.AlsoKnownAs))
for i, aka := range doc.AlsoKnownAs {
akaAsString[i] = aka.String()
signerKey, err := jwk.FromRaw(publicKey)
if err != nil {
return err
}
thumbprintRight, err := signerKey.Thumbprint(crypto.SHA256)
if err != nil {
return err
}
if !slices.Contains(akaAsString, clientId) {
return nil, oauth.OAuth2Error{Code: oauth.InvalidRequestObject, Description: "client_id does not match signer of authorization request"}
if bytes.Compare(thumbprintLeft, thumbprintRight) != 0 {
return errors.New("key thumbprints do not match")
}
return params, nil
return nil
}
67 changes: 51 additions & 16 deletions auth/api/iam/jar_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
"crypto"
"errors"
"fmt"
ssi "github.com/nuts-foundation/go-did"
"github.com/lestrrat-go/jwx/v2/jwk"
"github.com/nuts-foundation/nuts-node/crypto/storage/spi"
"github.com/nuts-foundation/nuts-node/test"
"testing"
Expand Down Expand Up @@ -109,6 +109,10 @@ func TestJar_Parse(t *testing.T) {
// setup did document and keys
privateKey, _ := spi.GenerateKeyPair()
kid := fmt.Sprintf("%s#%s", holderDID.String(), "key")
jwkKey, _ := jwk.FromRaw(privateKey.Public())
jwkKey.Set(jwk.KeyIDKey, kid)
jwkSet := jwk.NewSet()
_ = jwkSet.AddKey(jwkKey)

bytes, err := createSignedRequestObject(t, kid, privateKey, oauthParameters{
jwt.IssuerKey: holderDID.String(),
Expand All @@ -118,18 +122,16 @@ func TestJar_Parse(t *testing.T) {
token := string(bytes)
walletIssuerURL := test.MustParseURL(holderDID.String())
verifierMetadata := authorizationServerMetadata(verifierURL, []string{"web"})
didDocument := did.Document{
ID: holderDID,
AlsoKnownAs: []ssi.URI{ssi.MustParseURI(holderClientID)},
configuration := &oauth.OpenIDConfiguration{
JWKs: jwkSet,
}

t.Run("request_uri_method", func(t *testing.T) {

t.Run("ok - get", func(t *testing.T) {
ctx := newJarTestCtx(t)
ctx.iamClient.EXPECT().RequestObjectByGet(context.Background(), "request_uri").Return(token, nil)
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)
ctx.resolver.EXPECT().Resolve(holderDID, nil).Return(&didDocument, nil, nil)
ctx.iamClient.EXPECT().OpenIDConfiguration(gomock.Any(), holderClientID).Return(configuration, nil)

res, err := ctx.jar.Parse(context.Background(), verifierMetadata,
map[string][]string{
Expand All @@ -145,7 +147,7 @@ func TestJar_Parse(t *testing.T) {
ctx := newJarTestCtx(t)
ctx.iamClient.EXPECT().RequestObjectByGet(context.Background(), "request_uri").Return(token, nil)
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)
ctx.resolver.EXPECT().Resolve(holderDID, nil).Return(&didDocument, nil, nil)
ctx.iamClient.EXPECT().OpenIDConfiguration(gomock.Any(), holderClientID).Return(configuration, nil)

res, err := ctx.jar.Parse(context.Background(), verifierMetadata,
map[string][]string{
Expand All @@ -162,7 +164,7 @@ func TestJar_Parse(t *testing.T) {
md := authorizationServerMetadata(walletIssuerURL, []string{"web"})
ctx.iamClient.EXPECT().RequestObjectByPost(context.Background(), "request_uri", md).Return(token, nil)
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)
ctx.resolver.EXPECT().Resolve(holderDID, nil).Return(&didDocument, nil, nil)
ctx.iamClient.EXPECT().OpenIDConfiguration(gomock.Any(), holderClientID).Return(configuration, nil)

res, err := ctx.jar.Parse(context.Background(), md,
map[string][]string{
Expand Down Expand Up @@ -190,7 +192,7 @@ func TestJar_Parse(t *testing.T) {
t.Run("ok - 'request'", func(t *testing.T) {
ctx := newJarTestCtx(t)
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)
ctx.resolver.EXPECT().Resolve(holderDID, nil).Return(&didDocument, nil, nil)
ctx.iamClient.EXPECT().OpenIDConfiguration(gomock.Any(), holderClientID).Return(configuration, nil)

res, err := ctx.jar.Parse(context.Background(), verifierMetadata,
map[string][]string{
Expand Down Expand Up @@ -276,16 +278,52 @@ func TestJar_Parse(t *testing.T) {
oauth.ClientIDParam: verifierDID.String(),
})
require.NoError(t, err)
ctx.resolver.EXPECT().Resolve(holderDID, nil).Return(&did.Document{ID: holderDID}, nil, nil)
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)

res, err := ctx.jar.Parse(context.Background(), verifierMetadata,
map[string][]string{
oauth.ClientIDParam: {verifierDID.String()},
oauth.ClientIDParam: {verifierClientID},
oauth.RequestParam: {string(bytes)},
})

requireOAuthError(t, err, oauth.InvalidRequestObject, "client_id does not match signer of authorization request")
requireOAuthError(t, err, oauth.InvalidRequestObject, "invalid client_id claim in signed authorization request")
assert.Nil(t, res)
})
t.Run("error - retrieving OpenID configuration", func(t *testing.T) {
ctx := newJarTestCtx(t)
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)
ctx.iamClient.EXPECT().OpenIDConfiguration(gomock.Any(), holderClientID).Return(nil, assert.AnError)

res, err := ctx.jar.Parse(context.Background(), verifierMetadata,
map[string][]string{
oauth.ClientIDParam: {holderClientID},
oauth.RequestParam: {token},
})

requireOAuthError(t, err, oauth.ServerError, "failed to retrieve OpenID configuration")
assert.Nil(t, res)
})
t.Run("error - openID configuration key mismatch", func(t *testing.T) {
ctx := newJarTestCtx(t)
alternateKey, _ := spi.GenerateKeyPair()
jwkKey, _ := jwk.FromRaw(alternateKey.Public())
jwkKey.Set(jwk.KeyIDKey, kid)
jwkSet := jwk.NewSet()
_ = jwkSet.AddKey(jwkKey)

configuration := &oauth.OpenIDConfiguration{
JWKs: jwkSet,
}
ctx.keyResolver.EXPECT().ResolveKeyByID(kid, nil, resolver.AssertionMethod).Return(privateKey.Public(), nil)
ctx.iamClient.EXPECT().OpenIDConfiguration(gomock.Any(), holderClientID).Return(configuration, nil)

res, err := ctx.jar.Parse(context.Background(), verifierMetadata,
map[string][]string{
oauth.ClientIDParam: {holderClientID},
oauth.RequestParam: {token},
})

requireOAuthError(t, err, oauth.InvalidRequestObject, "key mismatch between OpenID configuration and signer key")
assert.Nil(t, res)
})
}
Expand All @@ -306,7 +344,6 @@ type testJarCtx struct {
iamClient *iam.MockClient
jwtSigner *cryptoNuts.MockJWTSigner
keyResolver *resolver.MockKeyResolver
resolver *resolver.MockDIDResolver
}

func newJarTestCtx(t testing.TB) testJarCtx {
Expand All @@ -316,18 +353,16 @@ func newJarTestCtx(t testing.TB) testJarCtx {
mockAuth.EXPECT().IAMClient().Return(mockIAMClient).AnyTimes()
mockSigner := cryptoNuts.NewMockJWTSigner(ctrl)
mockKeyResolver := resolver.NewMockKeyResolver(ctrl)
mockResolver := resolver.NewMockDIDResolver(ctrl)
return testJarCtx{
jar: &jar{
auth: mockAuth,
jwtSigner: mockSigner,
keyResolver: mockKeyResolver,
resolver: mockResolver,
client: mockIAMClient,
},
auth: mockAuth,
iamClient: mockIAMClient,
keyResolver: mockKeyResolver,
jwtSigner: mockSigner,
resolver: mockResolver,
}
}
Loading

0 comments on commit c4bf2bc

Please sign in to comment.