Crypto: alter Storage interface to create keys inside key store

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
 	}

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")

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!"))

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)

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"))

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
 	}

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!")
 	})

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.

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"})

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"
 }

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

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)