From a2473cdb42bde8cfde4623b346ea060a20356a12 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 3 Oct 2023 14:34:26 +0200 Subject: [PATCH] replace JWT with secure random code --- auth/api/iam/api.go | 10 +++---- auth/api/iam/api_test.go | 17 +++++------ auth/api/iam/openid4vp_test.go | 4 +-- auth/api/iam/s2s_vptoken.go | 49 +++++++++++--------------------- auth/api/iam/s2s_vptoken_test.go | 45 +++++++++-------------------- auth/api/iam/types.go | 9 ++++++ cmd/root.go | 2 +- storage/test.go | 2 +- 8 files changed, 57 insertions(+), 81 deletions(-) diff --git a/auth/api/iam/api.go b/auth/api/iam/api.go index 3a06280dba..a88e1c64eb 100644 --- a/auth/api/iam/api.go +++ b/auth/api/iam/api.go @@ -28,7 +28,7 @@ import ( "github.com/nuts-foundation/nuts-node/auth" "github.com/nuts-foundation/nuts-node/auth/log" "github.com/nuts-foundation/nuts-node/core" - "github.com/nuts-foundation/nuts-node/crypto" + "github.com/nuts-foundation/nuts-node/storage" "github.com/nuts-foundation/nuts-node/vcr" "github.com/nuts-foundation/nuts-node/vdr" "github.com/nuts-foundation/nuts-node/vdr/resolver" @@ -53,12 +53,12 @@ type Wrapper struct { vcr vcr.VCR vdr vdr.VDR auth auth.AuthenticationServices - privateKeyStore crypto.KeyStore sessions *SessionManager + storageInstance storage.Engine templates *template.Template } -func New(authInstance auth.AuthenticationServices, vcrInstance vcr.VCR, vdrInstance vdr.VDR, privateKeyStore crypto.KeyStore) *Wrapper { +func New(authInstance auth.AuthenticationServices, vcrInstance vcr.VCR, vdrInstance vdr.VDR, storageInstance storage.Engine) *Wrapper { templates := template.New("oauth2 templates") _, err := templates.ParseFS(assets, "assets/*.html") if err != nil { @@ -66,10 +66,10 @@ func New(authInstance auth.AuthenticationServices, vcrInstance vcr.VCR, vdrInsta } return &Wrapper{ sessions: &SessionManager{sessions: new(sync.Map)}, + storageInstance: storageInstance, auth: authInstance, vcr: vcrInstance, vdr: vdrInstance, - privateKeyStore: privateKeyStore, templates: templates, } } @@ -146,7 +146,7 @@ func (r Wrapper) HandleTokenRequest(ctx context.Context, request HandleTokenRequ } case "vp_token-bearer": // Nuts RFC021 vp_token bearer flow - return r.handleS2SAccessTokenRequest(ctx, *ownDID, request.Body.AdditionalProperties) + return r.handleS2SAccessTokenRequest(*ownDID, request.Body.AdditionalProperties) default: return nil, OAuth2Error{ Code: UnsupportedGrantType, diff --git a/auth/api/iam/api_test.go b/auth/api/iam/api_test.go index 47c5ffa596..00ee665d06 100644 --- a/auth/api/iam/api_test.go +++ b/auth/api/iam/api_test.go @@ -27,7 +27,7 @@ import ( "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/auth" "github.com/nuts-foundation/nuts-node/core" - "github.com/nuts-foundation/nuts-node/crypto" + "github.com/nuts-foundation/nuts-node/storage" "github.com/nuts-foundation/nuts-node/vcr" "github.com/nuts-foundation/nuts-node/vcr/pe" "github.com/nuts-foundation/nuts-node/vcr/verifier" @@ -166,9 +166,10 @@ func TestWrapper_GetOAuthClientMetadata(t *testing.T) { func TestWrapper_HandleAuthorizeRequest(t *testing.T) { t.Run("missing redirect_uri", func(t *testing.T) { ctx := newTestClient(t) + ctx.vdr.EXPECT().IsOwner(gomock.Any(), nutsDID).Return(true, nil) res, err := ctx.client.HandleAuthorizeRequest(requestContext(map[string]string{}), HandleAuthorizeRequestRequestObject{ - Id: nutsDID.String(), + Id: nutsDID.ID, }) requireOAuthError(t, err, InvalidRequest, "redirect_uri is required") @@ -176,12 +177,13 @@ func TestWrapper_HandleAuthorizeRequest(t *testing.T) { }) t.Run("unsupported response type", func(t *testing.T) { ctx := newTestClient(t) + ctx.vdr.EXPECT().IsOwner(gomock.Any(), nutsDID).Return(true, nil) res, err := ctx.client.HandleAuthorizeRequest(requestContext(map[string]string{ "redirect_uri": "https://example.com", "response_type": "unsupported", }), HandleAuthorizeRequestRequestObject{ - Id: nutsDID.String(), + Id: nutsDID.ID, }) requireOAuthError(t, err, UnsupportedResponseType, "") @@ -192,9 +194,10 @@ func TestWrapper_HandleAuthorizeRequest(t *testing.T) { func TestWrapper_HandleTokenRequest(t *testing.T) { t.Run("unsupported grant type", func(t *testing.T) { ctx := newTestClient(t) + ctx.vdr.EXPECT().IsOwner(gomock.Any(), nutsDID).Return(true, nil) res, err := ctx.client.HandleTokenRequest(nil, HandleTokenRequestRequestObject{ - Id: nutsDID.String(), + Id: nutsDID.ID, Body: &HandleTokenRequestFormdataRequestBody{ GrantType: "unsupported", }, @@ -238,7 +241,6 @@ type testCtx struct { client *Wrapper authnServices *auth.MockAuthenticationServices vdr *vdr.MockVDR - keyStore *crypto.MockKeyStore resolver *resolver.MockDIDResolver verifier *verifier.MockVerifier vcr *vcr.MockVCR @@ -248,7 +250,7 @@ func newTestClient(t testing.TB) *testCtx { publicURL, err := url.Parse("https://example.com") require.NoError(t, err) ctrl := gomock.NewController(t) - keyStore := crypto.NewMockKeyStore(ctrl) + storageEngine := storage.NewTestStorageEngine(t) mockVerifier := verifier.NewMockVerifier(ctrl) mockVCR := vcr.NewMockVCR(ctrl) mockVCR.EXPECT().Verifier().Return(mockVerifier).AnyTimes() @@ -267,14 +269,13 @@ func newTestClient(t testing.TB) *testCtx { authnServices: authnServices, resolver: resolver, vdr: vdr, - keyStore: keyStore, verifier: mockVerifier, vcr: mockVCR, client: &Wrapper{ auth: authnServices, vdr: vdr, vcr: mockVCR, - privateKeyStore: keyStore, + storageInstance: storageEngine, }, } } diff --git a/auth/api/iam/openid4vp_test.go b/auth/api/iam/openid4vp_test.go index 181bf94036..1cb513ab9a 100644 --- a/auth/api/iam/openid4vp_test.go +++ b/auth/api/iam/openid4vp_test.go @@ -124,7 +124,7 @@ func TestWrapper_handlePresentationRequest(t *testing.T) { _ = peStore.LoadFromFile("test/presentation_definition_mapping.json") mockAuth := auth.NewMockAuthenticationServices(ctrl) mockAuth.EXPECT().PresentationDefinitions().Return(peStore) - instance := New(mockAuth, nil, nil) + instance := New(mockAuth, nil, nil, nil) params := map[string]string{ "scope": "unsupported", @@ -139,7 +139,7 @@ func TestWrapper_handlePresentationRequest(t *testing.T) { assert.Nil(t, response) }) t.Run("invalid response_mode", func(t *testing.T) { - instance := New(nil, nil, nil) + instance := New(nil, nil, nil, nil) params := map[string]string{ "scope": "eOverdracht-overdrachtsbericht", "response_type": "code", diff --git a/auth/api/iam/s2s_vptoken.go b/auth/api/iam/s2s_vptoken.go index d824fabd02..8488083029 100644 --- a/auth/api/iam/s2s_vptoken.go +++ b/auth/api/iam/s2s_vptoken.go @@ -24,8 +24,6 @@ import ( "encoding/json" "errors" "fmt" - "github.com/lestrrat-go/jwx/jws" - "github.com/lestrrat-go/jwx/jwt" "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/go-did/vc" "github.com/nuts-foundation/nuts-node/core" @@ -49,7 +47,7 @@ const scopeCredentialsClaimKey = "vcs" // handleS2SAccessTokenRequest handles the /token request with vp_token bearer grant type, intended for service-to-service exchanges. // It performs cheap checks first (parameter presence and validity), then the more expensive ones (checking signatures and matching VCs to the presentation definition). -func (s Wrapper) handleS2SAccessTokenRequest(ctx context.Context, issuer did.DID, params map[string]string) (HandleTokenRequestResponseObject, error) { +func (s Wrapper) handleS2SAccessTokenRequest(issuer did.DID, params map[string]string) (HandleTokenRequestResponseObject, error) { submissionEncoded := params["presentation_submission"] scope := params[scopeParam] assertionEncoded := params["assertion"] @@ -146,44 +144,29 @@ func (s Wrapper) handleS2SAccessTokenRequest(ctx context.Context, issuer did.DID Description: "presentation_submission parameter is invalid: re-evaluation of the credentials against the presentation definition yields different credentials (bug or attempted hack)", } } - - // TODO: Check that the returned credentials fulfill the presentation definition of the scope. - // TODO: Only include credentials from the presentation submission - response, err := s.createAccessToken(ctx, issuer, issueTime, matchedCredentials, scope) + // All OK, allow access + response, err := s.createAccessToken(issuer, issueTime, vp, scope) if err != nil { - return nil, fmt.Errorf("creating access token: %w", err) + return nil, err } return HandleTokenRequest200JSONResponse(*response), nil } -func (s Wrapper) createAccessToken(ctx context.Context, issuer did.DID, issueTime time.Time, credentials []vc.VerifiableCredential, scope string) (*TokenResponse, error) { - // Sign with the private key of the issuer - keyResolver := resolver.DIDKeyResolver{Resolver: s.vdr.Resolver()} - signingKeyID, _, err := keyResolver.ResolveKey(issuer, &issueTime, resolver.NutsSigningKeyType) - if err != nil { - return nil, fmt.Errorf("resolve issuer signing key: %w", err) - } - // TODO: The JWT becomes quite large due to inclusion of the complete VCs; - // do we need to have an opaque token with backend storage of tokens? - claims := map[string]interface{}{ - jwt.IssuedAtKey: issueTime.Unix(), - jwt.IssuerKey: issuer.String(), - jwt.ExpirationKey: issueTime.Add(accessTokenValidity).Unix(), - jwt.NotBeforeKey: issueTime.Unix(), - scopeJWTClaimKey: scope, - scopeCredentialsClaimKey: credentials, - } - headers := map[string]interface{}{ - jws.KeyIDKey: signingKeyID.String(), - } - token, err := s.privateKeyStore.SignJWT(ctx, claims, headers, signingKeyID.String()) +func (s Wrapper) createAccessToken(issuer did.DID, issueTime time.Time, presentation vc.VerifiablePresentation, scope string) (*TokenResponse, error) { + accessToken := AccessToken{ + Token: generateCode(), + Issuer: issuer.String(), + Expiration: issueTime.Add(accessTokenValidity), + Presentation: presentation, + } + err := s.storageInstance.GetSessionDatabase().GetStore(accessTokenValidity, issuer.String(), "accesstoken").Put(accessToken.Token, accessToken) if err != nil { - return nil, fmt.Errorf("sign token: %w", err) + return nil, fmt.Errorf("unable to store access token: %w", err) } - expiredIn := int(accessTokenValidity.Seconds()) + expiresIn := int(accessTokenValidity.Seconds()) return &TokenResponse{ - AccessToken: token, - ExpiresIn: &expiredIn, + AccessToken: accessToken.Token, + ExpiresIn: &expiresIn, Scope: &scope, TokenType: "bearer", }, nil diff --git a/auth/api/iam/s2s_vptoken_test.go b/auth/api/iam/s2s_vptoken_test.go index e6e5241563..edbc9897df 100644 --- a/auth/api/iam/s2s_vptoken_test.go +++ b/auth/api/iam/s2s_vptoken_test.go @@ -19,25 +19,21 @@ package iam import ( - "context" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "encoding/json" ssi "github.com/nuts-foundation/go-did" + "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/go-did/vc" - "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/vcr/pe" "github.com/nuts-foundation/nuts-node/vcr/test" "github.com/nuts-foundation/nuts-node/vdr/resolver" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "net/url" "testing" - "time" - - "github.com/nuts-foundation/go-did/did" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestWrapper_RequestAccessToken(t *testing.T) { @@ -103,7 +99,6 @@ func TestWrapper_RequestAccessToken(t *testing.T) { func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { issuerDID := did.MustParseDID("did:test:123") const requestedScope = "eOverdracht-overdrachtsbericht" - const expectedAccessToken = "access-token" // Create issuer DID document and keys keyPair, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) issuerDIDDocument := did.Document{ @@ -139,15 +134,6 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { t.Run("ok", func(t *testing.T) { ctx := newTestClient(t) ctx.verifier.EXPECT().VerifyVP(verifiablePresentation, true, false, gomock.Any()).Return(verifiablePresentation.VerifiableCredential, nil) - ctx.resolver.EXPECT().Resolve(issuerDID, gomock.Any()).Return(&issuerDIDDocument, &resolver.DocumentMetadata{}, nil) - var actualClaims map[string]interface{} - var actualHeaders map[string]interface{} - ctx.keyStore.EXPECT().SignJWT(gomock.Any(), gomock.Any(), gomock.Any(), keyID.String()). - DoAndReturn(func(_ context.Context, claims map[string]interface{}, headers map[string]interface{}, _ interface{}) (string, error) { - actualClaims = claims - actualHeaders = headers - return expectedAccessToken, nil - }) params := map[string]string{ "assertion": url.QueryEscape(string(verifiablePresentationJSON)), @@ -155,7 +141,7 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { "scope": requestedScope, } - resp, err := ctx.client.handleS2SAccessTokenRequest(audit.TestContext(), issuerDID, params) + resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, params) require.NoError(t, err) require.IsType(t, HandleTokenRequest200JSONResponse{}, resp) @@ -163,10 +149,7 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { assert.Equal(t, "bearer", tokenResponse.TokenType) assert.Equal(t, requestedScope, *tokenResponse.Scope) assert.Equal(t, int(accessTokenValidity.Seconds()), *tokenResponse.ExpiresIn) - assert.Equal(t, expectedAccessToken, tokenResponse.AccessToken) - assert.Equal(t, keyID.String(), actualHeaders["kid"]) - // Make sure token is not valid for too long - assert.GreaterOrEqual(t, time.Now().Add(accessTokenValidity).Add(time.Minute).Unix(), actualClaims["exp"]) + assert.NotEmpty(t, tokenResponse.AccessToken) }) t.Run("VP is in JWT format (not supported)", func(t *testing.T) { ctx := newTestClient(t) @@ -176,7 +159,7 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { "scope": requestedScope, } - resp, err := ctx.client.handleS2SAccessTokenRequest(audit.TestContext(), issuerDID, params) + resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, params) assert.EqualError(t, err, "TODO: VPs in JWT format not yet supported") assert.Nil(t, resp) @@ -189,9 +172,9 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { "scope": requestedScope, } - resp, err := ctx.client.handleS2SAccessTokenRequest(audit.TestContext(), issuerDID, params) + resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, params) - assert.EqualError(t, err, "assertion parameter is invalid: json: cannot unmarshal array into Go value of type map[string]interface {}") + assert.EqualError(t, err, "invalid_request - json: cannot unmarshal array into Go value of type map[string]interface {} - assertion parameter is invalid: invalid JSON") assert.Nil(t, resp) }) t.Run("submission is not valid JSON", func(t *testing.T) { @@ -202,9 +185,9 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { "scope": requestedScope, } - resp, err := ctx.client.handleS2SAccessTokenRequest(audit.TestContext(), issuerDID, params) + resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, params) - assert.EqualError(t, err, `presentation_submission parameter is invalid: invalid character 'o' in literal null (expecting 'u')`) + assert.EqualError(t, err, `invalid_request - invalid character 'o' in literal null (expecting 'u') - presentation_submission parameter is invalid: invalid JSON`) assert.Nil(t, resp) }) t.Run("unsupported scope", func(t *testing.T) { @@ -216,9 +199,9 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { "scope": "everything", } - resp, err := ctx.client.handleS2SAccessTokenRequest(audit.TestContext(), issuerDID, params) + resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, params) - assert.EqualError(t, err, `unsupported scope for presentation exchange: everything`) + assert.EqualError(t, err, `invalid_scope - unsupported scope for presentation exchange: everything`) assert.Nil(t, resp) }) t.Run("re-evaluation of presentation definition yields different credentials", func(t *testing.T) { @@ -246,9 +229,9 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { "scope": requestedScope, } - resp, err := ctx.client.handleS2SAccessTokenRequest(audit.TestContext(), issuerDID, params) + resp, err := ctx.client.handleS2SAccessTokenRequest(issuerDID, params) - assert.EqualError(t, err, "presentation_submission parameter is invalid: re-evaluation of the credentials against the presentation definition yields different credentials (bug or attempted hack)") + assert.EqualError(t, err, "invalid_request - presentation_submission parameter is invalid: re-evaluation of the credentials against the presentation definition yields different credentials (bug or attempted hack)") assert.Nil(t, resp) }) } diff --git a/auth/api/iam/types.go b/auth/api/iam/types.go index 76c128ba28..867925b831 100644 --- a/auth/api/iam/types.go +++ b/auth/api/iam/types.go @@ -20,7 +20,9 @@ package iam import ( "github.com/nuts-foundation/go-did/did" + "github.com/nuts-foundation/go-did/vc" "github.com/nuts-foundation/nuts-node/vdr/resolver" + "time" ) // DIDDocument is an alias @@ -32,6 +34,13 @@ type DIDDocumentMetadata = resolver.DocumentMetadata // ErrorResponse is an alias type ErrorResponse = OAuth2Error +type AccessToken struct { + Token string + Issuer string + Expiration time.Time + Presentation vc.VerifiablePresentation +} + const ( // responseTypeParam is the name of the response_type parameter. // Specified by https://datatracker.ietf.org/doc/html/rfc6749#section-3.1.1 diff --git a/cmd/root.go b/cmd/root.go index c8e800ef3b..4c3c6b302e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -214,7 +214,7 @@ func CreateSystem(shutdownCallback context.CancelFunc) *core.System { system.RegisterRoutes(statusEngine.(core.Routable)) system.RegisterRoutes(metricsEngine.(core.Routable)) system.RegisterRoutes(&authAPIv1.Wrapper{Auth: authInstance, CredentialResolver: credentialInstance}) - system.RegisterRoutes(authIAMAPI.New(authInstance, credentialInstance, vdrInstance, cryptoInstance)) + system.RegisterRoutes(authIAMAPI.New(authInstance, credentialInstance, vdrInstance, storageInstance)) system.RegisterRoutes(&authMeansAPI.Wrapper{Auth: authInstance}) system.RegisterRoutes(&didmanAPI.Wrapper{Didman: didmanInstance}) diff --git a/storage/test.go b/storage/test.go index d1c6c07116..95fb052a18 100644 --- a/storage/test.go +++ b/storage/test.go @@ -34,7 +34,7 @@ func NewTestStorageEngineInDir(dir string) Engine { return result } -func NewTestStorageEngine(t *testing.T) Engine { +func NewTestStorageEngine(t testing.TB) Engine { oldOpts := append(DefaultBBoltOptions[:]) t.Cleanup(func() { DefaultBBoltOptions = oldOpts