diff --git a/auth/services/notary/notary.go b/auth/services/notary/notary.go index 71a75d167c..6f5fb9cb73 100644 --- a/auth/services/notary/notary.go +++ b/auth/services/notary/notary.go @@ -112,7 +112,11 @@ func (n *notary) DrawUpContract(ctx context.Context, template contract.Template, return nil, fmt.Errorf("could not draw up contract: %w", err) } - if !n.privateKeyStore.Exists(ctx, signingKeyID.String()) { + exists, err := n.privateKeyStore.Exists(ctx, signingKeyID.String()) + if err != nil { + return nil, err + } + if !exists { return nil, services.InvalidContractRequestError{Message: fmt.Errorf("organization is not managed by this node: %w", ErrMissingOrganizationKey)} } diff --git a/auth/services/notary/notary_test.go b/auth/services/notary/notary_test.go index baaadc33f5..11a3782b43 100644 --- a/auth/services/notary/notary_test.go +++ b/auth/services/notary/notary_test.go @@ -68,7 +68,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, resolver.NutsSigningKeyType).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return([]vc.VerifiableCredential{testCredential}, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, nil) @@ -83,7 +83,7 @@ func TestContract_DrawUpContract(t *testing.T) { defer test.ctrl.Finish() test.keyResolver.EXPECT().ResolveKey(orgID, gomock.Any(), resolver.NutsSigningKeyType).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, &testCredential) require.NoError(t, err) @@ -96,7 +96,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return([]vc.VerifiableCredential{testCredential}, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, 0, nil) @@ -110,7 +110,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &time.Time{}, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return([]vc.VerifiableCredential{testCredential}, nil) timeNow = func() time.Time { @@ -139,7 +139,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(false) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(false, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, nil) @@ -162,7 +162,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return(nil, errors.New("error occurred")) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, nil) @@ -175,7 +175,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return([]vc.VerifiableCredential{testCredential}, nil) template := contract.Template{ @@ -192,7 +192,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return([]vc.VerifiableCredential{testCredential, testCredential}, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, nil) @@ -209,7 +209,7 @@ func TestContract_DrawUpContract(t *testing.T) { _ = json.Unmarshal([]byte(jsonld.TestCredential), &testCredential2) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) test.vcr.EXPECT().Search(context.Background(), searchTerms, false, nil).Return([]vc.VerifiableCredential{testCredential, testCredential2}, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, nil) @@ -222,7 +222,7 @@ func TestContract_DrawUpContract(t *testing.T) { test := buildContext(t) test.keyResolver.EXPECT().ResolveKey(orgID, &validFrom, gomock.Any()).Return(keyID, nil, nil) - test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true) + test.keyStore.EXPECT().Exists(ctx, keyID.String()).Return(true, nil) drawnUpContract, err := test.notary.DrawUpContract(ctx, template, orgID, validFrom, duration, &vc.VerifiableCredential{}) diff --git a/auth/services/oauth/authz_server.go b/auth/services/oauth/authz_server.go index 44da727db1..b1e1dd3c8a 100644 --- a/auth/services/oauth/authz_server.go +++ b/auth/services/oauth/authz_server.go @@ -407,7 +407,11 @@ func (s *authzServer) validateSubject(ctx context.Context, validationCtx *valida if err != nil { return err } - if !s.privateKeyStore.Exists(ctx, signingKeyID.String()) { + exists, err := s.privateKeyStore.Exists(ctx, signingKeyID.String()) + if err != nil { + return fmt.Errorf("could not check if JWT signing key exists: %w", err) + } + if !exists { return fmt.Errorf("subject.vendor: %s is not managed by this node", subject) } @@ -487,7 +491,11 @@ func (s *authzServer) parseAndValidateJwtBearerToken(context *validationContext) // IntrospectAccessToken fills the fields in NutsAccessToken from the given Jwt Access Token func (s *authzServer) IntrospectAccessToken(ctx context.Context, accessToken string) (*services.NutsAccessToken, error) { token, err := nutsCrypto.ParseJWT(accessToken, func(kid string) (crypto.PublicKey, error) { - if !s.privateKeyStore.Exists(ctx, kid) { + exists, err := s.privateKeyStore.Exists(ctx, kid) + if err != nil { + return nil, fmt.Errorf("could not check if JWT signing key exists: %w", err) + } + if !exists { return nil, fmt.Errorf("JWT signing key not present on this node (kid=%s)", kid) } return s.keyResolver.ResolveKeyByID(kid, nil, resolver.NutsSigningKeyType) diff --git a/auth/services/oauth/authz_server_test.go b/auth/services/oauth/authz_server_test.go index b2cb3899a5..cb93d9d9ac 100644 --- a/auth/services/oauth/authz_server_test.go +++ b/auth/services/oauth/authz_server_test.go @@ -127,7 +127,7 @@ func TestAuth_CreateAccessToken(t *testing.T) { ctx.contractNotary.EXPECT().VerifyVP(gomock.Any(), nil).Return(nil, errors.New("identity validation failed")) ctx.keyResolver.EXPECT().ResolveKeyByID(requesterSigningKeyID.String(), gomock.Any(), resolver.NutsSigningKeyType).MinTimes(1).Return(requesterSigningKey.Public(), nil) ctx.keyResolver.EXPECT().ResolveKey(authorizerDID, gomock.Any(), resolver.NutsSigningKeyType).MinTimes(1).Return(authorizerSigningKeyID, authorizerSigningKey, nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true, nil) tokenCtx := validContext(t) signToken(tokenCtx) @@ -158,7 +158,7 @@ func TestAuth_CreateAccessToken(t *testing.T) { ctx := createContext(t) ctx.nameResolver.EXPECT().Search(context.Background(), searchTerms, false, gomock.Any()).Return([]vc.VerifiableCredential{testCredential}, nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true, nil) ctx.keyResolver.EXPECT().ResolveKeyByID(requesterSigningKeyID.String(), gomock.Any(), resolver.NutsSigningKeyType).MinTimes(1).Return(requesterSigningKey.Public(), nil) ctx.keyResolver.EXPECT().ResolveKey(authorizerDID, gomock.Any(), resolver.NutsSigningKeyType).MinTimes(1).Return(authorizerSigningKeyID, authorizerSigningKey, nil) ctx.contractNotary.EXPECT().VerifyVP(gomock.Any(), nil).Return(services.TestVPVerificationResult{Val: contract.Invalid, FailureReason: "because of reasons"}, nil) @@ -180,7 +180,7 @@ func TestAuth_CreateAccessToken(t *testing.T) { ctx.nameResolver.EXPECT().Search(context.Background(), searchTerms, false, gomock.Any()).Return([]vc.VerifiableCredential{testCredential}, nil).AnyTimes() ctx.didResolver.EXPECT().Resolve(authorizerDID, gomock.Any()).Return(getAuthorizerDIDDocument(), nil, nil).AnyTimes() ctx.serviceResolver.EXPECT().GetCompoundServiceEndpoint(authorizerDID, expectedService, services.OAuthEndpointType, true).Return(expectedAudience, nil).AnyTimes() - ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true).AnyTimes() + ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true, nil).AnyTimes() ctx.verifier.EXPECT().Verify(gomock.Any(), true, true, gomock.Any()).Return(nil).AnyTimes() ctx.contractNotary.EXPECT().VerifyVP(gomock.Any(), nil).Return(services.TestVPVerificationResult{ Val: contract.Valid, @@ -226,7 +226,7 @@ func TestAuth_CreateAccessToken(t *testing.T) { testCtx.nameResolver.EXPECT().Search(context.Background(), searchTerms, false, gomock.Any()).Return([]vc.VerifiableCredential{testCredential}, nil) testCtx.didResolver.EXPECT().Resolve(authorizerDID, gomock.Any()).Return(getAuthorizerDIDDocument(), nil, nil).AnyTimes() testCtx.serviceResolver.EXPECT().GetCompoundServiceEndpoint(authorizerDID, expectedService, services.OAuthEndpointType, true).Return(expectedAudience, nil) - testCtx.keyStore.EXPECT().Exists(testCtx.audit, authorizerSigningKeyID.String()).Return(true) + testCtx.keyStore.EXPECT().Exists(testCtx.audit, authorizerSigningKeyID.String()).Return(true, nil) testCtx.keyStore.EXPECT().SignJWT(gomock.Any(), gomock.Any(), nil, authorizerSigningKeyID.String()).Return("expectedAccessToken", nil) testCtx.verifier.EXPECT().Verify(gomock.Any(), true, true, gomock.Any()).Return(nil) @@ -248,7 +248,7 @@ func TestAuth_CreateAccessToken(t *testing.T) { ctx.nameResolver.EXPECT().Search(context.Background(), searchTerms, false, gomock.Any()).Return([]vc.VerifiableCredential{testCredential}, nil) ctx.didResolver.EXPECT().Resolve(authorizerDID, gomock.Any()).Return(getAuthorizerDIDDocument(), nil, nil).AnyTimes() ctx.serviceResolver.EXPECT().GetCompoundServiceEndpoint(authorizerDID, expectedService, services.OAuthEndpointType, true).Return(expectedAudience, nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true, nil) ctx.keyStore.EXPECT().SignJWT(gomock.Any(), gomock.Any(), nil, authorizerSigningKeyID.String()).Return("expectedAT", nil) ctx.contractNotary.EXPECT().VerifyVP(gomock.Any(), nil).Return(services.TestVPVerificationResult{ Val: contract.Valid, @@ -361,7 +361,7 @@ func TestService_validateSubject(t *testing.T) { tokenCtx.jwtBearerToken.Set(jwt.SubjectKey, authorizerDID.String()) ctx.keyResolver.EXPECT().ResolveKey(authorizerDID, gomock.Any(), resolver.NutsSigningKeyType).MinTimes(1).Return(authorizerSigningKeyID, authorizerSigningKey, nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(true, nil) err := ctx.oauthService.validateSubject(ctx.audit, tokenCtx) assert.NoError(t, err) @@ -391,7 +391,7 @@ func TestService_validateSubject(t *testing.T) { tokenCtx.jwtBearerToken.Set(jwt.SubjectKey, authorizerDID.String()) ctx.keyResolver.EXPECT().ResolveKey(authorizerDID, gomock.Any(), resolver.NutsSigningKeyType).MinTimes(1).Return(authorizerSigningKeyID, authorizerSigningKey, nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(false) + ctx.keyStore.EXPECT().Exists(ctx.audit, authorizerSigningKeyID.String()).Return(false, nil) err := ctx.oauthService.validateSubject(ctx.audit, tokenCtx) @@ -685,7 +685,7 @@ func TestService_IntrospectAccessToken(t *testing.T) { ctx := createContext(t) ctx.keyResolver.EXPECT().ResolveKeyByID(requesterSigningKeyID.String(), nil, resolver.NutsSigningKeyType).MinTimes(1).Return(requesterSigningKey.Public(), nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(true, nil) // First build an access token tokenCtx := validAccessToken() @@ -706,7 +706,7 @@ func TestService_IntrospectAccessToken(t *testing.T) { ctx := createContext(t) ctx.keyResolver.EXPECT().ResolveKeyByID(requesterSigningKeyID.String(), nil, resolver.NutsSigningKeyType).MinTimes(1).Return(requesterSigningKey.Public(), nil) - ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(true, nil) // First build an access token tokenCtx := validAccessToken() @@ -723,7 +723,7 @@ func TestService_IntrospectAccessToken(t *testing.T) { t.Run("private key not present", func(t *testing.T) { ctx := createContext(t) - ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(false) + ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(false, nil) // First build an access token tokenCtx := validContext(t) @@ -737,7 +737,7 @@ func TestService_IntrospectAccessToken(t *testing.T) { t.Run("key not present on DID", func(t *testing.T) { ctx := createContext(t) - ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(true) + ctx.keyStore.EXPECT().Exists(ctx.audit, requesterSigningKeyID.String()).Return(true, nil) ctx.keyResolver.EXPECT().ResolveKeyByID(requesterSigningKeyID.String(), nil, resolver.NutsSigningKeyType).MinTimes(1).Return(nil, resolver.ErrNotFound) // First build an access token diff --git a/crypto/crypto.go b/crypto/crypto.go index b5c700373e..9d732c4194 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -159,7 +159,11 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (Key, e } audit.Log(ctx, log.Logger(), audit.CryptoNewKeyEvent).Infof("Generating new key pair: %s", kid) - if client.storage.PrivateKeyExists(ctx, kid) { + exists, err := client.storage.PrivateKeyExists(ctx, kid) + if err != nil { + return nil, err + } + if exists { return nil, errors.New("key with the given ID already exists") } if err = client.storage.SavePrivateKey(ctx, kid, keyPair); err != nil { @@ -212,8 +216,12 @@ func GenerateJWK() (jwk.Key, error) { } // Exists checks storage for an entry for the given legal entity and returns true if it exists -func (client *Crypto) Exists(ctx context.Context, kid string) bool { - return client.storage.PrivateKeyExists(ctx, kid) +func (client *Crypto) Exists(ctx context.Context, kid string) (bool, error) { + exists, err := client.storage.PrivateKeyExists(ctx, kid) + if err != nil { + return false, fmt.Errorf("could not check if private key exists: %w", err) + } + return exists, nil } func (client *Crypto) Resolve(ctx context.Context, kid string) (Key, error) { diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index c48fe585c8..1f0d70fe5a 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -49,15 +49,21 @@ func TestCrypto_Exists(t *testing.T) { require.NoError(t, err) t.Run("returns true for existing key", func(t *testing.T) { - assert.True(t, client.Exists(ctx, kid)) + exists, err := client.Exists(ctx, kid) + require.NoError(t, err) + assert.True(t, exists) }) t.Run("returns false for non-existing key", func(t *testing.T) { - assert.False(t, client.Exists(ctx, "unknown")) + exists, err := client.Exists(ctx, "unknown") + require.NoError(t, err) + assert.False(t, exists) }) t.Run("returns false for invalid kid", func(t *testing.T) { - assert.False(t, client.Exists(ctx, "../")) + exists, err := client.Exists(ctx, "../") + require.Error(t, err) + assert.False(t, exists) }) } @@ -98,7 +104,7 @@ func TestCrypto_New(t *testing.T) { t.Run("error - save public key returns an error", func(t *testing.T) { ctrl := gomock.NewController(t) storageMock := spi.NewMockStorage(ctrl) - storageMock.EXPECT().PrivateKeyExists(ctx, "123").Return(false) + storageMock.EXPECT().PrivateKeyExists(ctx, "123").Return(false, nil) storageMock.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo")) client := &Crypto{storage: storageMock} @@ -111,7 +117,7 @@ func TestCrypto_New(t *testing.T) { t.Run("error - ID already in use", func(t *testing.T) { ctrl := gomock.NewController(t) storageMock := spi.NewMockStorage(ctrl) - storageMock.EXPECT().PrivateKeyExists(ctx, "123").Return(true) + storageMock.EXPECT().PrivateKeyExists(ctx, "123").Return(true, nil) client := &Crypto{storage: storageMock} key, err := client.New(ctx, StringNamingFunc("123")) diff --git a/crypto/interface.go b/crypto/interface.go index 93434d7ffa..ccd49bb38e 100644 --- a/crypto/interface.go +++ b/crypto/interface.go @@ -42,7 +42,7 @@ type KeyCreator interface { type KeyResolver interface { // Exists returns if the specified private key exists. // If an error occurs, false is also returned - Exists(ctx context.Context, kid string) bool + Exists(ctx context.Context, kid string) (bool, error) // Resolve returns a Key for the given KID. ErrPrivateKeyNotFound is returned for an unknown KID. Resolve(ctx context.Context, kid string) (Key, error) // List returns the KIDs of the private keys that are present in the KeyStore. diff --git a/crypto/mock.go b/crypto/mock.go index 5ec55244d2..f39d2b9e89 100644 --- a/crypto/mock.go +++ b/crypto/mock.go @@ -80,11 +80,12 @@ func (m *MockKeyResolver) EXPECT() *MockKeyResolverMockRecorder { } // Exists mocks base method. -func (m *MockKeyResolver) Exists(ctx context.Context, kid string) bool { +func (m *MockKeyResolver) Exists(ctx context.Context, kid string) (bool, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Exists", ctx, kid) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // Exists indicates an expected call of Exists. @@ -206,11 +207,12 @@ func (mr *MockKeyStoreMockRecorder) EncryptJWE(ctx, payload, headers, publicKey } // Exists mocks base method. -func (m *MockKeyStore) Exists(ctx context.Context, kid string) bool { +func (m *MockKeyStore) Exists(ctx context.Context, kid string) (bool, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "Exists", ctx, kid) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // Exists indicates an expected call of Exists. diff --git a/crypto/storage/external/client.go b/crypto/storage/external/client.go index 65bb689066..e54498b414 100644 --- a/crypto/storage/external/client.go +++ b/crypto/storage/external/client.go @@ -108,12 +108,17 @@ func (c APIClient) GetPrivateKey(ctx context.Context, kid string) (crypto.Signer } } -func (c APIClient) PrivateKeyExists(ctx context.Context, kid string) bool { +func (c APIClient) PrivateKeyExists(ctx context.Context, kid string) (bool, error) { response, err := c.httpClient.LookupSecretWithResponse(ctx, url.PathEscape(kid)) if err != nil { - return false + return false, err } - return response.StatusCode() == http.StatusOK + if response.StatusCode() == http.StatusOK { + return true, nil + } else if response.StatusCode() == http.StatusNotFound { + return false, nil + } + return false, fmt.Errorf("unable to check if private key exists: server returned HTTP %d", response.StatusCode()) } func (c APIClient) SavePrivateKey(ctx context.Context, kid string, key crypto.PrivateKey) error { diff --git a/crypto/storage/external/client_test.go b/crypto/storage/external/client_test.go index ef746222aa..59f940d99b 100644 --- a/crypto/storage/external/client_test.go +++ b/crypto/storage/external/client_test.go @@ -385,35 +385,40 @@ func TestAPIClient_PrivateKeyExists(t *testing.T) { t.Run("ok - it should return true if the key exists", func(t *testing.T) { client, _ := NewAPIClient(Config{s.URL, time.Second}) - exists := client.PrivateKeyExists(ctx, "test") + exists, err := client.PrivateKeyExists(ctx, "test") + require.NoError(t, err) require.True(t, exists) }) t.Run("ok - key with a slash in it (should be encoded)", func(t *testing.T) { client, _ := NewAPIClient(Config{s.URL, time.Second}) - exists := client.PrivateKeyExists(ctx, "key/with/slashes") + exists, err := client.PrivateKeyExists(ctx, "key/with/slashes") + require.NoError(t, err) require.True(t, exists) }) t.Run("ok - it should return false if the key does not exist", func(t *testing.T) { client, _ := NewAPIClient(Config{s.URL, time.Second}) - exists := client.PrivateKeyExists(ctx, "unknown-key") + exists, err := client.PrivateKeyExists(ctx, "unknown-key") + require.NoError(t, err) require.False(t, exists) }) t.Run("error - it returns false if the server returns an error", func(t *testing.T) { client, _ := NewAPIClient(Config{s.URL, time.Second}) - exists := client.PrivateKeyExists(ctx, "server-error") + exists, err := client.PrivateKeyExists(ctx, "server-error") + require.Error(t, err) require.False(t, exists) }) t.Run("error - it returns false if the response has an invalid format", func(t *testing.T) { client, _ := NewAPIClient(Config{s.URL, time.Second}) - exists := client.PrivateKeyExists(ctx, "invalid-response") + exists, err := client.PrivateKeyExists(ctx, "invalid-response") + require.Error(t, err) require.False(t, exists) }) } diff --git a/crypto/storage/fs/fs.go b/crypto/storage/fs/fs.go index 68f706e778..984d8db6d5 100644 --- a/crypto/storage/fs/fs.go +++ b/crypto/storage/fs/fs.go @@ -92,9 +92,12 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) { return fsc, nil } -func (fsc fileSystemBackend) PrivateKeyExists(_ context.Context, kid string) bool { +func (fsc fileSystemBackend) PrivateKeyExists(_ context.Context, kid string) (bool, error) { _, err := os.Stat(fsc.getEntryPath(kid, privateKeyEntry)) - return err == nil + if errors.Is(err, os.ErrNotExist) { + return false, nil + } + return err == nil, err } // GetPrivateKey loads the private key for the given legalEntity from disk. Since a legalEntity has a URI as identifier, the URI is base64 encoded and postfixed with '_private.pem'. Keys are stored in pem format and are 2k RSA keys. diff --git a/crypto/storage/fs/fs_test.go b/crypto/storage/fs/fs_test.go index 23a28e32e6..a01106a586 100644 --- a/crypto/storage/fs/fs_test.go +++ b/crypto/storage/fs/fs_test.go @@ -157,14 +157,18 @@ func Test_fs_GetPrivateKey(t *testing.T) { func Test_fs_KeyExistsFor(t *testing.T) { t.Run("non-existing entry", func(t *testing.T) { storage, _ := NewFileSystemBackend(io.TestDirectory(t)) - assert.False(t, storage.PrivateKeyExists(nil, "unknown")) + exists, err := storage.PrivateKeyExists(nil, "unknown") + assert.NoError(t, err) + assert.False(t, exists) }) t.Run("existing entry", func(t *testing.T) { storage, _ := NewFileSystemBackend(io.TestDirectory(t)) pk := test.GenerateECKey() kid := "kid" storage.SavePrivateKey(nil, kid, pk) - assert.True(t, storage.PrivateKeyExists(nil, kid)) + exists, err := storage.PrivateKeyExists(nil, kid) + assert.NoError(t, err) + assert.True(t, exists) }) } diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index 7182cb0131..5afd51b608 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -45,7 +45,7 @@ type Storage interface { // GetPrivateKey from the storage backend and return its handler as an implementation of crypto.Signer. GetPrivateKey(ctx context.Context, kid string) (crypto.Signer, error) // PrivateKeyExists checks if the private key indicated with the kid is stored in the storage backend. - PrivateKeyExists(ctx context.Context, kid string) bool + PrivateKeyExists(ctx context.Context, kid string) (bool, error) // SavePrivateKey stores the key under the kid in the storage backend. SavePrivateKey(ctx context.Context, kid string, key crypto.PrivateKey) error // ListPrivateKeys returns the KIDs of the private keys that are present. Returns a []string(nil) if there was a problem. diff --git a/crypto/storage/spi/mock.go b/crypto/storage/spi/mock.go index 59f039e79e..a5095259b5 100644 --- a/crypto/storage/spi/mock.go +++ b/crypto/storage/spi/mock.go @@ -113,11 +113,12 @@ func (mr *MockStorageMockRecorder) Name() *gomock.Call { } // PrivateKeyExists mocks base method. -func (m *MockStorage) PrivateKeyExists(ctx context.Context, kid string) bool { +func (m *MockStorage) PrivateKeyExists(ctx context.Context, kid string) (bool, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "PrivateKeyExists", ctx, kid) ret0, _ := ret[0].(bool) - return ret0 + ret1, _ := ret[1].(error) + return ret0, ret1 } // PrivateKeyExists indicates an expected call of PrivateKeyExists. diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 40a0118cf9..50e55c506f 100644 --- a/crypto/storage/spi/wrapper.go +++ b/crypto/storage/spi/wrapper.go @@ -64,9 +64,9 @@ func (w wrapper) GetPrivateKey(ctx context.Context, kid string) (crypto.Signer, return w.wrappedBackend.GetPrivateKey(ctx, kid) } -func (w wrapper) PrivateKeyExists(ctx context.Context, kid string) bool { +func (w wrapper) PrivateKeyExists(ctx context.Context, kid string) (bool, error) { if err := w.validateKID(kid); err != nil { - return false + return false, err } return w.wrappedBackend.PrivateKeyExists(ctx, kid) } diff --git a/crypto/storage/spi/wrapper_test.go b/crypto/storage/spi/wrapper_test.go index 6efe67095d..406c835214 100644 --- a/crypto/storage/spi/wrapper_test.go +++ b/crypto/storage/spi/wrapper_test.go @@ -83,7 +83,8 @@ func TestWrapper_PrivateKeyExists(t *testing.T) { t.Run("expect error for bad KIDs", func(t *testing.T) { w := wrapper{kidPattern: KidPattern} for _, kid := range badKIDs { - exists := w.PrivateKeyExists(ctx, kid) + exists, err := w.PrivateKeyExists(ctx, kid) + assert.Error(t, err) assert.False(t, exists) } }) @@ -93,8 +94,9 @@ func TestWrapper_PrivateKeyExists(t *testing.T) { w := NewValidatedKIDBackendWrapper(mockStorage, KidPattern) for _, kid := range goodKIDs { - mockStorage.EXPECT().PrivateKeyExists(ctx, kid).Return(true) - exists := w.PrivateKeyExists(ctx, kid) + mockStorage.EXPECT().PrivateKeyExists(ctx, kid).Return(true, nil) + exists, err := w.PrivateKeyExists(ctx, kid) + assert.NoError(t, err) assert.True(t, exists) } ctrl.Finish() diff --git a/crypto/storage/vault/vault.go b/crypto/storage/vault/vault.go index 2a67c50dbc..ad242ce83e 100644 --- a/crypto/storage/vault/vault.go +++ b/crypto/storage/vault/vault.go @@ -21,6 +21,7 @@ package vault import ( "context" "crypto" + "errors" "fmt" vault "github.com/hashicorp/vault/api" "github.com/nuts-foundation/nuts-node/core" @@ -175,10 +176,13 @@ func (v vaultKVStorage) storeValue(ctx context.Context, path, key string, value return nil } -func (v vaultKVStorage) PrivateKeyExists(ctx context.Context, kid string) bool { +func (v vaultKVStorage) PrivateKeyExists(ctx context.Context, kid string) (bool, error) { path := privateKeyPath(v.config.PathPrefix, kid) _, err := v.getValue(ctx, path, keyName) - return err == nil + if errors.Is(err, spi.ErrNotFound) { + return false, nil + } + return err == nil, err } func (v vaultKVStorage) ListPrivateKeys(ctx context.Context) []string { diff --git a/crypto/storage/vault/vault_test.go b/crypto/storage/vault/vault_test.go index 816766b0b7..8f7f0b4ce1 100644 --- a/crypto/storage/vault/vault_test.go +++ b/crypto/storage/vault/vault_test.go @@ -90,9 +90,13 @@ func TestVaultKVStorage(t *testing.T) { t.Run("ok - store and retrieve private key", func(t *testing.T) { vaultStorage := vaultKVStorage{config: DefaultConfig(), client: mockVaultClient{store: map[string]map[string]interface{}{}}} - assert.False(t, vaultStorage.PrivateKeyExists(ctx, kid), "key should not be in vault") + exists, err := vaultStorage.PrivateKeyExists(ctx, kid) + assert.NoError(t, err, "checking should work") + assert.False(t, exists, "key should not be in vault") assert.NoError(t, vaultStorage.SavePrivateKey(ctx, kid, privateKey), "saving should work") - assert.True(t, vaultStorage.PrivateKeyExists(ctx, kid), "key should be in vault") + exists, err = vaultStorage.PrivateKeyExists(ctx, kid) + assert.NoError(t, err, "checking should work") + assert.True(t, exists, "key should be in vault") result, err := vaultStorage.GetPrivateKey(ctx, kid) assert.NoError(t, err, "getting key should work") assert.Equal(t, privateKey, result, "expected retrieved key to equal original") @@ -103,7 +107,9 @@ func TestVaultKVStorage(t *testing.T) { vaultStorage := vaultKVStorage{client: mockVaultClient{store: map[string]map[string]interface{}{"kv/nuts-private-keys/did:nuts:123#abc": {}}}} err := vaultStorage.DeletePrivateKey(ctx, kid) assert.NoError(t, err) - assert.False(t, vaultStorage.PrivateKeyExists(ctx, kid), "key should not be in vault") + exists, err := vaultStorage.PrivateKeyExists(ctx, kid) + assert.NoError(t, err) + assert.False(t, exists, "key should not be in vault") }) t.Run("key does not exist", func(t *testing.T) { vaultStorage := vaultKVStorage{client: mockVaultClient{store: map[string]map[string]interface{}{}}} @@ -131,10 +137,11 @@ func TestVaultKVStorage(t *testing.T) { assert.ErrorIs(t, err, vaultError) }) - t.Run("ok - keyExists return false in case of vault error", func(t *testing.T) { + t.Run("ok - vault error", func(t *testing.T) { vaultStorage := vaultKVStorage{client: mockVaultClient{err: vaultError}} - result := vaultStorage.PrivateKeyExists(ctx, kid) - assert.False(t, result, "expected PrivateKeyExists to return false") + result, err := vaultStorage.PrivateKeyExists(ctx, kid) + assert.EqualError(t, err, "unable to read key from vault: vault error") + assert.False(t, result) }) t.Run("error - key not found (empty response)", func(t *testing.T) { @@ -143,7 +150,8 @@ func TestVaultKVStorage(t *testing.T) { assert.Error(t, err, "expected error on unknown kid") assert.EqualError(t, err, "entry not found") - exists := vaultStorage.PrivateKeyExists(ctx, kid) + exists, err := vaultStorage.PrivateKeyExists(ctx, kid) + assert.NoError(t, err) assert.False(t, exists) }) @@ -156,7 +164,8 @@ func TestVaultKVStorage(t *testing.T) { assert.Error(t, err, "expected error on unknown kid") assert.EqualError(t, err, "entry not found") - exists := vaultStorage.PrivateKeyExists(ctx, kid) + exists, err := vaultStorage.PrivateKeyExists(ctx, kid) + assert.NoError(t, err) assert.False(t, exists) }) diff --git a/crypto/test.go b/crypto/test.go index 4c0d65332e..577984ecbf 100644 --- a/crypto/test.go +++ b/crypto/test.go @@ -81,9 +81,9 @@ func (m memoryStorage) GetPrivateKey(_ context.Context, kid string) (crypto.Sign return pk.(crypto.Signer), nil } -func (m memoryStorage) PrivateKeyExists(_ context.Context, kid string) bool { +func (m memoryStorage) PrivateKeyExists(_ context.Context, kid string) (bool, error) { _, ok := m[kid] - return ok + return ok, nil } func (m memoryStorage) DeletePrivateKey(_ context.Context, kid string) error { diff --git a/http/cmd/cmd.go b/http/cmd/cmd.go index 37667697da..caf0a42764 100644 --- a/http/cmd/cmd.go +++ b/http/cmd/cmd.go @@ -78,7 +78,11 @@ func createTokenCommand() *cobra.Command { return err } - if !instance.Exists(cmd.Context(), http.AdminTokenSigningKID) { + exists, err := instance.Exists(cmd.Context(), http.AdminTokenSigningKID) + if err != nil { + return err + } + if !exists { cmd.Println("Token signing key not found, generating new key...") _, err := instance.New(ctx, func(key crypto.PublicKey) (string, error) { return http.AdminTokenSigningKID, nil diff --git a/network/network.go b/network/network.go index eb6791672c..17e63898b1 100644 --- a/network/network.go +++ b/network/network.go @@ -464,7 +464,11 @@ func (n *Network) validateNodeDIDKeys(ctx context.Context, nodeDID did.DID) erro return fmt.Errorf("DID document does not contain a keyAgreement key, register a keyAgreement key (did=%s)", nodeDID) } for _, keyAgreement := range document.KeyAgreement { - if !n.keyStore.Exists(ctx, keyAgreement.ID.String()) { + exists, err := n.keyStore.Exists(ctx, keyAgreement.ID.String()) + if err != nil { + return fmt.Errorf("error checking keyAgreement key existence (did=%s,kid=%s): %w", nodeDID, keyAgreement.ID, err) + } + if !exists { return fmt.Errorf("keyAgreement private key is not present in key store, recover your key material or register a new keyAgreement key (did=%s,kid=%s)", nodeDID, keyAgreement.ID) } } diff --git a/vdr/didweb/manager_test.go b/vdr/didweb/manager_test.go index a009f75e8a..2fa41928c9 100644 --- a/vdr/didweb/manager_test.go +++ b/vdr/didweb/manager_test.go @@ -476,7 +476,9 @@ func TestManager_Deactivate(t *testing.T) { require.NoError(t, err) // Sanity check for assertion after deactivation, check that we can find the private key - require.True(t, cryptoInstance.Exists(ctx, document.VerificationMethod[0].ID.String())) + exists, err := cryptoInstance.Exists(ctx, document.VerificationMethod[0].ID.String()) + require.NoError(t, err) + require.True(t, exists) err = m.Deactivate(ctx, document.ID) require.NoError(t, err) @@ -485,7 +487,9 @@ func TestManager_Deactivate(t *testing.T) { require.ErrorIs(t, err, resolver.ErrNotFound) // Make sure it cleans up private keys - require.False(t, cryptoInstance.Exists(ctx, document.VerificationMethod[0].ID.String())) + exists, err = cryptoInstance.Exists(ctx, document.VerificationMethod[0].ID.String()) + require.NoError(t, err) + require.False(t, exists) }) t.Run("unable to delete private key", func(t *testing.T) { resetStore(t, storageEngine.GetSQLDatabase()) @@ -494,7 +498,9 @@ func TestManager_Deactivate(t *testing.T) { document, _, err := m.Create(ctx, DefaultCreationOptions()) require.NoError(t, err) - require.True(t, cryptoInstance.Exists(ctx, document.VerificationMethod[0].ID.String())) + exists, err := cryptoInstance.Exists(ctx, document.VerificationMethod[0].ID.String()) + require.NoError(t, err) + require.True(t, exists) require.NoError(t, cryptoInstance.Delete(ctx, document.VerificationMethod[0].ID.String())) err = m.Deactivate(ctx, document.ID)