From 692d7f875f15d6a57ffa8a56335e3625d36ff22c Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 10 Sep 2026 13:00:39 +0200 Subject: [PATCH 01/20] fix(vdr): reflect key store backend capability in DID document key usage did:nuts always tagged a newly generated key's VerificationMethod with DefaultKeyFlags(), which includes KeyAgreementUsage, regardless of whether the underlying key store backend can actually use that key for decryption. Azure Key Vault EC keys can only sign; Azure doesn't support decryption/ECDH with them. A DID document could therefore advertise KeyAgreement for a key nobody can actually decrypt with, breaking gRPC network private-transaction delivery for the sender resolving the recipient's KeyAgreement key. crypto.KeyCreator.New() (and the storage.spi.Storage.NewPrivateKey() backend call it wraps) now also returns the DIDKeyFlags the generated key can actually be used for. fs, vault and the external adapter all report every usage (they hand back plain, exportable EC keys); Azure Key Vault reports AssertionKeyUsage only. did:nuts and did:web now intersect the requested key usage with what the backend actually reports before persisting a VerificationMethod's key usage, so a verification relationship is only added to a DID document when the key backing it actually supports it. Assisted by AI --- crypto/crypto.go | 7 ++-- crypto/crypto_test.go | 8 ++-- crypto/interface.go | 4 +- crypto/mock.go | 14 ++++--- crypto/storage/azure/keyvault.go | 12 ++++-- crypto/storage/azure/keyvault_test.go | 12 ++++-- crypto/storage/external/client.go | 3 +- crypto/storage/fs/fs.go | 3 +- crypto/storage/spi/interface.go | 20 ++++++---- crypto/storage/spi/interface_test.go | 6 +-- crypto/storage/spi/mock.go | 8 ++-- crypto/storage/spi/wrapper.go | 9 +++-- crypto/storage/vault/vault.go | 3 +- crypto/test.go | 4 +- network/network_integration_test.go | 6 +-- network/network_test.go | 18 ++++----- storage/orm/keyflag.go | 6 +++ vcr/issuer/issuer_test.go | 12 +++--- vcr/issuer/openid_test.go | 2 +- vcr/signature/json_web_signature_test.go | 2 +- vcr/signature/proof/jsonld_test.go | 2 +- vcr/test/test.go | 2 +- vcr/verifier/signature_verifier_test.go | 2 +- vdr/didnuts/ambassador_test.go | 8 ++-- vdr/didnuts/manager.go | 32 +++++++++------ vdr/didnuts/manager_test.go | 50 +++++++++++++++++++++--- vdr/didsubject/interface.go | 8 ++-- vdr/didsubject/manager.go | 4 +- vdr/didsubject/manager_test.go | 4 +- vdr/didsubject/mock.go | 7 ++-- vdr/didweb/manager.go | 16 ++++---- vdr/vdr_test.go | 8 ++-- 32 files changed, 192 insertions(+), 110 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index cd9d4b4181..9586738d9f 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -224,14 +224,15 @@ func (client *Crypto) Migrate() error { // New generates a new key pair. // Stores the private key, returns the public key and DB reference. // It returns an error when a key with the resulting ID already exists. -func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { var ref *orm.KeyReference var publicKey crypto.PublicKey + var keyUsage orm.DIDKeyFlags err := client.continueTransaction(ctx, func(tx *gorm.DB) error { keyName := uuid.New().String() var err error var version string - publicKey, version, err = client.backend.NewPrivateKey(ctx, keyName) + publicKey, version, keyUsage, err = client.backend.NewPrivateKey(ctx, keyName) if err != nil { return err } @@ -248,7 +249,7 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.K audit.Log(ctx, log.Logger(), audit.CryptoNewKeyEvent).Infof("Generated new key pair: %s", kid) return tx.Save(ref).Error }) - return ref, publicKey, err + return ref, publicKey, keyUsage, err } // Delete removes the private key with the given KID from the KeyStore. diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index f4b113d3b1..d1172d735c 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -109,7 +109,7 @@ func TestCrypto_New(t *testing.T) { t.Run("ok", func(t *testing.T) { auditLogs := audit.CaptureAuditLogs(t) - ref, pubKey, err := client.New(ctx, StringNamingFunc("kid")) + ref, pubKey, _, err := client.New(ctx, StringNamingFunc("kid")) assert.NoError(t, err) assert.NotNil(t, ref) @@ -117,7 +117,7 @@ func TestCrypto_New(t *testing.T) { auditLogs.AssertContains(t, ModuleName, "CreateNewKey", audit.TestActor, "Generated new key pair: "+ref.KID) }) t.Run("error - invalid naming function", func(t *testing.T) { - _, _, err := client.New(ctx, ErrorNamingFunc(assert.AnError)) + _, _, _, err := client.New(ctx, ErrorNamingFunc(assert.AnError)) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) @@ -125,11 +125,11 @@ func TestCrypto_New(t *testing.T) { t.Run("error from backend", func(t *testing.T) { ctrl := gomock.NewController(t) storageMock := spi.NewMockStorage(ctrl) - storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", assert.AnError) + storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", orm.DIDKeyFlags(0), assert.AnError) client := createCrypto(t) client.backend = storageMock - _, _, err := client.New(ctx, StringNamingFunc("kid")) + _, _, _, err := client.New(ctx, StringNamingFunc("kid")) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) diff --git a/crypto/interface.go b/crypto/interface.go index e925ebb677..ebcbd27019 100644 --- a/crypto/interface.go +++ b/crypto/interface.go @@ -40,7 +40,9 @@ type KeyCreator interface { // New generates a keypair and returns a reference. The context is used to pass audit information. // It generates a key at the backend and stores its reference in the SQL DB. // A DB transaction may be passed through the context using `orm.TransactionKey`. - New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) + // It also returns the DIDKeyFlags the generated key can actually be used for, as reported by the + // storage backend, so callers don't add a verification method (e.g. KeyAgreement) the key can't back. + New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) } // KeyResolver is the interface for resolving keys. diff --git a/crypto/mock.go b/crypto/mock.go index 44046d64aa..a15937acd8 100644 --- a/crypto/mock.go +++ b/crypto/mock.go @@ -44,13 +44,14 @@ func (m *MockKeyCreator) EXPECT() *MockKeyCreatorMockRecorder { } // New mocks base method. -func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "New", ctx, namingFunc) ret0, _ := ret[0].(*orm.KeyReference) ret1, _ := ret[1].(crypto.PublicKey) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret2, _ := ret[2].(orm.DIDKeyFlags) + ret3, _ := ret[3].(error) + return ret0, ret1, ret2, ret3 } // New indicates an expected call of New. @@ -255,13 +256,14 @@ func (mr *MockKeyStoreMockRecorder) List(ctx any) *gomock.Call { } // New mocks base method. -func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "New", ctx, namingFunc) ret0, _ := ret[0].(*orm.KeyReference) ret1, _ := ret[1].(crypto.PublicKey) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret2, _ := ret[2].(orm.DIDKeyFlags) + ret3, _ := ret[3].(error) + return ret0, ret1, ret2, ret3 } // New indicates an expected call of New. diff --git a/crypto/storage/azure/keyvault.go b/crypto/storage/azure/keyvault.go index 0ce74e218b..430bd26e13 100644 --- a/crypto/storage/azure/keyvault.go +++ b/crypto/storage/azure/keyvault.go @@ -37,6 +37,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto/log" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" ) @@ -102,7 +103,10 @@ func (a Keyvault) CheckHealth() map[string]core.Health { return nil } -func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { +// NewPrivateKey creates a new EC key in Azure Key Vault. It only reports AssertionKeyUsage (not +// EncryptionKeyUsage/KeyAgreementUsage): Azure Key Vault EC keys can only be used for signing, they +// can't be used for decryption/ECDH, so such a key can't back a KeyAgreement verification method. +func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { var keyType azkeys.KeyType if a.useHSM { keyType = azkeys.KeyTypeECHSM @@ -119,13 +123,13 @@ func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.Pub }, }, nil) if err != nil { - return nil, "", fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) + return nil, "", 0, fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) } publicKey, _, version, err := parseKey(response.Key) if err != nil { - return nil, "", err + return nil, "", 0, err } - return publicKey, version, nil + return publicKey, version, orm.AssertionKeyUsage(), nil } func (a Keyvault) GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) { diff --git a/crypto/storage/azure/keyvault_test.go b/crypto/storage/azure/keyvault_test.go index 2dd583cb8b..c6b6056685 100644 --- a/crypto/storage/azure/keyvault_test.go +++ b/crypto/storage/azure/keyvault_test.go @@ -40,6 +40,7 @@ import ( "github.com/google/uuid" "github.com/lestrrat-go/jwx/v3/jwk" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -66,7 +67,7 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { }) store := Keyvault{client: vaultClient} - privateKey, version, err := store.NewPrivateKey(context.Background(), "did-web-example-com-0") + privateKey, version, keyUsage, err := store.NewPrivateKey(context.Background(), "did-web-example-com-0") require.NoError(t, err) assert.NotNil(t, privateKey) assert.Equal(t, "b86c2e6ad9054f4abf69cc185b99aa60", version) @@ -74,6 +75,9 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { assert.Equal(t, azkeys.CurveNameP256, *capturedParams.Curve) assert.True(t, *capturedParams.KeyAttributes.Enabled) assert.False(t, *capturedParams.KeyAttributes.Exportable) + // Azure Key Vault EC keys can sign, but not decrypt, so they can't back KeyAgreement. + assert.Equal(t, orm.AssertionKeyUsage(), keyUsage) + assert.False(t, keyUsage.Is(orm.KeyAgreementUsage)) }) } @@ -278,14 +282,14 @@ func TestIntegrationTest(t *testing.T) { var keyName = uuid.NewString() ctx := context.Background() - _, version, err := store.NewPrivateKey(ctx, keyName) + _, version, _, err := store.NewPrivateKey(ctx, keyName) if !errors.Is(err, spi.ErrKeyAlreadyExists) { assert.NoError(t, err) } t.Run("New", func(t *testing.T) { t.Run("already exists", func(t *testing.T) { - _, _, err := store.NewPrivateKey(ctx, keyName) + _, _, _, err := store.NewPrivateKey(ctx, keyName) assert.ErrorIs(t, err, spi.ErrKeyAlreadyExists) }) }) @@ -328,7 +332,7 @@ func TestIntegrationTest(t *testing.T) { t.Run("DeletePrivateKey", func(t *testing.T) { t.Run("ok", func(t *testing.T) { otherKeyName := uuid.NewString() - _, version, err := store.NewPrivateKey(ctx, otherKeyName) + _, version, _, err := store.NewPrivateKey(ctx, otherKeyName) assert.NoError(t, err) err = store.DeletePrivateKey(ctx, otherKeyName) diff --git a/crypto/storage/external/client.go b/crypto/storage/external/client.go index 27d23d4a2f..09a8873970 100644 --- a/crypto/storage/external/client.go +++ b/crypto/storage/external/client.go @@ -30,6 +30,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/crypto/util" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/tracing" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) @@ -44,7 +45,7 @@ type APIClient struct { httpClient *ClientWithResponses } -func (c APIClient) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { +func (c APIClient) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { return spi.GenerateAndStore(ctx, c, keyName) } diff --git a/crypto/storage/fs/fs.go b/crypto/storage/fs/fs.go index f73662bffb..8def7e716c 100644 --- a/crypto/storage/fs/fs.go +++ b/crypto/storage/fs/fs.go @@ -32,6 +32,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/crypto/util" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" ) type entryType string @@ -92,7 +93,7 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) { return fsc, nil } -func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { +func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { return spi.GenerateAndStore(ctx, fsc, keyName) } diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index a54230e85d..d25b14929c 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -31,6 +31,7 @@ import ( "github.com/lestrrat-go/jwx/v3/jwk" "github.com/nuts-foundation/nuts-node/v6/core" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" ) // ErrNotFound indicates that the specified crypto storage entry couldn't be found. @@ -48,7 +49,10 @@ type Storage interface { // NewPrivateKey creates a new private key. The backend will create the version and publicKey. // It should be preferred over generating a key in the application and saving it to the storage, // as it allows for unexportable (safer) keys. - NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) + // It also returns the DIDKeyFlags the generated key can actually be used for, e.g. an Azure Key Vault + // EC key can only be used for signing (not KeyAgreementUsage), since Azure Key Vault doesn't support + // decryption/ECDH with it. + NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) // GetPrivateKey from the storage backend and return its handler as an implementation of crypto.Signer. GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) // PrivateKeyExists checks if the private key indicated with the keyname/version is stored in the storage backend. @@ -116,22 +120,24 @@ func (pke PublicKeyEntry) JWK() jwk.Key { } // GenerateAndStore generates a new key pair and stores it in the provided storage. -func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, error) { +// It always generates a plain, exportable EC key, so it supports every DIDKeyFlags usage, including +// KeyAgreementUsage. +func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { keyPair, err := GenerateKeyPair() if err != nil { - return nil, "", err + return nil, "", 0, err } exists, err := store.PrivateKeyExists(ctx, keyName, "1") if err != nil { - return nil, "", fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) + return nil, "", 0, fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) } if exists { - return nil, "", errors.New("key with the given ID already exists") + return nil, "", 0, errors.New("key with the given ID already exists") } if err = store.SavePrivateKey(ctx, keyName, keyPair); err != nil { - return nil, "", fmt.Errorf("could not create new keypair: could not save private key: %w", err) + return nil, "", 0, fmt.Errorf("could not create new keypair: could not save private key: %w", err) } - return keyPair.Public(), "1", nil + return keyPair.Public(), "1", orm.AllKeyUsage(), nil } // GenerateKeyPair generates a new key pair using the default key type. diff --git a/crypto/storage/spi/interface_test.go b/crypto/storage/spi/interface_test.go index 8671d6ada7..5061467325 100644 --- a/crypto/storage/spi/interface_test.go +++ b/crypto/storage/spi/interface_test.go @@ -70,7 +70,7 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(nil) keyName := "123" - key, version, err := GenerateAndStore(ctx, store, keyName) + key, version, _, err := GenerateAndStore(ctx, store, keyName) assert.NoError(t, err) assert.NotNil(t, key) @@ -84,7 +84,7 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo")) keyName := "123" - _, _, err := GenerateAndStore(ctx, store, keyName) + _, _, _, err := GenerateAndStore(ctx, store, keyName) assert.ErrorContains(t, err, "could not create new keypair: could not save private key: foo") }) @@ -95,7 +95,7 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().PrivateKeyExists(ctx, "123", "1").Return(true, nil) keyName := "123" - _, _, err := GenerateAndStore(ctx, store, keyName) + _, _, _, err := GenerateAndStore(ctx, store, keyName) 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 15d4214c15..551042e6a4 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/v6/core" + orm "github.com/nuts-foundation/nuts-node/v6/storage/orm" gomock "go.uber.org/mock/gomock" ) @@ -114,13 +115,14 @@ func (mr *MockStorageMockRecorder) Name() *gomock.Call { } // NewPrivateKey mocks base method. -func (m *MockStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { +func (m *MockStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewPrivateKey", ctx, keyName) ret0, _ := ret[0].(crypto.PublicKey) ret1, _ := ret[1].(string) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret2, _ := ret[2].(orm.DIDKeyFlags) + ret3, _ := ret[3].(error) + return ret0, ret1, ret2, ret3 } // NewPrivateKey indicates an expected call of NewPrivateKey. diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 71a695e76e..48b15b16ac 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/v6/core" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "regexp" ) @@ -89,10 +90,10 @@ func (w wrapper) ListPrivateKeys(ctx context.Context) []KeyNameVersion { return w.wrappedBackend.ListPrivateKeys(ctx) } -func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { - publicKey, version, err := w.wrappedBackend.NewPrivateKey(ctx, keyName) +func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { + publicKey, version, keyUsage, err := w.wrappedBackend.NewPrivateKey(ctx, keyName) if err != nil { - return nil, "", err + return nil, "", 0, err } - return publicKey, version, err + return publicKey, version, keyUsage, err } diff --git a/crypto/storage/vault/vault.go b/crypto/storage/vault/vault.go index e78b057d66..32fc456fc1 100644 --- a/crypto/storage/vault/vault.go +++ b/crypto/storage/vault/vault.go @@ -32,6 +32,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/crypto/log" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/crypto/util" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/tracing" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) @@ -107,7 +108,7 @@ func NewVaultKVStorage(config Config) (spi.Storage, error) { return vaultStorage, nil } -func (v vaultKVStorage) NewPrivateKey(ctx context.Context, keyPath string) (crypto.PublicKey, string, error) { +func (v vaultKVStorage) NewPrivateKey(ctx context.Context, keyPath string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { return spi.GenerateAndStore(ctx, v, keyPath) } diff --git a/crypto/test.go b/crypto/test.go index d910fe0024..fdb6c5451a 100644 --- a/crypto/test.go +++ b/crypto/test.go @@ -68,7 +68,7 @@ var _ spi.Storage = &memoryStorage{} type memoryStorage map[string]crypto.PrivateKey -func (m memoryStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { +func (m memoryStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { return spi.GenerateAndStore(ctx, m, keyName) } @@ -143,7 +143,7 @@ func (t TestKey) Private() crypto.PrivateKey { // newKeyReference creates a new DID, DIDocument, VerificationMethod and KeyReference in the DB // It does not create valid DID Document data func newKeyReference(t *testing.T, client *Crypto, kid string) (*orm.KeyReference, crypto.PublicKey) { - ref, publicKey, err := client.New(audit.TestContext(), StringNamingFunc(kid)) + ref, publicKey, _, err := client.New(audit.TestContext(), StringNamingFunc(kid)) require.NoError(t, err) DID := orm.DID{ID: "did:test:" + t.Name(), Subject: "subject"} DIDDoc := orm.DidDocument{ diff --git a/network/network_integration_test.go b/network/network_integration_test.go index 01481f711d..cff7976876 100644 --- a/network/network_integration_test.go +++ b/network/network_integration_test.go @@ -201,7 +201,7 @@ func TestNetworkIntegration_Messages(t *testing.T) { }) // set root - _, key, _ := bootstrap.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("key1")) + _, key, _, _ := bootstrap.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("key1")) rootTx, err := bootstrap.network.CreateTransaction(audit.TestContext(), TransactionTemplate(payloadType, []byte("root_tx"), "key1").WithAttachKey(key)) require.NoError(t, err) require.NoError(t, node1.network.state.Add(context.Background(), rootTx, []byte("root_tx"))) @@ -976,7 +976,7 @@ func resetIntegrationTest(t *testing.T) { document := did.Document{ID: nodeDID} kid := did.DIDURL{DID: nodeDID} kid.Fragment = "key-1" - _, key, _ := keyStore.New(audit.TestContext(), func(_ crypto.PublicKey) (string, error) { + _, key, _, _ := keyStore.New(audit.TestContext(), func(_ crypto.PublicKey) (string, error) { return kid.String(), nil }) verificationMethod, _ := did.NewVerificationMethod(kid, ssi.JsonWebKey2020, nodeDID, key) @@ -1024,7 +1024,7 @@ func addBootstrapDIDDocument(t *testing.T, n node, subject string) hash.SHA256Ha } func addTransactionAndWaitForItToArrive(t *testing.T, payload string, sender node, receivers ...string) bool { - keyRef, key, _ := sender.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(uuid.New().String())) + keyRef, key, _, _ := sender.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(uuid.New().String())) expectedTransaction, err := sender.network.CreateTransaction(audit.TestContext(), TransactionTemplate(payloadType, []byte(payload), keyRef.KID).WithAttachKey(key)) if !assert.NoError(t, err) { return false diff --git a/network/network_test.go b/network/network_test.go index 7b13c68a8d..d8a4dfd5e0 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -412,7 +412,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, key, _, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) cxt.state.EXPECT().Head(gomock.Any()) cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) @@ -426,7 +426,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) cxt.state.EXPECT().Head(gomock.Any()) cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) @@ -452,7 +452,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) tx, err := cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithAdditionalPrevs([]hash.SHA256Hash{additionalPrev.Ref()})) @@ -467,7 +467,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) // 'Register' prev on DAG prev, _, _ := dag.CreateTestTransaction(1) @@ -491,7 +491,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) cxt.network.nodeDID = *nodeDID @@ -510,7 +510,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) + _, key, _, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) cxt.network.nodeDID = *nodeDID _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "did:nuts:sender#signing-key").WithAttachKey(key).WithPrivate([]did.DID{*sender, *receiver})) @@ -522,7 +522,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) cxt.network.nodeDID = *nodeDID _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithPrivate([]did.DID{*sender, *receiver})) @@ -534,7 +534,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithPrivate([]did.DID{*sender, *receiver})) assert.EqualError(t, err, "node DID must be configured to create private transactions") @@ -552,7 +552,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt.state.EXPECT().GetTransaction(gomock.Any(), additionalPrev.Ref()).Return(additionalPrev, nil) cxt.state.EXPECT().IsPayloadPresent(gomock.Any(), additionalPrev.PayloadHash()).Return(true, nil) cxt.state.EXPECT().Head(gomock.Any()).Return(rootTX.Ref(), nil) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) _, err := cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithAdditionalPrevs([]hash.SHA256Hash{additionalPrev.Ref()})) diff --git a/storage/orm/keyflag.go b/storage/orm/keyflag.go index b04f8dc433..dec86bf0d4 100644 --- a/storage/orm/keyflag.go +++ b/storage/orm/keyflag.go @@ -49,6 +49,12 @@ func EncryptionKeyUsage() DIDKeyFlags { return KeyAgreementUsage } +// AllKeyUsage returns every DIDKeyFlags bit. It's the usage reported by a key store backend whose +// keys support every verification relationship, e.g. because it hands back plain, exportable EC keys. +func AllKeyUsage() DIDKeyFlags { + return AssertionKeyUsage() | EncryptionKeyUsage() +} + // verificationMethodToKeyFlags creates DIDKeyFlags for a did.VerificationMethod based on its usage in the did.Document. func verificationMethodToKeyFlags(document did.Document, vm *did.VerificationMethod) DIDKeyFlags { var flags DIDKeyFlags diff --git a/vcr/issuer/issuer_test.go b/vcr/issuer/issuer_test.go index b9cd8cef16..27cdacbb79 100644 --- a/vcr/issuer/issuer_test.go +++ b/vcr/issuer/issuer_test.go @@ -82,7 +82,7 @@ func Test_issuer_buildAndSignVC(t *testing.T) { }}, } keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(kid)) + _, signingKey, _, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(kid)) require.NoError(t, err) t.Run("JSON-LD", func(t *testing.T) { @@ -289,7 +289,7 @@ func Test_issuer_Issue(t *testing.T) { ctx := audit.TestContext() jsonldManager := jsonld.NewTestJSONLDManager(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - _, issuerKey, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc(issuerKeyID)) + _, issuerKey, _, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc(issuerKeyID)) t.Run("ok - unpublished", func(t *testing.T) { ctrl := gomock.NewController(t) @@ -550,7 +550,7 @@ func Test_issuer_buildRevocation(t *testing.T) { t.Run("ok", func(t *testing.T) { ctrl := gomock.NewController(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) + kid, key, _, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) keyResolverMock := resolver.NewMockKeyResolver(ctrl) keyResolverMock.EXPECT().ResolveKey(issuerDID, nil, resolver.AssertionMethod).Return(kid.KID, key, nil) @@ -771,7 +771,7 @@ func Test_issuer_revokeNetwork(t *testing.T) { issuerURI := issuerDID.URI() jsonldManager := jsonld.NewTestJSONLDManager(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) + kid, key, _, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) ctx := audit.TestContext() t.Run("for a known credential", func(t *testing.T) { @@ -927,7 +927,7 @@ func TestIssuer_revokeStatusList(t *testing.T) { ctx := audit.TestContext() keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) + _, signingKey, _, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) require.NoError(t, err) t.Run("ok", func(t *testing.T) { @@ -1052,7 +1052,7 @@ func TestIssuer_StatusList(t *testing.T) { ctx := audit.TestContext() db := orm.NewTestDatabase(t) keyStore := nutsCrypto.NewDatabaseCryptoInstance(db) - _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) + _, signingKey, _, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) require.NoError(t, err) jsonldManager := jsonld.NewTestJSONLDManager(t) diff --git a/vcr/issuer/openid_test.go b/vcr/issuer/openid_test.go index 593ea395a6..3b12e32179 100644 --- a/vcr/issuer/openid_test.go +++ b/vcr/issuer/openid_test.go @@ -117,7 +117,7 @@ func Test_memoryIssuer_ProviderMetadata(t *testing.T) { func Test_memoryIssuer_HandleCredentialRequest(t *testing.T) { keyStore := crypto.NewMemoryCryptoInstance(t) ctx := audit.TestContext() - _, signerKey, _ := keyStore.New(ctx, crypto.StringNamingFunc(keyID)) + _, signerKey, _, _ := keyStore.New(ctx, crypto.StringNamingFunc(keyID)) ctrl := gomock.NewController(t) keyResolver := resolver.NewMockKeyResolver(ctrl) keyResolver.EXPECT().ResolveKeyByID(keyID, nil, resolver.NutsSigningKeyType).AnyTimes().Return(signerKey, nil) diff --git a/vcr/signature/json_web_signature_test.go b/vcr/signature/json_web_signature_test.go index f5c1f73df4..7355e16468 100644 --- a/vcr/signature/json_web_signature_test.go +++ b/vcr/signature/json_web_signature_test.go @@ -121,7 +121,7 @@ func TestJsonWebSignature2020_Sign(t *testing.T) { doc := []byte("foo") cryptoInstance := crypto.NewMemoryCryptoInstance(t) const keyID = "did:nuts:123#abc" - _, _, _ = cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(keyID)) + _, _, _, _ = cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(keyID)) sig := JSONWebSignature2020{Signer: cryptoInstance} result, err := sig.Sign(audit.TestContext(), doc, keyID) diff --git a/vcr/signature/proof/jsonld_test.go b/vcr/signature/proof/jsonld_test.go index 78f7f641b2..5cecec944c 100644 --- a/vcr/signature/proof/jsonld_test.go +++ b/vcr/signature/proof/jsonld_test.go @@ -170,7 +170,7 @@ func TestLDProof_Sign(t *testing.T) { contextLoader := jsonld.NewTestJSONLDManager(t).DocumentLoader() cryptoInstance := crypto.NewMemoryCryptoInstance(t) - _, key, _ := cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(kid)) + _, key, _, _ := cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(kid)) t.Run("sign and verify a document", func(t *testing.T) { now := time.Now() expires := now.Add(20 * time.Hour) diff --git a/vcr/test/test.go b/vcr/test/test.go index b7b52a41ef..b10f8660f2 100644 --- a/vcr/test/test.go +++ b/vcr/test/test.go @@ -58,7 +58,7 @@ func CreateJWTPresentation(t *testing.T, subjectDID did.DID, tokenVisitor func(t tokenVisitor(unsignedToken) } keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, key, err := keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(kid)) + _, key, _, err := keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(kid)) require.NoError(t, err) claims, err = jwx.ClaimsAsMap(unsignedToken) require.NoError(t, err) diff --git a/vcr/verifier/signature_verifier_test.go b/vcr/verifier/signature_verifier_test.go index 3527309638..c7ac7785a6 100644 --- a/vcr/verifier/signature_verifier_test.go +++ b/vcr/verifier/signature_verifier_test.go @@ -119,7 +119,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { t.Run("JWT", func(t *testing.T) { // Create did:jwk for issuer, and sign credential keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, err := keyStore.New(audit.TestContext(), func(key crypto.PublicKey) (string, error) { + kid, key, _, err := keyStore.New(audit.TestContext(), func(key crypto.PublicKey) (string, error) { keyAsJWK, _ := jwk.Import(key) keyJSON, _ := json.Marshal(keyAsJWK) return "did:jwk:" + base64.RawStdEncoding.EncodeToString(keyJSON) + "#0", nil diff --git a/vdr/didnuts/ambassador_test.go b/vdr/didnuts/ambassador_test.go index d6cbf7096e..80661db6a3 100644 --- a/vdr/didnuts/ambassador_test.go +++ b/vdr/didnuts/ambassador_test.go @@ -55,7 +55,7 @@ type mockKeyStore struct { } // New creates a new valid key with the correct KID -func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { if m.privateKey == nil { m.privateKey, _ = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) @@ -66,7 +66,7 @@ func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc) (*orm Version: uuid.NewString(), } } - return m.keyReference, m.privateKey.Public(), nil + return m.keyReference, m.privateKey.Public(), orm.AssertionKeyUsage() | orm.EncryptionKeyUsage(), nil } func (m *mockKeyStore) Link(_ context.Context, _ string, _ string, _ string) error { @@ -434,7 +434,7 @@ func TestAmbassador_handleUpdateDIDDocument(t *testing.T) { currentDoc, signingKey := newDidDoc(t) newDoc := did.Document{Context: []interface{}{did.DIDContextV1URI()}, ID: currentDoc.ID} - newCapInv, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}) + newCapInv, _, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}) newDoc.AddCapabilityInvocation(newCapInv) didDocPayload, _ := json.Marshal(newDoc) @@ -469,7 +469,7 @@ func TestAmbassador_handleUpdateDIDDocument(t *testing.T) { currentDoc, signingKey := newDidDoc(t) newDoc := did.Document{Context: []interface{}{did.DIDContextV1URI()}, ID: currentDoc.ID} - newCapInv, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}) + newCapInv, _, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}) newDoc.AddCapabilityInvocation(newCapInv) didDocPayload, _ := json.Marshal(newDoc) diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index d44caf7920..633adaa822 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -157,21 +157,22 @@ func (m Manager) RemoveVerificationMethod(ctx context.Context, id did.DID, keyID } // CreateNewVerificationMethodForDID creates a new VerificationMethod of type JsonWebKey2020 -// with a freshly generated key for a given DID. -func CreateNewVerificationMethodForDID(ctx context.Context, id did.DID, keyCreator nutsCrypto.KeyCreator) (*did.VerificationMethod, error) { - keyRef, publicKey, err := keyCreator.New(ctx, didSubKIDNamingFunc(id)) +// with a freshly generated key for a given DID. It also returns the DIDKeyFlags the key can actually +// be used for, as reported by the key store backend. +func CreateNewVerificationMethodForDID(ctx context.Context, id did.DID, keyCreator nutsCrypto.KeyCreator) (*did.VerificationMethod, orm.DIDKeyFlags, error) { + keyRef, publicKey, keyUsage, err := keyCreator.New(ctx, didSubKIDNamingFunc(id)) if err != nil { - return nil, err + return nil, 0, err } keyID, err := did.ParseDIDURL(keyRef.KID) if err != nil { - return nil, err + return nil, 0, err } method, err := did.NewVerificationMethod(*keyID, ssi.JsonWebKey2020, id, publicKey) if err != nil { - return nil, err + return nil, 0, err } - return method, nil + return method, keyUsage, nil } // Update updates a DID Document based on the DID. @@ -253,11 +254,13 @@ func (m Manager) Update(ctx context.Context, id did.DID, next did.Document) erro ******************************/ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDocument, error) { - keyRef, publicKey, err := m.keyStore.New(ctx, DIDKIDNamingFunc) + keyRef, publicKey, actualUsage, err := m.keyStore.New(ctx, DIDKIDNamingFunc) if err != nil { return nil, err } - keyFlags := DefaultKeyFlags() + // Only claim the verification relationships (e.g. KeyAgreement) the key store backend can actually + // back for this key; e.g. an Azure Key Vault EC key can't be used for KeyAgreement (decryption). + keyFlags := DefaultKeyFlags() & actualUsage keyID, err := did.ParseDIDURL(keyRef.KID) if err != nil { @@ -290,9 +293,14 @@ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDo return &sqlDoc, nil } -func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, _ orm.DIDKeyFlags) (*did.VerificationMethod, error) { - // did:nuts uses EC keys for everything, so it doesn't use the DIDKeyFlags - return CreateNewVerificationMethodForDID(ctx, id, m.keyStore) +func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, requestedFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { + // did:nuts uses EC keys for everything, so it doesn't select a key type based on the requested DIDKeyFlags. + method, actualUsage, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) + if err != nil { + return nil, 0, err + } + // Only grant what was requested AND what the key store backend can actually back. + return method, requestedFlags & actualUsage, nil } func (m Manager) Commit(ctx context.Context, change orm.DIDChangeLog) error { diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index 41575f899f..5cacf0d876 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -37,6 +37,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/audit" nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/crypto/hash" + "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/network" "github.com/nuts-foundation/nuts-node/v6/network/dag" "github.com/nuts-foundation/nuts-node/v6/storage" @@ -106,7 +107,7 @@ func TestManager_RemoveVerificationMethod(t *testing.T) { t.Run("ok", func(t *testing.T) { ctx := newTestContext(t) - _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) + _, pubKey, _, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) doc1 := createDoc(pubKey) doc2 := createDoc(pubKey) ctx.didResolver.EXPECT().Resolve(*id123, &resolver.ResolveMetadata{AllowDeactivated: true}).Return(&doc1, &resolver.DocumentMetadata{}, nil) @@ -135,7 +136,7 @@ func TestManager_RemoveVerificationMethod(t *testing.T) { t.Run("error - document is deactivated", func(t *testing.T) { ctx := newTestContext(t) - _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) + _, pubKey, _, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) doc1 := createDoc(pubKey) doc2 := createDoc(pubKey) ctx.didResolver.EXPECT().Resolve(*id123, &resolver.ResolveMetadata{AllowDeactivated: true}).Return(&doc1, &resolver.DocumentMetadata{Deactivated: true}, nil) @@ -155,7 +156,7 @@ func TestManager_CreateNewAuthenticationMethodForDID(t *testing.T) { t.Run("ok", func(t *testing.T) { // Prepare a document with an authenticationMethod: document := &did.Document{ID: *id123} - method, err := CreateNewVerificationMethodForDID(audit.TestContext(), document.ID, kc) + method, _, err := CreateNewVerificationMethodForDID(audit.TestContext(), document.ID, kc) require.NoError(t, err) document.AddCapabilityInvocation(method) @@ -196,7 +197,7 @@ func TestManager_GenerateDocument(t *testing.T) { t.Run("additional verification method", func(t *testing.T) { asDID := did.MustParseDID(doc.DID.ID) - verificationMethod, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) + verificationMethod, _, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) require.NoError(t, err) @@ -317,7 +318,7 @@ func TestManager_NewDocument(t *testing.T) { t.Run("additional verification method", func(t *testing.T) { asDID := did.MustParseDID(doc.DID.ID) - verificationMethod, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) + verificationMethod, _, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) require.NoError(t, err) @@ -328,6 +329,45 @@ func TestManager_NewDocument(t *testing.T) { assert.Equal(t, fmt.Sprintf("%s#%s", doc.DID.ID, asJWKKeyID), verificationMethod.ID.String()) }) }) + + t.Run("key store backend can't back KeyAgreement", func(t *testing.T) { + // Mimics an Azure Key Vault backend: it can generate EC keys, but they can't be used for + // decryption/ECDH, so a key it generates can't back a KeyAgreement verification method. + backend := signOnlyStorage{nutsCrypto.NewMemoryStorage()} + signOnlyKeyStore := nutsCrypto.NewTestCryptoInstance(db, backend) + signOnlyManager := NewManager(signOnlyKeyStore, nil, nil, nil, db) + + doc, err := signOnlyManager.NewDocument(ctx, orm.AssertionKeyUsage()) + + require.NoError(t, err) + require.Len(t, doc.VerificationMethods, 1) + assert.False(t, orm.DIDKeyFlags(doc.VerificationMethods[0].KeyTypes).Is(orm.KeyAgreementUsage), + "KeyAgreement must not be persisted when the key store backend can't back it") + + generatedDoc, err := doc.ToDIDDocument() + require.NoError(t, err) + assert.Empty(t, generatedDoc.KeyAgreement) + assert.NotEmpty(t, generatedDoc.CapabilityInvocation) + + asDID := did.MustParseDID(doc.DID.ID) + _, actualUsage, err := signOnlyManager.NewVerificationMethod(ctx, asDID, orm.EncryptionKeyUsage()) + + require.NoError(t, err) + assert.False(t, actualUsage.Is(orm.KeyAgreementUsage), + "requesting KeyAgreement for a new VerificationMethod must not be granted when the backend can't back it") + }) +} + +// signOnlyStorage wraps a spi.Storage but reports that its generated keys can only be used for +// signing, mimicking an Azure Key Vault EC key: it can sign, but Azure Key Vault doesn't support +// decryption/ECDH with it, so it can't back a KeyAgreement verification method. +type signOnlyStorage struct { + spi.Storage +} + +func (s signOnlyStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { + publicKey, version, _, err := s.Storage.NewPrivateKey(ctx, keyName) + return publicKey, version, orm.AssertionKeyUsage(), err } func TestManager_Commit(t *testing.T) { diff --git a/vdr/didsubject/interface.go b/vdr/didsubject/interface.go index 7a019e73d5..86c3eaef0c 100644 --- a/vdr/didsubject/interface.go +++ b/vdr/didsubject/interface.go @@ -52,9 +52,11 @@ type MethodManager interface { NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*orm.DidDocument, error) // NewVerificationMethod generates a new VerificationMethod for the given subject. // This is done by the method manager since the VM ID might depend on method specific rules. - // If keyUsage includes management.KeyAgreement, an RSA key is generated, otherwise an EC key. - // RSA keys are not yet fully supported, see https://github.com/nuts-foundation/nuts-node/issues/1948 - NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, error) + // It also returns the DIDKeyFlags the VerificationMethod can actually be used for, which may be a + // subset of the requested keyFlags: the underlying key store backend might not support every + // requested usage for the generated key (e.g. an Azure Key Vault EC key can sign but can't back + // KeyAgreement, since Azure Key Vault doesn't support decryption/ECDH with it). + NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) // Commit is called after changes are made to the primary db. // On success, the caller will remove/update the DID changelog. Commit(ctx context.Context, event orm.DIDChangeLog) error diff --git a/vdr/didsubject/manager.go b/vdr/didsubject/manager.go index 32774d8c6f..37fb9c5a26 100644 --- a/vdr/didsubject/manager.go +++ b/vdr/didsubject/manager.go @@ -402,7 +402,7 @@ func (r *SqlManager) AddVerificationMethod(ctx context.Context, subject string, } transactionContext := context.WithValue(ctx, storage.TransactionKey{}, tx) - vm, err := r.MethodManagers[id.Method].NewVerificationMethod(transactionContext, id, keyUsage) + vm, actualKeyUsage, err := r.MethodManagers[id.Method].NewVerificationMethod(transactionContext, id, keyUsage) if err != nil { return nil, err } @@ -410,7 +410,7 @@ func (r *SqlManager) AddVerificationMethod(ctx context.Context, subject string, data, _ := json.Marshal(*vm) sqlMethod := orm.VerificationMethod{ ID: vm.ID.String(), - KeyTypes: orm.VerificationMethodKeyType(keyUsage), + KeyTypes: orm.VerificationMethodKeyType(actualKeyUsage), Data: data, } current.VerificationMethods = append(current.VerificationMethods, sqlMethod) diff --git a/vdr/didsubject/manager_test.go b/vdr/didsubject/manager_test.go index cc14f0a6e7..3a27a0608b 100644 --- a/vdr/didsubject/manager_test.go +++ b/vdr/didsubject/manager_test.go @@ -473,10 +473,10 @@ func (t testMethod) NewDocument(_ context.Context, _ orm.DIDKeyFlags) (*orm.DidD return &orm.DidDocument{DID: orm.DID{ID: id}}, t.error } -func (t testMethod) NewVerificationMethod(_ context.Context, controller did.DID, _ orm.DIDKeyFlags) (*did.VerificationMethod, error) { +func (t testMethod) NewVerificationMethod(_ context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { return &did.VerificationMethod{ ID: did.MustParseDIDURL(fmt.Sprintf("%s#%s", controller.String(), uuid.New().String())), - }, t.error + }, keyFlags, t.error } func (t testMethod) Commit(_ context.Context, _ orm.DIDChangeLog) error { diff --git a/vdr/didsubject/mock.go b/vdr/didsubject/mock.go index f64ce1591d..4b68afcd47 100644 --- a/vdr/didsubject/mock.go +++ b/vdr/didsubject/mock.go @@ -88,12 +88,13 @@ func (mr *MockMethodManagerMockRecorder) NewDocument(ctx, keyFlags any) *gomock. } // NewVerificationMethod mocks base method. -func (m *MockMethodManager) NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, error) { +func (m *MockMethodManager) NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewVerificationMethod", ctx, controller, keyFlags) ret0, _ := ret[0].(*did.VerificationMethod) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret1, _ := ret[1].(orm.DIDKeyFlags) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // NewVerificationMethod indicates an expected call of NewVerificationMethod. diff --git a/vdr/didweb/manager.go b/vdr/didweb/manager.go index 4ba4eb28fe..24516047ea 100644 --- a/vdr/didweb/manager.go +++ b/vdr/didweb/manager.go @@ -62,14 +62,14 @@ func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*or keyTypes := []orm.DIDKeyFlags{orm.AssertionKeyUsage(), orm.EncryptionKeyUsage()} for _, keyType := range keyTypes { if keyType.Is(keyFlags) { - verificationMethod, err := m.NewVerificationMethod(ctx, *newDID, keyType) + verificationMethod, actualUsage, err := m.NewVerificationMethod(ctx, *newDID, keyType) if err != nil { return nil, err } asJson, _ := json.Marshal(verificationMethod) sqlVerificationMethods = append(sqlVerificationMethods, orm.VerificationMethod{ ID: verificationMethod.ID.String(), - KeyTypes: orm.VerificationMethodKeyType(keyType), + KeyTypes: orm.VerificationMethodKeyType(actualUsage), Data: asJson, }) } @@ -90,7 +90,7 @@ func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*or return &sqlDoc, nil } -func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, keyUsage orm.DIDKeyFlags) (*did.VerificationMethod, error) { +func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, keyUsage orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { verificationMethodID := did.DIDURL{ DID: controller, Fragment: uuid.New().String(), @@ -98,25 +98,25 @@ func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, var publicKey crypto.PublicKey var err error if keyUsage.Is(orm.KeyAgreementUsage) { - return nil, errors.New("key agreement not supported for did:web") + return nil, 0, errors.New("key agreement not supported for did:web") // todo requires update to nutsCrypto module //verificationMethodKey, err = m.keyStore.NewRSA(ctx, func(key crypt.PublicKey) (string, error) { // return verificationMethodID.String(), nil //}) } else { - _, publicKey, err = m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { + _, publicKey, _, err = m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { return verificationMethodID.String(), nil }) } if err != nil { - return nil, err + return nil, 0, err } verificationMethod, err := did.NewVerificationMethod(verificationMethodID, ssi.JsonWebKey2020, controller, publicKey) if err != nil { - return nil, err + return nil, 0, err } - return verificationMethod, nil + return verificationMethod, keyUsage, nil } // Commit does nothing for did:web. This is important since only the one of the method managers may have a failing commit. diff --git a/vdr/vdr_test.go b/vdr/vdr_test.go index 89c7153322..47f5cddcae 100644 --- a/vdr/vdr_test.go +++ b/vdr/vdr_test.go @@ -140,7 +140,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { client := nutsCrypto.NewDatabaseCryptoInstance(db) keyID := did.DIDURL{DID: TestDIDA} keyID.Fragment = "1" - _, _, _ = client.New(audit.TestContext(), nutsCrypto.StringNamingFunc(keyID.String())) + _, _, _, _ = client.New(audit.TestContext(), nutsCrypto.StringNamingFunc(keyID.String())) ctrl := gomock.NewController(t) pkiMock := pki.NewMockValidator(ctrl) vdr := NewVDR(client, nil, didstore.NewTestStore(t), nil, storageEngine, pkiMock) @@ -160,7 +160,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { t.Run("ok - 1 owned conflict in controlled document", func(t *testing.T) { // vendor test := newVDRTestCtx(t) - _, keyVendor, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:vendor#keyVendor-1")) + _, keyVendor, _, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:vendor#keyVendor-1")) didDocVendor := &did.Document{ID: did.MustParseDID("did:nuts:vendor")} vendorVM, err := did.NewVerificationMethod(did.MustParseDIDURL("did:nuts:vendor#keyVendor-1"), ssi.JsonWebKey2020, didDocVendor.ID, keyVendor) @@ -168,7 +168,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { didDocVendor.AddCapabilityInvocation(vendorVM) // organization - _, keyOrg, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:org#keyOrg-1")) + _, keyOrg, _, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:org#keyOrg-1")) didDocOrg := &did.Document{ID: did.MustParseDID("did:nuts:org")} didDocOrg.Controller = []did.DID{didDocVendor.ID} orgVM, err := did.NewVerificationMethod(did.MustParseDIDURL("did:nuts:org#keyOrg-1"), ssi.JsonWebKey2020, didDocOrg.ID, keyOrg) @@ -343,7 +343,7 @@ func TestVDR_Migrate(t *testing.T) { t.Run("makes documents self-controlled", func(t *testing.T) { ctx := controllerMigrationSetup(t) keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - keyRef, publicKey, err := keyStore.New(ctx.ctx, didnuts.DIDKIDNamingFunc) + keyRef, publicKey, _, err := keyStore.New(ctx.ctx, didnuts.DIDKIDNamingFunc) require.NoError(t, err) methodID := did.MustParseDIDURL(keyRef.KID) methodID.ID = TestDIDA.ID From 30cff920a2c310670576100ebd17b0f082130b8d Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 10 Sep 2026 14:19:36 +0200 Subject: [PATCH 02/20] refactor(crypto): keep DID key-usage semantics out of the storage backend layer The first version of this fix had crypto/storage/spi.Storage.NewPrivateKey() (and its fs/vault/azure/external implementations) return orm.DIDKeyFlags directly, so a raw key-storage backend had to speak DID Core vocabulary (AssertionMethod, KeyAgreement) that has nothing to do with storing a key. Storage.NewPrivateKey() now reports a crypto-native spi.KeyCapability (SigningOnly or SigningAndDecryption) instead, with no storage/orm dependency. crypto.Crypto.New() is the translation boundary: it already persists orm.KeyReference, so it now also translates the reported capability into orm.DIDKeyFlags and persists it as KeyReference.KeyUsage (migration 012), rather than returning it as a separate value that would just duplicate what's already on the returned KeyReference. did:nuts and did:web read requestedFlags & keyRef.KeyUsage to decide what a VerificationMethod actually gets to claim. crypto.Migrate() corrects existing KeyReferences to sign-only for nodes configured with the Azure Key Vault backend: switching crypto storage backends for an existing node isn't supported (KeyName/Version are backend-specific and become unreachable), so every managed key under an Azure-configured node is known to have been created by Azure Key Vault. Assisted by AI --- crypto/crypto.go | 52 +++++++++++++++---- crypto/crypto_test.go | 28 ++++++++-- crypto/interface.go | 7 +-- crypto/mock.go | 14 +++-- crypto/storage/azure/keyvault.go | 14 +++-- crypto/storage/azure/keyvault_test.go | 6 +-- crypto/storage/external/client.go | 3 +- crypto/storage/fs/fs.go | 3 +- crypto/storage/spi/interface.go | 34 +++++++----- crypto/storage/spi/interface_test.go | 3 +- crypto/storage/spi/mock.go | 5 +- crypto/storage/spi/wrapper.go | 9 ++-- crypto/storage/vault/vault.go | 3 +- crypto/test.go | 4 +- network/network_integration_test.go | 6 +-- network/network_test.go | 18 +++---- storage/orm/key_reference.go | 4 ++ storage/orm/keyflag.go | 6 --- .../012_key_reference_key_usage.sql | 18 +++++++ vcr/issuer/issuer_test.go | 12 ++--- vcr/issuer/openid_test.go | 2 +- vcr/signature/json_web_signature_test.go | 2 +- vcr/signature/proof/jsonld_test.go | 2 +- vcr/test/test.go | 2 +- vcr/verifier/signature_verifier_test.go | 2 +- vdr/didnuts/ambassador_test.go | 11 ++-- vdr/didnuts/manager.go | 8 +-- vdr/didnuts/manager_test.go | 8 +-- vdr/didweb/manager.go | 2 +- vdr/vdr_test.go | 8 +-- 30 files changed, 182 insertions(+), 114 deletions(-) create mode 100644 storage/sql_migrations/012_key_reference_key_usage.sql diff --git a/crypto/crypto.go b/crypto/crypto.go index 9586738d9f..18d4763090 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -190,6 +190,13 @@ func (client *Crypto) Migrate() error { // else do nothing outerContext := context.TODO() + // Azure Key Vault EC keys can't be used for decryption; every other backend hands back plain, + // exportable EC keys that support both signing and decryption. + capability := spi.SigningAndDecryption + if client.config.Storage == azure.StorageType { + capability = spi.SigningOnly + } + keyUsage := orm.VerificationMethodKeyType(keyUsageForCapability(capability)) // run everything in a single transaction // we do not expect to have a lot of keys, so this should be fine @@ -204,9 +211,10 @@ func (client *Crypto) Migrate() error { if errors.Is(err, gorm.ErrRecordNotFound) { // create a new key reference ref := &orm.KeyReference{ - KID: keyNameVersion.KeyName, - KeyName: keyNameVersion.KeyName, - Version: keyNameVersion.Version, + KID: keyNameVersion.KeyName, + KeyName: keyNameVersion.KeyName, + Version: keyNameVersion.Version, + KeyUsage: keyUsage, } err := tx.Save(ref).Error if err != nil { @@ -217,6 +225,18 @@ func (client *Crypto) Migrate() error { } } } + if capability == spi.SigningOnly { + // Correct KeyReferences created before this backend reported per-key usage (the SQL + // migration defaults key_usage to "everything"): on a node configured with the Azure Key + // Vault backend, every managed key was created by Azure Key Vault and can't decrypt. + // Switching crypto storage backends for an existing node isn't supported (KeyName/Version + // are backend-specific and become unreachable), so this is safe to assume unconditionally. + allUsage := orm.VerificationMethodKeyType(keyUsageForCapability(spi.SigningAndDecryption)) + err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", allUsage).Update("key_usage", keyUsage).Error + if err != nil { + return fmt.Errorf("could not correct existing KeyReferences for the Azure Key Vault backend: %w", err) + } + } return nil }) } @@ -224,15 +244,15 @@ func (client *Crypto) Migrate() error { // New generates a new key pair. // Stores the private key, returns the public key and DB reference. // It returns an error when a key with the resulting ID already exists. -func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { +func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { var ref *orm.KeyReference var publicKey crypto.PublicKey - var keyUsage orm.DIDKeyFlags err := client.continueTransaction(ctx, func(tx *gorm.DB) error { keyName := uuid.New().String() var err error var version string - publicKey, version, keyUsage, err = client.backend.NewPrivateKey(ctx, keyName) + var capability spi.KeyCapability + publicKey, version, capability, err = client.backend.NewPrivateKey(ctx, keyName) if err != nil { return err } @@ -242,14 +262,26 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.K return err } ref = &orm.KeyReference{ - KID: kid, - KeyName: keyName, - Version: version, + KID: kid, + KeyName: keyName, + Version: version, + KeyUsage: orm.VerificationMethodKeyType(keyUsageForCapability(capability)), } audit.Log(ctx, log.Logger(), audit.CryptoNewKeyEvent).Infof("Generated new key pair: %s", kid) return tx.Save(ref).Error }) - return ref, publicKey, keyUsage, err + return ref, publicKey, err +} + +// keyUsageForCapability derives the DIDKeyFlags a key with the given KeyCapability can back. +// Every key can be used for signing (AssertionKeyUsage); only a key that also supports +// decryption/ECDH (SigningAndDecryption) can additionally back KeyAgreement (EncryptionKeyUsage). +func keyUsageForCapability(capability spi.KeyCapability) orm.DIDKeyFlags { + keyUsage := orm.AssertionKeyUsage() + if capability == spi.SigningAndDecryption { + keyUsage |= orm.EncryptionKeyUsage() + } + return keyUsage } // Delete removes the private key with the given KID from the KeyStore. diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index d1172d735c..c0cdc56bb8 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -21,6 +21,7 @@ package crypto import ( "context" "github.com/nuts-foundation/nuts-node/v6/audit" + "github.com/nuts-foundation/nuts-node/v6/crypto/storage/azure" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/fs" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/storage" @@ -99,6 +100,24 @@ func TestCrypto_Migrate(t *testing.T) { keys := client.List(context.Background()) require.Len(t, keys, 1) }) + t.Run("corrects existing KeyReferences for the Azure Key Vault backend", func(t *testing.T) { + backend := NewMemoryStorage() + db := orm.NewTestDatabase(t) + client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} + allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) + signOnlyUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage()) + // Simulates a KeyReference created before the Azure Key Vault backend reported per-key usage, + // i.e. one still holding the SQL migration's default of "everything". + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1", KeyUsage: allUsage}).Error + require.NoError(t, err) + + err = client.Migrate() + require.NoError(t, err) + + var keyRef orm.KeyReference + require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) + assert.Equal(t, signOnlyUsage, keyRef.KeyUsage) + }) } func TestCrypto_New(t *testing.T) { @@ -109,15 +128,16 @@ func TestCrypto_New(t *testing.T) { t.Run("ok", func(t *testing.T) { auditLogs := audit.CaptureAuditLogs(t) - ref, pubKey, _, err := client.New(ctx, StringNamingFunc("kid")) + ref, pubKey, err := client.New(ctx, StringNamingFunc("kid")) assert.NoError(t, err) assert.NotNil(t, ref) assert.NotNil(t, pubKey) + assert.Equal(t, orm.VerificationMethodKeyType(orm.AssertionKeyUsage()|orm.EncryptionKeyUsage()), ref.KeyUsage) auditLogs.AssertContains(t, ModuleName, "CreateNewKey", audit.TestActor, "Generated new key pair: "+ref.KID) }) t.Run("error - invalid naming function", func(t *testing.T) { - _, _, _, err := client.New(ctx, ErrorNamingFunc(assert.AnError)) + _, _, err := client.New(ctx, ErrorNamingFunc(assert.AnError)) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) @@ -125,11 +145,11 @@ func TestCrypto_New(t *testing.T) { t.Run("error from backend", func(t *testing.T) { ctrl := gomock.NewController(t) storageMock := spi.NewMockStorage(ctrl) - storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", orm.DIDKeyFlags(0), assert.AnError) + storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", spi.SigningOnly, assert.AnError) client := createCrypto(t) client.backend = storageMock - _, _, _, err := client.New(ctx, StringNamingFunc("kid")) + _, _, err := client.New(ctx, StringNamingFunc("kid")) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) diff --git a/crypto/interface.go b/crypto/interface.go index ebcbd27019..ce673b45fb 100644 --- a/crypto/interface.go +++ b/crypto/interface.go @@ -40,9 +40,10 @@ type KeyCreator interface { // New generates a keypair and returns a reference. The context is used to pass audit information. // It generates a key at the backend and stores its reference in the SQL DB. // A DB transaction may be passed through the context using `orm.TransactionKey`. - // It also returns the DIDKeyFlags the generated key can actually be used for, as reported by the - // storage backend, so callers don't add a verification method (e.g. KeyAgreement) the key can't back. - New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) + // The returned KeyReference's KeyUsage reports the DIDKeyFlags the generated key can actually be + // used for, as reported by the storage backend, so callers don't add a verification method (e.g. + // KeyAgreement) the key can't back. + New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) } // KeyResolver is the interface for resolving keys. diff --git a/crypto/mock.go b/crypto/mock.go index a15937acd8..44046d64aa 100644 --- a/crypto/mock.go +++ b/crypto/mock.go @@ -44,14 +44,13 @@ func (m *MockKeyCreator) EXPECT() *MockKeyCreatorMockRecorder { } // New mocks base method. -func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { +func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "New", ctx, namingFunc) ret0, _ := ret[0].(*orm.KeyReference) ret1, _ := ret[1].(crypto.PublicKey) - ret2, _ := ret[2].(orm.DIDKeyFlags) - ret3, _ := ret[3].(error) - return ret0, ret1, ret2, ret3 + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // New indicates an expected call of New. @@ -256,14 +255,13 @@ func (mr *MockKeyStoreMockRecorder) List(ctx any) *gomock.Call { } // New mocks base method. -func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { +func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "New", ctx, namingFunc) ret0, _ := ret[0].(*orm.KeyReference) ret1, _ := ret[1].(crypto.PublicKey) - ret2, _ := ret[2].(orm.DIDKeyFlags) - ret3, _ := ret[3].(error) - return ret0, ret1, ret2, ret3 + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // New indicates an expected call of New. diff --git a/crypto/storage/azure/keyvault.go b/crypto/storage/azure/keyvault.go index 430bd26e13..8a311616a8 100644 --- a/crypto/storage/azure/keyvault.go +++ b/crypto/storage/azure/keyvault.go @@ -37,7 +37,6 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto/log" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" "golang.org/x/crypto/cryptobyte" "golang.org/x/crypto/cryptobyte/asn1" ) @@ -103,10 +102,9 @@ func (a Keyvault) CheckHealth() map[string]core.Health { return nil } -// NewPrivateKey creates a new EC key in Azure Key Vault. It only reports AssertionKeyUsage (not -// EncryptionKeyUsage/KeyAgreementUsage): Azure Key Vault EC keys can only be used for signing, they -// can't be used for decryption/ECDH, so such a key can't back a KeyAgreement verification method. -func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +// NewPrivateKey creates a new EC key in Azure Key Vault. It reports SigningOnly: Azure Key Vault EC +// keys can only be used for signing, they can't be used for decryption/ECDH. +func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { var keyType azkeys.KeyType if a.useHSM { keyType = azkeys.KeyTypeECHSM @@ -123,13 +121,13 @@ func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.Pub }, }, nil) if err != nil { - return nil, "", 0, fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) + return nil, "", spi.SigningOnly, fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) } publicKey, _, version, err := parseKey(response.Key) if err != nil { - return nil, "", 0, err + return nil, "", spi.SigningOnly, err } - return publicKey, version, orm.AssertionKeyUsage(), nil + return publicKey, version, spi.SigningOnly, nil } func (a Keyvault) GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) { diff --git a/crypto/storage/azure/keyvault_test.go b/crypto/storage/azure/keyvault_test.go index c6b6056685..09591e79cc 100644 --- a/crypto/storage/azure/keyvault_test.go +++ b/crypto/storage/azure/keyvault_test.go @@ -40,7 +40,6 @@ import ( "github.com/google/uuid" "github.com/lestrrat-go/jwx/v3/jwk" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -67,7 +66,7 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { }) store := Keyvault{client: vaultClient} - privateKey, version, keyUsage, err := store.NewPrivateKey(context.Background(), "did-web-example-com-0") + privateKey, version, capability, err := store.NewPrivateKey(context.Background(), "did-web-example-com-0") require.NoError(t, err) assert.NotNil(t, privateKey) assert.Equal(t, "b86c2e6ad9054f4abf69cc185b99aa60", version) @@ -76,8 +75,7 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { assert.True(t, *capturedParams.KeyAttributes.Enabled) assert.False(t, *capturedParams.KeyAttributes.Exportable) // Azure Key Vault EC keys can sign, but not decrypt, so they can't back KeyAgreement. - assert.Equal(t, orm.AssertionKeyUsage(), keyUsage) - assert.False(t, keyUsage.Is(orm.KeyAgreementUsage)) + assert.Equal(t, spi.SigningOnly, capability) }) } diff --git a/crypto/storage/external/client.go b/crypto/storage/external/client.go index 09a8873970..a88bba2337 100644 --- a/crypto/storage/external/client.go +++ b/crypto/storage/external/client.go @@ -30,7 +30,6 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/crypto/util" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/tracing" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) @@ -45,7 +44,7 @@ type APIClient struct { httpClient *ClientWithResponses } -func (c APIClient) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +func (c APIClient) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { return spi.GenerateAndStore(ctx, c, keyName) } diff --git a/crypto/storage/fs/fs.go b/crypto/storage/fs/fs.go index 8def7e716c..50dbd2bfcb 100644 --- a/crypto/storage/fs/fs.go +++ b/crypto/storage/fs/fs.go @@ -32,7 +32,6 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/crypto/util" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" ) type entryType string @@ -93,7 +92,7 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) { return fsc, nil } -func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { return spi.GenerateAndStore(ctx, fsc, keyName) } diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index d25b14929c..cc00f288b4 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -31,7 +31,6 @@ import ( "github.com/lestrrat-go/jwx/v3/jwk" "github.com/nuts-foundation/nuts-node/v6/core" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" ) // ErrNotFound indicates that the specified crypto storage entry couldn't be found. @@ -43,16 +42,26 @@ var ErrKeyAlreadyExists = errors.New("key already exists") // KidPattern is the regexp for acceptable kids var KidPattern = regexp.MustCompile(`^(?:(?:[\da-zA-Z_\- :#.])|(?:%[0-9a-fA-F]{2}))+$`) +// KeyCapability describes what a newly generated key can be used for. +type KeyCapability int + +const ( + // SigningOnly means the key can only be used for signing, e.g. an Azure Key Vault EC key: Azure + // Key Vault doesn't support decryption/ECDH with it. + SigningOnly KeyCapability = iota + // SigningAndDecryption means the key can be used for both signing and decryption/ECDH key + // agreement, e.g. a plain, exportable EC key. + SigningAndDecryption +) + // Storage interface containing functions for storing and retrieving keys. type Storage interface { core.HealthCheckable // NewPrivateKey creates a new private key. The backend will create the version and publicKey. // It should be preferred over generating a key in the application and saving it to the storage, // as it allows for unexportable (safer) keys. - // It also returns the DIDKeyFlags the generated key can actually be used for, e.g. an Azure Key Vault - // EC key can only be used for signing (not KeyAgreementUsage), since Azure Key Vault doesn't support - // decryption/ECDH with it. - NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) + // It also reports the KeyCapability of the generated key. + NewPrivateKey(ctx context.Context, keyName string) (publicKey crypto.PublicKey, version string, capability KeyCapability, err error) // GetPrivateKey from the storage backend and return its handler as an implementation of crypto.Signer. GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) // PrivateKeyExists checks if the private key indicated with the keyname/version is stored in the storage backend. @@ -120,24 +129,23 @@ func (pke PublicKeyEntry) JWK() jwk.Key { } // GenerateAndStore generates a new key pair and stores it in the provided storage. -// It always generates a plain, exportable EC key, so it supports every DIDKeyFlags usage, including -// KeyAgreementUsage. -func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +// It always generates a plain, exportable EC key, which can be used for both signing and decryption. +func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, KeyCapability, error) { keyPair, err := GenerateKeyPair() if err != nil { - return nil, "", 0, err + return nil, "", SigningOnly, err } exists, err := store.PrivateKeyExists(ctx, keyName, "1") if err != nil { - return nil, "", 0, fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) + return nil, "", SigningOnly, fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) } if exists { - return nil, "", 0, errors.New("key with the given ID already exists") + return nil, "", SigningOnly, errors.New("key with the given ID already exists") } if err = store.SavePrivateKey(ctx, keyName, keyPair); err != nil { - return nil, "", 0, fmt.Errorf("could not create new keypair: could not save private key: %w", err) + return nil, "", SigningOnly, fmt.Errorf("could not create new keypair: could not save private key: %w", err) } - return keyPair.Public(), "1", orm.AllKeyUsage(), nil + return keyPair.Public(), "1", SigningAndDecryption, nil } // GenerateKeyPair generates a new key pair using the default key type. diff --git a/crypto/storage/spi/interface_test.go b/crypto/storage/spi/interface_test.go index 5061467325..96735b68da 100644 --- a/crypto/storage/spi/interface_test.go +++ b/crypto/storage/spi/interface_test.go @@ -70,11 +70,12 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(nil) keyName := "123" - key, version, _, err := GenerateAndStore(ctx, store, keyName) + key, version, capability, err := GenerateAndStore(ctx, store, keyName) assert.NoError(t, err) assert.NotNil(t, key) assert.Equal(t, "1", version) + assert.Equal(t, SigningAndDecryption, capability) }) t.Run("error - save public key returns an error", func(t *testing.T) { diff --git a/crypto/storage/spi/mock.go b/crypto/storage/spi/mock.go index 551042e6a4..439cf42b5f 100644 --- a/crypto/storage/spi/mock.go +++ b/crypto/storage/spi/mock.go @@ -15,7 +15,6 @@ import ( reflect "reflect" core "github.com/nuts-foundation/nuts-node/v6/core" - orm "github.com/nuts-foundation/nuts-node/v6/storage/orm" gomock "go.uber.org/mock/gomock" ) @@ -115,12 +114,12 @@ func (mr *MockStorageMockRecorder) Name() *gomock.Call { } // NewPrivateKey mocks base method. -func (m *MockStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +func (m *MockStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, KeyCapability, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewPrivateKey", ctx, keyName) ret0, _ := ret[0].(crypto.PublicKey) ret1, _ := ret[1].(string) - ret2, _ := ret[2].(orm.DIDKeyFlags) + ret2, _ := ret[2].(KeyCapability) ret3, _ := ret[3].(error) return ret0, ret1, ret2, ret3 } diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 48b15b16ac..3d0c4088e9 100644 --- a/crypto/storage/spi/wrapper.go +++ b/crypto/storage/spi/wrapper.go @@ -23,7 +23,6 @@ import ( "crypto" "fmt" "github.com/nuts-foundation/nuts-node/v6/core" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" "regexp" ) @@ -90,10 +89,10 @@ func (w wrapper) ListPrivateKeys(ctx context.Context) []KeyNameVersion { return w.wrappedBackend.ListPrivateKeys(ctx) } -func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { - publicKey, version, keyUsage, err := w.wrappedBackend.NewPrivateKey(ctx, keyName) +func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, KeyCapability, error) { + publicKey, version, capability, err := w.wrappedBackend.NewPrivateKey(ctx, keyName) if err != nil { - return nil, "", 0, err + return nil, "", SigningOnly, err } - return publicKey, version, keyUsage, err + return publicKey, version, capability, err } diff --git a/crypto/storage/vault/vault.go b/crypto/storage/vault/vault.go index 32fc456fc1..c9fe36c6bb 100644 --- a/crypto/storage/vault/vault.go +++ b/crypto/storage/vault/vault.go @@ -32,7 +32,6 @@ import ( "github.com/nuts-foundation/nuts-node/v6/crypto/log" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/crypto/util" - "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/tracing" "go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp" ) @@ -108,7 +107,7 @@ func NewVaultKVStorage(config Config) (spi.Storage, error) { return vaultStorage, nil } -func (v vaultKVStorage) NewPrivateKey(ctx context.Context, keyPath string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +func (v vaultKVStorage) NewPrivateKey(ctx context.Context, keyPath string) (crypto.PublicKey, string, spi.KeyCapability, error) { return spi.GenerateAndStore(ctx, v, keyPath) } diff --git a/crypto/test.go b/crypto/test.go index fdb6c5451a..91da32e1cb 100644 --- a/crypto/test.go +++ b/crypto/test.go @@ -68,7 +68,7 @@ var _ spi.Storage = &memoryStorage{} type memoryStorage map[string]crypto.PrivateKey -func (m memoryStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +func (m memoryStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { return spi.GenerateAndStore(ctx, m, keyName) } @@ -143,7 +143,7 @@ func (t TestKey) Private() crypto.PrivateKey { // newKeyReference creates a new DID, DIDocument, VerificationMethod and KeyReference in the DB // It does not create valid DID Document data func newKeyReference(t *testing.T, client *Crypto, kid string) (*orm.KeyReference, crypto.PublicKey) { - ref, publicKey, _, err := client.New(audit.TestContext(), StringNamingFunc(kid)) + ref, publicKey, err := client.New(audit.TestContext(), StringNamingFunc(kid)) require.NoError(t, err) DID := orm.DID{ID: "did:test:" + t.Name(), Subject: "subject"} DIDDoc := orm.DidDocument{ diff --git a/network/network_integration_test.go b/network/network_integration_test.go index cff7976876..01481f711d 100644 --- a/network/network_integration_test.go +++ b/network/network_integration_test.go @@ -201,7 +201,7 @@ func TestNetworkIntegration_Messages(t *testing.T) { }) // set root - _, key, _, _ := bootstrap.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("key1")) + _, key, _ := bootstrap.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("key1")) rootTx, err := bootstrap.network.CreateTransaction(audit.TestContext(), TransactionTemplate(payloadType, []byte("root_tx"), "key1").WithAttachKey(key)) require.NoError(t, err) require.NoError(t, node1.network.state.Add(context.Background(), rootTx, []byte("root_tx"))) @@ -976,7 +976,7 @@ func resetIntegrationTest(t *testing.T) { document := did.Document{ID: nodeDID} kid := did.DIDURL{DID: nodeDID} kid.Fragment = "key-1" - _, key, _, _ := keyStore.New(audit.TestContext(), func(_ crypto.PublicKey) (string, error) { + _, key, _ := keyStore.New(audit.TestContext(), func(_ crypto.PublicKey) (string, error) { return kid.String(), nil }) verificationMethod, _ := did.NewVerificationMethod(kid, ssi.JsonWebKey2020, nodeDID, key) @@ -1024,7 +1024,7 @@ func addBootstrapDIDDocument(t *testing.T, n node, subject string) hash.SHA256Ha } func addTransactionAndWaitForItToArrive(t *testing.T, payload string, sender node, receivers ...string) bool { - keyRef, key, _, _ := sender.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(uuid.New().String())) + keyRef, key, _ := sender.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(uuid.New().String())) expectedTransaction, err := sender.network.CreateTransaction(audit.TestContext(), TransactionTemplate(payloadType, []byte(payload), keyRef.KID).WithAttachKey(key)) if !assert.NoError(t, err) { return false diff --git a/network/network_test.go b/network/network_test.go index d8a4dfd5e0..7b13c68a8d 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -412,7 +412,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, key, _, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) cxt.state.EXPECT().Head(gomock.Any()) cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) @@ -426,7 +426,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) cxt.state.EXPECT().Head(gomock.Any()) cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) @@ -452,7 +452,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) err := cxt.start() require.NoError(t, err) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) tx, err := cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithAdditionalPrevs([]hash.SHA256Hash{additionalPrev.Ref()})) @@ -467,7 +467,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) // 'Register' prev on DAG prev, _, _ := dag.CreateTestTransaction(1) @@ -491,7 +491,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) cxt.network.nodeDID = *nodeDID @@ -510,7 +510,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, key, _, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) + _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) cxt.network.nodeDID = *nodeDID _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "did:nuts:sender#signing-key").WithAttachKey(key).WithPrivate([]did.DID{*sender, *receiver})) @@ -522,7 +522,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) cxt.network.nodeDID = *nodeDID _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithPrivate([]did.DID{*sender, *receiver})) @@ -534,7 +534,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithPrivate([]did.DID{*sender, *receiver})) assert.EqualError(t, err, "node DID must be configured to create private transactions") @@ -552,7 +552,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt.state.EXPECT().GetTransaction(gomock.Any(), additionalPrev.Ref()).Return(additionalPrev, nil) cxt.state.EXPECT().IsPayloadPresent(gomock.Any(), additionalPrev.PayloadHash()).Return(true, nil) cxt.state.EXPECT().Head(gomock.Any()).Return(rootTX.Ref(), nil) - _, _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) _, err := cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithAdditionalPrevs([]hash.SHA256Hash{additionalPrev.Ref()})) diff --git a/storage/orm/key_reference.go b/storage/orm/key_reference.go index f6dc930cf2..a4315b2f2f 100644 --- a/storage/orm/key_reference.go +++ b/storage/orm/key_reference.go @@ -24,6 +24,10 @@ type KeyReference struct { KID string `gorm:"column:kid;primaryKey"` KeyName string Version string + // KeyUsage is the bitmask of DIDKeyFlags the key can actually be used for, as reported by the key + // store backend. E.g. an Azure Key Vault EC key can only be used for signing, not KeyAgreement, + // since Azure Key Vault doesn't support decryption/ECDH with it. + KeyUsage VerificationMethodKeyType } func (d KeyReference) TableName() string { diff --git a/storage/orm/keyflag.go b/storage/orm/keyflag.go index dec86bf0d4..b04f8dc433 100644 --- a/storage/orm/keyflag.go +++ b/storage/orm/keyflag.go @@ -49,12 +49,6 @@ func EncryptionKeyUsage() DIDKeyFlags { return KeyAgreementUsage } -// AllKeyUsage returns every DIDKeyFlags bit. It's the usage reported by a key store backend whose -// keys support every verification relationship, e.g. because it hands back plain, exportable EC keys. -func AllKeyUsage() DIDKeyFlags { - return AssertionKeyUsage() | EncryptionKeyUsage() -} - // verificationMethodToKeyFlags creates DIDKeyFlags for a did.VerificationMethod based on its usage in the did.Document. func verificationMethodToKeyFlags(document did.Document, vm *did.VerificationMethod) DIDKeyFlags { var flags DIDKeyFlags diff --git a/storage/sql_migrations/012_key_reference_key_usage.sql b/storage/sql_migrations/012_key_reference_key_usage.sql new file mode 100644 index 0000000000..22c9b461cc --- /dev/null +++ b/storage/sql_migrations/012_key_reference_key_usage.sql @@ -0,0 +1,18 @@ +-- +goose Up +-- key_usage is a bitmask of the DIDKeyFlags the key can actually be used for, using the same +-- encoding as did_verification_method.key_types: +-- 0x01 - AssertionMethod +-- 0x02 - Authentication +-- 0x04 - CapabilityDelegation +-- 0x08 - CapabilityInvocation +-- 0x10 - KeyAgreement +-- Defaults to all flags (31): the common case, since fs/vault/external backends hand back plain, +-- exportable EC keys that support every usage. crypto.Migrate() corrects existing rows to 15 +-- (everything except KeyAgreement) for nodes configured with the Azure Key Vault backend, whose EC +-- keys can only be used for signing. +alter table key_reference + add column key_usage SMALLINT not null default 31; + +-- +goose Down +alter table key_reference + drop column key_usage; diff --git a/vcr/issuer/issuer_test.go b/vcr/issuer/issuer_test.go index 27cdacbb79..b9cd8cef16 100644 --- a/vcr/issuer/issuer_test.go +++ b/vcr/issuer/issuer_test.go @@ -82,7 +82,7 @@ func Test_issuer_buildAndSignVC(t *testing.T) { }}, } keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, signingKey, _, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(kid)) + _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(kid)) require.NoError(t, err) t.Run("JSON-LD", func(t *testing.T) { @@ -289,7 +289,7 @@ func Test_issuer_Issue(t *testing.T) { ctx := audit.TestContext() jsonldManager := jsonld.NewTestJSONLDManager(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - _, issuerKey, _, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc(issuerKeyID)) + _, issuerKey, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc(issuerKeyID)) t.Run("ok - unpublished", func(t *testing.T) { ctrl := gomock.NewController(t) @@ -550,7 +550,7 @@ func Test_issuer_buildRevocation(t *testing.T) { t.Run("ok", func(t *testing.T) { ctrl := gomock.NewController(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) + kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) keyResolverMock := resolver.NewMockKeyResolver(ctrl) keyResolverMock.EXPECT().ResolveKey(issuerDID, nil, resolver.AssertionMethod).Return(kid.KID, key, nil) @@ -771,7 +771,7 @@ func Test_issuer_revokeNetwork(t *testing.T) { issuerURI := issuerDID.URI() jsonldManager := jsonld.NewTestJSONLDManager(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) + kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) ctx := audit.TestContext() t.Run("for a known credential", func(t *testing.T) { @@ -927,7 +927,7 @@ func TestIssuer_revokeStatusList(t *testing.T) { ctx := audit.TestContext() keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, signingKey, _, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) + _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) require.NoError(t, err) t.Run("ok", func(t *testing.T) { @@ -1052,7 +1052,7 @@ func TestIssuer_StatusList(t *testing.T) { ctx := audit.TestContext() db := orm.NewTestDatabase(t) keyStore := nutsCrypto.NewDatabaseCryptoInstance(db) - _, signingKey, _, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) + _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) require.NoError(t, err) jsonldManager := jsonld.NewTestJSONLDManager(t) diff --git a/vcr/issuer/openid_test.go b/vcr/issuer/openid_test.go index 3b12e32179..593ea395a6 100644 --- a/vcr/issuer/openid_test.go +++ b/vcr/issuer/openid_test.go @@ -117,7 +117,7 @@ func Test_memoryIssuer_ProviderMetadata(t *testing.T) { func Test_memoryIssuer_HandleCredentialRequest(t *testing.T) { keyStore := crypto.NewMemoryCryptoInstance(t) ctx := audit.TestContext() - _, signerKey, _, _ := keyStore.New(ctx, crypto.StringNamingFunc(keyID)) + _, signerKey, _ := keyStore.New(ctx, crypto.StringNamingFunc(keyID)) ctrl := gomock.NewController(t) keyResolver := resolver.NewMockKeyResolver(ctrl) keyResolver.EXPECT().ResolveKeyByID(keyID, nil, resolver.NutsSigningKeyType).AnyTimes().Return(signerKey, nil) diff --git a/vcr/signature/json_web_signature_test.go b/vcr/signature/json_web_signature_test.go index 7355e16468..f5c1f73df4 100644 --- a/vcr/signature/json_web_signature_test.go +++ b/vcr/signature/json_web_signature_test.go @@ -121,7 +121,7 @@ func TestJsonWebSignature2020_Sign(t *testing.T) { doc := []byte("foo") cryptoInstance := crypto.NewMemoryCryptoInstance(t) const keyID = "did:nuts:123#abc" - _, _, _, _ = cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(keyID)) + _, _, _ = cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(keyID)) sig := JSONWebSignature2020{Signer: cryptoInstance} result, err := sig.Sign(audit.TestContext(), doc, keyID) diff --git a/vcr/signature/proof/jsonld_test.go b/vcr/signature/proof/jsonld_test.go index 5cecec944c..78f7f641b2 100644 --- a/vcr/signature/proof/jsonld_test.go +++ b/vcr/signature/proof/jsonld_test.go @@ -170,7 +170,7 @@ func TestLDProof_Sign(t *testing.T) { contextLoader := jsonld.NewTestJSONLDManager(t).DocumentLoader() cryptoInstance := crypto.NewMemoryCryptoInstance(t) - _, key, _, _ := cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(kid)) + _, key, _ := cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(kid)) t.Run("sign and verify a document", func(t *testing.T) { now := time.Now() expires := now.Add(20 * time.Hour) diff --git a/vcr/test/test.go b/vcr/test/test.go index b10f8660f2..b7b52a41ef 100644 --- a/vcr/test/test.go +++ b/vcr/test/test.go @@ -58,7 +58,7 @@ func CreateJWTPresentation(t *testing.T, subjectDID did.DID, tokenVisitor func(t tokenVisitor(unsignedToken) } keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, key, _, err := keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(kid)) + _, key, err := keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(kid)) require.NoError(t, err) claims, err = jwx.ClaimsAsMap(unsignedToken) require.NoError(t, err) diff --git a/vcr/verifier/signature_verifier_test.go b/vcr/verifier/signature_verifier_test.go index c7ac7785a6..3527309638 100644 --- a/vcr/verifier/signature_verifier_test.go +++ b/vcr/verifier/signature_verifier_test.go @@ -119,7 +119,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { t.Run("JWT", func(t *testing.T) { // Create did:jwk for issuer, and sign credential keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _, err := keyStore.New(audit.TestContext(), func(key crypto.PublicKey) (string, error) { + kid, key, err := keyStore.New(audit.TestContext(), func(key crypto.PublicKey) (string, error) { keyAsJWK, _ := jwk.Import(key) keyJSON, _ := json.Marshal(keyAsJWK) return "did:jwk:" + base64.RawStdEncoding.EncodeToString(keyJSON) + "#0", nil diff --git a/vdr/didnuts/ambassador_test.go b/vdr/didnuts/ambassador_test.go index 80661db6a3..3d26b06efe 100644 --- a/vdr/didnuts/ambassador_test.go +++ b/vdr/didnuts/ambassador_test.go @@ -55,18 +55,19 @@ type mockKeyStore struct { } // New creates a new valid key with the correct KID -func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, orm.DIDKeyFlags, error) { +func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { if m.privateKey == nil { m.privateKey, _ = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) kid, _ := nf(m.privateKey.PublicKey) m.keyReference = &orm.KeyReference{ - KID: kid, - KeyName: uuid.NewString(), - Version: uuid.NewString(), + KID: kid, + KeyName: uuid.NewString(), + Version: uuid.NewString(), + KeyUsage: orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()), } } - return m.keyReference, m.privateKey.Public(), orm.AssertionKeyUsage() | orm.EncryptionKeyUsage(), nil + return m.keyReference, m.privateKey.Public(), nil } func (m *mockKeyStore) Link(_ context.Context, _ string, _ string, _ string) error { diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index 633adaa822..8bc6e06a1c 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -160,7 +160,7 @@ func (m Manager) RemoveVerificationMethod(ctx context.Context, id did.DID, keyID // with a freshly generated key for a given DID. It also returns the DIDKeyFlags the key can actually // be used for, as reported by the key store backend. func CreateNewVerificationMethodForDID(ctx context.Context, id did.DID, keyCreator nutsCrypto.KeyCreator) (*did.VerificationMethod, orm.DIDKeyFlags, error) { - keyRef, publicKey, keyUsage, err := keyCreator.New(ctx, didSubKIDNamingFunc(id)) + keyRef, publicKey, err := keyCreator.New(ctx, didSubKIDNamingFunc(id)) if err != nil { return nil, 0, err } @@ -172,7 +172,7 @@ func CreateNewVerificationMethodForDID(ctx context.Context, id did.DID, keyCreat if err != nil { return nil, 0, err } - return method, keyUsage, nil + return method, orm.DIDKeyFlags(keyRef.KeyUsage), nil } // Update updates a DID Document based on the DID. @@ -254,13 +254,13 @@ func (m Manager) Update(ctx context.Context, id did.DID, next did.Document) erro ******************************/ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDocument, error) { - keyRef, publicKey, actualUsage, err := m.keyStore.New(ctx, DIDKIDNamingFunc) + keyRef, publicKey, err := m.keyStore.New(ctx, DIDKIDNamingFunc) if err != nil { return nil, err } // Only claim the verification relationships (e.g. KeyAgreement) the key store backend can actually // back for this key; e.g. an Azure Key Vault EC key can't be used for KeyAgreement (decryption). - keyFlags := DefaultKeyFlags() & actualUsage + keyFlags := DefaultKeyFlags() & orm.DIDKeyFlags(keyRef.KeyUsage) keyID, err := did.ParseDIDURL(keyRef.KID) if err != nil { diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index 5cacf0d876..5f07639600 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -107,7 +107,7 @@ func TestManager_RemoveVerificationMethod(t *testing.T) { t.Run("ok", func(t *testing.T) { ctx := newTestContext(t) - _, pubKey, _, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) + _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) doc1 := createDoc(pubKey) doc2 := createDoc(pubKey) ctx.didResolver.EXPECT().Resolve(*id123, &resolver.ResolveMetadata{AllowDeactivated: true}).Return(&doc1, &resolver.DocumentMetadata{}, nil) @@ -136,7 +136,7 @@ func TestManager_RemoveVerificationMethod(t *testing.T) { t.Run("error - document is deactivated", func(t *testing.T) { ctx := newTestContext(t) - _, pubKey, _, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) + _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) doc1 := createDoc(pubKey) doc2 := createDoc(pubKey) ctx.didResolver.EXPECT().Resolve(*id123, &resolver.ResolveMetadata{AllowDeactivated: true}).Return(&doc1, &resolver.DocumentMetadata{Deactivated: true}, nil) @@ -365,9 +365,9 @@ type signOnlyStorage struct { spi.Storage } -func (s signOnlyStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, orm.DIDKeyFlags, error) { +func (s signOnlyStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { publicKey, version, _, err := s.Storage.NewPrivateKey(ctx, keyName) - return publicKey, version, orm.AssertionKeyUsage(), err + return publicKey, version, spi.SigningOnly, err } func TestManager_Commit(t *testing.T) { diff --git a/vdr/didweb/manager.go b/vdr/didweb/manager.go index 24516047ea..7ab6fb159b 100644 --- a/vdr/didweb/manager.go +++ b/vdr/didweb/manager.go @@ -104,7 +104,7 @@ func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, // return verificationMethodID.String(), nil //}) } else { - _, publicKey, _, err = m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { + _, publicKey, err = m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { return verificationMethodID.String(), nil }) } diff --git a/vdr/vdr_test.go b/vdr/vdr_test.go index 47f5cddcae..89c7153322 100644 --- a/vdr/vdr_test.go +++ b/vdr/vdr_test.go @@ -140,7 +140,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { client := nutsCrypto.NewDatabaseCryptoInstance(db) keyID := did.DIDURL{DID: TestDIDA} keyID.Fragment = "1" - _, _, _, _ = client.New(audit.TestContext(), nutsCrypto.StringNamingFunc(keyID.String())) + _, _, _ = client.New(audit.TestContext(), nutsCrypto.StringNamingFunc(keyID.String())) ctrl := gomock.NewController(t) pkiMock := pki.NewMockValidator(ctrl) vdr := NewVDR(client, nil, didstore.NewTestStore(t), nil, storageEngine, pkiMock) @@ -160,7 +160,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { t.Run("ok - 1 owned conflict in controlled document", func(t *testing.T) { // vendor test := newVDRTestCtx(t) - _, keyVendor, _, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:vendor#keyVendor-1")) + _, keyVendor, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:vendor#keyVendor-1")) didDocVendor := &did.Document{ID: did.MustParseDID("did:nuts:vendor")} vendorVM, err := did.NewVerificationMethod(did.MustParseDIDURL("did:nuts:vendor#keyVendor-1"), ssi.JsonWebKey2020, didDocVendor.ID, keyVendor) @@ -168,7 +168,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { didDocVendor.AddCapabilityInvocation(vendorVM) // organization - _, keyOrg, _, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:org#keyOrg-1")) + _, keyOrg, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:org#keyOrg-1")) didDocOrg := &did.Document{ID: did.MustParseDID("did:nuts:org")} didDocOrg.Controller = []did.DID{didDocVendor.ID} orgVM, err := did.NewVerificationMethod(did.MustParseDIDURL("did:nuts:org#keyOrg-1"), ssi.JsonWebKey2020, didDocOrg.ID, keyOrg) @@ -343,7 +343,7 @@ func TestVDR_Migrate(t *testing.T) { t.Run("makes documents self-controlled", func(t *testing.T) { ctx := controllerMigrationSetup(t) keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - keyRef, publicKey, _, err := keyStore.New(ctx.ctx, didnuts.DIDKIDNamingFunc) + keyRef, publicKey, err := keyStore.New(ctx.ctx, didnuts.DIDKIDNamingFunc) require.NoError(t, err) methodID := did.MustParseDIDURL(keyRef.KID) methodID.ID = TestDIDA.ID From 1045cb3d531e9d5116bacf44183d19b35b5f81bf Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 10 Sep 2026 14:26:49 +0200 Subject: [PATCH 03/20] refactor(crypto): make KeyCapability an actual bitmask KeyCapability had two named states, SigningOnly and SigningAndDecryption, that read like a bitmask (the combined-word name) without being one - membership was checked with ==, not bitwise. Give it real bits, Signing and Decryption, combined the same way orm.DIDKeyFlags already is (Signing | Decryption), with a matching Is() helper. Every generated key sets Signing; only decryption-capable keys also set Decryption, so a key that can't sign at all (e.g. a future AES-like backend) remains representable without it. Assisted by AI --- crypto/crypto.go | 12 ++++++------ crypto/crypto_test.go | 2 +- crypto/storage/azure/keyvault.go | 10 +++++----- crypto/storage/azure/keyvault_test.go | 2 +- crypto/storage/spi/interface.go | 28 +++++++++++++++------------ crypto/storage/spi/interface_test.go | 2 +- crypto/storage/spi/wrapper.go | 2 +- vdr/didnuts/manager_test.go | 2 +- 8 files changed, 32 insertions(+), 28 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index 18d4763090..56fd4c8221 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -192,9 +192,9 @@ func (client *Crypto) Migrate() error { outerContext := context.TODO() // Azure Key Vault EC keys can't be used for decryption; every other backend hands back plain, // exportable EC keys that support both signing and decryption. - capability := spi.SigningAndDecryption + capability := spi.Signing | spi.Decryption if client.config.Storage == azure.StorageType { - capability = spi.SigningOnly + capability = spi.Signing } keyUsage := orm.VerificationMethodKeyType(keyUsageForCapability(capability)) @@ -225,13 +225,13 @@ func (client *Crypto) Migrate() error { } } } - if capability == spi.SigningOnly { + if !capability.Is(spi.Decryption) { // Correct KeyReferences created before this backend reported per-key usage (the SQL // migration defaults key_usage to "everything"): on a node configured with the Azure Key // Vault backend, every managed key was created by Azure Key Vault and can't decrypt. // Switching crypto storage backends for an existing node isn't supported (KeyName/Version // are backend-specific and become unreachable), so this is safe to assume unconditionally. - allUsage := orm.VerificationMethodKeyType(keyUsageForCapability(spi.SigningAndDecryption)) + allUsage := orm.VerificationMethodKeyType(keyUsageForCapability(spi.Signing | spi.Decryption)) err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", allUsage).Update("key_usage", keyUsage).Error if err != nil { return fmt.Errorf("could not correct existing KeyReferences for the Azure Key Vault backend: %w", err) @@ -275,10 +275,10 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.K // keyUsageForCapability derives the DIDKeyFlags a key with the given KeyCapability can back. // Every key can be used for signing (AssertionKeyUsage); only a key that also supports -// decryption/ECDH (SigningAndDecryption) can additionally back KeyAgreement (EncryptionKeyUsage). +// decryption/ECDH can additionally back KeyAgreement (EncryptionKeyUsage). func keyUsageForCapability(capability spi.KeyCapability) orm.DIDKeyFlags { keyUsage := orm.AssertionKeyUsage() - if capability == spi.SigningAndDecryption { + if capability.Is(spi.Decryption) { keyUsage |= orm.EncryptionKeyUsage() } return keyUsage diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index c0cdc56bb8..64cef2aff5 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -145,7 +145,7 @@ func TestCrypto_New(t *testing.T) { t.Run("error from backend", func(t *testing.T) { ctrl := gomock.NewController(t) storageMock := spi.NewMockStorage(ctrl) - storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", spi.SigningOnly, assert.AnError) + storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", spi.Signing, assert.AnError) client := createCrypto(t) client.backend = storageMock diff --git a/crypto/storage/azure/keyvault.go b/crypto/storage/azure/keyvault.go index 8a311616a8..fdd934ee3f 100644 --- a/crypto/storage/azure/keyvault.go +++ b/crypto/storage/azure/keyvault.go @@ -102,8 +102,8 @@ func (a Keyvault) CheckHealth() map[string]core.Health { return nil } -// NewPrivateKey creates a new EC key in Azure Key Vault. It reports SigningOnly: Azure Key Vault EC -// keys can only be used for signing, they can't be used for decryption/ECDH. +// NewPrivateKey creates a new EC key in Azure Key Vault. It reports only spi.Signing: Azure Key +// Vault EC keys can only be used for signing, they can't be used for decryption/ECDH. func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { var keyType azkeys.KeyType if a.useHSM { @@ -121,13 +121,13 @@ func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.Pub }, }, nil) if err != nil { - return nil, "", spi.SigningOnly, fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) + return nil, "", 0, fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) } publicKey, _, version, err := parseKey(response.Key) if err != nil { - return nil, "", spi.SigningOnly, err + return nil, "", 0, err } - return publicKey, version, spi.SigningOnly, nil + return publicKey, version, spi.Signing, nil } func (a Keyvault) GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) { diff --git a/crypto/storage/azure/keyvault_test.go b/crypto/storage/azure/keyvault_test.go index 09591e79cc..01a6837735 100644 --- a/crypto/storage/azure/keyvault_test.go +++ b/crypto/storage/azure/keyvault_test.go @@ -75,7 +75,7 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { assert.True(t, *capturedParams.KeyAttributes.Enabled) assert.False(t, *capturedParams.KeyAttributes.Exportable) // Azure Key Vault EC keys can sign, but not decrypt, so they can't back KeyAgreement. - assert.Equal(t, spi.SigningOnly, capability) + assert.Equal(t, spi.Signing, capability) }) } diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index cc00f288b4..0728790b13 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -42,18 +42,22 @@ var ErrKeyAlreadyExists = errors.New("key already exists") // KidPattern is the regexp for acceptable kids var KidPattern = regexp.MustCompile(`^(?:(?:[\da-zA-Z_\- :#.])|(?:%[0-9a-fA-F]{2}))+$`) -// KeyCapability describes what a newly generated key can be used for. +// KeyCapability is a bitmask describing what a newly generated key can be used for. type KeyCapability int const ( - // SigningOnly means the key can only be used for signing, e.g. an Azure Key Vault EC key: Azure - // Key Vault doesn't support decryption/ECDH with it. - SigningOnly KeyCapability = iota - // SigningAndDecryption means the key can be used for both signing and decryption/ECDH key - // agreement, e.g. a plain, exportable EC key. - SigningAndDecryption + // Signing means the key can be used for signing. Every generated key can do this. + Signing KeyCapability = 1 << iota + // Decryption means the key can be used for decryption/ECDH key agreement. E.g. an Azure Key + // Vault EC key can't do this: Azure Key Vault doesn't support decryption/ECDH with it. + Decryption ) +// Is returns whether the specified KeyCapability is enabled. +func (k KeyCapability) Is(other KeyCapability) bool { + return k&other > 0 +} + // Storage interface containing functions for storing and retrieving keys. type Storage interface { core.HealthCheckable @@ -133,19 +137,19 @@ func (pke PublicKeyEntry) JWK() jwk.Key { func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, KeyCapability, error) { keyPair, err := GenerateKeyPair() if err != nil { - return nil, "", SigningOnly, err + return nil, "", 0, err } exists, err := store.PrivateKeyExists(ctx, keyName, "1") if err != nil { - return nil, "", SigningOnly, fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) + return nil, "", 0, fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) } if exists { - return nil, "", SigningOnly, errors.New("key with the given ID already exists") + return nil, "", 0, errors.New("key with the given ID already exists") } if err = store.SavePrivateKey(ctx, keyName, keyPair); err != nil { - return nil, "", SigningOnly, fmt.Errorf("could not create new keypair: could not save private key: %w", err) + return nil, "", 0, fmt.Errorf("could not create new keypair: could not save private key: %w", err) } - return keyPair.Public(), "1", SigningAndDecryption, nil + return keyPair.Public(), "1", Signing | Decryption, nil } // GenerateKeyPair generates a new key pair using the default key type. diff --git a/crypto/storage/spi/interface_test.go b/crypto/storage/spi/interface_test.go index 96735b68da..9e7d67ee08 100644 --- a/crypto/storage/spi/interface_test.go +++ b/crypto/storage/spi/interface_test.go @@ -75,7 +75,7 @@ func TestGenerateAndStore(t *testing.T) { assert.NoError(t, err) assert.NotNil(t, key) assert.Equal(t, "1", version) - assert.Equal(t, SigningAndDecryption, capability) + assert.Equal(t, Signing|Decryption, capability) }) t.Run("error - save public key returns an error", func(t *testing.T) { diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 3d0c4088e9..4eee1bbb91 100644 --- a/crypto/storage/spi/wrapper.go +++ b/crypto/storage/spi/wrapper.go @@ -92,7 +92,7 @@ func (w wrapper) ListPrivateKeys(ctx context.Context) []KeyNameVersion { func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, KeyCapability, error) { publicKey, version, capability, err := w.wrappedBackend.NewPrivateKey(ctx, keyName) if err != nil { - return nil, "", SigningOnly, err + return nil, "", 0, err } return publicKey, version, capability, err } diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index 5f07639600..786a7c72d4 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -367,7 +367,7 @@ type signOnlyStorage struct { func (s signOnlyStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { publicKey, version, _, err := s.Storage.NewPrivateKey(ctx, keyName) - return publicKey, version, spi.SigningOnly, err + return publicKey, version, spi.Signing, err } func TestManager_Commit(t *testing.T) { From 95aad298ceff41ab389a4a64c85a8822b6704666 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Thu, 10 Sep 2026 14:52:36 +0200 Subject: [PATCH 04/20] fix(crypto): default key_usage to 0, not "everything", until it's known The migration previously defaulted key_reference.key_usage to 31 ("everything") and crypto.Migrate() only corrected it down to sign-only for the Azure Key Vault backend specifically. That fails open: any future backend that also can't decrypt, but isn't recognized by that Azure-only check, would silently keep the "everything" default and offer a KeyAgreement verification method it can't back. Default to 0 ("not yet determined") instead, and have Migrate() set any row still at 0 to the currently configured backend's real usage, unconditionally rather than only for Azure. Existing rows never end up assuming a capability that hasn't actually been confirmed. Assisted by AI --- crypto/crypto.go | 17 +++++-------- crypto/crypto_test.go | 24 +++++++++++++++---- .../012_key_reference_key_usage.sql | 9 ++++--- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index 56fd4c8221..8c87e1879f 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -225,17 +225,12 @@ func (client *Crypto) Migrate() error { } } } - if !capability.Is(spi.Decryption) { - // Correct KeyReferences created before this backend reported per-key usage (the SQL - // migration defaults key_usage to "everything"): on a node configured with the Azure Key - // Vault backend, every managed key was created by Azure Key Vault and can't decrypt. - // Switching crypto storage backends for an existing node isn't supported (KeyName/Version - // are backend-specific and become unreachable), so this is safe to assume unconditionally. - allUsage := orm.VerificationMethodKeyType(keyUsageForCapability(spi.Signing | spi.Decryption)) - err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", allUsage).Update("key_usage", keyUsage).Error - if err != nil { - return fmt.Errorf("could not correct existing KeyReferences for the Azure Key Vault backend: %w", err) - } + // Set the usage for KeyReferences created before this backend reported per-key usage: the SQL + // migration defaults key_usage to 0 ("not yet determined") rather than assuming every existing + // key supports every usage. Every row still at 0 gets the currently configured backend's usage. + err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", orm.VerificationMethodKeyType(0)).Update("key_usage", keyUsage).Error + if err != nil { + return fmt.Errorf("could not set key usage for existing KeyReferences: %w", err) } return nil }) diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 64cef2aff5..61bfddc761 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -100,15 +100,31 @@ func TestCrypto_Migrate(t *testing.T) { keys := client.List(context.Background()) require.Len(t, keys, 1) }) - t.Run("corrects existing KeyReferences for the Azure Key Vault backend", func(t *testing.T) { + t.Run("sets key usage for existing KeyReferences still at the SQL migration's default of 0", func(t *testing.T) { backend := NewMemoryStorage() db := orm.NewTestDatabase(t) - client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} + client := &Crypto{backend: backend, db: db} allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) + // Simulates a KeyReference created before the backend reported per-key usage, i.e. one still + // holding the SQL migration's default of 0 ("not yet determined"). + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error + require.NoError(t, err) + + err = client.Migrate() + require.NoError(t, err) + + var keyRef orm.KeyReference + require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) + assert.Equal(t, allUsage, keyRef.KeyUsage) + }) + t.Run("sets key usage to sign-only for existing KeyReferences on the Azure Key Vault backend", func(t *testing.T) { + backend := NewMemoryStorage() + db := orm.NewTestDatabase(t) + client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} signOnlyUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage()) // Simulates a KeyReference created before the Azure Key Vault backend reported per-key usage, - // i.e. one still holding the SQL migration's default of "everything". - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1", KeyUsage: allUsage}).Error + // i.e. one still holding the SQL migration's default of 0 ("not yet determined"). + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error require.NoError(t, err) err = client.Migrate() diff --git a/storage/sql_migrations/012_key_reference_key_usage.sql b/storage/sql_migrations/012_key_reference_key_usage.sql index 22c9b461cc..c6834c2ea5 100644 --- a/storage/sql_migrations/012_key_reference_key_usage.sql +++ b/storage/sql_migrations/012_key_reference_key_usage.sql @@ -6,12 +6,11 @@ -- 0x04 - CapabilityDelegation -- 0x08 - CapabilityInvocation -- 0x10 - KeyAgreement --- Defaults to all flags (31): the common case, since fs/vault/external backends hand back plain, --- exportable EC keys that support every usage. crypto.Migrate() corrects existing rows to 15 --- (everything except KeyAgreement) for nodes configured with the Azure Key Vault backend, whose EC --- keys can only be used for signing. +-- Defaults to 0 ("not yet determined") rather than assuming every existing key supports every +-- usage: crypto.Migrate() sets it to the currently configured backend's usage for every row still +-- at 0, on every startup. alter table key_reference - add column key_usage SMALLINT not null default 31; + add column key_usage SMALLINT not null default 0; -- +goose Down alter table key_reference From 734822b92c1d048eb28df822bece441f423f822f Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 07:04:41 +0200 Subject: [PATCH 05/20] fix(crypto): make key_reference.key_usage NOT NULL with no default A DEFAULT clause on key_usage would let the database silently manufacture a value application code never actually chose - the exact kind of hidden default (did:nuts's DefaultKeyFlags() blindly assuming KeyAgreement) that caused this whole issue. crypto.Crypto.New() already always sets this value explicitly for every key it creates, so the only place that ever needs to fill one in is a one-time migration for rows that predate this column. Migration 012 is now a Go migration rather than a .sql file (matching Migration011CredentialPropValueType) so it can add the column nullable, backfill existing rows to 31 ("everything": what every key was assumed to support before this column existed) in one UPDATE, then set NOT NULL - something a single .sql statement can't do on a non-empty table. As before, that "everything" assumption is wrong for a backend like Azure Key Vault whose keys can't actually decrypt, so crypto.Migrate() still corrects it for that backend afterwards. SQLite has no ALTER COLUMN syntax, so it can't add the NOT NULL constraint after the fact; the column stays nullable there, same carve-out Migration011 already uses. Assisted by AI --- crypto/crypto.go | 19 +++-- crypto/crypto_test.go | 26 ++----- storage/engine.go | 5 +- .../012_key_reference_key_usage.go | 70 +++++++++++++++++++ .../012_key_reference_key_usage.sql | 17 ----- 5 files changed, 92 insertions(+), 45 deletions(-) create mode 100644 storage/sql_migrations/012_key_reference_key_usage.go delete mode 100644 storage/sql_migrations/012_key_reference_key_usage.sql diff --git a/crypto/crypto.go b/crypto/crypto.go index 8c87e1879f..331715bf31 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -225,12 +225,19 @@ func (client *Crypto) Migrate() error { } } } - // Set the usage for KeyReferences created before this backend reported per-key usage: the SQL - // migration defaults key_usage to 0 ("not yet determined") rather than assuming every existing - // key supports every usage. Every row still at 0 gets the currently configured backend's usage. - err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", orm.VerificationMethodKeyType(0)).Update("key_usage", keyUsage).Error - if err != nil { - return fmt.Errorf("could not set key usage for existing KeyReferences: %w", err) + if !capability.Is(spi.Decryption) { + // Correct KeyReferences created before this backend reported per-key usage: the migration + // that added key_usage backfilled existing rows to "everything" (what every key was + // assumed to support before that column existed), which is wrong for a backend, like + // Azure Key Vault, whose keys can't actually decrypt. Switching crypto storage backends + // for an existing node isn't supported (KeyName/Version are backend-specific and become + // unreachable), so every managed key under this backend is known to have been created by + // it, regardless of when the row was created. + allUsage := orm.VerificationMethodKeyType(keyUsageForCapability(spi.Signing | spi.Decryption)) + err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", allUsage).Update("key_usage", keyUsage).Error + if err != nil { + return fmt.Errorf("could not correct existing KeyReferences for the Azure Key Vault backend: %w", err) + } } return nil }) diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 61bfddc761..0dfdbd688b 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -100,31 +100,15 @@ func TestCrypto_Migrate(t *testing.T) { keys := client.List(context.Background()) require.Len(t, keys, 1) }) - t.Run("sets key usage for existing KeyReferences still at the SQL migration's default of 0", func(t *testing.T) { - backend := NewMemoryStorage() - db := orm.NewTestDatabase(t) - client := &Crypto{backend: backend, db: db} - allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) - // Simulates a KeyReference created before the backend reported per-key usage, i.e. one still - // holding the SQL migration's default of 0 ("not yet determined"). - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error - require.NoError(t, err) - - err = client.Migrate() - require.NoError(t, err) - - var keyRef orm.KeyReference - require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) - assert.Equal(t, allUsage, keyRef.KeyUsage) - }) - t.Run("sets key usage to sign-only for existing KeyReferences on the Azure Key Vault backend", func(t *testing.T) { + t.Run("corrects existing KeyReferences for the Azure Key Vault backend", func(t *testing.T) { backend := NewMemoryStorage() db := orm.NewTestDatabase(t) client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} + allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) signOnlyUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage()) - // Simulates a KeyReference created before the Azure Key Vault backend reported per-key usage, - // i.e. one still holding the SQL migration's default of 0 ("not yet determined"). - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error + // Simulates a KeyReference created before the Azure Key Vault backend reported per-key usage: + // the migration that added key_usage backfills existing rows to "everything". + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1", KeyUsage: allUsage}).Error require.NoError(t, err) err = client.Migrate() diff --git a/storage/engine.go b/storage/engine.go index b2ce9cdd15..955dd4c296 100644 --- a/storage/engine.go +++ b/storage/engine.go @@ -431,7 +431,10 @@ func (e *engine) initSQLDatabase(strictmode bool) error { return err } gooseProvider, err := goose.NewProvider(dialect, db, sql_migrations.SQLMigrationsFS, - goose.WithGoMigrations(sql_migrations.Migration011CredentialPropValueType(dbType)), + goose.WithGoMigrations( + sql_migrations.Migration011CredentialPropValueType(dbType), + sql_migrations.Migration012KeyReferenceKeyUsage(dbType), + ), ) if err != nil { return err diff --git a/storage/sql_migrations/012_key_reference_key_usage.go b/storage/sql_migrations/012_key_reference_key_usage.go new file mode 100644 index 0000000000..1be1df95dd --- /dev/null +++ b/storage/sql_migrations/012_key_reference_key_usage.go @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2026 Nuts community + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + * + */ + +package sql_migrations + +import ( + "context" + "database/sql" + + "github.com/pressly/goose/v3" +) + +// Migration012KeyReferenceKeyUsage returns the goose Go migration (version 12) that adds +// key_reference.key_usage: a bitmask of the DIDKeyFlags the key can actually be used for, using +// the same encoding as did_verification_method.key_types: +// +// 0x01 - AssertionMethod +// 0x02 - Authentication +// 0x04 - CapabilityDelegation +// 0x08 - CapabilityInvocation +// 0x10 - KeyAgreement +// +// This is a Go migration (rather than a .sql file) so the column can end up NOT NULL with no +// DEFAULT. A DEFAULT would let the database silently manufacture a value application code never +// actually chose - crypto.Crypto.New() always sets this value explicitly for every key it creates, +// so the only place that ever needs to fill in a value on its own is this one-time migration, +// backfilling rows that predate this column to 31 ("everything": what every key was assumed to +// support before this column existed). crypto.Crypto.Migrate() corrects that assumption afterwards +// for backends, like Azure Key Vault, whose keys can't actually do everything. +// +// SQLite has no ALTER COLUMN syntax, so it can't add the NOT NULL constraint to the existing column +// without rebuilding the whole table; the column stays nullable there, same carve-out as +// Migration011CredentialPropValueType. Application code still always writes a real value. +func Migration012KeyReferenceKeyUsage(dbType string) *goose.Migration { + return goose.NewGoMigration(12, + &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { + if _, err := tx.ExecContext(ctx, "alter table key_reference add column key_usage SMALLINT"); err != nil { + return err + } + if _, err := tx.ExecContext(ctx, "update key_reference set key_usage = 31"); err != nil { + return err + } + if dbType == "postgres" { + if _, err := tx.ExecContext(ctx, "alter table key_reference alter column key_usage set not null"); err != nil { + return err + } + } + return nil + }}, + &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, "alter table key_reference drop column key_usage") + return err + }}, + ) +} diff --git a/storage/sql_migrations/012_key_reference_key_usage.sql b/storage/sql_migrations/012_key_reference_key_usage.sql deleted file mode 100644 index c6834c2ea5..0000000000 --- a/storage/sql_migrations/012_key_reference_key_usage.sql +++ /dev/null @@ -1,17 +0,0 @@ --- +goose Up --- key_usage is a bitmask of the DIDKeyFlags the key can actually be used for, using the same --- encoding as did_verification_method.key_types: --- 0x01 - AssertionMethod --- 0x02 - Authentication --- 0x04 - CapabilityDelegation --- 0x08 - CapabilityInvocation --- 0x10 - KeyAgreement --- Defaults to 0 ("not yet determined") rather than assuming every existing key supports every --- usage: crypto.Migrate() sets it to the currently configured backend's usage for every row still --- at 0, on every startup. -alter table key_reference - add column key_usage SMALLINT not null default 0; - --- +goose Down -alter table key_reference - drop column key_usage; From 41ba62879e1e8f2d9d81b8e83e8eb3ccbcb9e6ea Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 10:15:30 +0200 Subject: [PATCH 06/20] fix(storage): cover mysql/sqlserver/azuresql in the key_usage migration, drop the lingering default Two fixes to migration 012: - It only branched on dbType == "postgres", missing that this node also supports mysql, sqlserver and azuresql as configured backends (see the dialect switch in storage/engine.go), same set Migration011 CredentialPropValueType already handles. Give it the same per-dialect statement map. - Simplify to two statements instead of three: ADD COLUMN ... NOT NULL needs a DEFAULT to backfill existing rows in the first place - there's no way around that in a single statement - so add it with DEFAULT 31, then drop the default immediately after so it doesn't linger for future inserts. Postgres and MySQL support dropping a column default directly; SQL Server ties a default to a separately named constraint, so adding the column now names that constraint explicitly so it can be dropped by name afterwards. SQLite has neither ALTER COLUMN nor DROP CONSTRAINT, so it keeps the default (harmless: crypto.Crypto.New() always writes a real value explicitly). Assisted by AI --- .../012_key_reference_key_usage.go | 65 ++++++++++++++----- 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/storage/sql_migrations/012_key_reference_key_usage.go b/storage/sql_migrations/012_key_reference_key_usage.go index 1be1df95dd..943c4c71d1 100644 --- a/storage/sql_migrations/012_key_reference_key_usage.go +++ b/storage/sql_migrations/012_key_reference_key_usage.go @@ -21,10 +21,49 @@ package sql_migrations import ( "context" "database/sql" + "fmt" "github.com/pressly/goose/v3" ) +// keyReferenceKeyUsage012 provides the per-database-type statements for adding +// key_reference.key_usage as NOT NULL with no default that lingers for future inserts, keyed by +// database type: +// +// - Adding a NOT NULL column to a non-empty table needs a DEFAULT to backfill existing rows with +// - there's no way around that in a single ADD COLUMN statement - so every dialect's addColumn +// statement backfills existing rows to 31 ("everything": what every key was assumed to support +// before this column existed) via `... NOT NULL DEFAULT 31`. +// - Postgres and MySQL can then drop that default again immediately afterward with a plain +// `ALTER COLUMN ... DROP DEFAULT`, so it doesn't linger for future inserts. +// - SQL Server ties a default to a separate, named constraint object rather than to the column +// itself, so dropping it means naming that constraint explicitly when adding the column, then +// dropping the constraint by name. +// - SQLite has no ALTER COLUMN or DROP CONSTRAINT syntax at all, so it's stuck with a permanent +// default. That's harmless in practice: crypto.Crypto.New() always writes a real value +// explicitly for every key it creates, so nothing ever relies on it. +var keyReferenceKeyUsage012 = map[string]struct{ addColumn, dropDefault string }{ + "sqlite": { + addColumn: "alter table key_reference add column key_usage SMALLINT not null default 31", + }, + "postgres": { + addColumn: "alter table key_reference add column key_usage SMALLINT not null default 31", + dropDefault: "alter table key_reference alter column key_usage drop default", + }, + "mysql": { + addColumn: "alter table key_reference add column key_usage SMALLINT not null default 31", + dropDefault: "alter table key_reference alter column key_usage drop default", + }, + "sqlserver": { + addColumn: "alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default 31", + dropDefault: "alter table key_reference drop constraint df_key_reference_key_usage", + }, + "azuresql": { + addColumn: "alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default 31", + dropDefault: "alter table key_reference drop constraint df_key_reference_key_usage", + }, +} + // Migration012KeyReferenceKeyUsage returns the goose Go migration (version 12) that adds // key_reference.key_usage: a bitmask of the DIDKeyFlags the key can actually be used for, using // the same encoding as did_verification_method.key_types: @@ -35,28 +74,22 @@ import ( // 0x08 - CapabilityInvocation // 0x10 - KeyAgreement // -// This is a Go migration (rather than a .sql file) so the column can end up NOT NULL with no -// DEFAULT. A DEFAULT would let the database silently manufacture a value application code never -// actually chose - crypto.Crypto.New() always sets this value explicitly for every key it creates, -// so the only place that ever needs to fill in a value on its own is this one-time migration, -// backfilling rows that predate this column to 31 ("everything": what every key was assumed to -// support before this column existed). crypto.Crypto.Migrate() corrects that assumption afterwards -// for backends, like Azure Key Vault, whose keys can't actually do everything. -// -// SQLite has no ALTER COLUMN syntax, so it can't add the NOT NULL constraint to the existing column -// without rebuilding the whole table; the column stays nullable there, same carve-out as -// Migration011CredentialPropValueType. Application code still always writes a real value. +// This is a Go migration (rather than a .sql file) because, like Migration011CredentialPropValueType, +// the required syntax differs per database (see keyReferenceKeyUsage012). crypto.Crypto.Migrate() +// corrects the "everything" backfill assumption afterwards for backends, like Azure Key Vault, whose +// keys can't actually do everything. func Migration012KeyReferenceKeyUsage(dbType string) *goose.Migration { + statements, ok := keyReferenceKeyUsage012[dbType] return goose.NewGoMigration(12, &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { - if _, err := tx.ExecContext(ctx, "alter table key_reference add column key_usage SMALLINT"); err != nil { - return err + if !ok { + return fmt.Errorf("unsupported database type: %s", dbType) } - if _, err := tx.ExecContext(ctx, "update key_reference set key_usage = 31"); err != nil { + if _, err := tx.ExecContext(ctx, statements.addColumn); err != nil { return err } - if dbType == "postgres" { - if _, err := tx.ExecContext(ctx, "alter table key_reference alter column key_usage set not null"); err != nil { + if statements.dropDefault != "" { + if _, err := tx.ExecContext(ctx, statements.dropDefault); err != nil { return err } } From da50419f5b1b56413f89184702e4990d17a50a1e Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 10:27:01 +0200 Subject: [PATCH 07/20] refactor(storage): simplify key_usage migration statement table to a switch Replace the map of {addColumn, dropDefault} structs (one empty field for SQLite) with a function returning an ordered []string of statements per database type, grouping the dialects that share identical statements (postgres/mysql; sqlserver/azuresql) into single switch cases instead of repeating the same SQL text under each dialect's key. Also names the magic backfill value (31) as keyReferenceKeyUsage012AllUsage, spelled out as the OR of the same bit values already documented on Migration012KeyReferenceKeyUsage. Assisted by AI --- .../012_key_reference_key_usage.go | 82 ++++++++++--------- 1 file changed, 42 insertions(+), 40 deletions(-) diff --git a/storage/sql_migrations/012_key_reference_key_usage.go b/storage/sql_migrations/012_key_reference_key_usage.go index 943c4c71d1..9b404a92a5 100644 --- a/storage/sql_migrations/012_key_reference_key_usage.go +++ b/storage/sql_migrations/012_key_reference_key_usage.go @@ -26,42 +26,50 @@ import ( "github.com/pressly/goose/v3" ) -// keyReferenceKeyUsage012 provides the per-database-type statements for adding -// key_reference.key_usage as NOT NULL with no default that lingers for future inserts, keyed by -// database type: +// keyReferenceKeyUsage012AllUsage is every DIDKeyFlags bit combined - AssertionMethod, Authentication, +// CapabilityDelegation, CapabilityInvocation and KeyAgreement, using the same bit values as +// did_verification_method.key_types (see Migration012KeyReferenceKeyUsage's doc comment). It's what +// every key was assumed to support before key_reference.key_usage existed, so it's the value used to +// backfill existing rows when the column is added. +const keyReferenceKeyUsage012AllUsage = 0x01 | 0x02 | 0x04 | 0x08 | 0x10 + +// keyReferenceKeyUsage012Statements returns the statements, run in order, that add +// key_reference.key_usage as NOT NULL with no default that lingers for future inserts, for the +// given database type: // // - Adding a NOT NULL column to a non-empty table needs a DEFAULT to backfill existing rows with -// - there's no way around that in a single ADD COLUMN statement - so every dialect's addColumn -// statement backfills existing rows to 31 ("everything": what every key was assumed to support -// before this column existed) via `... NOT NULL DEFAULT 31`. +// - there's no way around that in a single ADD COLUMN statement - so every dialect's first +// statement backfills existing rows to keyReferenceKeyUsage012AllUsage via +// `... NOT NULL DEFAULT `. // - Postgres and MySQL can then drop that default again immediately afterward with a plain // `ALTER COLUMN ... DROP DEFAULT`, so it doesn't linger for future inserts. // - SQL Server ties a default to a separate, named constraint object rather than to the column // itself, so dropping it means naming that constraint explicitly when adding the column, then // dropping the constraint by name. // - SQLite has no ALTER COLUMN or DROP CONSTRAINT syntax at all, so it's stuck with a permanent -// default. That's harmless in practice: crypto.Crypto.New() always writes a real value -// explicitly for every key it creates, so nothing ever relies on it. -var keyReferenceKeyUsage012 = map[string]struct{ addColumn, dropDefault string }{ - "sqlite": { - addColumn: "alter table key_reference add column key_usage SMALLINT not null default 31", - }, - "postgres": { - addColumn: "alter table key_reference add column key_usage SMALLINT not null default 31", - dropDefault: "alter table key_reference alter column key_usage drop default", - }, - "mysql": { - addColumn: "alter table key_reference add column key_usage SMALLINT not null default 31", - dropDefault: "alter table key_reference alter column key_usage drop default", - }, - "sqlserver": { - addColumn: "alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default 31", - dropDefault: "alter table key_reference drop constraint df_key_reference_key_usage", - }, - "azuresql": { - addColumn: "alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default 31", - dropDefault: "alter table key_reference drop constraint df_key_reference_key_usage", - }, +// default (a single statement, nothing to drop it with afterwards). That's harmless in +// practice: crypto.Crypto.New() always writes a real value explicitly for every key it +// creates, so nothing ever relies on it. +func keyReferenceKeyUsage012Statements(dbType string) []string { + switch dbType { + case "postgres", "mysql": + return []string{ + fmt.Sprintf("alter table key_reference add column key_usage SMALLINT not null default %d", keyReferenceKeyUsage012AllUsage), + "alter table key_reference alter column key_usage drop default", + } + case "sqlserver", "azuresql": + return []string{ + fmt.Sprintf("alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default %d", keyReferenceKeyUsage012AllUsage), + "alter table key_reference drop constraint df_key_reference_key_usage", + } + default: + // SQLite (and, defensively, anything else storage.Engine's own dialect switch didn't already + // reject before migrations ever run): just the DEFAULT-backfilling ADD COLUMN, nothing to + // drop the default with afterwards. + return []string{ + fmt.Sprintf("alter table key_reference add column key_usage SMALLINT not null default %d", keyReferenceKeyUsage012AllUsage), + } + } } // Migration012KeyReferenceKeyUsage returns the goose Go migration (version 12) that adds @@ -75,21 +83,15 @@ var keyReferenceKeyUsage012 = map[string]struct{ addColumn, dropDefault string } // 0x10 - KeyAgreement // // This is a Go migration (rather than a .sql file) because, like Migration011CredentialPropValueType, -// the required syntax differs per database (see keyReferenceKeyUsage012). crypto.Crypto.Migrate() -// corrects the "everything" backfill assumption afterwards for backends, like Azure Key Vault, whose -// keys can't actually do everything. +// the required syntax differs per database (see keyReferenceKeyUsage012Statements). +// crypto.Crypto.Migrate() corrects the "everything" backfill assumption afterwards for backends, +// like Azure Key Vault, whose keys can't actually do everything. func Migration012KeyReferenceKeyUsage(dbType string) *goose.Migration { - statements, ok := keyReferenceKeyUsage012[dbType] + statements := keyReferenceKeyUsage012Statements(dbType) return goose.NewGoMigration(12, &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { - if !ok { - return fmt.Errorf("unsupported database type: %s", dbType) - } - if _, err := tx.ExecContext(ctx, statements.addColumn); err != nil { - return err - } - if statements.dropDefault != "" { - if _, err := tx.ExecContext(ctx, statements.dropDefault); err != nil { + for _, statement := range statements { + if _, err := tx.ExecContext(ctx, statement); err != nil { return err } } From e279b9621f7b582793337f88585f41baf294e308 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 10:45:48 +0200 Subject: [PATCH 08/20] fix(crypto): use 0, not "everything", as the not-yet-migrated marker The migration backfilled existing key_reference rows to 31 ("everything"), and crypto.Crypto.Migrate() corrected rows still at 31 to sign-only for the Azure Key Vault backend. But 31 is also a perfectly valid, real usage value - a KeyReference already correctly set to "everything" (e.g. created under a different backend before a node was reconfigured to use Azure Key Vault) would be indistinguishable from one that just hadn't been migrated yet, and Migrate() would wrongly re-correct it every time it runs. Backfill to 0 instead: no real key usage can ever be 0, since every key supports at least AssertionKeyUsage, so it can never collide with an already-correct value. Migrate() now corrects any row still at 0 to the currently configured backend's real usage, unconditionally rather than only for Azure Key Vault, since the check no longer needs to guess which backend a "wrong" value came from. Assisted by AI --- crypto/crypto.go | 22 ++++----- crypto/crypto_test.go | 43 ++++++++++++++++-- .../012_key_reference_key_usage.go | 45 +++++++++---------- 3 files changed, 68 insertions(+), 42 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index 331715bf31..2e751a76f5 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -225,19 +225,15 @@ func (client *Crypto) Migrate() error { } } } - if !capability.Is(spi.Decryption) { - // Correct KeyReferences created before this backend reported per-key usage: the migration - // that added key_usage backfilled existing rows to "everything" (what every key was - // assumed to support before that column existed), which is wrong for a backend, like - // Azure Key Vault, whose keys can't actually decrypt. Switching crypto storage backends - // for an existing node isn't supported (KeyName/Version are backend-specific and become - // unreachable), so every managed key under this backend is known to have been created by - // it, regardless of when the row was created. - allUsage := orm.VerificationMethodKeyType(keyUsageForCapability(spi.Signing | spi.Decryption)) - err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", allUsage).Update("key_usage", keyUsage).Error - if err != nil { - return fmt.Errorf("could not correct existing KeyReferences for the Azure Key Vault backend: %w", err) - } + // Set the usage for KeyReferences created before this backend reported per-key usage: the + // migration that added key_usage backfills existing rows to 0, a value no real key usage can + // ever have (every key supports at least AssertionKeyUsage), so it unambiguously means "not + // yet set" rather than colliding with a real, already-corrected value. Switching crypto + // storage backends for an existing node isn't supported (KeyName/Version are backend-specific + // and become unreachable), so every managed key under this backend is known to have been + // created by it, regardless of when the row was created. + if err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", orm.VerificationMethodKeyType(0)).Update("key_usage", keyUsage).Error; err != nil { + return fmt.Errorf("could not set key usage for existing KeyReferences: %w", err) } return nil }) diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 0dfdbd688b..e79935a927 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -100,15 +100,31 @@ func TestCrypto_Migrate(t *testing.T) { keys := client.List(context.Background()) require.Len(t, keys, 1) }) - t.Run("corrects existing KeyReferences for the Azure Key Vault backend", func(t *testing.T) { + t.Run("sets key usage for existing KeyReferences not yet migrated", func(t *testing.T) { backend := NewMemoryStorage() db := orm.NewTestDatabase(t) - client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} + client := &Crypto{backend: backend, db: db} allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) + // Simulates a KeyReference created before this backend reported per-key usage: the migration + // that added key_usage backfills existing rows to 0 ("not yet migrated"). + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error + require.NoError(t, err) + + err = client.Migrate() + require.NoError(t, err) + + var keyRef orm.KeyReference + require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) + assert.Equal(t, allUsage, keyRef.KeyUsage) + }) + t.Run("sets key usage to sign-only for existing KeyReferences on the Azure Key Vault backend", func(t *testing.T) { + backend := NewMemoryStorage() + db := orm.NewTestDatabase(t) + client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} signOnlyUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage()) // Simulates a KeyReference created before the Azure Key Vault backend reported per-key usage: - // the migration that added key_usage backfills existing rows to "everything". - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1", KeyUsage: allUsage}).Error + // the migration that added key_usage backfills existing rows to 0 ("not yet migrated"). + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error require.NoError(t, err) err = client.Migrate() @@ -118,6 +134,25 @@ func TestCrypto_Migrate(t *testing.T) { require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) assert.Equal(t, signOnlyUsage, keyRef.KeyUsage) }) + t.Run("does not touch KeyReferences that already have a real usage", func(t *testing.T) { + backend := NewMemoryStorage() + db := orm.NewTestDatabase(t) + client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} + allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) + // A KeyReference already correctly set to "everything" (e.g. created under a different + // backend before this node was reconfigured to use Azure Key Vault) must not be + // re-corrected: 0, not a real usage value like "everything", is what marks a row as needing + // correction. + err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1", KeyUsage: allUsage}).Error + require.NoError(t, err) + + err = client.Migrate() + require.NoError(t, err) + + var keyRef orm.KeyReference + require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) + assert.Equal(t, allUsage, keyRef.KeyUsage) + }) } func TestCrypto_New(t *testing.T) { diff --git a/storage/sql_migrations/012_key_reference_key_usage.go b/storage/sql_migrations/012_key_reference_key_usage.go index 9b404a92a5..4c69ba12f3 100644 --- a/storage/sql_migrations/012_key_reference_key_usage.go +++ b/storage/sql_migrations/012_key_reference_key_usage.go @@ -26,49 +26,44 @@ import ( "github.com/pressly/goose/v3" ) -// keyReferenceKeyUsage012AllUsage is every DIDKeyFlags bit combined - AssertionMethod, Authentication, -// CapabilityDelegation, CapabilityInvocation and KeyAgreement, using the same bit values as -// did_verification_method.key_types (see Migration012KeyReferenceKeyUsage's doc comment). It's what -// every key was assumed to support before key_reference.key_usage existed, so it's the value used to -// backfill existing rows when the column is added. -const keyReferenceKeyUsage012AllUsage = 0x01 | 0x02 | 0x04 | 0x08 | 0x10 +// keyReferenceKeyUsage012NotYetMigrated is the value key_reference.key_usage is backfilled to for +// rows that predate that column. It's not a real DIDKeyFlags value - every actual key supports at +// least AssertionKeyUsage (0x0F), so 0 can never collide with a genuinely computed value - which is +// exactly why crypto.Crypto.Migrate() can safely use "still at 0" to mean "not yet corrected to the +// currently configured backend's real usage", without ever mistaking an already-corrected row (which +// could legitimately end up back at a "full usage" value) for one that still needs correcting. +const keyReferenceKeyUsage012NotYetMigrated = 0 // keyReferenceKeyUsage012Statements returns the statements, run in order, that add // key_reference.key_usage as NOT NULL with no default that lingers for future inserts, for the // given database type: // // - Adding a NOT NULL column to a non-empty table needs a DEFAULT to backfill existing rows with -// - there's no way around that in a single ADD COLUMN statement - so every dialect's first -// statement backfills existing rows to keyReferenceKeyUsage012AllUsage via +// - there's no way around that in a single ADD COLUMN statement - so every dialect's first +// statement backfills existing rows to keyReferenceKeyUsage012NotYetMigrated via // `... NOT NULL DEFAULT `. // - Postgres and MySQL can then drop that default again immediately afterward with a plain // `ALTER COLUMN ... DROP DEFAULT`, so it doesn't linger for future inserts. // - SQL Server ties a default to a separate, named constraint object rather than to the column // itself, so dropping it means naming that constraint explicitly when adding the column, then // dropping the constraint by name. -// - SQLite has no ALTER COLUMN or DROP CONSTRAINT syntax at all, so it's stuck with a permanent -// default (a single statement, nothing to drop it with afterwards). That's harmless in -// practice: crypto.Crypto.New() always writes a real value explicitly for every key it -// creates, so nothing ever relies on it. +// - SQLite (and, defensively, any other database type storage.Engine's own dialect switch didn't +// already reject before migrations ever run) has no ALTER COLUMN or DROP CONSTRAINT syntax at +// all, so it's stuck with a permanent default (a single statement, nothing to drop it with +// afterwards). That's harmless in practice: crypto.Crypto.New() always writes a real value +// explicitly for every key it creates, so nothing ever relies on it. func keyReferenceKeyUsage012Statements(dbType string) []string { + addColumn := fmt.Sprintf("alter table key_reference add column key_usage SMALLINT not null default %d", keyReferenceKeyUsage012NotYetMigrated) switch dbType { case "postgres", "mysql": - return []string{ - fmt.Sprintf("alter table key_reference add column key_usage SMALLINT not null default %d", keyReferenceKeyUsage012AllUsage), - "alter table key_reference alter column key_usage drop default", - } + return []string{addColumn, "alter table key_reference alter column key_usage drop default"} case "sqlserver", "azuresql": return []string{ - fmt.Sprintf("alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default %d", keyReferenceKeyUsage012AllUsage), + fmt.Sprintf("alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default %d", keyReferenceKeyUsage012NotYetMigrated), "alter table key_reference drop constraint df_key_reference_key_usage", } default: - // SQLite (and, defensively, anything else storage.Engine's own dialect switch didn't already - // reject before migrations ever run): just the DEFAULT-backfilling ADD COLUMN, nothing to - // drop the default with afterwards. - return []string{ - fmt.Sprintf("alter table key_reference add column key_usage SMALLINT not null default %d", keyReferenceKeyUsage012AllUsage), - } + return []string{addColumn} } } @@ -84,8 +79,8 @@ func keyReferenceKeyUsage012Statements(dbType string) []string { // // This is a Go migration (rather than a .sql file) because, like Migration011CredentialPropValueType, // the required syntax differs per database (see keyReferenceKeyUsage012Statements). -// crypto.Crypto.Migrate() corrects the "everything" backfill assumption afterwards for backends, -// like Azure Key Vault, whose keys can't actually do everything. +// crypto.Crypto.Migrate() sets the real value for existing rows this migration backfills to +// keyReferenceKeyUsage012NotYetMigrated, since only it knows the currently configured backend. func Migration012KeyReferenceKeyUsage(dbType string) *goose.Migration { statements := keyReferenceKeyUsage012Statements(dbType) return goose.NewGoMigration(12, From 5743a4bb83cad24e6587deb176bcf2ea416fe4f9 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 10:53:12 +0200 Subject: [PATCH 09/20] refactor(storage): drop the unneeded constant for the 0 marker 0 doesn't need a name the way the previous 31 backfill value did (that one only made sense spelled out as the OR of specific bit values); it's self-evidently "unset". Inline it and drop the now-unused fmt import. Assisted by AI --- .../012_key_reference_key_usage.go | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/storage/sql_migrations/012_key_reference_key_usage.go b/storage/sql_migrations/012_key_reference_key_usage.go index 4c69ba12f3..ab644cbf02 100644 --- a/storage/sql_migrations/012_key_reference_key_usage.go +++ b/storage/sql_migrations/012_key_reference_key_usage.go @@ -21,27 +21,20 @@ package sql_migrations import ( "context" "database/sql" - "fmt" "github.com/pressly/goose/v3" ) -// keyReferenceKeyUsage012NotYetMigrated is the value key_reference.key_usage is backfilled to for -// rows that predate that column. It's not a real DIDKeyFlags value - every actual key supports at -// least AssertionKeyUsage (0x0F), so 0 can never collide with a genuinely computed value - which is -// exactly why crypto.Crypto.Migrate() can safely use "still at 0" to mean "not yet corrected to the -// currently configured backend's real usage", without ever mistaking an already-corrected row (which -// could legitimately end up back at a "full usage" value) for one that still needs correcting. -const keyReferenceKeyUsage012NotYetMigrated = 0 - // keyReferenceKeyUsage012Statements returns the statements, run in order, that add // key_reference.key_usage as NOT NULL with no default that lingers for future inserts, for the // given database type: // // - Adding a NOT NULL column to a non-empty table needs a DEFAULT to backfill existing rows with // - there's no way around that in a single ADD COLUMN statement - so every dialect's first -// statement backfills existing rows to keyReferenceKeyUsage012NotYetMigrated via -// `... NOT NULL DEFAULT `. +// statement backfills existing rows to 0. That's not a real DIDKeyFlags value - every actual +// key supports at least AssertionKeyUsage (0x0F) - so crypto.Crypto.Migrate() can use "still at +// 0" to unambiguously mean "not yet corrected to the currently configured backend's real +// usage", without ever mistaking an already-corrected row for one that still needs correcting. // - Postgres and MySQL can then drop that default again immediately afterward with a plain // `ALTER COLUMN ... DROP DEFAULT`, so it doesn't linger for future inserts. // - SQL Server ties a default to a separate, named constraint object rather than to the column @@ -53,13 +46,13 @@ const keyReferenceKeyUsage012NotYetMigrated = 0 // afterwards). That's harmless in practice: crypto.Crypto.New() always writes a real value // explicitly for every key it creates, so nothing ever relies on it. func keyReferenceKeyUsage012Statements(dbType string) []string { - addColumn := fmt.Sprintf("alter table key_reference add column key_usage SMALLINT not null default %d", keyReferenceKeyUsage012NotYetMigrated) + const addColumn = "alter table key_reference add column key_usage SMALLINT not null default 0" switch dbType { case "postgres", "mysql": return []string{addColumn, "alter table key_reference alter column key_usage drop default"} case "sqlserver", "azuresql": return []string{ - fmt.Sprintf("alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default %d", keyReferenceKeyUsage012NotYetMigrated), + "alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default 0", "alter table key_reference drop constraint df_key_reference_key_usage", } default: @@ -79,8 +72,8 @@ func keyReferenceKeyUsage012Statements(dbType string) []string { // // This is a Go migration (rather than a .sql file) because, like Migration011CredentialPropValueType, // the required syntax differs per database (see keyReferenceKeyUsage012Statements). -// crypto.Crypto.Migrate() sets the real value for existing rows this migration backfills to -// keyReferenceKeyUsage012NotYetMigrated, since only it knows the currently configured backend. +// crypto.Crypto.Migrate() sets the real value for existing rows this migration backfills to 0, +// since only it knows the currently configured backend. func Migration012KeyReferenceKeyUsage(dbType string) *goose.Migration { statements := keyReferenceKeyUsage012Statements(dbType) return goose.NewGoMigration(12, From f013735b8c227d3d09c4301323a7a4f0b6b1f7f6 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 11:00:12 +0200 Subject: [PATCH 10/20] refactor(vdr): rename actualUsage to allowedKeyUsage Assisted by AI --- vdr/didnuts/manager.go | 4 ++-- vdr/didnuts/manager_test.go | 4 ++-- vdr/didweb/manager.go | 7 ++++--- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index 8bc6e06a1c..284b02ea1c 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -295,12 +295,12 @@ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDo func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, requestedFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { // did:nuts uses EC keys for everything, so it doesn't select a key type based on the requested DIDKeyFlags. - method, actualUsage, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) + method, allowedKeyUsage, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) if err != nil { return nil, 0, err } // Only grant what was requested AND what the key store backend can actually back. - return method, requestedFlags & actualUsage, nil + return method, requestedFlags & allowedKeyUsage, nil } func (m Manager) Commit(ctx context.Context, change orm.DIDChangeLog) error { diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index 786a7c72d4..0491b49356 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -350,10 +350,10 @@ func TestManager_NewDocument(t *testing.T) { assert.NotEmpty(t, generatedDoc.CapabilityInvocation) asDID := did.MustParseDID(doc.DID.ID) - _, actualUsage, err := signOnlyManager.NewVerificationMethod(ctx, asDID, orm.EncryptionKeyUsage()) + _, allowedKeyUsage, err := signOnlyManager.NewVerificationMethod(ctx, asDID, orm.EncryptionKeyUsage()) require.NoError(t, err) - assert.False(t, actualUsage.Is(orm.KeyAgreementUsage), + assert.False(t, allowedKeyUsage.Is(orm.KeyAgreementUsage), "requesting KeyAgreement for a new VerificationMethod must not be granted when the backend can't back it") }) } diff --git a/vdr/didweb/manager.go b/vdr/didweb/manager.go index 7ab6fb159b..a6c8d5b109 100644 --- a/vdr/didweb/manager.go +++ b/vdr/didweb/manager.go @@ -24,9 +24,10 @@ import ( "encoding/json" "errors" "fmt" + "time" + nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/storage/orm" - "time" "github.com/google/uuid" ssi "github.com/nuts-foundation/go-did" @@ -62,14 +63,14 @@ func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*or keyTypes := []orm.DIDKeyFlags{orm.AssertionKeyUsage(), orm.EncryptionKeyUsage()} for _, keyType := range keyTypes { if keyType.Is(keyFlags) { - verificationMethod, actualUsage, err := m.NewVerificationMethod(ctx, *newDID, keyType) + verificationMethod, allowedKeyUsage, err := m.NewVerificationMethod(ctx, *newDID, keyType) if err != nil { return nil, err } asJson, _ := json.Marshal(verificationMethod) sqlVerificationMethods = append(sqlVerificationMethods, orm.VerificationMethod{ ID: verificationMethod.ID.String(), - KeyTypes: orm.VerificationMethodKeyType(actualUsage), + KeyTypes: orm.VerificationMethodKeyType(allowedKeyUsage), Data: asJson, }) } From 88a331b260d147b1eafe674b03675f5e27ffff62 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 11:13:38 +0200 Subject: [PATCH 11/20] refactor(vdr): rename actualKeyFlags; let did:web report the flags it actually granted did:web's NewVerificationMethod always returned the caller's requested keyUsage unchanged, rather than what the generated key can actually back. It always generated an EC key via the shared crypto.KeyStore, so this made no practical difference for AssertionMethod-family flags (every backend can sign) - the pre-existing KeyAgreementUsage rejection in didsubject.SqlManager already short-circuits before this function is ever reached for that case - but it's now consistent with did:nuts, which does the same intersection against keyRef.KeyUsage. Also renames the local variable holding that value (allowedKeyUsage -> actualKeyFlags) in both did:nuts and did:web for consistency. Assisted by AI --- vdr/didnuts/manager.go | 4 ++-- vdr/didnuts/manager_test.go | 4 ++-- vdr/didweb/manager.go | 21 +++++---------------- 3 files changed, 9 insertions(+), 20 deletions(-) diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index 284b02ea1c..0335090982 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -295,12 +295,12 @@ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDo func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, requestedFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { // did:nuts uses EC keys for everything, so it doesn't select a key type based on the requested DIDKeyFlags. - method, allowedKeyUsage, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) + method, actualKeyFlags, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) if err != nil { return nil, 0, err } // Only grant what was requested AND what the key store backend can actually back. - return method, requestedFlags & allowedKeyUsage, nil + return method, requestedFlags & actualKeyFlags, nil } func (m Manager) Commit(ctx context.Context, change orm.DIDChangeLog) error { diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index 0491b49356..ad9db1f8a0 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -350,10 +350,10 @@ func TestManager_NewDocument(t *testing.T) { assert.NotEmpty(t, generatedDoc.CapabilityInvocation) asDID := did.MustParseDID(doc.DID.ID) - _, allowedKeyUsage, err := signOnlyManager.NewVerificationMethod(ctx, asDID, orm.EncryptionKeyUsage()) + _, actualKeyFlags, err := signOnlyManager.NewVerificationMethod(ctx, asDID, orm.EncryptionKeyUsage()) require.NoError(t, err) - assert.False(t, allowedKeyUsage.Is(orm.KeyAgreementUsage), + assert.False(t, actualKeyFlags.Is(orm.KeyAgreementUsage), "requesting KeyAgreement for a new VerificationMethod must not be granted when the backend can't back it") }) } diff --git a/vdr/didweb/manager.go b/vdr/didweb/manager.go index a6c8d5b109..8222a34f1d 100644 --- a/vdr/didweb/manager.go +++ b/vdr/didweb/manager.go @@ -22,7 +22,6 @@ import ( "context" "crypto" "encoding/json" - "errors" "fmt" "time" @@ -91,24 +90,14 @@ func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*or return &sqlDoc, nil } -func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, keyUsage orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { +func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, requestedKeyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { verificationMethodID := did.DIDURL{ DID: controller, Fragment: uuid.New().String(), } - var publicKey crypto.PublicKey - var err error - if keyUsage.Is(orm.KeyAgreementUsage) { - return nil, 0, errors.New("key agreement not supported for did:web") - // todo requires update to nutsCrypto module - //verificationMethodKey, err = m.keyStore.NewRSA(ctx, func(key crypt.PublicKey) (string, error) { - // return verificationMethodID.String(), nil - //}) - } else { - _, publicKey, err = m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { - return verificationMethodID.String(), nil - }) - } + keyRef, publicKey, err := m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { + return verificationMethodID.String(), nil + }) if err != nil { return nil, 0, err } @@ -117,7 +106,7 @@ func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, return nil, 0, err } - return verificationMethod, keyUsage, nil + return verificationMethod, orm.DIDKeyFlags(keyRef.KeyUsage) & requestedKeyFlags, nil } // Commit does nothing for did:web. This is important since only the one of the method managers may have a failing commit. From 26cf5aa3a92bde8845b3aef06ae150db146f99f5 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 12:46:36 +0200 Subject: [PATCH 12/20] refactor(vdr): simplify NewDocument/NewVerificationMethod for did:nuts DefaultKeyFlags() (AssertionKeyUsage|EncryptionKeyUsage) is every bit DIDKeyFlags has; ANDing it with keyRef.KeyUsage, itself always a subset of that same domain, can never remove anything, so it was a no-op. Also drops a comment left over from before NewVerificationMethod started intersecting requestedFlags with the backend's actual capability. Assisted by AI --- vdr/didnuts/manager.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index 0335090982..9fba6442dc 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -260,7 +260,7 @@ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDo } // Only claim the verification relationships (e.g. KeyAgreement) the key store backend can actually // back for this key; e.g. an Azure Key Vault EC key can't be used for KeyAgreement (decryption). - keyFlags := DefaultKeyFlags() & orm.DIDKeyFlags(keyRef.KeyUsage) + keyFlags := orm.DIDKeyFlags(keyRef.KeyUsage) keyID, err := did.ParseDIDURL(keyRef.KID) if err != nil { @@ -294,7 +294,6 @@ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDo } func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, requestedFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { - // did:nuts uses EC keys for everything, so it doesn't select a key type based on the requested DIDKeyFlags. method, actualKeyFlags, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) if err != nil { return nil, 0, err From 7ea274524606c9bfca4b1ed2a258924b363a1e0e Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 14:44:21 +0200 Subject: [PATCH 13/20] fix(crypto): key usage should also require signing capability keyUsageForCapability unconditionally granted AssertionKeyUsage regardless of whether the backend capability included Signing, breaking symmetry with the Decryption check right below it. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ji5QQpcavwWb54ygCddr1h --- crypto/crypto.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index 2e751a76f5..14585b3c45 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -23,13 +23,14 @@ import ( "crypto" "errors" "fmt" + "path" + "time" + "github.com/google/uuid" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/azure" "github.com/nuts-foundation/nuts-node/v6/storage" "github.com/nuts-foundation/nuts-node/v6/storage/orm" "gorm.io/gorm" - "path" - "time" "github.com/nuts-foundation/nuts-node/v6/audit" "github.com/nuts-foundation/nuts-node/v6/core" @@ -271,11 +272,14 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.K return ref, publicKey, err } -// keyUsageForCapability derives the DIDKeyFlags a key with the given KeyCapability can back. -// Every key can be used for signing (AssertionKeyUsage); only a key that also supports -// decryption/ECDH can additionally back KeyAgreement (EncryptionKeyUsage). +// keyUsageForCapability derives the DIDKeyFlags a key with the given KeyCapability can back: +// a key that can sign can back AssertionKeyUsage; a key that can decrypt/ECDH can back +// KeyAgreement (EncryptionKeyUsage). func keyUsageForCapability(capability spi.KeyCapability) orm.DIDKeyFlags { - keyUsage := orm.AssertionKeyUsage() + var keyUsage orm.DIDKeyFlags + if capability.Is(spi.Signing) { + keyUsage |= orm.AssertionKeyUsage() + } if capability.Is(spi.Decryption) { keyUsage |= orm.EncryptionKeyUsage() } From f51cab1c76e66549ce5785570570979270684c2f Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 15:25:57 +0200 Subject: [PATCH 14/20] refactor(crypto,vdr): fail fast on unsupported key usage instead of persisting it Nothing ever read key_reference.key_usage back from the DB after the KeyCreator.New() call that wrote it, so the migration, its 0-vs-real-value sentinel, and the Migrate() backfill logic were pure overhead. Replace all of it with a single check, before any key is created: crypto.Crypto.New() now takes the required DIDKeyFlags and compares them against what the configured backend can back (Azure Key Vault: signing only; every other backend: signing + decryption), refusing with ErrKeyUsageNotSupported if it can't fully back them. This also drops spi.KeyCapability from the backend interface entirely: what a backend can do turned out to be a static fact of which backend is configured, not something to discover per generated key. Since a successful New()/NewVerificationMethod() call now always grants exactly what was requested, the "achieved flags" return value that threaded through KeyCreator.New(), MethodManager.NewVerificationMethod() and AddVerificationMethod()'s intersection check is gone; those signatures are back to their pre-fix shape. Behavior change: a did:nuts document requires KeyAgreement on its single key, so did:nuts document creation now fails outright on Azure Key Vault instead of silently publishing a document without KeyAgreement. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ji5QQpcavwWb54ygCddr1h --- crypto/crypto.go | 64 +++++-------- crypto/crypto_test.go | 74 ++++----------- crypto/interface.go | 14 ++- crypto/mock.go | 16 ++-- crypto/storage/azure/keyvault.go | 12 +-- crypto/storage/azure/keyvault_test.go | 10 +- crypto/storage/external/client.go | 2 +- crypto/storage/fs/fs.go | 2 +- crypto/storage/spi/interface.go | 31 ++----- crypto/storage/spi/interface_test.go | 7 +- crypto/storage/spi/mock.go | 7 +- crypto/storage/spi/wrapper.go | 8 +- crypto/storage/vault/vault.go | 2 +- crypto/test.go | 14 ++- network/network_integration_test.go | 7 +- network/network_test.go | 19 ++-- storage/engine.go | 1 - storage/orm/key_reference.go | 4 - .../012_key_reference_key_usage.go | 93 ------------------- vcr/issuer/issuer_test.go | 12 +-- vcr/issuer/openid_test.go | 3 +- vcr/signature/json_web_signature_test.go | 3 +- vcr/signature/proof/jsonld_test.go | 3 +- vcr/test/test.go | 3 +- vcr/verifier/signature_verifier_test.go | 3 +- vdr/api/v1/api.go | 2 + vdr/api/v2/api.go | 2 + vdr/didnuts/ambassador_test.go | 13 ++- vdr/didnuts/manager.go | 36 ++++--- vdr/didnuts/manager_test.go | 51 +++------- vdr/didsubject/interface.go | 10 +- vdr/didsubject/manager.go | 4 +- vdr/didsubject/manager_test.go | 4 +- vdr/didweb/manager.go | 19 ++-- vdr/vdr_test.go | 8 +- 35 files changed, 190 insertions(+), 373 deletions(-) delete mode 100644 storage/sql_migrations/012_key_reference_key_usage.go diff --git a/crypto/crypto.go b/crypto/crypto.go index 14585b3c45..a4658b7a71 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -191,13 +191,6 @@ func (client *Crypto) Migrate() error { // else do nothing outerContext := context.TODO() - // Azure Key Vault EC keys can't be used for decryption; every other backend hands back plain, - // exportable EC keys that support both signing and decryption. - capability := spi.Signing | spi.Decryption - if client.config.Storage == azure.StorageType { - capability = spi.Signing - } - keyUsage := orm.VerificationMethodKeyType(keyUsageForCapability(capability)) // run everything in a single transaction // we do not expect to have a lot of keys, so this should be fine @@ -212,10 +205,9 @@ func (client *Crypto) Migrate() error { if errors.Is(err, gorm.ErrRecordNotFound) { // create a new key reference ref := &orm.KeyReference{ - KID: keyNameVersion.KeyName, - KeyName: keyNameVersion.KeyName, - Version: keyNameVersion.Version, - KeyUsage: keyUsage, + KID: keyNameVersion.KeyName, + KeyName: keyNameVersion.KeyName, + Version: keyNameVersion.Version, } err := tx.Save(ref).Error if err != nil { @@ -226,32 +218,28 @@ func (client *Crypto) Migrate() error { } } } - // Set the usage for KeyReferences created before this backend reported per-key usage: the - // migration that added key_usage backfills existing rows to 0, a value no real key usage can - // ever have (every key supports at least AssertionKeyUsage), so it unambiguously means "not - // yet set" rather than colliding with a real, already-corrected value. Switching crypto - // storage backends for an existing node isn't supported (KeyName/Version are backend-specific - // and become unreachable), so every managed key under this backend is known to have been - // created by it, regardless of when the row was created. - if err := tx.Model(&orm.KeyReference{}).Where("key_usage = ?", orm.VerificationMethodKeyType(0)).Update("key_usage", keyUsage).Error; err != nil { - return fmt.Errorf("could not set key usage for existing KeyReferences: %w", err) - } return nil }) } // New generates a new key pair. // Stores the private key, returns the public key and DB reference. -// It returns an error when a key with the resulting ID already exists. -func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +// requiredUsage is checked against supportedKeyUsage before any key is created: if the configured +// backend can't fully back it (e.g. Azure Key Vault can't back KeyAgreement, since it doesn't support +// decryption/ECDH), no key is created and ErrKeyUsageNotSupported is returned. +// It also returns an error when a key with the resulting ID already exists. +func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc, requiredUsage orm.DIDKeyFlags) (*orm.KeyReference, crypto.PublicKey, error) { + if supported := client.supportedKeyUsage(); requiredUsage&supported != requiredUsage { + return nil, nil, ErrKeyUsageNotSupported + } + var ref *orm.KeyReference var publicKey crypto.PublicKey err := client.continueTransaction(ctx, func(tx *gorm.DB) error { keyName := uuid.New().String() var err error var version string - var capability spi.KeyCapability - publicKey, version, capability, err = client.backend.NewPrivateKey(ctx, keyName) + publicKey, version, err = client.backend.NewPrivateKey(ctx, keyName) if err != nil { return err } @@ -261,10 +249,9 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.K return err } ref = &orm.KeyReference{ - KID: kid, - KeyName: keyName, - Version: version, - KeyUsage: orm.VerificationMethodKeyType(keyUsageForCapability(capability)), + KID: kid, + KeyName: keyName, + Version: version, } audit.Log(ctx, log.Logger(), audit.CryptoNewKeyEvent).Infof("Generated new key pair: %s", kid) return tx.Save(ref).Error @@ -272,18 +259,15 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.K return ref, publicKey, err } -// keyUsageForCapability derives the DIDKeyFlags a key with the given KeyCapability can back: -// a key that can sign can back AssertionKeyUsage; a key that can decrypt/ECDH can back -// KeyAgreement (EncryptionKeyUsage). -func keyUsageForCapability(capability spi.KeyCapability) orm.DIDKeyFlags { - var keyUsage orm.DIDKeyFlags - if capability.Is(spi.Signing) { - keyUsage |= orm.AssertionKeyUsage() - } - if capability.Is(spi.Decryption) { - keyUsage |= orm.EncryptionKeyUsage() +// supportedKeyUsage returns the DIDKeyFlags a key generated by the configured key store backend can +// back. Every backend can back AssertionKeyUsage (signing); only Azure Key Vault can't also back +// EncryptionKeyUsage (KeyAgreement), since it doesn't support decryption/ECDH with its EC keys. +func (client *Crypto) supportedKeyUsage() orm.DIDKeyFlags { + usage := orm.AssertionKeyUsage() + if client.config.Storage != azure.StorageType { + usage |= orm.EncryptionKeyUsage() } - return keyUsage + return usage } // Delete removes the private key with the given KID from the KeyStore. diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index e79935a927..029034bf6e 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -100,59 +100,6 @@ func TestCrypto_Migrate(t *testing.T) { keys := client.List(context.Background()) require.Len(t, keys, 1) }) - t.Run("sets key usage for existing KeyReferences not yet migrated", func(t *testing.T) { - backend := NewMemoryStorage() - db := orm.NewTestDatabase(t) - client := &Crypto{backend: backend, db: db} - allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) - // Simulates a KeyReference created before this backend reported per-key usage: the migration - // that added key_usage backfills existing rows to 0 ("not yet migrated"). - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error - require.NoError(t, err) - - err = client.Migrate() - require.NoError(t, err) - - var keyRef orm.KeyReference - require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) - assert.Equal(t, allUsage, keyRef.KeyUsage) - }) - t.Run("sets key usage to sign-only for existing KeyReferences on the Azure Key Vault backend", func(t *testing.T) { - backend := NewMemoryStorage() - db := orm.NewTestDatabase(t) - client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} - signOnlyUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage()) - // Simulates a KeyReference created before the Azure Key Vault backend reported per-key usage: - // the migration that added key_usage backfills existing rows to 0 ("not yet migrated"). - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1"}).Error - require.NoError(t, err) - - err = client.Migrate() - require.NoError(t, err) - - var keyRef orm.KeyReference - require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) - assert.Equal(t, signOnlyUsage, keyRef.KeyUsage) - }) - t.Run("does not touch KeyReferences that already have a real usage", func(t *testing.T) { - backend := NewMemoryStorage() - db := orm.NewTestDatabase(t) - client := &Crypto{backend: backend, db: db, config: Config{Storage: azure.StorageType}} - allUsage := orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()) - // A KeyReference already correctly set to "everything" (e.g. created under a different - // backend before this node was reconfigured to use Azure Key Vault) must not be - // re-corrected: 0, not a real usage value like "everything", is what marks a row as needing - // correction. - err := db.Save(&orm.KeyReference{KID: "vm-id", KeyName: "some-uuid", Version: "1", KeyUsage: allUsage}).Error - require.NoError(t, err) - - err = client.Migrate() - require.NoError(t, err) - - var keyRef orm.KeyReference - require.NoError(t, db.Where("kid = ?", "vm-id").First(&keyRef).Error) - assert.Equal(t, allUsage, keyRef.KeyUsage) - }) } func TestCrypto_New(t *testing.T) { @@ -163,16 +110,15 @@ func TestCrypto_New(t *testing.T) { t.Run("ok", func(t *testing.T) { auditLogs := audit.CaptureAuditLogs(t) - ref, pubKey, err := client.New(ctx, StringNamingFunc("kid")) + ref, pubKey, err := client.New(ctx, StringNamingFunc("kid"), orm.AssertionKeyUsage()|orm.EncryptionKeyUsage()) assert.NoError(t, err) assert.NotNil(t, ref) assert.NotNil(t, pubKey) - assert.Equal(t, orm.VerificationMethodKeyType(orm.AssertionKeyUsage()|orm.EncryptionKeyUsage()), ref.KeyUsage) auditLogs.AssertContains(t, ModuleName, "CreateNewKey", audit.TestActor, "Generated new key pair: "+ref.KID) }) t.Run("error - invalid naming function", func(t *testing.T) { - _, _, err := client.New(ctx, ErrorNamingFunc(assert.AnError)) + _, _, err := client.New(ctx, ErrorNamingFunc(assert.AnError), orm.AssertionKeyUsage()) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) @@ -180,15 +126,27 @@ func TestCrypto_New(t *testing.T) { t.Run("error from backend", func(t *testing.T) { ctrl := gomock.NewController(t) storageMock := spi.NewMockStorage(ctrl) - storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", spi.Signing, assert.AnError) + storageMock.EXPECT().NewPrivateKey(ctx, gomock.Any()).Return(nil, "", assert.AnError) client := createCrypto(t) client.backend = storageMock - _, _, err := client.New(ctx, StringNamingFunc("kid")) + _, _, err := client.New(ctx, StringNamingFunc("kid"), orm.AssertionKeyUsage()) require.Error(t, err) assert.ErrorIs(t, err, assert.AnError) }) + t.Run("required usage not supported by backend: no key is created", func(t *testing.T) { + ctrl := gomock.NewController(t) + storageMock := spi.NewMockStorage(ctrl) + // NewPrivateKey is deliberately not stubbed: it must not be called. + client := createCrypto(t) + client.backend = storageMock + client.config = Config{Storage: azure.StorageType} + + _, _, err := client.New(ctx, StringNamingFunc("kid"), orm.EncryptionKeyUsage()) + + assert.ErrorIs(t, err, ErrKeyUsageNotSupported) + }) } func TestCrypto_Delete(t *testing.T) { diff --git a/crypto/interface.go b/crypto/interface.go index ce673b45fb..eac526b1b1 100644 --- a/crypto/interface.go +++ b/crypto/interface.go @@ -29,6 +29,11 @@ import ( // ErrPrivateKeyNotFound is returned when the private key doesn't exist var ErrPrivateKeyNotFound = errors.New("private key not found") +// ErrKeyUsageNotSupported is returned when the configured key store backend can't create a key that +// backs the requested DIDKeyFlags, e.g. Azure Key Vault can't create keys usable for +// decryption/KeyAgreement. No key is created when this is returned. +var ErrKeyUsageNotSupported = errors.New("the key store can't create a key that supports the requested key usage") + // ErrorInvalidNumberOfSignatures indicates that the number of signatures present in the JWT is invalid. var ErrorInvalidNumberOfSignatures = errors.New("invalid number of signatures") @@ -40,10 +45,11 @@ type KeyCreator interface { // New generates a keypair and returns a reference. The context is used to pass audit information. // It generates a key at the backend and stores its reference in the SQL DB. // A DB transaction may be passed through the context using `orm.TransactionKey`. - // The returned KeyReference's KeyUsage reports the DIDKeyFlags the generated key can actually be - // used for, as reported by the storage backend, so callers don't add a verification method (e.g. - // KeyAgreement) the key can't back. - New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) + // requiredUsage is checked against what the configured key store backend can actually back (e.g. + // an Azure Key Vault EC key can't be used for KeyAgreement, since Azure Key Vault doesn't support + // decryption/ECDH with it) before any key is created. If the backend can't fully satisfy it, no + // key is created and ErrKeyUsageNotSupported is returned. + New(ctx context.Context, namingFunc KIDNamingFunc, requiredUsage orm.DIDKeyFlags) (*orm.KeyReference, crypto.PublicKey, error) } // KeyResolver is the interface for resolving keys. diff --git a/crypto/mock.go b/crypto/mock.go index 44046d64aa..16ad08ca34 100644 --- a/crypto/mock.go +++ b/crypto/mock.go @@ -44,9 +44,9 @@ func (m *MockKeyCreator) EXPECT() *MockKeyCreatorMockRecorder { } // New mocks base method. -func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc, requiredUsage orm.DIDKeyFlags) (*orm.KeyReference, crypto.PublicKey, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "New", ctx, namingFunc) + ret := m.ctrl.Call(m, "New", ctx, namingFunc, requiredUsage) ret0, _ := ret[0].(*orm.KeyReference) ret1, _ := ret[1].(crypto.PublicKey) ret2, _ := ret[2].(error) @@ -54,9 +54,9 @@ func (m *MockKeyCreator) New(ctx context.Context, namingFunc KIDNamingFunc) (*or } // New indicates an expected call of New. -func (mr *MockKeyCreatorMockRecorder) New(ctx, namingFunc any) *gomock.Call { +func (mr *MockKeyCreatorMockRecorder) New(ctx, namingFunc, requiredUsage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockKeyCreator)(nil).New), ctx, namingFunc) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockKeyCreator)(nil).New), ctx, namingFunc, requiredUsage) } // MockKeyResolver is a mock of KeyResolver interface. @@ -255,9 +255,9 @@ func (mr *MockKeyStoreMockRecorder) List(ctx any) *gomock.Call { } // New mocks base method. -func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc, requiredUsage orm.DIDKeyFlags) (*orm.KeyReference, crypto.PublicKey, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "New", ctx, namingFunc) + ret := m.ctrl.Call(m, "New", ctx, namingFunc, requiredUsage) ret0, _ := ret[0].(*orm.KeyReference) ret1, _ := ret[1].(crypto.PublicKey) ret2, _ := ret[2].(error) @@ -265,9 +265,9 @@ func (m *MockKeyStore) New(ctx context.Context, namingFunc KIDNamingFunc) (*orm. } // New indicates an expected call of New. -func (mr *MockKeyStoreMockRecorder) New(ctx, namingFunc any) *gomock.Call { +func (mr *MockKeyStoreMockRecorder) New(ctx, namingFunc, requiredUsage any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockKeyStore)(nil).New), ctx, namingFunc) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "New", reflect.TypeOf((*MockKeyStore)(nil).New), ctx, namingFunc, requiredUsage) } // Resolve mocks base method. diff --git a/crypto/storage/azure/keyvault.go b/crypto/storage/azure/keyvault.go index fdd934ee3f..de94eb67ad 100644 --- a/crypto/storage/azure/keyvault.go +++ b/crypto/storage/azure/keyvault.go @@ -102,9 +102,9 @@ func (a Keyvault) CheckHealth() map[string]core.Health { return nil } -// NewPrivateKey creates a new EC key in Azure Key Vault. It reports only spi.Signing: Azure Key -// Vault EC keys can only be used for signing, they can't be used for decryption/ECDH. -func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { +// NewPrivateKey creates a new EC key in Azure Key Vault. Azure Key Vault EC keys can only be used +// for signing, they can't be used for decryption/ECDH. +func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { var keyType azkeys.KeyType if a.useHSM { keyType = azkeys.KeyTypeECHSM @@ -121,13 +121,13 @@ func (a Keyvault) NewPrivateKey(ctx context.Context, keyName string) (crypto.Pub }, }, nil) if err != nil { - return nil, "", 0, fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) + return nil, "", fmt.Errorf("unable to create key in Azure Key Vault (name=%s): %w", keyName, err) } publicKey, _, version, err := parseKey(response.Key) if err != nil { - return nil, "", 0, err + return nil, "", err } - return publicKey, version, spi.Signing, nil + return publicKey, version, nil } func (a Keyvault) GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) { diff --git a/crypto/storage/azure/keyvault_test.go b/crypto/storage/azure/keyvault_test.go index 01a6837735..2dd583cb8b 100644 --- a/crypto/storage/azure/keyvault_test.go +++ b/crypto/storage/azure/keyvault_test.go @@ -66,7 +66,7 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { }) store := Keyvault{client: vaultClient} - privateKey, version, capability, err := store.NewPrivateKey(context.Background(), "did-web-example-com-0") + privateKey, version, err := store.NewPrivateKey(context.Background(), "did-web-example-com-0") require.NoError(t, err) assert.NotNil(t, privateKey) assert.Equal(t, "b86c2e6ad9054f4abf69cc185b99aa60", version) @@ -74,8 +74,6 @@ func Test_Keyvault_NewPrivateKey(t *testing.T) { assert.Equal(t, azkeys.CurveNameP256, *capturedParams.Curve) assert.True(t, *capturedParams.KeyAttributes.Enabled) assert.False(t, *capturedParams.KeyAttributes.Exportable) - // Azure Key Vault EC keys can sign, but not decrypt, so they can't back KeyAgreement. - assert.Equal(t, spi.Signing, capability) }) } @@ -280,14 +278,14 @@ func TestIntegrationTest(t *testing.T) { var keyName = uuid.NewString() ctx := context.Background() - _, version, _, err := store.NewPrivateKey(ctx, keyName) + _, version, err := store.NewPrivateKey(ctx, keyName) if !errors.Is(err, spi.ErrKeyAlreadyExists) { assert.NoError(t, err) } t.Run("New", func(t *testing.T) { t.Run("already exists", func(t *testing.T) { - _, _, _, err := store.NewPrivateKey(ctx, keyName) + _, _, err := store.NewPrivateKey(ctx, keyName) assert.ErrorIs(t, err, spi.ErrKeyAlreadyExists) }) }) @@ -330,7 +328,7 @@ func TestIntegrationTest(t *testing.T) { t.Run("DeletePrivateKey", func(t *testing.T) { t.Run("ok", func(t *testing.T) { otherKeyName := uuid.NewString() - _, version, _, err := store.NewPrivateKey(ctx, otherKeyName) + _, version, err := store.NewPrivateKey(ctx, otherKeyName) assert.NoError(t, err) err = store.DeletePrivateKey(ctx, otherKeyName) diff --git a/crypto/storage/external/client.go b/crypto/storage/external/client.go index a88bba2337..27d23d4a2f 100644 --- a/crypto/storage/external/client.go +++ b/crypto/storage/external/client.go @@ -44,7 +44,7 @@ type APIClient struct { httpClient *ClientWithResponses } -func (c APIClient) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { +func (c APIClient) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { return spi.GenerateAndStore(ctx, c, keyName) } diff --git a/crypto/storage/fs/fs.go b/crypto/storage/fs/fs.go index 50dbd2bfcb..f73662bffb 100644 --- a/crypto/storage/fs/fs.go +++ b/crypto/storage/fs/fs.go @@ -92,7 +92,7 @@ func NewFileSystemBackend(fspath string) (spi.Storage, error) { return fsc, nil } -func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { +func (fsc fileSystemBackend) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { return spi.GenerateAndStore(ctx, fsc, keyName) } diff --git a/crypto/storage/spi/interface.go b/crypto/storage/spi/interface.go index 0728790b13..5cfe7253e4 100644 --- a/crypto/storage/spi/interface.go +++ b/crypto/storage/spi/interface.go @@ -42,30 +42,13 @@ var ErrKeyAlreadyExists = errors.New("key already exists") // KidPattern is the regexp for acceptable kids var KidPattern = regexp.MustCompile(`^(?:(?:[\da-zA-Z_\- :#.])|(?:%[0-9a-fA-F]{2}))+$`) -// KeyCapability is a bitmask describing what a newly generated key can be used for. -type KeyCapability int - -const ( - // Signing means the key can be used for signing. Every generated key can do this. - Signing KeyCapability = 1 << iota - // Decryption means the key can be used for decryption/ECDH key agreement. E.g. an Azure Key - // Vault EC key can't do this: Azure Key Vault doesn't support decryption/ECDH with it. - Decryption -) - -// Is returns whether the specified KeyCapability is enabled. -func (k KeyCapability) Is(other KeyCapability) bool { - return k&other > 0 -} - // Storage interface containing functions for storing and retrieving keys. type Storage interface { core.HealthCheckable // NewPrivateKey creates a new private key. The backend will create the version and publicKey. // It should be preferred over generating a key in the application and saving it to the storage, // as it allows for unexportable (safer) keys. - // It also reports the KeyCapability of the generated key. - NewPrivateKey(ctx context.Context, keyName string) (publicKey crypto.PublicKey, version string, capability KeyCapability, err error) + NewPrivateKey(ctx context.Context, keyName string) (publicKey crypto.PublicKey, version string, err error) // GetPrivateKey from the storage backend and return its handler as an implementation of crypto.Signer. GetPrivateKey(ctx context.Context, keyName string, version string) (crypto.Signer, error) // PrivateKeyExists checks if the private key indicated with the keyname/version is stored in the storage backend. @@ -134,22 +117,22 @@ func (pke PublicKeyEntry) JWK() jwk.Key { // GenerateAndStore generates a new key pair and stores it in the provided storage. // It always generates a plain, exportable EC key, which can be used for both signing and decryption. -func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, KeyCapability, error) { +func GenerateAndStore(ctx context.Context, store Storage, keyName string) (crypto.PublicKey, string, error) { keyPair, err := GenerateKeyPair() if err != nil { - return nil, "", 0, err + return nil, "", err } exists, err := store.PrivateKeyExists(ctx, keyName, "1") if err != nil { - return nil, "", 0, fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) + return nil, "", fmt.Errorf("could not create new keypair: could not check if key already exists: %w", err) } if exists { - return nil, "", 0, errors.New("key with the given ID already exists") + return nil, "", errors.New("key with the given ID already exists") } if err = store.SavePrivateKey(ctx, keyName, keyPair); err != nil { - return nil, "", 0, fmt.Errorf("could not create new keypair: could not save private key: %w", err) + return nil, "", fmt.Errorf("could not create new keypair: could not save private key: %w", err) } - return keyPair.Public(), "1", Signing | Decryption, nil + return keyPair.Public(), "1", nil } // GenerateKeyPair generates a new key pair using the default key type. diff --git a/crypto/storage/spi/interface_test.go b/crypto/storage/spi/interface_test.go index 9e7d67ee08..8671d6ada7 100644 --- a/crypto/storage/spi/interface_test.go +++ b/crypto/storage/spi/interface_test.go @@ -70,12 +70,11 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(nil) keyName := "123" - key, version, capability, err := GenerateAndStore(ctx, store, keyName) + key, version, err := GenerateAndStore(ctx, store, keyName) assert.NoError(t, err) assert.NotNil(t, key) assert.Equal(t, "1", version) - assert.Equal(t, Signing|Decryption, capability) }) t.Run("error - save public key returns an error", func(t *testing.T) { @@ -85,7 +84,7 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().SavePrivateKey(ctx, gomock.Any(), gomock.Any()).Return(errors.New("foo")) keyName := "123" - _, _, _, err := GenerateAndStore(ctx, store, keyName) + _, _, err := GenerateAndStore(ctx, store, keyName) assert.ErrorContains(t, err, "could not create new keypair: could not save private key: foo") }) @@ -96,7 +95,7 @@ func TestGenerateAndStore(t *testing.T) { store.EXPECT().PrivateKeyExists(ctx, "123", "1").Return(true, nil) keyName := "123" - _, _, _, err := GenerateAndStore(ctx, store, keyName) + _, _, err := GenerateAndStore(ctx, store, keyName) 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 439cf42b5f..15d4214c15 100644 --- a/crypto/storage/spi/mock.go +++ b/crypto/storage/spi/mock.go @@ -114,14 +114,13 @@ func (mr *MockStorageMockRecorder) Name() *gomock.Call { } // NewPrivateKey mocks base method. -func (m *MockStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, KeyCapability, error) { +func (m *MockStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewPrivateKey", ctx, keyName) ret0, _ := ret[0].(crypto.PublicKey) ret1, _ := ret[1].(string) - ret2, _ := ret[2].(KeyCapability) - ret3, _ := ret[3].(error) - return ret0, ret1, ret2, ret3 + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 } // NewPrivateKey indicates an expected call of NewPrivateKey. diff --git a/crypto/storage/spi/wrapper.go b/crypto/storage/spi/wrapper.go index 4eee1bbb91..ea072d0162 100644 --- a/crypto/storage/spi/wrapper.go +++ b/crypto/storage/spi/wrapper.go @@ -89,10 +89,6 @@ func (w wrapper) ListPrivateKeys(ctx context.Context) []KeyNameVersion { return w.wrappedBackend.ListPrivateKeys(ctx) } -func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, KeyCapability, error) { - publicKey, version, capability, err := w.wrappedBackend.NewPrivateKey(ctx, keyName) - if err != nil { - return nil, "", 0, err - } - return publicKey, version, capability, err +func (w wrapper) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { + return w.wrappedBackend.NewPrivateKey(ctx, keyName) } diff --git a/crypto/storage/vault/vault.go b/crypto/storage/vault/vault.go index c9fe36c6bb..e78b057d66 100644 --- a/crypto/storage/vault/vault.go +++ b/crypto/storage/vault/vault.go @@ -107,7 +107,7 @@ func NewVaultKVStorage(config Config) (spi.Storage, error) { return vaultStorage, nil } -func (v vaultKVStorage) NewPrivateKey(ctx context.Context, keyPath string) (crypto.PublicKey, string, spi.KeyCapability, error) { +func (v vaultKVStorage) NewPrivateKey(ctx context.Context, keyPath string) (crypto.PublicKey, string, error) { return spi.GenerateAndStore(ctx, v, keyPath) } diff --git a/crypto/test.go b/crypto/test.go index 91da32e1cb..25e185b0d4 100644 --- a/crypto/test.go +++ b/crypto/test.go @@ -23,6 +23,7 @@ import ( "crypto" "github.com/nuts-foundation/nuts-node/v6/audit" "github.com/nuts-foundation/nuts-node/v6/core" + "github.com/nuts-foundation/nuts-node/v6/crypto/storage/azure" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/stretchr/testify/require" @@ -48,6 +49,15 @@ func NewTestCryptoInstance(db *gorm.DB, storage spi.Storage) *Crypto { return newInstance } +// NewAzureKeyVaultLikeCryptoInstance returns a Crypto test instance configured as if it were using +// the Azure Key Vault backend, without needing a real Azure connection: it can back signing, but not +// KeyAgreement (decryption/ECDH). +func NewAzureKeyVaultLikeCryptoInstance(db *gorm.DB) *Crypto { + newInstance := NewTestCryptoInstance(db, NewMemoryStorage()) + newInstance.config = Config{Storage: azure.StorageType} + return newInstance +} + func StringNamingFunc(name string) KIDNamingFunc { return func(key crypto.PublicKey) (string, error) { return name, nil @@ -68,7 +78,7 @@ var _ spi.Storage = &memoryStorage{} type memoryStorage map[string]crypto.PrivateKey -func (m memoryStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { +func (m memoryStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, error) { return spi.GenerateAndStore(ctx, m, keyName) } @@ -143,7 +153,7 @@ func (t TestKey) Private() crypto.PrivateKey { // newKeyReference creates a new DID, DIDocument, VerificationMethod and KeyReference in the DB // It does not create valid DID Document data func newKeyReference(t *testing.T, client *Crypto, kid string) (*orm.KeyReference, crypto.PublicKey) { - ref, publicKey, err := client.New(audit.TestContext(), StringNamingFunc(kid)) + ref, publicKey, err := client.New(audit.TestContext(), StringNamingFunc(kid), orm.AssertionKeyUsage()) require.NoError(t, err) DID := orm.DID{ID: "did:test:" + t.Name(), Subject: "subject"} DIDDoc := orm.DidDocument{ diff --git a/network/network_integration_test.go b/network/network_integration_test.go index 01481f711d..76c7df34be 100644 --- a/network/network_integration_test.go +++ b/network/network_integration_test.go @@ -50,6 +50,7 @@ import ( v2 "github.com/nuts-foundation/nuts-node/v6/network/transport/v2" "github.com/nuts-foundation/nuts-node/v6/pki" "github.com/nuts-foundation/nuts-node/v6/storage" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/test" "github.com/nuts-foundation/nuts-node/v6/test/io" "github.com/stretchr/testify/assert" @@ -201,7 +202,7 @@ func TestNetworkIntegration_Messages(t *testing.T) { }) // set root - _, key, _ := bootstrap.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("key1")) + _, key, _ := bootstrap.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("key1"), orm.AssertionKeyUsage()) rootTx, err := bootstrap.network.CreateTransaction(audit.TestContext(), TransactionTemplate(payloadType, []byte("root_tx"), "key1").WithAttachKey(key)) require.NoError(t, err) require.NoError(t, node1.network.state.Add(context.Background(), rootTx, []byte("root_tx"))) @@ -978,7 +979,7 @@ func resetIntegrationTest(t *testing.T) { kid.Fragment = "key-1" _, key, _ := keyStore.New(audit.TestContext(), func(_ crypto.PublicKey) (string, error) { return kid.String(), nil - }) + }, orm.AssertionKeyUsage()) verificationMethod, _ := did.NewVerificationMethod(kid, ssi.JsonWebKey2020, nodeDID, key) document.VerificationMethod.Add(verificationMethod) document.KeyAgreement.Add(verificationMethod) @@ -1024,7 +1025,7 @@ func addBootstrapDIDDocument(t *testing.T, n node, subject string) hash.SHA256Ha } func addTransactionAndWaitForItToArrive(t *testing.T, payload string, sender node, receivers ...string) bool { - keyRef, key, _ := sender.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(uuid.New().String())) + keyRef, key, _ := sender.network.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(uuid.New().String()), orm.AssertionKeyUsage()) expectedTransaction, err := sender.network.CreateTransaction(audit.TestContext(), TransactionTemplate(payloadType, []byte(payload), keyRef.KID).WithAttachKey(key)) if !assert.NoError(t, err) { return false diff --git a/network/network_test.go b/network/network_test.go index 7b13c68a8d..fffeddf9d8 100644 --- a/network/network_test.go +++ b/network/network_test.go @@ -49,6 +49,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/network/transport" "github.com/nuts-foundation/nuts-node/v6/pki" "github.com/nuts-foundation/nuts-node/v6/storage" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/test/io" testPKI "github.com/nuts-foundation/nuts-node/v6/test/pki" "github.com/nuts-foundation/nuts-node/v6/vdr/didnuts/didstore" @@ -412,7 +413,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) cxt.state.EXPECT().Head(gomock.Any()) cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) @@ -426,7 +427,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) cxt.state.EXPECT().Head(gomock.Any()) cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) @@ -452,7 +453,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt.state.EXPECT().Add(gomock.Any(), gomock.Any(), payload) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) tx, err := cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithAdditionalPrevs([]hash.SHA256Hash{additionalPrev.Ref()})) @@ -467,7 +468,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) // 'Register' prev on DAG prev, _, _ := dag.CreateTestTransaction(1) @@ -491,7 +492,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key"), orm.AssertionKeyUsage()) cxt.network.nodeDID = *nodeDID @@ -510,7 +511,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key")) + _, key, _ := cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("did:nuts:sender#signing-key"), orm.AssertionKeyUsage()) cxt.network.nodeDID = *nodeDID _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "did:nuts:sender#signing-key").WithAttachKey(key).WithPrivate([]did.DID{*sender, *receiver})) @@ -522,7 +523,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) cxt.network.nodeDID = *nodeDID _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithPrivate([]did.DID{*sender, *receiver})) @@ -534,7 +535,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt := createNetwork(t, ctrl) err := cxt.start() require.NoError(t, err) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) _, err = cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithPrivate([]did.DID{*sender, *receiver})) assert.EqualError(t, err, "node DID must be configured to create private transactions") @@ -552,7 +553,7 @@ func TestNetwork_CreateTransaction(t *testing.T) { cxt.state.EXPECT().GetTransaction(gomock.Any(), additionalPrev.Ref()).Return(additionalPrev, nil) cxt.state.EXPECT().IsPayloadPresent(gomock.Any(), additionalPrev.PayloadHash()).Return(true, nil) cxt.state.EXPECT().Head(gomock.Any()).Return(rootTX.Ref(), nil) - _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key")) + _, _, _ = cxt.keyStore.New(audit.TestContext(), crypto.StringNamingFunc("signing-key"), orm.AssertionKeyUsage()) _, err := cxt.network.CreateTransaction(ctx, TransactionTemplate(payloadType, payload, "signing-key").WithAdditionalPrevs([]hash.SHA256Hash{additionalPrev.Ref()})) diff --git a/storage/engine.go b/storage/engine.go index 955dd4c296..751bc064fa 100644 --- a/storage/engine.go +++ b/storage/engine.go @@ -433,7 +433,6 @@ func (e *engine) initSQLDatabase(strictmode bool) error { gooseProvider, err := goose.NewProvider(dialect, db, sql_migrations.SQLMigrationsFS, goose.WithGoMigrations( sql_migrations.Migration011CredentialPropValueType(dbType), - sql_migrations.Migration012KeyReferenceKeyUsage(dbType), ), ) if err != nil { diff --git a/storage/orm/key_reference.go b/storage/orm/key_reference.go index a4315b2f2f..f6dc930cf2 100644 --- a/storage/orm/key_reference.go +++ b/storage/orm/key_reference.go @@ -24,10 +24,6 @@ type KeyReference struct { KID string `gorm:"column:kid;primaryKey"` KeyName string Version string - // KeyUsage is the bitmask of DIDKeyFlags the key can actually be used for, as reported by the key - // store backend. E.g. an Azure Key Vault EC key can only be used for signing, not KeyAgreement, - // since Azure Key Vault doesn't support decryption/ECDH with it. - KeyUsage VerificationMethodKeyType } func (d KeyReference) TableName() string { diff --git a/storage/sql_migrations/012_key_reference_key_usage.go b/storage/sql_migrations/012_key_reference_key_usage.go deleted file mode 100644 index ab644cbf02..0000000000 --- a/storage/sql_migrations/012_key_reference_key_usage.go +++ /dev/null @@ -1,93 +0,0 @@ -/* - * Copyright (C) 2026 Nuts community - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see . - * - */ - -package sql_migrations - -import ( - "context" - "database/sql" - - "github.com/pressly/goose/v3" -) - -// keyReferenceKeyUsage012Statements returns the statements, run in order, that add -// key_reference.key_usage as NOT NULL with no default that lingers for future inserts, for the -// given database type: -// -// - Adding a NOT NULL column to a non-empty table needs a DEFAULT to backfill existing rows with -// - there's no way around that in a single ADD COLUMN statement - so every dialect's first -// statement backfills existing rows to 0. That's not a real DIDKeyFlags value - every actual -// key supports at least AssertionKeyUsage (0x0F) - so crypto.Crypto.Migrate() can use "still at -// 0" to unambiguously mean "not yet corrected to the currently configured backend's real -// usage", without ever mistaking an already-corrected row for one that still needs correcting. -// - Postgres and MySQL can then drop that default again immediately afterward with a plain -// `ALTER COLUMN ... DROP DEFAULT`, so it doesn't linger for future inserts. -// - SQL Server ties a default to a separate, named constraint object rather than to the column -// itself, so dropping it means naming that constraint explicitly when adding the column, then -// dropping the constraint by name. -// - SQLite (and, defensively, any other database type storage.Engine's own dialect switch didn't -// already reject before migrations ever run) has no ALTER COLUMN or DROP CONSTRAINT syntax at -// all, so it's stuck with a permanent default (a single statement, nothing to drop it with -// afterwards). That's harmless in practice: crypto.Crypto.New() always writes a real value -// explicitly for every key it creates, so nothing ever relies on it. -func keyReferenceKeyUsage012Statements(dbType string) []string { - const addColumn = "alter table key_reference add column key_usage SMALLINT not null default 0" - switch dbType { - case "postgres", "mysql": - return []string{addColumn, "alter table key_reference alter column key_usage drop default"} - case "sqlserver", "azuresql": - return []string{ - "alter table key_reference add key_usage SMALLINT not null constraint df_key_reference_key_usage default 0", - "alter table key_reference drop constraint df_key_reference_key_usage", - } - default: - return []string{addColumn} - } -} - -// Migration012KeyReferenceKeyUsage returns the goose Go migration (version 12) that adds -// key_reference.key_usage: a bitmask of the DIDKeyFlags the key can actually be used for, using -// the same encoding as did_verification_method.key_types: -// -// 0x01 - AssertionMethod -// 0x02 - Authentication -// 0x04 - CapabilityDelegation -// 0x08 - CapabilityInvocation -// 0x10 - KeyAgreement -// -// This is a Go migration (rather than a .sql file) because, like Migration011CredentialPropValueType, -// the required syntax differs per database (see keyReferenceKeyUsage012Statements). -// crypto.Crypto.Migrate() sets the real value for existing rows this migration backfills to 0, -// since only it knows the currently configured backend. -func Migration012KeyReferenceKeyUsage(dbType string) *goose.Migration { - statements := keyReferenceKeyUsage012Statements(dbType) - return goose.NewGoMigration(12, - &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { - for _, statement := range statements { - if _, err := tx.ExecContext(ctx, statement); err != nil { - return err - } - } - return nil - }}, - &goose.GoFunc{RunTx: func(ctx context.Context, tx *sql.Tx) error { - _, err := tx.ExecContext(ctx, "alter table key_reference drop column key_usage") - return err - }}, - ) -} diff --git a/vcr/issuer/issuer_test.go b/vcr/issuer/issuer_test.go index b9cd8cef16..1063252805 100644 --- a/vcr/issuer/issuer_test.go +++ b/vcr/issuer/issuer_test.go @@ -82,7 +82,7 @@ func Test_issuer_buildAndSignVC(t *testing.T) { }}, } keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(kid)) + _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(kid), orm.AssertionKeyUsage()) require.NoError(t, err) t.Run("JSON-LD", func(t *testing.T) { @@ -289,7 +289,7 @@ func Test_issuer_Issue(t *testing.T) { ctx := audit.TestContext() jsonldManager := jsonld.NewTestJSONLDManager(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - _, issuerKey, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc(issuerKeyID)) + _, issuerKey, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc(issuerKeyID), orm.AssertionKeyUsage()) t.Run("ok - unpublished", func(t *testing.T) { ctrl := gomock.NewController(t) @@ -550,7 +550,7 @@ func Test_issuer_buildRevocation(t *testing.T) { t.Run("ok", func(t *testing.T) { ctrl := gomock.NewController(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) + kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc"), orm.AssertionKeyUsage()) keyResolverMock := resolver.NewMockKeyResolver(ctrl) keyResolverMock.EXPECT().ResolveKey(issuerDID, nil, resolver.AssertionMethod).Return(kid.KID, key, nil) @@ -771,7 +771,7 @@ func Test_issuer_revokeNetwork(t *testing.T) { issuerURI := issuerDID.URI() jsonldManager := jsonld.NewTestJSONLDManager(t) nutsCryptoInstance := nutsCrypto.NewMemoryCryptoInstance(t) - kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc")) + kid, key, _ := nutsCryptoInstance.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:issuer#abc"), orm.AssertionKeyUsage()) ctx := audit.TestContext() t.Run("for a known credential", func(t *testing.T) { @@ -927,7 +927,7 @@ func TestIssuer_revokeStatusList(t *testing.T) { ctx := audit.TestContext() keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) + _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc"), orm.AssertionKeyUsage()) require.NoError(t, err) t.Run("ok", func(t *testing.T) { @@ -1052,7 +1052,7 @@ func TestIssuer_StatusList(t *testing.T) { ctx := audit.TestContext() db := orm.NewTestDatabase(t) keyStore := nutsCrypto.NewDatabaseCryptoInstance(db) - _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc")) + _, signingKey, err := keyStore.New(ctx, nutsCrypto.StringNamingFunc(webIssuerDID.String()+"#abc"), orm.AssertionKeyUsage()) require.NoError(t, err) jsonldManager := jsonld.NewTestJSONLDManager(t) diff --git a/vcr/issuer/openid_test.go b/vcr/issuer/openid_test.go index 593ea395a6..3388bcd40b 100644 --- a/vcr/issuer/openid_test.go +++ b/vcr/issuer/openid_test.go @@ -28,6 +28,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/core" "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/storage" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/vcr/openid4vci" "github.com/nuts-foundation/nuts-node/v6/vdr/resolver" "github.com/stretchr/testify/assert" @@ -117,7 +118,7 @@ func Test_memoryIssuer_ProviderMetadata(t *testing.T) { func Test_memoryIssuer_HandleCredentialRequest(t *testing.T) { keyStore := crypto.NewMemoryCryptoInstance(t) ctx := audit.TestContext() - _, signerKey, _ := keyStore.New(ctx, crypto.StringNamingFunc(keyID)) + _, signerKey, _ := keyStore.New(ctx, crypto.StringNamingFunc(keyID), orm.AssertionKeyUsage()) ctrl := gomock.NewController(t) keyResolver := resolver.NewMockKeyResolver(ctrl) keyResolver.EXPECT().ResolveKeyByID(keyID, nil, resolver.NutsSigningKeyType).AnyTimes().Return(signerKey, nil) diff --git a/vcr/signature/json_web_signature_test.go b/vcr/signature/json_web_signature_test.go index f5c1f73df4..6d04b32f74 100644 --- a/vcr/signature/json_web_signature_test.go +++ b/vcr/signature/json_web_signature_test.go @@ -26,6 +26,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/audit" "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/jsonld" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "testing" @@ -121,7 +122,7 @@ func TestJsonWebSignature2020_Sign(t *testing.T) { doc := []byte("foo") cryptoInstance := crypto.NewMemoryCryptoInstance(t) const keyID = "did:nuts:123#abc" - _, _, _ = cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(keyID)) + _, _, _ = cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(keyID), orm.AssertionKeyUsage()) sig := JSONWebSignature2020{Signer: cryptoInstance} result, err := sig.Sign(audit.TestContext(), doc, keyID) diff --git a/vcr/signature/proof/jsonld_test.go b/vcr/signature/proof/jsonld_test.go index 78f7f641b2..fcbae87b4a 100644 --- a/vcr/signature/proof/jsonld_test.go +++ b/vcr/signature/proof/jsonld_test.go @@ -30,6 +30,7 @@ import ( ssi "github.com/nuts-foundation/go-did" "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/nuts-node/v6/crypto" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/vcr/signature" "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" @@ -170,7 +171,7 @@ func TestLDProof_Sign(t *testing.T) { contextLoader := jsonld.NewTestJSONLDManager(t).DocumentLoader() cryptoInstance := crypto.NewMemoryCryptoInstance(t) - _, key, _ := cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(kid)) + _, key, _ := cryptoInstance.New(audit.TestContext(), crypto.StringNamingFunc(kid), orm.AssertionKeyUsage()) t.Run("sign and verify a document", func(t *testing.T) { now := time.Now() expires := now.Add(20 * time.Hour) diff --git a/vcr/test/test.go b/vcr/test/test.go index b7b52a41ef..6d9313fff8 100644 --- a/vcr/test/test.go +++ b/vcr/test/test.go @@ -29,6 +29,7 @@ import ( "github.com/nuts-foundation/nuts-node/v6/audit" nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/crypto/jwx" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/vcr/signature/proof" "github.com/stretchr/testify/require" "testing" @@ -58,7 +59,7 @@ func CreateJWTPresentation(t *testing.T, subjectDID did.DID, tokenVisitor func(t tokenVisitor(unsignedToken) } keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - _, key, err := keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(kid)) + _, key, err := keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(kid), orm.AssertionKeyUsage()) require.NoError(t, err) claims, err = jwx.ClaimsAsMap(unsignedToken) require.NoError(t, err) diff --git a/vcr/verifier/signature_verifier_test.go b/vcr/verifier/signature_verifier_test.go index 3527309638..7f07efc1c4 100644 --- a/vcr/verifier/signature_verifier_test.go +++ b/vcr/verifier/signature_verifier_test.go @@ -50,6 +50,7 @@ import ( nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/jsonld" + "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/vdr/didjwk" "github.com/nuts-foundation/nuts-node/v6/vdr/resolver" "github.com/stretchr/testify/assert" @@ -123,7 +124,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { keyAsJWK, _ := jwk.Import(key) keyJSON, _ := json.Marshal(keyAsJWK) return "did:jwk:" + base64.RawStdEncoding.EncodeToString(keyJSON) + "#0", nil - }) + }, orm.AssertionKeyUsage()) require.NoError(t, err) template := testCredential(t) diff --git a/vdr/api/v1/api.go b/vdr/api/v1/api.go index 90ad9b0afa..2fc6b8d929 100644 --- a/vdr/api/v1/api.go +++ b/vdr/api/v1/api.go @@ -24,6 +24,7 @@ import ( "errors" "fmt" "github.com/nuts-foundation/nuts-node/v6/audit" + nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/vdr" "github.com/nuts-foundation/nuts-node/v6/vdr/didnuts" "github.com/nuts-foundation/nuts-node/v6/vdr/didsubject" @@ -55,6 +56,7 @@ func (a *Wrapper) ResolveStatusCode(err error) int { resolver.ErrNoActiveController: http.StatusConflict, resolver.ErrDuplicateService: http.StatusBadRequest, did.ErrInvalidDID: http.StatusBadRequest, + nutsCrypto.ErrKeyUsageNotSupported: http.StatusBadRequest, }) } diff --git a/vdr/api/v2/api.go b/vdr/api/v2/api.go index b5d4ac135f..510068bd5d 100644 --- a/vdr/api/v2/api.go +++ b/vdr/api/v2/api.go @@ -27,6 +27,7 @@ import ( "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/nuts-node/v6/audit" "github.com/nuts-foundation/nuts-node/v6/core" + nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/http/cache" "github.com/nuts-foundation/nuts-node/v6/storage/orm" "github.com/nuts-foundation/nuts-node/v6/vdr" @@ -64,6 +65,7 @@ func (w *Wrapper) ResolveStatusCode(err error) int { didsubject.ErrInvalidService: http.StatusBadRequest, didsubject.ErrUnsupportedDIDMethod: http.StatusBadRequest, didsubject.ErrKeyAgreementNotSupported: http.StatusBadRequest, + nutsCrypto.ErrKeyUsageNotSupported: http.StatusBadRequest, didsubject.ErrSubjectValidation: http.StatusBadRequest, resolver.ErrDeactivated: http.StatusConflict, did.ErrInvalidService: http.StatusBadRequest, diff --git a/vdr/didnuts/ambassador_test.go b/vdr/didnuts/ambassador_test.go index 3d26b06efe..150d570c97 100644 --- a/vdr/didnuts/ambassador_test.go +++ b/vdr/didnuts/ambassador_test.go @@ -55,16 +55,15 @@ type mockKeyStore struct { } // New creates a new valid key with the correct KID -func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc) (*orm.KeyReference, crypto.PublicKey, error) { +func (m *mockKeyStore) New(_ context.Context, nf nutsCrypto.KIDNamingFunc, _ orm.DIDKeyFlags) (*orm.KeyReference, crypto.PublicKey, error) { if m.privateKey == nil { m.privateKey, _ = ecdsa.GenerateKey(elliptic.P256(), rand.Reader) kid, _ := nf(m.privateKey.PublicKey) m.keyReference = &orm.KeyReference{ - KID: kid, - KeyName: uuid.NewString(), - Version: uuid.NewString(), - KeyUsage: orm.VerificationMethodKeyType(orm.AssertionKeyUsage() | orm.EncryptionKeyUsage()), + KID: kid, + KeyName: uuid.NewString(), + Version: uuid.NewString(), } } return m.keyReference, m.privateKey.Public(), nil @@ -435,7 +434,7 @@ func TestAmbassador_handleUpdateDIDDocument(t *testing.T) { currentDoc, signingKey := newDidDoc(t) newDoc := did.Document{Context: []interface{}{did.DIDContextV1URI()}, ID: currentDoc.ID} - newCapInv, _, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}) + newCapInv, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}, orm.AssertionKeyUsage()) newDoc.AddCapabilityInvocation(newCapInv) didDocPayload, _ := json.Marshal(newDoc) @@ -470,7 +469,7 @@ func TestAmbassador_handleUpdateDIDDocument(t *testing.T) { currentDoc, signingKey := newDidDoc(t) newDoc := did.Document{Context: []interface{}{did.DIDContextV1URI()}, ID: currentDoc.ID} - newCapInv, _, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}) + newCapInv, _ := CreateNewVerificationMethodForDID(audit.TestContext(), currentDoc.ID, &mockKeyStore{}, orm.AssertionKeyUsage()) newDoc.AddCapabilityInvocation(newCapInv) didDocPayload, _ := json.Marshal(newDoc) diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index 9fba6442dc..cf0622902d 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -157,22 +157,22 @@ func (m Manager) RemoveVerificationMethod(ctx context.Context, id did.DID, keyID } // CreateNewVerificationMethodForDID creates a new VerificationMethod of type JsonWebKey2020 -// with a freshly generated key for a given DID. It also returns the DIDKeyFlags the key can actually -// be used for, as reported by the key store backend. -func CreateNewVerificationMethodForDID(ctx context.Context, id did.DID, keyCreator nutsCrypto.KeyCreator) (*did.VerificationMethod, orm.DIDKeyFlags, error) { - keyRef, publicKey, err := keyCreator.New(ctx, didSubKIDNamingFunc(id)) +// with a freshly generated key for a given DID. requiredUsage is checked against the key store +// backend's capability before any key is created; see nutsCrypto.KeyCreator.New. +func CreateNewVerificationMethodForDID(ctx context.Context, id did.DID, keyCreator nutsCrypto.KeyCreator, requiredUsage orm.DIDKeyFlags) (*did.VerificationMethod, error) { + keyRef, publicKey, err := keyCreator.New(ctx, didSubKIDNamingFunc(id), requiredUsage) if err != nil { - return nil, 0, err + return nil, err } keyID, err := did.ParseDIDURL(keyRef.KID) if err != nil { - return nil, 0, err + return nil, err } method, err := did.NewVerificationMethod(*keyID, ssi.JsonWebKey2020, id, publicKey) if err != nil { - return nil, 0, err + return nil, err } - return method, orm.DIDKeyFlags(keyRef.KeyUsage), nil + return method, nil } // Update updates a DID Document based on the DID. @@ -253,14 +253,17 @@ func (m Manager) Update(ctx context.Context, id did.DID, next did.Document) erro * New style DID Method Manager ******************************/ +// NewDocument creates a new did:nuts document backed by a single key that must support every +// relationship in DefaultKeyFlags: a did:nuts key backs all of a document's relationships, so a key +// store backend that can't back all of them (e.g. Azure Key Vault, which can't back KeyAgreement) +// can't back a did:nuts document at all. The requested keyFlags are ignored: did:nuts always requires +// DefaultKeyFlags, regardless of what a subject-level creation request asked for. func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDocument, error) { - keyRef, publicKey, err := m.keyStore.New(ctx, DIDKIDNamingFunc) + keyFlags := DefaultKeyFlags() + keyRef, publicKey, err := m.keyStore.New(ctx, DIDKIDNamingFunc, keyFlags) if err != nil { return nil, err } - // Only claim the verification relationships (e.g. KeyAgreement) the key store backend can actually - // back for this key; e.g. an Azure Key Vault EC key can't be used for KeyAgreement (decryption). - keyFlags := orm.DIDKeyFlags(keyRef.KeyUsage) keyID, err := did.ParseDIDURL(keyRef.KID) if err != nil { @@ -293,13 +296,8 @@ func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDo return &sqlDoc, nil } -func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, requestedFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { - method, actualKeyFlags, err := CreateNewVerificationMethodForDID(ctx, id, m.keyStore) - if err != nil { - return nil, 0, err - } - // Only grant what was requested AND what the key store backend can actually back. - return method, requestedFlags & actualKeyFlags, nil +func (m Manager) NewVerificationMethod(ctx context.Context, id did.DID, requestedFlags orm.DIDKeyFlags) (*did.VerificationMethod, error) { + return CreateNewVerificationMethodForDID(ctx, id, m.keyStore, requestedFlags) } func (m Manager) Commit(ctx context.Context, change orm.DIDChangeLog) error { diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index ad9db1f8a0..a583cbace3 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -37,7 +37,6 @@ import ( "github.com/nuts-foundation/nuts-node/v6/audit" nutsCrypto "github.com/nuts-foundation/nuts-node/v6/crypto" "github.com/nuts-foundation/nuts-node/v6/crypto/hash" - "github.com/nuts-foundation/nuts-node/v6/crypto/storage/spi" "github.com/nuts-foundation/nuts-node/v6/network" "github.com/nuts-foundation/nuts-node/v6/network/dag" "github.com/nuts-foundation/nuts-node/v6/storage" @@ -107,7 +106,7 @@ func TestManager_RemoveVerificationMethod(t *testing.T) { t.Run("ok", func(t *testing.T) { ctx := newTestContext(t) - _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) + _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String()), orm.AssertionKeyUsage()) doc1 := createDoc(pubKey) doc2 := createDoc(pubKey) ctx.didResolver.EXPECT().Resolve(*id123, &resolver.ResolveMetadata{AllowDeactivated: true}).Return(&doc1, &resolver.DocumentMetadata{}, nil) @@ -136,7 +135,7 @@ func TestManager_RemoveVerificationMethod(t *testing.T) { t.Run("error - document is deactivated", func(t *testing.T) { ctx := newTestContext(t) - _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String())) + _, pubKey, _ := ctx.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc(id123Method.String()), orm.AssertionKeyUsage()) doc1 := createDoc(pubKey) doc2 := createDoc(pubKey) ctx.didResolver.EXPECT().Resolve(*id123, &resolver.ResolveMetadata{AllowDeactivated: true}).Return(&doc1, &resolver.DocumentMetadata{Deactivated: true}, nil) @@ -156,7 +155,7 @@ func TestManager_CreateNewAuthenticationMethodForDID(t *testing.T) { t.Run("ok", func(t *testing.T) { // Prepare a document with an authenticationMethod: document := &did.Document{ID: *id123} - method, _, err := CreateNewVerificationMethodForDID(audit.TestContext(), document.ID, kc) + method, err := CreateNewVerificationMethodForDID(audit.TestContext(), document.ID, kc, orm.AssertionKeyUsage()) require.NoError(t, err) document.AddCapabilityInvocation(method) @@ -197,7 +196,7 @@ func TestManager_GenerateDocument(t *testing.T) { t.Run("additional verification method", func(t *testing.T) { asDID := did.MustParseDID(doc.DID.ID) - verificationMethod, _, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) + verificationMethod, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) require.NoError(t, err) @@ -318,7 +317,7 @@ func TestManager_NewDocument(t *testing.T) { t.Run("additional verification method", func(t *testing.T) { asDID := did.MustParseDID(doc.DID.ID) - verificationMethod, _, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) + verificationMethod, err := manager.NewVerificationMethod(ctx, asDID, orm.AssertionKeyUsage()) require.NoError(t, err) @@ -332,44 +331,24 @@ func TestManager_NewDocument(t *testing.T) { t.Run("key store backend can't back KeyAgreement", func(t *testing.T) { // Mimics an Azure Key Vault backend: it can generate EC keys, but they can't be used for - // decryption/ECDH, so a key it generates can't back a KeyAgreement verification method. - backend := signOnlyStorage{nutsCrypto.NewMemoryStorage()} - signOnlyKeyStore := nutsCrypto.NewTestCryptoInstance(db, backend) + // decryption/ECDH, so a key it generates can't back a KeyAgreement verification method. A + // did:nuts document always needs a key that backs every relationship, so document creation + // must fail outright rather than silently publish a document without KeyAgreement. + signOnlyKeyStore := nutsCrypto.NewAzureKeyVaultLikeCryptoInstance(db) signOnlyManager := NewManager(signOnlyKeyStore, nil, nil, nil, db) - doc, err := signOnlyManager.NewDocument(ctx, orm.AssertionKeyUsage()) + _, err := signOnlyManager.NewDocument(ctx, orm.AssertionKeyUsage()) - require.NoError(t, err) - require.Len(t, doc.VerificationMethods, 1) - assert.False(t, orm.DIDKeyFlags(doc.VerificationMethods[0].KeyTypes).Is(orm.KeyAgreementUsage), - "KeyAgreement must not be persisted when the key store backend can't back it") - - generatedDoc, err := doc.ToDIDDocument() - require.NoError(t, err) - assert.Empty(t, generatedDoc.KeyAgreement) - assert.NotEmpty(t, generatedDoc.CapabilityInvocation) + assert.ErrorIs(t, err, nutsCrypto.ErrKeyUsageNotSupported) - asDID := did.MustParseDID(doc.DID.ID) - _, actualKeyFlags, err := signOnlyManager.NewVerificationMethod(ctx, asDID, orm.EncryptionKeyUsage()) + t.Run("explicit VerificationMethod request", func(t *testing.T) { + _, err := signOnlyManager.NewVerificationMethod(ctx, did.MustParseDID("did:nuts:test"), orm.EncryptionKeyUsage()) - require.NoError(t, err) - assert.False(t, actualKeyFlags.Is(orm.KeyAgreementUsage), - "requesting KeyAgreement for a new VerificationMethod must not be granted when the backend can't back it") + assert.ErrorIs(t, err, nutsCrypto.ErrKeyUsageNotSupported) + }) }) } -// signOnlyStorage wraps a spi.Storage but reports that its generated keys can only be used for -// signing, mimicking an Azure Key Vault EC key: it can sign, but Azure Key Vault doesn't support -// decryption/ECDH with it, so it can't back a KeyAgreement verification method. -type signOnlyStorage struct { - spi.Storage -} - -func (s signOnlyStorage) NewPrivateKey(ctx context.Context, keyName string) (crypto.PublicKey, string, spi.KeyCapability, error) { - publicKey, version, _, err := s.Storage.NewPrivateKey(ctx, keyName) - return publicKey, version, spi.Signing, err -} - func TestManager_Commit(t *testing.T) { newEventLog := func(ctx *testContext) orm.DIDChangeLog { document := newDidDocWithStore(t, ctx.manager) diff --git a/vdr/didsubject/interface.go b/vdr/didsubject/interface.go index 86c3eaef0c..552e2a4970 100644 --- a/vdr/didsubject/interface.go +++ b/vdr/didsubject/interface.go @@ -52,11 +52,11 @@ type MethodManager interface { NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*orm.DidDocument, error) // NewVerificationMethod generates a new VerificationMethod for the given subject. // This is done by the method manager since the VM ID might depend on method specific rules. - // It also returns the DIDKeyFlags the VerificationMethod can actually be used for, which may be a - // subset of the requested keyFlags: the underlying key store backend might not support every - // requested usage for the generated key (e.g. an Azure Key Vault EC key can sign but can't back - // KeyAgreement, since Azure Key Vault doesn't support decryption/ECDH with it). - NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) + // keyFlags is a hard requirement: if the underlying key store backend can't back every requested + // usage for the generated key (e.g. an Azure Key Vault EC key can sign but can't back + // KeyAgreement, since Azure Key Vault doesn't support decryption/ECDH with it), no key is created + // and an error is returned. + NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, error) // Commit is called after changes are made to the primary db. // On success, the caller will remove/update the DID changelog. Commit(ctx context.Context, event orm.DIDChangeLog) error diff --git a/vdr/didsubject/manager.go b/vdr/didsubject/manager.go index 37fb9c5a26..32774d8c6f 100644 --- a/vdr/didsubject/manager.go +++ b/vdr/didsubject/manager.go @@ -402,7 +402,7 @@ func (r *SqlManager) AddVerificationMethod(ctx context.Context, subject string, } transactionContext := context.WithValue(ctx, storage.TransactionKey{}, tx) - vm, actualKeyUsage, err := r.MethodManagers[id.Method].NewVerificationMethod(transactionContext, id, keyUsage) + vm, err := r.MethodManagers[id.Method].NewVerificationMethod(transactionContext, id, keyUsage) if err != nil { return nil, err } @@ -410,7 +410,7 @@ func (r *SqlManager) AddVerificationMethod(ctx context.Context, subject string, data, _ := json.Marshal(*vm) sqlMethod := orm.VerificationMethod{ ID: vm.ID.String(), - KeyTypes: orm.VerificationMethodKeyType(actualKeyUsage), + KeyTypes: orm.VerificationMethodKeyType(keyUsage), Data: data, } current.VerificationMethods = append(current.VerificationMethods, sqlMethod) diff --git a/vdr/didsubject/manager_test.go b/vdr/didsubject/manager_test.go index 3a27a0608b..cc14f0a6e7 100644 --- a/vdr/didsubject/manager_test.go +++ b/vdr/didsubject/manager_test.go @@ -473,10 +473,10 @@ func (t testMethod) NewDocument(_ context.Context, _ orm.DIDKeyFlags) (*orm.DidD return &orm.DidDocument{DID: orm.DID{ID: id}}, t.error } -func (t testMethod) NewVerificationMethod(_ context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { +func (t testMethod) NewVerificationMethod(_ context.Context, controller did.DID, _ orm.DIDKeyFlags) (*did.VerificationMethod, error) { return &did.VerificationMethod{ ID: did.MustParseDIDURL(fmt.Sprintf("%s#%s", controller.String(), uuid.New().String())), - }, keyFlags, t.error + }, t.error } func (t testMethod) Commit(_ context.Context, _ orm.DIDChangeLog) error { diff --git a/vdr/didweb/manager.go b/vdr/didweb/manager.go index 8222a34f1d..44b49067d5 100644 --- a/vdr/didweb/manager.go +++ b/vdr/didweb/manager.go @@ -62,14 +62,14 @@ func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*or keyTypes := []orm.DIDKeyFlags{orm.AssertionKeyUsage(), orm.EncryptionKeyUsage()} for _, keyType := range keyTypes { if keyType.Is(keyFlags) { - verificationMethod, allowedKeyUsage, err := m.NewVerificationMethod(ctx, *newDID, keyType) + verificationMethod, err := m.NewVerificationMethod(ctx, *newDID, keyType) if err != nil { return nil, err } asJson, _ := json.Marshal(verificationMethod) sqlVerificationMethods = append(sqlVerificationMethods, orm.VerificationMethod{ ID: verificationMethod.ID.String(), - KeyTypes: orm.VerificationMethodKeyType(allowedKeyUsage), + KeyTypes: orm.VerificationMethodKeyType(keyType), Data: asJson, }) } @@ -90,23 +90,18 @@ func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*or return &sqlDoc, nil } -func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, requestedKeyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { +func (m Manager) NewVerificationMethod(ctx context.Context, controller did.DID, requestedKeyFlags orm.DIDKeyFlags) (*did.VerificationMethod, error) { verificationMethodID := did.DIDURL{ DID: controller, Fragment: uuid.New().String(), } - keyRef, publicKey, err := m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { + _, publicKey, err := m.keyStore.New(ctx, func(key crypto.PublicKey) (string, error) { return verificationMethodID.String(), nil - }) + }, requestedKeyFlags) if err != nil { - return nil, 0, err + return nil, err } - verificationMethod, err := did.NewVerificationMethod(verificationMethodID, ssi.JsonWebKey2020, controller, publicKey) - if err != nil { - return nil, 0, err - } - - return verificationMethod, orm.DIDKeyFlags(keyRef.KeyUsage) & requestedKeyFlags, nil + return did.NewVerificationMethod(verificationMethodID, ssi.JsonWebKey2020, controller, publicKey) } // Commit does nothing for did:web. This is important since only the one of the method managers may have a failing commit. diff --git a/vdr/vdr_test.go b/vdr/vdr_test.go index 89c7153322..248b78d750 100644 --- a/vdr/vdr_test.go +++ b/vdr/vdr_test.go @@ -140,7 +140,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { client := nutsCrypto.NewDatabaseCryptoInstance(db) keyID := did.DIDURL{DID: TestDIDA} keyID.Fragment = "1" - _, _, _ = client.New(audit.TestContext(), nutsCrypto.StringNamingFunc(keyID.String())) + _, _, _ = client.New(audit.TestContext(), nutsCrypto.StringNamingFunc(keyID.String()), orm.AssertionKeyUsage()) ctrl := gomock.NewController(t) pkiMock := pki.NewMockValidator(ctrl) vdr := NewVDR(client, nil, didstore.NewTestStore(t), nil, storageEngine, pkiMock) @@ -160,7 +160,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { t.Run("ok - 1 owned conflict in controlled document", func(t *testing.T) { // vendor test := newVDRTestCtx(t) - _, keyVendor, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:vendor#keyVendor-1")) + _, keyVendor, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:vendor#keyVendor-1"), orm.AssertionKeyUsage()) didDocVendor := &did.Document{ID: did.MustParseDID("did:nuts:vendor")} vendorVM, err := did.NewVerificationMethod(did.MustParseDIDURL("did:nuts:vendor#keyVendor-1"), ssi.JsonWebKey2020, didDocVendor.ID, keyVendor) @@ -168,7 +168,7 @@ func TestVDR_ConflictingDocuments(t *testing.T) { didDocVendor.AddCapabilityInvocation(vendorVM) // organization - _, keyOrg, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:org#keyOrg-1")) + _, keyOrg, _ := test.keyStore.New(audit.TestContext(), nutsCrypto.StringNamingFunc("did:nuts:org#keyOrg-1"), orm.AssertionKeyUsage()) didDocOrg := &did.Document{ID: did.MustParseDID("did:nuts:org")} didDocOrg.Controller = []did.DID{didDocVendor.ID} orgVM, err := did.NewVerificationMethod(did.MustParseDIDURL("did:nuts:org#keyOrg-1"), ssi.JsonWebKey2020, didDocOrg.ID, keyOrg) @@ -343,7 +343,7 @@ func TestVDR_Migrate(t *testing.T) { t.Run("makes documents self-controlled", func(t *testing.T) { ctx := controllerMigrationSetup(t) keyStore := nutsCrypto.NewMemoryCryptoInstance(t) - keyRef, publicKey, err := keyStore.New(ctx.ctx, didnuts.DIDKIDNamingFunc) + keyRef, publicKey, err := keyStore.New(ctx.ctx, didnuts.DIDKIDNamingFunc, orm.AssertionKeyUsage()) require.NoError(t, err) methodID := did.MustParseDIDURL(keyRef.KID) methodID.ID = TestDIDA.ID From 6fbd4d3d2a482939a67d54a31ed0ac765e27d52f Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 16:05:43 +0200 Subject: [PATCH 15/20] fix(vdr): regenerate stale MockMethodManager, tidy migration formatting MockMethodManager.NewVerificationMethod still had the pre-refactor 3-return signature; nothing currently uses it as a didsubject.MethodManager so it didn't fail the build, but it was drifted, broken generated code. Regenerated via mockgen. Also collapses a goose.WithGoMigrations() call back to one line now that it only takes a single argument again, and notes on Crypto.supportedKeyUsage() why it's deliberately not backend-adapter-specific today. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ji5QQpcavwWb54ygCddr1h --- crypto/crypto.go | 3 +++ storage/engine.go | 4 +--- vdr/didsubject/mock.go | 7 +++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crypto/crypto.go b/crypto/crypto.go index a4658b7a71..e351d3cd33 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -262,6 +262,9 @@ func (client *Crypto) New(ctx context.Context, namingFunc KIDNamingFunc, require // supportedKeyUsage returns the DIDKeyFlags a key generated by the configured key store backend can // back. Every backend can back AssertionKeyUsage (signing); only Azure Key Vault can't also back // EncryptionKeyUsage (KeyAgreement), since it doesn't support decryption/ECDH with its EC keys. +// This is a simple, static, per-backend-type fact today. If RSA key support is ever added (Azure Key +// Vault RSA keys can do decryption, unlike its EC keys), this would need to become a decision that +// also depends on key type, at which point it likely belongs in the backend adapter instead. func (client *Crypto) supportedKeyUsage() orm.DIDKeyFlags { usage := orm.AssertionKeyUsage() if client.config.Storage != azure.StorageType { diff --git a/storage/engine.go b/storage/engine.go index 751bc064fa..b2ce9cdd15 100644 --- a/storage/engine.go +++ b/storage/engine.go @@ -431,9 +431,7 @@ func (e *engine) initSQLDatabase(strictmode bool) error { return err } gooseProvider, err := goose.NewProvider(dialect, db, sql_migrations.SQLMigrationsFS, - goose.WithGoMigrations( - sql_migrations.Migration011CredentialPropValueType(dbType), - ), + goose.WithGoMigrations(sql_migrations.Migration011CredentialPropValueType(dbType)), ) if err != nil { return err diff --git a/vdr/didsubject/mock.go b/vdr/didsubject/mock.go index 4b68afcd47..f64ce1591d 100644 --- a/vdr/didsubject/mock.go +++ b/vdr/didsubject/mock.go @@ -88,13 +88,12 @@ func (mr *MockMethodManagerMockRecorder) NewDocument(ctx, keyFlags any) *gomock. } // NewVerificationMethod mocks base method. -func (m *MockMethodManager) NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, orm.DIDKeyFlags, error) { +func (m *MockMethodManager) NewVerificationMethod(ctx context.Context, controller did.DID, keyFlags orm.DIDKeyFlags) (*did.VerificationMethod, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "NewVerificationMethod", ctx, controller, keyFlags) ret0, _ := ret[0].(*did.VerificationMethod) - ret1, _ := ret[1].(orm.DIDKeyFlags) - ret2, _ := ret[2].(error) - return ret0, ret1, ret2 + ret1, _ := ret[1].(error) + return ret0, ret1 } // NewVerificationMethod indicates an expected call of NewVerificationMethod. From d9a5c4f7a79b2a2f9c8ed3686d0f67e09581e1ea Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Fri, 11 Sep 2026 16:14:14 +0200 Subject: [PATCH 16/20] fix(vdr): stop forcing DefaultKeyFlags on did:nuts document creation didnuts.Manager.NewDocument ignored its keyFlags argument and always requested DefaultKeyFlags() (including KeyAgreement), regardless of what the caller actually asked for. With OpenID4VCI as an alternative to gRPC/DAG private-transaction delivery, a did:nuts document no longer strictly needs KeyAgreement, so it shouldn't be forced on backends that can't back it (e.g. Azure Key Vault) when nobody asked for it. NewDocument now honors keyFlags like didweb.Manager.NewDocument already did: the default subject-creation request (AssertionKeyUsage only, unless EncryptionKeyCreationOption is given) now reaches did:nuts too, so default did:nuts document creation on Azure Key Vault succeeds. An explicit request for KeyAgreement against a backend that can't back it still fails loudly via ErrKeyUsageNotSupported. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Ji5QQpcavwWb54ygCddr1h --- vdr/didnuts/manager.go | 13 ++++++------- vdr/didnuts/manager_test.go | 20 +++++++++++++++----- vdr/didsubject/interface.go | 1 + 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/vdr/didnuts/manager.go b/vdr/didnuts/manager.go index cf0622902d..e4b2739e39 100644 --- a/vdr/didnuts/manager.go +++ b/vdr/didnuts/manager.go @@ -253,13 +253,12 @@ func (m Manager) Update(ctx context.Context, id did.DID, next did.Document) erro * New style DID Method Manager ******************************/ -// NewDocument creates a new did:nuts document backed by a single key that must support every -// relationship in DefaultKeyFlags: a did:nuts key backs all of a document's relationships, so a key -// store backend that can't back all of them (e.g. Azure Key Vault, which can't back KeyAgreement) -// can't back a did:nuts document at all. The requested keyFlags are ignored: did:nuts always requires -// DefaultKeyFlags, regardless of what a subject-level creation request asked for. -func (m Manager) NewDocument(ctx context.Context, _ orm.DIDKeyFlags) (*orm.DidDocument, error) { - keyFlags := DefaultKeyFlags() +// NewDocument creates a new did:nuts document backed by a single key that backs every relationship +// in keyFlags. If the key store backend can't back a requested relationship (e.g. Azure Key Vault +// can't back KeyAgreement), no key is created and an error is returned: with OpenID4VCI as an +// alternative to gRPC/DAG private-transaction delivery, a did:nuts document no longer strictly needs +// KeyAgreement, so callers that don't need it shouldn't request it (see EncryptionKeyCreationOption). +func (m Manager) NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*orm.DidDocument, error) { keyRef, publicKey, err := m.keyStore.New(ctx, DIDKIDNamingFunc, keyFlags) if err != nil { return nil, err diff --git a/vdr/didnuts/manager_test.go b/vdr/didnuts/manager_test.go index a583cbace3..3042aff5b1 100644 --- a/vdr/didnuts/manager_test.go +++ b/vdr/didnuts/manager_test.go @@ -331,15 +331,25 @@ func TestManager_NewDocument(t *testing.T) { t.Run("key store backend can't back KeyAgreement", func(t *testing.T) { // Mimics an Azure Key Vault backend: it can generate EC keys, but they can't be used for - // decryption/ECDH, so a key it generates can't back a KeyAgreement verification method. A - // did:nuts document always needs a key that backs every relationship, so document creation - // must fail outright rather than silently publish a document without KeyAgreement. + // decryption/ECDH, so a key it generates can't back a KeyAgreement verification method. signOnlyKeyStore := nutsCrypto.NewAzureKeyVaultLikeCryptoInstance(db) signOnlyManager := NewManager(signOnlyKeyStore, nil, nil, nil, db) - _, err := signOnlyManager.NewDocument(ctx, orm.AssertionKeyUsage()) + t.Run("document creation without KeyAgreement still succeeds", func(t *testing.T) { + // With OpenID4VCI as an alternative to gRPC/DAG private-transaction delivery, a did:nuts + // document no longer strictly needs KeyAgreement, so this must not fail outright. + doc, err := signOnlyManager.NewDocument(ctx, orm.AssertionKeyUsage()) - assert.ErrorIs(t, err, nutsCrypto.ErrKeyUsageNotSupported) + require.NoError(t, err) + require.Len(t, doc.VerificationMethods, 1) + assert.False(t, orm.DIDKeyFlags(doc.VerificationMethods[0].KeyTypes).Is(orm.KeyAgreementUsage)) + }) + + t.Run("explicitly requesting KeyAgreement fails", func(t *testing.T) { + _, err := signOnlyManager.NewDocument(ctx, DefaultKeyFlags()) + + assert.ErrorIs(t, err, nutsCrypto.ErrKeyUsageNotSupported) + }) t.Run("explicit VerificationMethod request", func(t *testing.T) { _, err := signOnlyManager.NewVerificationMethod(ctx, did.MustParseDID("did:nuts:test"), orm.EncryptionKeyUsage()) diff --git a/vdr/didsubject/interface.go b/vdr/didsubject/interface.go index 552e2a4970..731e105f89 100644 --- a/vdr/didsubject/interface.go +++ b/vdr/didsubject/interface.go @@ -49,6 +49,7 @@ var ErrSubjectNotFound = errors.New("subject not found") type MethodManager interface { // NewDocument generates a new DID document for the given subject. // This is done by the method manager since the DID might depend on method specific rules. + // keyFlags is a hard requirement, same as NewVerificationMethod's. NewDocument(ctx context.Context, keyFlags orm.DIDKeyFlags) (*orm.DidDocument, error) // NewVerificationMethod generates a new VerificationMethod for the given subject. // This is done by the method manager since the VM ID might depend on method specific rules. From 8a5c58a0798a7ca5475bc63b2dca991ce6523704 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Mon, 14 Sep 2026 15:28:50 +0200 Subject: [PATCH 17/20] fix(vdr): restore KeyAgreement key on did:nuts documents created via V1 API didnuts.Manager.NewDocument used to ignore its keyFlags argument and always request DefaultKeyFlags() (including KeyAgreement), so the V1 CreateDID endpoint got a KeyAgreement key without ever asking for one. Now that NewDocument honors keyFlags, V1 CreateDID silently stopped creating one, breaking private-transaction (gRPC/DAG) delivery, which needs it to encrypt the PAL header. V1's documented contract is that the request body is ignored and defaults (including keyAgreement = true) are always used, so CreateDID now explicitly requests EncryptionKeyCreationOption to restore that default. Assisted by AI --- vdr/api/v1/api.go | 8 ++++++-- vdr/api/v1/api_test.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/vdr/api/v1/api.go b/vdr/api/v1/api.go index 2fc6b8d929..38971232d7 100644 --- a/vdr/api/v1/api.go +++ b/vdr/api/v1/api.go @@ -128,8 +128,12 @@ func (a *Wrapper) Routes(router core.EchoRouter) { // CreateDID creates a new DID Document and returns it. func (a *Wrapper) CreateDID(ctx context.Context, _ CreateDIDRequestObject) (CreateDIDResponseObject, error) { - // request body is ignored, defaults are used. - options := didsubject.DefaultCreationOptions().With(didsubject.NutsLegacyNamingOption{}) + // request body is ignored, defaults are used: selfControl, assertionMethod, keyAgreement and + // capabilityInvocation all default to true (see docs/_static/vdr/v1.yaml), so KeyAgreement must + // always be requested here. + options := didsubject.DefaultCreationOptions(). + With(didsubject.NutsLegacyNamingOption{}). + With(didsubject.EncryptionKeyCreationOption{}) docs, _, err := a.SubjectManager.Create(ctx, options) // if this operation leads to an error, it may return a 500 diff --git a/vdr/api/v1/api_test.go b/vdr/api/v1/api_test.go index 8c16e1636e..02164d7e57 100644 --- a/vdr/api/v1/api_test.go +++ b/vdr/api/v1/api_test.go @@ -50,7 +50,7 @@ func TestWrapper_CreateDID(t *testing.T) { t.Run("ok - defaults", func(t *testing.T) { ctx := newMockContext(t) request := DIDCreateRequest{SelfControl: to.Ptr(false)} // SelfControl value is overwritten with default - ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions().With(didsubject.NutsLegacyNamingOption{})).Return([]did.Document{*didDoc}, "subject", nil) + ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions().With(didsubject.NutsLegacyNamingOption{}).With(didsubject.EncryptionKeyCreationOption{})).Return([]did.Document{*didDoc}, "subject", nil) response, err := ctx.client.CreateDID(nil, CreateDIDRequestObject{Body: &request}) From 85e5ad892708f8ad717fd5061de22fc5cdbd7ab8 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 15 Sep 2026 14:35:17 +0200 Subject: [PATCH 18/20] fix(vdr): don't fail did:nuts creation because did:web can't back KeyAgreement SqlManager.Create used one shared keyFlags value for every DID method configured for a subject, and aborted creating the whole subject whenever KeyAgreement was requested and did:web was among the methods (did:web has never supported it: an RSA/SOGIS constraint, independent of key store backend). Restoring V1 CreateDID's default KeyAgreement request (previous commit) meant this now hard-failed subject creation for any party with did:web enabled alongside did:nuts. Strip KeyAgreement from did:web's flags instead of aborting: did:nuts still gets a working key (or a loud ErrKeyUsageNotSupported if its key store backend, e.g. Azure Key Vault, can't back it), while did:web's document is created without one, silently. Also documents in the V1 OpenAPI spec that CreateDID's keyAgreement default can't be overridden and always fails on a backend that can't provide it, pointing callers at the V2 API instead. Assisted by AI --- docs/_static/vdr/v1.yaml | 6 ++++-- vdr/didsubject/manager.go | 13 ++++++++----- vdr/didsubject/manager_test.go | 28 +++++++++++++++++++++++----- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/docs/_static/vdr/v1.yaml b/docs/_static/vdr/v1.yaml index 5808898ed4..fad2b7bc5b 100644 --- a/docs/_static/vdr/v1.yaml +++ b/docs/_static/vdr/v1.yaml @@ -16,11 +16,13 @@ paths: description: | Starting with v6.0.0, the entire body will be ignored and default values will be used. The default values are: selfControl = true, assertionMethod = true, keyAgreement = true, capabilityInvocation = true, capabilityDelegation = true, authentication = true and controllers = []. - + Only a single keypair will be generated. All enabled methods will reuse the same key pair. + keyAgreement = true can't be overridden through this endpoint. If the configured key store backend can't back a KeyAgreement key (e.g. Azure Key Vault, which doesn't support decryption/ECDH with its EC keys), this operation always fails with a 400. Use the V2 API instead (`POST /internal/vdr/v2/subject`), which lets a caller omit the encryption key request. + error returns: - * 400 - Invalid (combination of) options + * 400 - Invalid (combination of) options, or the key store backend can't back a requested key usage (e.g. KeyAgreement on Azure Key Vault) * 500 - An error occurred while processing the request operationId: "createDID" requestBody: diff --git a/vdr/didsubject/manager.go b/vdr/didsubject/manager.go index 32774d8c6f..357d047220 100644 --- a/vdr/didsubject/manager.go +++ b/vdr/didsubject/manager.go @@ -147,15 +147,18 @@ func (r *SqlManager) Create(ctx context.Context, options CreationOptions) ([]did // call generate on all managers for method, manager := range r.MethodManagers { - // known limitation, check is also done within the manager, but at this point we can return a known error for the API - // requires update to nutsCrypto module - if keyFlags.Is(orm.KeyAgreementUsage) && method == "web" { - return nil, ErrKeyAgreementNotSupported + methodKeyFlags := keyFlags + // did:web never supports KeyAgreement (RSA/SOGIS constraint, requires update to + // nutsCrypto module, see #1948), independent of the key store backend. Strip it here + // rather than failing creation of the whole subject over it: did:nuts (and any other + // method) should still get its key. + if method == "web" { + methodKeyFlags &^= orm.KeyAgreementUsage } // save tx in context to pass all the way down to KeyStore transactionContext := context.WithValue(ctx, storage.TransactionKey{}, tx) - sqlDoc, err := manager.NewDocument(transactionContext, keyFlags) + sqlDoc, err := manager.NewDocument(transactionContext, methodKeyFlags) if err != nil { return nil, fmt.Errorf("could not generate DID document (method %s): %w", method, err) } diff --git a/vdr/didsubject/manager_test.go b/vdr/didsubject/manager_test.go index cc14f0a6e7..45bb6ca921 100644 --- a/vdr/didsubject/manager_test.go +++ b/vdr/didsubject/manager_test.go @@ -138,6 +138,20 @@ func TestManager_Create(t *testing.T) { assert.True(t, strings.HasPrefix(IDs[0], "did:test:")) assert.True(t, strings.HasPrefix(IDs[1], "did:example:")) }) + t.Run("KeyAgreement requested: did:nuts gets it, did:web doesn't, subject creation still succeeds", func(t *testing.T) { + db := testDB(t) + var nutsKeyFlags, webKeyFlags orm.DIDKeyFlags + m := SqlManager{DB: db, MethodManagers: map[string]MethodManager{ + "nuts": testMethod{method: "nuts", keyFlagsCapture: &nutsKeyFlags}, + "web": testMethod{method: "web", keyFlagsCapture: &webKeyFlags}, + }} + + _, _, err := m.Create(audit.TestContext(), DefaultCreationOptions().With(EncryptionKeyCreationOption{})) + + require.NoError(t, err) + assert.True(t, nutsKeyFlags.Is(orm.KeyAgreementUsage), "did:nuts should get a KeyAgreement key") + assert.False(t, webKeyFlags.Is(orm.KeyAgreementUsage), "did:web should not be asked for a KeyAgreement key") + }) t.Run("with unknown option", func(t *testing.T) { db := testDB(t) m := SqlManager{DB: db, MethodManagers: map[string]MethodManager{"example": testMethod{}}} @@ -455,13 +469,17 @@ func TestNewIDForService(t *testing.T) { } type testMethod struct { - committed bool - error error - method string - document *orm.DidDocument + committed bool + error error + method string + document *orm.DidDocument + keyFlagsCapture *orm.DIDKeyFlags // if set, records the keyFlags NewDocument was called with } -func (t testMethod) NewDocument(_ context.Context, _ orm.DIDKeyFlags) (*orm.DidDocument, error) { +func (t testMethod) NewDocument(_ context.Context, keyFlags orm.DIDKeyFlags) (*orm.DidDocument, error) { + if t.keyFlagsCapture != nil { + *t.keyFlagsCapture = keyFlags + } if t.document != nil { return t.document, nil } From 16132f77409b06d756f4aa0cd676426be467a0bc Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 15 Sep 2026 14:45:05 +0200 Subject: [PATCH 19/20] docs(vdr): note V1/V2 mixing and flag the did:web KeyAgreement check as temporary docs/_static/vdr/v1.yaml: advise against mixing V1 and V2 for the same node's DIDs/subjects; callers hitting V1's Azure Key Vault limitation should switch to V2 entirely, not use both. vdr/didsubject/manager.go: mark the method == "web" check as a temporary hardcoded workaround, since did:web's KeyAgreement restriction is only a policy choice (RSA/SOGIS, #1948), not a technical one. If that's resolved, the whole check can be removed rather than replaced with a per-method capability query. Assisted by AI --- docs/_static/vdr/v1.yaml | 2 +- vdr/didsubject/manager.go | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/_static/vdr/v1.yaml b/docs/_static/vdr/v1.yaml index fad2b7bc5b..8a805b1c8b 100644 --- a/docs/_static/vdr/v1.yaml +++ b/docs/_static/vdr/v1.yaml @@ -19,7 +19,7 @@ paths: Only a single keypair will be generated. All enabled methods will reuse the same key pair. - keyAgreement = true can't be overridden through this endpoint. If the configured key store backend can't back a KeyAgreement key (e.g. Azure Key Vault, which doesn't support decryption/ECDH with its EC keys), this operation always fails with a 400. Use the V2 API instead (`POST /internal/vdr/v2/subject`), which lets a caller omit the encryption key request. + keyAgreement = true can't be overridden through this endpoint. If the configured key store backend can't back a KeyAgreement key (e.g. Azure Key Vault, which doesn't support decryption/ECDH with its EC keys), this operation always fails with a 400. Use the V2 API instead (`POST /internal/vdr/v2/subject`), which lets a caller omit the encryption key request. Don't mix V1 and V2 for creating and managing DIDs/subjects on the same node: switch over to V2 entirely rather than using both. error returns: * 400 - Invalid (combination of) options, or the key store backend can't back a requested key usage (e.g. KeyAgreement on Azure Key Vault) diff --git a/vdr/didsubject/manager.go b/vdr/didsubject/manager.go index 357d047220..f96fb1c94b 100644 --- a/vdr/didsubject/manager.go +++ b/vdr/didsubject/manager.go @@ -148,10 +148,13 @@ func (r *SqlManager) Create(ctx context.Context, options CreationOptions) ([]did // call generate on all managers for method, manager := range r.MethodManagers { methodKeyFlags := keyFlags - // did:web never supports KeyAgreement (RSA/SOGIS constraint, requires update to - // nutsCrypto module, see #1948), independent of the key store backend. Strip it here - // rather than failing creation of the whole subject over it: did:nuts (and any other - // method) should still get its key. + // did:web never supports KeyAgreement (RSA/SOGIS constraint, see #1948), independent of + // the key store backend. Strip it here rather than failing creation of the whole subject + // over it: did:nuts (and any other method) should still get its key. + // TEMPORARY: this hardcodes method == "web" instead of asking the manager what it + // supports. If #1948 is resolved (dropping the RSA/SOGIS requirement so did:web can back + // KeyAgreement with an EC key like every other method), this whole check can go away + // instead of being replaced with a per-method capability query. if method == "web" { methodKeyFlags &^= orm.KeyAgreementUsage } From b587fb1d3ffc595fc44ab2eb9551bd47a496b457 Mon Sep 17 00:00:00 2001 From: Rein Krul Date: Tue, 15 Sep 2026 16:15:55 +0200 Subject: [PATCH 20/20] fix(vdr): default V2 CreateSubject to requesting a KeyAgreement key too Before did:nuts started honoring its keyFlags argument, every did:nuts document got a KeyAgreement key regardless of what any caller (V1 or V2) asked for. V2's CreateSubject only ever requested one when the caller explicitly set keys.encryptionKey: true, which had no visible effect until did:nuts started honoring it: callers who never set that flag (the documented default) now silently stop getting a working KeyAgreement key on new subjects and key rotations, breaking gRPC/DAG private-transaction delivery with no error at creation time. Restore the old default: an encryption key is now requested unless the caller explicitly sets keys.encryptionKey: false. did:web keeps silently not getting one either way (SqlManager.Create already strips it); a key store backend that can't back it (Azure Key Vault) still fails loudly, whether the request came from the new default or an explicit true. Assisted by AI --- docs/_static/vdr/v2.yaml | 7 ++++++- vdr/api/v2/api.go | 9 +++++---- vdr/api/v2/api_test.go | 30 ++++++++++++++++++++++++++++-- 3 files changed, 39 insertions(+), 7 deletions(-) diff --git a/docs/_static/vdr/v2.yaml b/docs/_static/vdr/v2.yaml index e0bc54405e..d6b0b9d167 100644 --- a/docs/_static/vdr/v2.yaml +++ b/docs/_static/vdr/v2.yaml @@ -482,7 +482,12 @@ components: description: If true, an EC keypair is generated and added to the DID Documents as a assertion, authentication, capability invocation and capability delegation method. encryptionKey: type: boolean - description: If true, an RSA keypair is generated and added to the DID Documents as a key agreement method. + description: | + If true, a keypair is generated and added to the DID Documents as a key agreement method. + Defaults to true when the keys object (or the whole request body) is omitted; only an + explicit false opts out. did:web never supports this and is skipped without an error, + regardless of this setting. If the key store backend can't support it either (e.g. Azure + Key Vault), creation fails, whether this was left at its default or set explicitly. CreateSubjectOptions: type: object description: Options for the subject creation. diff --git a/vdr/api/v2/api.go b/vdr/api/v2/api.go index 510068bd5d..4602823788 100644 --- a/vdr/api/v2/api.go +++ b/vdr/api/v2/api.go @@ -121,10 +121,11 @@ func (w *Wrapper) CreateSubject(ctx context.Context, request CreateSubjectReques if request.Body.Subject != nil { options = options.With(didsubject.SubjectCreationOption{Subject: *request.Body.Subject}) } - if request.Body.Keys != nil { - if request.Body.Keys.EncryptionKey { - options = options.With(didsubject.EncryptionKeyCreationOption{}) - } + // An encryption key is requested by default (keys omitted, or keys.encryptionKey explicitly + // true), matching the behavior every did:nuts document used to get before key usage started + // reflecting backend capability. Only an explicit keys.encryptionKey: false opts out. + if request.Body.Keys == nil || request.Body.Keys.EncryptionKey { + options = options.With(didsubject.EncryptionKeyCreationOption{}) } docs, subject, err := w.SubjectManager.Create(ctx, options) diff --git a/vdr/api/v2/api_test.go b/vdr/api/v2/api_test.go index d2c7e34c76..f2abfe859b 100644 --- a/vdr/api/v2/api_test.go +++ b/vdr/api/v2/api_test.go @@ -50,7 +50,7 @@ var didDoc = did.Document{ func TestWrapper_CreateSubject(t *testing.T) { t.Run("ok - defaults", func(t *testing.T) { ctx := newMockContext(t) - ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions()).Return([]did.Document{didDoc}, "subject", nil) + ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions().With(didsubject.EncryptionKeyCreationOption{})).Return([]did.Document{didDoc}, "subject", nil) response, err := ctx.client.CreateSubject(nil, CreateSubjectRequestObject{Body: &CreateSubjectJSONRequestBody{}}) @@ -61,7 +61,7 @@ func TestWrapper_CreateSubject(t *testing.T) { t.Run("with Subject", func(t *testing.T) { ctx := newMockContext(t) subject := "subject" - ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions().With(didsubject.SubjectCreationOption{Subject: subject})).Return([]did.Document{didDoc}, "subject", nil) + ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions().With(didsubject.SubjectCreationOption{Subject: subject}).With(didsubject.EncryptionKeyCreationOption{})).Return([]did.Document{didDoc}, "subject", nil) response, err := ctx.client.CreateSubject(nil, CreateSubjectRequestObject{ Body: &CreateSubjectJSONRequestBody{ @@ -73,6 +73,32 @@ func TestWrapper_CreateSubject(t *testing.T) { assert.Len(t, response.(CreateSubject200JSONResponse).Documents, 1) assert.Equal(t, "subject", response.(CreateSubject200JSONResponse).Subject) }) + t.Run("ok - keys.encryptionKey explicitly true", func(t *testing.T) { + ctx := newMockContext(t) + ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions().With(didsubject.EncryptionKeyCreationOption{})).Return([]did.Document{didDoc}, "subject", nil) + + response, err := ctx.client.CreateSubject(nil, CreateSubjectRequestObject{ + Body: &CreateSubjectJSONRequestBody{ + Keys: &KeyCreationOptions{EncryptionKey: true}, + }, + }) + + require.NoError(t, err) + assert.Len(t, response.(CreateSubject200JSONResponse).Documents, 1) + }) + t.Run("ok - keys.encryptionKey explicitly false opts out", func(t *testing.T) { + ctx := newMockContext(t) + ctx.subjectManager.EXPECT().Create(gomock.Any(), didsubject.DefaultCreationOptions()).Return([]did.Document{didDoc}, "subject", nil) + + response, err := ctx.client.CreateSubject(nil, CreateSubjectRequestObject{ + Body: &CreateSubjectJSONRequestBody{ + Keys: &KeyCreationOptions{EncryptionKey: false}, + }, + }) + + require.NoError(t, err) + assert.Len(t, response.(CreateSubject200JSONResponse).Documents, 1) + }) t.Run("error - create fails", func(t *testing.T) { ctx := newMockContext(t) ctx.subjectManager.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil, "", assert.AnError)