diff --git a/crypto/crypto.go b/crypto/crypto.go index 9d732c4194..fedd80976a 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -21,9 +21,6 @@ package crypto import ( "context" "crypto" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" "errors" "fmt" "github.com/lestrrat-go/jwx/v2/jwk" @@ -153,24 +150,13 @@ func (client *Crypto) Configure(config core.ServerConfig) error { // 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) + 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) - 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 { - 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 } @@ -181,30 +167,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 1f0d70fe5a..8d83005be8 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -20,8 +20,6 @@ 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" @@ -81,7 +79,7 @@ func TestCrypto_New(t *testing.T) { 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) { @@ -92,38 +90,6 @@ func TestCrypto_New(t *testing.T) { 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, nil) - 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, nil) - - 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) { 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..616e7987df 100644 --- a/crypto/ephemeral.go +++ b/crypto/ephemeral.go @@ -19,9 +19,13 @@ package crypto +import ( + "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) + keyPair, kid, err := spi.GenerateKeyPairAndKID(namingFunc) if err != nil { return nil, err } diff --git a/crypto/storage/external/client.go b/crypto/storage/external/client.go index e54498b414..cf43a986db 100644 --- a/crypto/storage/external/client.go +++ b/crypto/storage/external/client.go @@ -42,6 +42,10 @@ type APIClient struct { httpClient *ClientWithResponses } +func (c APIClient) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (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 984d8db6d5..28afa78eab 100644 --- a/crypto/storage/fs/fs.go +++ b/crypto/storage/fs/fs.go @@ -92,6 +92,10 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) { return fsc, nil } +func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) { + return spi.GenerateAndStore(ctx, fsc, namingFunc) +} + func (fsc fileSystemBackend) PrivateKeyExists(_ context.Context, kid string) (bool, error) { _, err := os.Stat(fsc.getEntryPath(kid, privateKeyEntry)) if errors.Is(err, os.ErrNotExist) { diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index 5afd51b608..813543ca60 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -21,6 +21,9 @@ package spi import ( "context" "crypto" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" "encoding/json" "errors" "fmt" @@ -42,11 +45,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 func(crypto.PublicKey) (string, error)) (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, error) - // 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 +103,44 @@ func (pke *PublicKeyEntry) UnmarshalJSON(bytes []byte) error { func (pke PublicKeyEntry) JWK() jwk.Key { return pke.parsedJWK } + +// GenerateAndStore generates a new key pair and stores it in the provided storage. +func GenerateAndStore(ctx context.Context, store Storage, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) { + keyPair, kid, err := GenerateKeyPairAndKID(namingFunc) + if err != nil { + return nil, "", err + } + exists, err := store.PrivateKeyExists(ctx, kid) + if err != nil { + return nil, "", err + } + if exists { + 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 func(crypto.PublicKey) (string, error)) (*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..2b052ca6cb 100644 --- a/crypto/storage/spi/interface_test.go +++ b/crypto/storage/spi/interface_test.go @@ -17,11 +17,15 @@ package spi import ( + "context" + "crypto" "crypto/ecdsa" "crypto/elliptic" "crypto/rand" + "errors" "github.com/lestrrat-go/jwx/v2/jwk" "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" "testing" "github.com/stretchr/testify/assert" @@ -49,3 +53,70 @@ 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, nil) + store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(nil) + kid := "123" + + key, kid, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) { + return kid, nil + }) + + 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, nil) + store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo")) + kid := "123" + + _, _, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) { + return kid, nil + }) + + 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, nil) + kid := "123" + + _, _, err := GenerateAndStore(ctx, store, func(_ crypto.PublicKey) (string, error) { + return kid, nil + }) + + 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 a5095259b5..224863e6a2 100644 --- a/crypto/storage/spi/mock.go +++ b/crypto/storage/spi/mock.go @@ -112,6 +112,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 func(crypto.PublicKey) (string, error)) (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, error) { m.ctrl.T.Helper() diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 50e55c506f..ec08ee7809 100644 --- a/crypto/storage/spi/wrapper.go +++ b/crypto/storage/spi/wrapper.go @@ -88,3 +88,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 func(crypto.PublicKey) (string, error)) (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 ad242ce83e..a41a624d33 100644 --- a/crypto/storage/vault/vault.go +++ b/crypto/storage/vault/vault.go @@ -103,6 +103,10 @@ func NewVaultKVStorage(config Config) (spi.Storage, error) { return vaultStorage, nil } +func (v vaultKVStorage) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (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 577984ecbf..314069b8d3 100644 --- a/crypto/test.go +++ b/crypto/test.go @@ -55,8 +55,14 @@ func NewMemoryStorage() spi.Storage { return memoryStorage{} } +var _ spi.Storage = &memoryStorage{} + type memoryStorage map[string]crypto.PrivateKey +func (m memoryStorage) NewPrivateKey(ctx context.Context, namingFunc func(crypto.PublicKey) (string, error)) (crypto.PublicKey, string, error) { + return spi.GenerateAndStore(ctx, m, namingFunc) +} + func (m memoryStorage) Name() string { return "memory" }