From 92689d9b932f3799c707eb6f6f30b32f30712993 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 17 May 2024 13:25:32 +0200 Subject: [PATCH] Crypto: alter Storage interface to create keys inside key store --- crypto/crypto.go | 41 +++-------------- crypto/crypto_test.go | 45 +++---------------- crypto/decrypter_test.go | 3 +- crypto/dpop_test.go | 3 +- crypto/ecies_test.go | 5 ++- crypto/ephemeral.go | 9 +++- crypto/ephemeral_test.go | 5 ++- crypto/interface.go | 6 +-- crypto/jwx_test.go | 8 ++-- crypto/mock.go | 5 ++- crypto/storage/external/client.go | 5 +++ crypto/storage/fs/fs.go | 5 +++ crypto/storage/package.go | 5 +++ crypto/storage/spi/interface.go | 46 ++++++++++++++++++- crypto/storage/spi/interface_test.go | 66 ++++++++++++++++++++++++++++ crypto/storage/spi/mock.go | 17 +++++++ crypto/storage/spi/wrapper.go | 12 +++++ crypto/storage/vault/vault.go | 5 +++ crypto/test.go | 36 +++++++-------- crypto/test/keys.go | 15 +++++++ vcr/test/test.go | 3 +- vdr/didnuts/ambassador_test.go | 3 +- vdr/didnuts/creator.go | 3 +- vdr/didnuts/creator_test.go | 3 +- vdr/vdr_test.go | 7 +-- 25 files changed, 240 insertions(+), 121 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index b5c700373e..1643f0d85e 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -21,12 +21,10 @@ package crypto import ( "context" "crypto" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "errors" "fmt" "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/nuts-foundation/nuts-node/crypto/storage" "path" "time" @@ -152,21 +150,14 @@ func (client *Crypto) Configure(config core.ServerConfig) error { // New generates a new key pair. // Stores the private key, returns the public basicKey. // It returns an error when a key with the resulting ID already exists. -func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (Key, error) { - keyPair, kid, err := generateKeyPairAndKID(namingFunc) +func (client *Crypto) New(ctx context.Context, namingFunc storage.KIDNamingFunc) (Key, error) { + publicKey, kid, err := client.storage.NewPrivateKey(ctx, namingFunc) if err != nil { return nil, err } - - audit.Log(ctx, log.Logger(), audit.CryptoNewKeyEvent).Infof("Generating new key pair: %s", kid) - if client.storage.PrivateKeyExists(ctx, kid) { - return nil, errors.New("key with the given ID already exists") - } - if err = client.storage.SavePrivateKey(ctx, kid, keyPair); err != nil { - return nil, fmt.Errorf("could not create new keypair: could not save private key: %w", err) - } + audit.Log(ctx, log.Logger(), audit.CryptoNewKeyEvent).Infof("Generated new key pair: %s", kid) return basicKey{ - publicKey: keyPair.Public(), + publicKey: publicKey, kid: kid, }, nil } @@ -177,30 +168,10 @@ func (client *Crypto) Delete(ctx context.Context, kid string) error { return client.storage.DeletePrivateKey(ctx, kid) } -func generateKeyPairAndKID(namingFunc KIDNamingFunc) (*ecdsa.PrivateKey, string, error) { - keyPair, err := generateECKeyPair() - if err != nil { - return nil, "", err - } - - kid, err := namingFunc(keyPair.Public()) - if err != nil { - return nil, "", err - } - log.Logger(). - WithField(core.LogFieldKeyID, kid). - Debug("Generated new key pair") - return keyPair, kid, nil -} - -func generateECKeyPair() (*ecdsa.PrivateKey, error) { - return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) -} - // GenerateJWK a new in-memory key pair and returns it as JWK. // It sets the alg field of the JWK. func GenerateJWK() (jwk.Key, error) { - keyPair, err := generateECKeyPair() + keyPair, err := spi.GenerateKeyPair() if err != nil { return nil, nil } diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index c48fe585c8..badf6bb2a4 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -20,11 +20,10 @@ package crypto import ( "context" - "crypto" - "errors" "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/crypto/storage/fs" "github.com/nuts-foundation/nuts-node/crypto/storage/spi" + "github.com/nuts-foundation/nuts-node/crypto/test" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" "net/http" @@ -45,7 +44,7 @@ func TestCrypto_Exists(t *testing.T) { client := createCrypto(t) kid := "kid" - _, err := client.New(audit.TestContext(), StringNamingFunc(kid)) + _, err := client.New(audit.TestContext(), test.StringNamingFunc(kid)) require.NoError(t, err) t.Run("returns true for existing key", func(t *testing.T) { @@ -70,54 +69,22 @@ func TestCrypto_New(t *testing.T) { kid := "kid" auditLogs := audit.CaptureLogs(t) - key, err := client.New(ctx, StringNamingFunc(kid)) + key, err := client.New(ctx, test.StringNamingFunc(kid)) assert.NoError(t, err) assert.NotNil(t, key.Public()) assert.Equal(t, kid, key.KID()) - auditLogs.AssertContains(t, ModuleName, "CreateNewKey", audit.TestActor, "Generating new key pair: kid") + auditLogs.AssertContains(t, ModuleName, "CreateNewKey", audit.TestActor, "Generated new key pair: kid") }) t.Run("error - invalid KID", func(t *testing.T) { kid := "../certificate" - key, err := client.New(ctx, StringNamingFunc(kid)) + key, err := client.New(ctx, test.StringNamingFunc(kid)) assert.ErrorContains(t, err, "invalid key ID") assert.Nil(t, key) }) - - t.Run("error - NamingFunction returns err", func(t *testing.T) { - errorNamingFunc := func(key crypto.PublicKey) (string, error) { - return "", errors.New("b00m!") - } - _, err := client.New(ctx, errorNamingFunc) - assert.Error(t, err) - }) - - 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().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo")) - - client := &Crypto{storage: storageMock} - key, err := client.New(ctx, StringNamingFunc("123")) - assert.Nil(t, key) - assert.Error(t, err) - assert.Equal(t, "could not create new keypair: could not save private key: foo", err.Error()) - }) - - 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) - - client := &Crypto{storage: storageMock} - key, err := client.New(ctx, StringNamingFunc("123")) - assert.Nil(t, key) - assert.EqualError(t, err, "key with the given ID already exists", err) - }) } func TestCrypto_Delete(t *testing.T) { @@ -143,7 +110,7 @@ func TestCrypto_Resolve(t *testing.T) { ctx := context.Background() client := createCrypto(t) kid := "kid" - key, _ := client.New(audit.TestContext(), StringNamingFunc(kid)) + key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid)) t.Run("ok", func(t *testing.T) { resolvedKey, err := client.Resolve(ctx, "kid") diff --git a/crypto/decrypter_test.go b/crypto/decrypter_test.go index d07a18a241..24ba9303a9 100644 --- a/crypto/decrypter_test.go +++ b/crypto/decrypter_test.go @@ -22,6 +22,7 @@ import ( "context" "crypto/ecdsa" "github.com/nuts-foundation/nuts-node/audit" + "github.com/nuts-foundation/nuts-node/crypto/test" "github.com/stretchr/testify/assert" "testing" ) @@ -31,7 +32,7 @@ func TestCrypto_Decrypt(t *testing.T) { t.Run("ok", func(t *testing.T) { client := createCrypto(t) kid := "kid" - key, _ := client.New(audit.TestContext(), StringNamingFunc(kid)) + key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid)) pubKey := key.Public().(*ecdsa.PublicKey) cipherText, err := EciesEncrypt(pubKey, []byte("hello!")) diff --git a/crypto/dpop_test.go b/crypto/dpop_test.go index 113fb40e27..7dd2c81933 100644 --- a/crypto/dpop_test.go +++ b/crypto/dpop_test.go @@ -20,6 +20,7 @@ package crypto import ( "encoding/base64" + "github.com/nuts-foundation/nuts-node/crypto/test" "net/http" "testing" @@ -34,7 +35,7 @@ import ( func TestDPOP(t *testing.T) { client := createCrypto(t) - privateKey, _ := client.New(audit.TestContext(), StringNamingFunc("kid")) + privateKey, _ := client.New(audit.TestContext(), test.StringNamingFunc("kid")) keyAsJWK, _ := jwk.FromRaw(privateKey.Public()) _ = keyAsJWK.Set(jwk.AlgorithmKey, jwa.ES256) request, _ := http.NewRequest("POST", "https://server.example.com/token", nil) diff --git a/crypto/ecies_test.go b/crypto/ecies_test.go index 8a902a03a3..01e2ebec3a 100644 --- a/crypto/ecies_test.go +++ b/crypto/ecies_test.go @@ -19,13 +19,14 @@ package crypto import ( + "github.com/nuts-foundation/nuts-node/crypto/storage/spi" "testing" "github.com/stretchr/testify/assert" ) func TestEciesEncrypt(t *testing.T) { - key, err := generateECKeyPair() + key, err := spi.GenerateKeyPair() assert.NoError(t, err) cipherText1, err := EciesEncrypt(&key.PublicKey, []byte("hello world")) @@ -38,7 +39,7 @@ func TestEciesEncrypt(t *testing.T) { } func TestEciesDecrypt(t *testing.T) { - key, err := generateECKeyPair() + key, err := spi.GenerateKeyPair() assert.NoError(t, err) cipherText, err := EciesEncrypt(&key.PublicKey, []byte("hello world")) diff --git a/crypto/ephemeral.go b/crypto/ephemeral.go index 43262498f4..dbe498d957 100644 --- a/crypto/ephemeral.go +++ b/crypto/ephemeral.go @@ -19,9 +19,14 @@ package crypto +import ( + "github.com/nuts-foundation/nuts-node/crypto/storage" + "github.com/nuts-foundation/nuts-node/crypto/storage/spi" +) + // NewEphemeralKey returns a Key for single use. -func NewEphemeralKey(namingFunc KIDNamingFunc) (Key, error) { - keyPair, kid, err := generateKeyPairAndKID(namingFunc) +func NewEphemeralKey(namingFunc storage.KIDNamingFunc) (Key, error) { + keyPair, kid, err := spi.GenerateKeyPairAndKID(namingFunc) if err != nil { return nil, err } diff --git a/crypto/ephemeral_test.go b/crypto/ephemeral_test.go index b5c7e7d9ed..d4d000849c 100644 --- a/crypto/ephemeral_test.go +++ b/crypto/ephemeral_test.go @@ -21,6 +21,7 @@ package crypto import ( "errors" + "github.com/nuts-foundation/nuts-node/crypto/test" "github.com/stretchr/testify/require" "testing" @@ -29,7 +30,7 @@ import ( func TestNewEphemeralKey(t *testing.T) { t.Run("ok", func(t *testing.T) { - key, err := NewEphemeralKey(StringNamingFunc("kid")) + key, err := NewEphemeralKey(test.StringNamingFunc("kid")) require.NoError(t, err) @@ -39,7 +40,7 @@ func TestNewEphemeralKey(t *testing.T) { }) t.Run("error", func(t *testing.T) { - _, err := NewEphemeralKey(ErrorNamingFunc(errors.New("b00m!"))) + _, err := NewEphemeralKey(test.ErrorNamingFunc(errors.New("b00m!"))) assert.EqualError(t, err, "b00m!") }) diff --git a/crypto/interface.go b/crypto/interface.go index 93434d7ffa..361bff6d33 100644 --- a/crypto/interface.go +++ b/crypto/interface.go @@ -23,19 +23,17 @@ import ( "crypto" "errors" "github.com/nuts-foundation/nuts-node/crypto/dpop" + "github.com/nuts-foundation/nuts-node/crypto/storage" ) // ErrPrivateKeyNotFound is returned when the private key doesn't exist var ErrPrivateKeyNotFound = errors.New("private key not found") -// KIDNamingFunc is a function passed to New() which generates the kid for the pub/priv key -type KIDNamingFunc func(key crypto.PublicKey) (string, error) - // KeyCreator is the interface for creating key pairs. type KeyCreator interface { // New generates a keypair and returns a Key. The context is used to pass audit information. // The KIDNamingFunc will provide the kid. - New(ctx context.Context, namingFunc KIDNamingFunc) (Key, error) + New(ctx context.Context, namingFunc storage.KIDNamingFunc) (Key, error) } // KeyResolver is the interface for resolving keys. diff --git a/crypto/jwx_test.go b/crypto/jwx_test.go index 5586dea7cf..80a985dfa8 100644 --- a/crypto/jwx_test.go +++ b/crypto/jwx_test.go @@ -154,7 +154,7 @@ func TestCrypto_SignJWT(t *testing.T) { client := createCrypto(t) kid := "kid" - key, _ := client.New(audit.TestContext(), StringNamingFunc(kid)) + key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid)) t.Run("creates valid JWT", func(t *testing.T) { tokenString, err := client.SignJWT(audit.TestContext(), map[string]interface{}{"iss": "nuts", "sub": "subject"}, nil, key) @@ -198,7 +198,7 @@ func TestCrypto_SignJWS(t *testing.T) { client := createCrypto(t) kid := "kid" - key, _ := client.New(audit.TestContext(), StringNamingFunc(kid)) + key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid)) t.Run("creates valid JWS", func(t *testing.T) { payload, _ := json.Marshal(map[string]interface{}{"iss": "nuts"}) @@ -245,7 +245,7 @@ func TestCrypto_SignJWS(t *testing.T) { func TestCrypto_EncryptJWE(t *testing.T) { client := createCrypto(t) - key, _ := client.New(audit.TestContext(), StringNamingFunc("did:nuts:1234#key-1")) + key, _ := client.New(audit.TestContext(), test.StringNamingFunc("did:nuts:1234#key-1")) public := key.Public() headers := map[string]interface{}{"typ": "JWT", "kid": key.KID()} @@ -345,7 +345,7 @@ func TestCrypto_DecryptJWE(t *testing.T) { client := createCrypto(t) kid := "did:nuts:1234#key-1" - key, _ := client.New(audit.TestContext(), StringNamingFunc(kid)) + key, _ := client.New(audit.TestContext(), test.StringNamingFunc(kid)) t.Run("decrypts valid JWE", func(t *testing.T) { payload, _ := json.Marshal(map[string]interface{}{"iss": "nuts"}) diff --git a/crypto/mock.go b/crypto/mock.go index 5ec55244d2..cd76c146bc 100644 --- a/crypto/mock.go +++ b/crypto/mock.go @@ -15,6 +15,7 @@ import ( reflect "reflect" dpop "github.com/nuts-foundation/nuts-node/crypto/dpop" + storage "github.com/nuts-foundation/nuts-node/crypto/storage" gomock "go.uber.org/mock/gomock" ) @@ -42,7 +43,7 @@ func (m *MockKeyCreator) EXPECT() *MockKeyCreatorMockRecorder { } // New mocks base method. -func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (Key, error) { +func (m *MockKeyCreator) New(ctx context.Context, namingFunc storage.KIDNamingFunc) (Key, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "New", ctx, namingFunc) ret0, _ := ret[0].(Key) @@ -234,7 +235,7 @@ func (mr *MockKeyStoreMockRecorder) List(ctx any) *gomock.Call { } // New mocks base method. -func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (Key, error) { +func (m *MockKeyStore) New(ctx context.Context, namingFunc storage.KIDNamingFunc) (Key, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "New", ctx, namingFunc) ret0, _ := ret[0].(Key) diff --git a/crypto/storage/external/client.go b/crypto/storage/external/client.go index 65bb689066..449c665edd 100644 --- a/crypto/storage/external/client.go +++ b/crypto/storage/external/client.go @@ -23,6 +23,7 @@ import ( "crypto" "errors" "fmt" + "github.com/nuts-foundation/nuts-node/crypto/storage" "net/http" "net/url" "time" @@ -42,6 +43,10 @@ type APIClient struct { httpClient *ClientWithResponses } +func (c APIClient) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + return spi.GenerateAndStore(ctx, c, namingFunc) +} + func (c APIClient) Name() string { return "Crypto" } diff --git a/crypto/storage/fs/fs.go b/crypto/storage/fs/fs.go index 68f706e778..a506689d2d 100644 --- a/crypto/storage/fs/fs.go +++ b/crypto/storage/fs/fs.go @@ -24,6 +24,7 @@ import ( "errors" "fmt" "github.com/nuts-foundation/nuts-node/crypto/log" + "github.com/nuts-foundation/nuts-node/crypto/storage" "io/fs" "os" "path/filepath" @@ -92,6 +93,10 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) { return fsc, nil } +func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + return spi.GenerateAndStore(ctx, fsc, namingFunc) +} + func (fsc fileSystemBackend) PrivateKeyExists(_ context.Context, kid string) bool { _, err := os.Stat(fsc.getEntryPath(kid, privateKeyEntry)) return err == nil diff --git a/crypto/storage/package.go b/crypto/storage/package.go index f5ed13365c..eaa552fa28 100644 --- a/crypto/storage/package.go +++ b/crypto/storage/package.go @@ -23,3 +23,8 @@ // Will be removed in a future release, in favor of the `external` storage backend. // - `external` (External): a secret storage backend that stores secrets externally (e.g. Vault). package storage + +import "crypto" + +// KIDNamingFunc is a function passed to New() which generates the kid for the pub/priv key +type KIDNamingFunc func(key crypto.PublicKey) (string, error) diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index 7182cb0131..db35635175 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -21,9 +21,13 @@ package spi import ( "context" "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" "encoding/json" "errors" "fmt" + "github.com/nuts-foundation/nuts-node/crypto/storage" "regexp" "github.com/lestrrat-go/jwx/v2/jwk" @@ -42,11 +46,15 @@ var KidPattern = regexp.MustCompile(`^(?:(?:[\da-zA-Z_\- :#.])|(?:%[0-9a-fA-F]{2 // Storage interface containing functions for storing and retrieving keys. type Storage interface { core.HealthCheckable + // NewPrivateKey creates a new private key and returns its handler as an implementation of crypto.Signer. + // It should be preferred over generating a key in the application and saving it to the storage, + // as it allows for unexportable (safer) keys. If the resulting kid already exists, it returns an error + NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) // 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 - // SavePrivateKey stores the key under the kid in the storage backend. + // SavePrivateKey imports 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. ListPrivateKeys(ctx context.Context) []string @@ -96,3 +104,39 @@ func (pke *PublicKeyEntry) UnmarshalJSON(bytes []byte) error { func (pke PublicKeyEntry) JWK() jwk.Key { return pke.parsedJWK } + +func GenerateAndStore(ctx context.Context, store Storage, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + keyPair, kid, err := GenerateKeyPairAndKID(namingFunc) + if err != nil { + return nil, "", err + } + if store.PrivateKeyExists(ctx, kid) { + return nil, "", errors.New("key with the given ID already exists") + } + if err = store.SavePrivateKey(ctx, kid, keyPair); err != nil { + return nil, "", fmt.Errorf("could not create new keypair: could not save private key: %w", err) + } + return keyPair.Public(), kid, nil +} + +// GenerateKeyPair generates a new key pair using the default key type. +// It's intended to be used by crypto backends that don't create unexportable keys. +func GenerateKeyPair() (*ecdsa.PrivateKey, error) { + return ecdsa.GenerateKey(elliptic.P256(), rand.Reader) +} + +// GenerateKeyPairAndKID generates a new key pair and a KID using the provided naming function. +// It's intended to be used by crypto backends that don't create unexportable keys. +func GenerateKeyPairAndKID(namingFunc storage.KIDNamingFunc) (*ecdsa.PrivateKey, string, error) { + keyPair, err := GenerateKeyPair() + if err != nil { + return nil, "", err + } + + kid, err := namingFunc(keyPair.Public()) + if err != nil { + return nil, "", err + } + + return keyPair, kid, nil +} diff --git a/crypto/storage/spi/interface_test.go b/crypto/storage/spi/interface_test.go index 5d34ccf501..3e5cebf616 100644 --- a/crypto/storage/spi/interface_test.go +++ b/crypto/storage/spi/interface_test.go @@ -17,11 +17,16 @@ package spi import ( + "context" + "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "errors" "github.com/lestrrat-go/jwx/v2/jwk" + "github.com/nuts-foundation/nuts-node/crypto/test" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "testing" "github.com/stretchr/testify/assert" @@ -49,3 +54,64 @@ func TestPublicKeyEntry_FromJWK(t *testing.T) { assert.NotEmpty(t, entry.Key) assert.Same(t, pk, entry.parsedJWK) } + +func TestGenerateKeyPairAndKID(t *testing.T) { + t.Run("success", func(t *testing.T) { + keyPair, kid, err := GenerateKeyPairAndKID(func(_ crypto.PublicKey) (string, error) { + return "keyname", nil + }) + require.NoError(t, err) + assert.NotNil(t, keyPair) + assert.Equal(t, "keyname", kid) + }) +} + +func TestGenerateAndStore(t *testing.T) { + ctx := context.Background() + t.Run("ok", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockStorage(ctrl) + store.EXPECT().PrivateKeyExists(ctx, "123").Return(false) + store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(nil) + kid := "123" + + key, kid, err := GenerateAndStore(ctx, store, test.StringNamingFunc(kid)) + + assert.NoError(t, err) + assert.NotNil(t, key) + assert.Equal(t, "123", kid) + }) + t.Run("error - NamingFunction returns err", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockStorage(ctrl) + + _, _, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) { + return "", errors.New("foo") + }) + + assert.ErrorContains(t, err, "foo") + }) + + t.Run("error - save public key returns an error", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockStorage(ctrl) + store.EXPECT().PrivateKeyExists(ctx, "123").Return(false) + store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo")) + kid := "123" + + _, _, err := GenerateAndStore(ctx, store, test.StringNamingFunc(kid)) + + assert.ErrorContains(t, err, "could not create new keypair: could not save private key: foo") + }) + + t.Run("error - ID already in use", func(t *testing.T) { + ctrl := gomock.NewController(t) + store := NewMockStorage(ctrl) + store.EXPECT().PrivateKeyExists(ctx, "123").Return(true) + kid := "123" + + _, _, err := GenerateAndStore(ctx, store, test.StringNamingFunc(kid)) + + assert.ErrorContains(t, err, "key with the given ID already exists") + }) +} diff --git a/crypto/storage/spi/mock.go b/crypto/storage/spi/mock.go index 59f039e79e..f8209b08a5 100644 --- a/crypto/storage/spi/mock.go +++ b/crypto/storage/spi/mock.go @@ -15,6 +15,7 @@ import ( reflect "reflect" core "github.com/nuts-foundation/nuts-node/core" + storage "github.com/nuts-foundation/nuts-node/crypto/storage" gomock "go.uber.org/mock/gomock" ) @@ -112,6 +113,22 @@ func (mr *MockStorageMockRecorder) Name() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Name", reflect.TypeOf((*MockStorage)(nil).Name)) } +// NewPrivateKey mocks base method. +func (m *MockStorage) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NewPrivateKey", ctx, namingFunc) + ret0, _ := ret[0].(crypto.PublicKey) + ret1, _ := ret[1].(string) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// NewPrivateKey indicates an expected call of NewPrivateKey. +func (mr *MockStorageMockRecorder) NewPrivateKey(ctx, namingFunc any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NewPrivateKey", reflect.TypeOf((*MockStorage)(nil).NewPrivateKey), ctx, namingFunc) +} + // PrivateKeyExists mocks base method. func (m *MockStorage) PrivateKeyExists(ctx context.Context, kid string) bool { m.ctrl.T.Helper() diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 40a0118cf9..56e15eb0b4 100644 --- a/crypto/storage/spi/wrapper.go +++ b/crypto/storage/spi/wrapper.go @@ -23,6 +23,7 @@ import ( "crypto" "fmt" "github.com/nuts-foundation/nuts-node/core" + "github.com/nuts-foundation/nuts-node/crypto/storage" "regexp" ) @@ -88,3 +89,14 @@ func (w wrapper) DeletePrivateKey(ctx context.Context, kid string) error { func (w wrapper) ListPrivateKeys(ctx context.Context) []string { return w.wrappedBackend.ListPrivateKeys(ctx) } + +func (w wrapper) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + publicKey, kid, err := w.wrappedBackend.NewPrivateKey(ctx, namingFunc) + if err != nil { + return nil, "", err + } + if err := w.validateKID(kid); err != nil { + return nil, "", err + } + return publicKey, kid, err +} diff --git a/crypto/storage/vault/vault.go b/crypto/storage/vault/vault.go index 2a67c50dbc..08793c952d 100644 --- a/crypto/storage/vault/vault.go +++ b/crypto/storage/vault/vault.go @@ -25,6 +25,7 @@ import ( vault "github.com/hashicorp/vault/api" "github.com/nuts-foundation/nuts-node/core" "github.com/nuts-foundation/nuts-node/crypto/log" + "github.com/nuts-foundation/nuts-node/crypto/storage" "github.com/nuts-foundation/nuts-node/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/crypto/util" "path/filepath" @@ -102,6 +103,10 @@ func NewVaultKVStorage(config Config) (spi.Storage, error) { return vaultStorage, nil } +func (v vaultKVStorage) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + return spi.GenerateAndStore(ctx, v, namingFunc) +} + func configureVaultClient(cfg Config) (*vault.Client, error) { vaultConfig := vault.DefaultConfig() vaultConfig.Timeout = cfg.Timeout diff --git a/crypto/test.go b/crypto/test.go index 4c0d65332e..9da3f3d216 100644 --- a/crypto/test.go +++ b/crypto/test.go @@ -22,6 +22,7 @@ import ( "context" "crypto" "github.com/nuts-foundation/nuts-node/core" + "github.com/nuts-foundation/nuts-node/crypto/storage" "github.com/nuts-foundation/nuts-node/crypto/storage/spi" log "github.com/sirupsen/logrus" ) @@ -38,34 +39,27 @@ func NewTestCryptoInstance(storage spi.Storage) *Crypto { return newInstance } -// StringNamingFunc can be used to give a key a simple string name -func StringNamingFunc(name string) KIDNamingFunc { - return func(key crypto.PublicKey) (string, error) { - return name, nil - } +func NewMemoryStorage() MemoryStorage { + return MemoryStorage{} } -func ErrorNamingFunc(err error) KIDNamingFunc { - return func(key crypto.PublicKey) (string, error) { - return "", err - } -} +var _ spi.Storage = &MemoryStorage{} -func NewMemoryStorage() spi.Storage { - return memoryStorage{} -} +type MemoryStorage map[string]crypto.PrivateKey -type memoryStorage map[string]crypto.PrivateKey +func (m MemoryStorage) NewPrivateKey(ctx context.Context, namingFunc storage.KIDNamingFunc) (crypto.PublicKey, string, error) { + return spi.GenerateAndStore(ctx, m, namingFunc) +} -func (m memoryStorage) Name() string { +func (m MemoryStorage) Name() string { return "memory" } -func (m memoryStorage) CheckHealth() map[string]core.Health { +func (m MemoryStorage) CheckHealth() map[string]core.Health { return map[string]core.Health{"memory": {Status: core.HealthStatusUp}} } -func (m memoryStorage) ListPrivateKeys(_ context.Context) []string { +func (m MemoryStorage) ListPrivateKeys(_ context.Context) []string { var result []string for key := range m { result = append(result, key) @@ -73,7 +67,7 @@ func (m memoryStorage) ListPrivateKeys(_ context.Context) []string { return result } -func (m memoryStorage) GetPrivateKey(_ context.Context, kid string) (crypto.Signer, error) { +func (m MemoryStorage) GetPrivateKey(_ context.Context, kid string) (crypto.Signer, error) { pk, ok := m[kid] if !ok { return nil, ErrPrivateKeyNotFound @@ -81,12 +75,12 @@ 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 { _, ok := m[kid] return ok } -func (m memoryStorage) DeletePrivateKey(_ context.Context, kid string) error { +func (m MemoryStorage) DeletePrivateKey(_ context.Context, kid string) error { _, ok := m[kid] if !ok { return ErrPrivateKeyNotFound @@ -95,7 +89,7 @@ func (m memoryStorage) DeletePrivateKey(_ context.Context, kid string) error { return nil } -func (m memoryStorage) SavePrivateKey(_ context.Context, kid string, key crypto.PrivateKey) error { +func (m MemoryStorage) SavePrivateKey(_ context.Context, kid string, key crypto.PrivateKey) error { m[kid] = key return nil } diff --git a/crypto/test/keys.go b/crypto/test/keys.go index 39c729ffe2..606f3aba4b 100644 --- a/crypto/test/keys.go +++ b/crypto/test/keys.go @@ -19,10 +19,12 @@ package test import ( + "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" "crypto/rsa" + "github.com/nuts-foundation/nuts-node/crypto/storage" ) // GenerateRSAKey generates a 1024 bits RSA key @@ -39,3 +41,16 @@ func GenerateECKey() *ecdsa.PrivateKey { key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) return key } + +// StringNamingFunc can be used to give a key a simple string name +func StringNamingFunc(name string) storage.KIDNamingFunc { + return func(key crypto.PublicKey) (string, error) { + return name, nil + } +} + +func ErrorNamingFunc(err error) storage.KIDNamingFunc { + return func(key crypto.PublicKey) (string, error) { + return "", err + } +} diff --git a/vcr/test/test.go b/vcr/test/test.go index 9a5b23971e..1bc4cd52a7 100644 --- a/vcr/test/test.go +++ b/vcr/test/test.go @@ -28,6 +28,7 @@ import ( "github.com/nuts-foundation/go-did/vc" "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/crypto" + "github.com/nuts-foundation/nuts-node/crypto/test" "github.com/nuts-foundation/nuts-node/vcr/signature/proof" "github.com/stretchr/testify/require" "testing" @@ -56,7 +57,7 @@ func CreateJWTPresentation(t *testing.T, subjectDID did.DID, tokenVisitor func(t tokenVisitor(unsignedToken) } keyStore := crypto.NewMemoryCryptoInstance() - key, err := keyStore.New(audit.TestContext(), crypto.StringNamingFunc(subjectDID.String())) + key, err := keyStore.New(audit.TestContext(), test.StringNamingFunc(subjectDID.String())) require.NoError(t, err) claims, err = unsignedToken.AsMap(context.Background()) signedToken, err := keyStore.SignJWT(audit.TestContext(), claims, headers, key.KID()) diff --git a/vdr/didnuts/ambassador_test.go b/vdr/didnuts/ambassador_test.go index bb9bff2698..559209106a 100644 --- a/vdr/didnuts/ambassador_test.go +++ b/vdr/didnuts/ambassador_test.go @@ -28,6 +28,7 @@ import ( "errors" "fmt" "github.com/nuts-foundation/nuts-node/audit" + "github.com/nuts-foundation/nuts-node/crypto/storage" "github.com/nuts-foundation/nuts-node/network" "github.com/nuts-foundation/nuts-node/vdr/resolver" "testing" @@ -52,7 +53,7 @@ type mockKeyCreator struct { } // New creates a new valid key with the correct KID -func (m *mockKeyCreator) New(_ context.Context, fn crypto.KIDNamingFunc) (crypto.Key, error) { +func (m *mockKeyCreator) New(_ context.Context, fn storage.KIDNamingFunc) (crypto.Key, error) { if m.key == nil { privateKey, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) kid, _ := fn(privateKey.Public()) diff --git a/vdr/didnuts/creator.go b/vdr/didnuts/creator.go index 679a775ccf..21b41a3414 100644 --- a/vdr/didnuts/creator.go +++ b/vdr/didnuts/creator.go @@ -24,6 +24,7 @@ import ( "errors" "fmt" "github.com/nuts-foundation/nuts-node/crypto/hash" + "github.com/nuts-foundation/nuts-node/crypto/storage" "github.com/nuts-foundation/nuts-node/network" "github.com/nuts-foundation/nuts-node/vdr/management" "github.com/nuts-foundation/nuts-node/vdr/resolver" @@ -119,7 +120,7 @@ func didKIDNamingFunc(pKey crypto.PublicKey) (string, error) { // It wraps the KIDNamingFunc with the context of the DID of the document. // It returns a keyID in the form of the documents DID with the new keys thumbprint as fragment. // E.g. for a assertionMethod key that differs from the key the DID document was created with. -func didSubKIDNamingFunc(owningDID did.DID) nutsCrypto.KIDNamingFunc { +func didSubKIDNamingFunc(owningDID did.DID) storage.KIDNamingFunc { return func(pKey crypto.PublicKey) (string, error) { return getKIDName(pKey, func(_ jwk.Key) (string, error) { return owningDID.ID, nil diff --git a/vdr/didnuts/creator_test.go b/vdr/didnuts/creator_test.go index f273739382..9f9b28ef69 100644 --- a/vdr/didnuts/creator_test.go +++ b/vdr/didnuts/creator_test.go @@ -26,6 +26,7 @@ import ( "encoding/json" "errors" "github.com/nuts-foundation/nuts-node/crypto/hash" + "github.com/nuts-foundation/nuts-node/crypto/storage" "github.com/nuts-foundation/nuts-node/network" "github.com/nuts-foundation/nuts-node/vdr/management" "github.com/nuts-foundation/nuts-node/vdr/resolver" @@ -170,7 +171,7 @@ func TestCreator_Create(t *testing.T) { keyCreator := nutsCrypto.NewMockKeyCreator(ctrl) creator := Creator{KeyStore: keyCreator} - keyCreator.EXPECT().New(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, fn nutsCrypto.KIDNamingFunc) (nutsCrypto.Key, error) { + keyCreator.EXPECT().New(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, fn storage.KIDNamingFunc) (nutsCrypto.Key, error) { key, _ := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) keyName, _ := fn(key.Public()) return nutsCrypto.TestKey{ diff --git a/vdr/vdr_test.go b/vdr/vdr_test.go index acc16b68ff..7e799c5983 100644 --- a/vdr/vdr_test.go +++ b/vdr/vdr_test.go @@ -30,6 +30,7 @@ import ( ssi "github.com/nuts-foundation/go-did" "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/core" + cryptoTest "github.com/nuts-foundation/nuts-node/crypto/test" "github.com/nuts-foundation/nuts-node/storage" "github.com/nuts-foundation/nuts-node/vdr/didnuts" "github.com/nuts-foundation/nuts-node/vdr/didnuts/didstore" @@ -257,7 +258,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { client := crypto.NewMemoryCryptoInstance() keyID := did.DIDURL{DID: TestDIDA} keyID.Fragment = "1" - _, _ = client.New(audit.TestContext(), crypto.StringNamingFunc(keyID.String())) + _, _ = client.New(audit.TestContext(), cryptoTest.StringNamingFunc(keyID.String())) vdr := NewVDR(client, nil, didstore.NewTestStore(t), nil, storage.NewTestStorageEngine(t)) _ = vdr.Configure(core.TestServerConfig()) //vdr.didResolver.Register(didnuts.MethodName, didnuts.Resolver{Store: vdr.store}) @@ -303,8 +304,8 @@ func TestVDR_ConflictingDocuments(t *testing.T) { require.NoError(t, err) client := crypto.NewMemoryCryptoInstance() - _, _ = client.New(audit.TestContext(), crypto.StringNamingFunc(keyVendor.KID())) - _, _ = client.New(audit.TestContext(), crypto.StringNamingFunc(keyOrg.KID())) + _, _ = client.New(audit.TestContext(), cryptoTest.StringNamingFunc(keyVendor.KID())) + _, _ = client.New(audit.TestContext(), cryptoTest.StringNamingFunc(keyOrg.KID())) vdr := NewVDR(client, nil, didstore.NewTestStore(t), nil, storage.NewTestStorageEngine(t)) _ = vdr.Configure(*core.NewServerConfig()) vdr.didResolver.Register(didnuts.MethodName, didnuts.Resolver{Store: vdr.store})