From 7e59b8f9e991e187131450e9ac7644493ea3dfd4 Mon Sep 17 00:00:00 2001 From: Zerotens <12968743+zerotens@users.noreply.github.com> Date: Sun, 16 Feb 2025 09:46:07 +0100 Subject: [PATCH] rbd: fix encrypted PVC with metadata KMS cannot be deleted Signed-off-by: Zerotens <12968743+zerotens@users.noreply.github.com> --- PendingReleaseNotes.md | 1 + internal/kms/dummy.go | 7 +++- internal/kms/secretskms.go | 72 +++++++++++++++++++++------------ internal/kms/secretskms_test.go | 31 ++++++++------ 4 files changed, 71 insertions(+), 40 deletions(-) diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index 76de0dfb873..0c5ec25adfb 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -11,5 +11,6 @@ value [PR](https://github.com/ceph/ceph-csi/pull/4887) - cephfs: support omap data store in radosnamespace [PR](https://github.com/ceph/ceph-csi/pull/4661) - helm: Support setting nodepluigin and provisioner annotations +- rbd: fix encrypted PVC with metadata KMS cannot be deleted ## NOTE diff --git a/internal/kms/dummy.go b/internal/kms/dummy.go index fdf7ac45921..87b9cee2fa5 100644 --- a/internal/kms/dummy.go +++ b/internal/kms/dummy.go @@ -54,8 +54,11 @@ func newDefaultTestDummy() EncryptionKMS { func newSecretsMetadataTestDummy() EncryptionKMS { smKMS := secretsMetadataKMS{} - smKMS.secretsKMS = secretsKMS{passphrase: base64.URLEncoding.EncodeToString( - []byte("test dummy passphrase"))} + smKMS.Secrets = map[string]string{ + encryptionPassphraseKey: "test dummy passphrase", + } + smKMS.Tenant = "tenant" + smKMS.Config = nil return smKMS } diff --git a/internal/kms/secretskms.go b/internal/kms/secretskms.go index d20b53a0c33..08241cb2951 100644 --- a/internal/kms/secretskms.go +++ b/internal/kms/secretskms.go @@ -97,7 +97,7 @@ func (kms secretsKMS) RemoveDEK(ctx context.Context, key string) error { // secretsMetadataKMS is a KMS based on the secretKMS, but stores the // Data-Encryption-Key (DEK) in the metadata of the volume. type secretsMetadataKMS struct { - secretsKMS + ProviderInitArgs } var _ = RegisterProvider(Provider{ @@ -109,28 +109,9 @@ var _ = RegisterProvider(Provider{ // so that the passphrase from the user provided or StorageClass secrets can be used // for encrypting/decrypting DEKs that are stored in a detached DEKStore. func initSecretsMetadataKMS(args ProviderInitArgs) (EncryptionKMS, error) { - var ( - smKMS secretsMetadataKMS - encryptionPassphrase string - ok bool - err error - ) + var smKMS secretsMetadataKMS - encryptionPassphrase, err = smKMS.fetchEncryptionPassphrase( - args.Config, args.Tenant) - if err != nil { - if !errors.Is(err, errConfigOptionMissing) { - return nil, err - } - // if 'userSecret' option is not specified, fetch encryptionPassphrase - // from storageclass secrets. - encryptionPassphrase, ok = args.Secrets[encryptionPassphraseKey] - if !ok { - return nil, fmt.Errorf( - "missing %q in storageclass secret", encryptionPassphraseKey) - } - } - smKMS.secretsKMS = secretsKMS{passphrase: encryptionPassphrase} + smKMS.ProviderInitArgs = args return smKMS, nil } @@ -184,7 +165,46 @@ func (kms secretsMetadataKMS) fetchEncryptionPassphrase( // Destroy frees all used resources. func (kms secretsMetadataKMS) Destroy() { - kms.secretsKMS.Destroy() + // nothing to do +} + +// FetchDEK returns passphrase from Kubernetes secrets. +func (kms secretsMetadataKMS) FetchDEK(ctx context.Context, key string) (string, error) { + var ( + smKMS secretsMetadataKMS + encryptionPassphrase string + ok bool + err error + ) + + encryptionPassphrase, err = smKMS.fetchEncryptionPassphrase( + kms.Config, kms.Tenant) + if err != nil { + if !errors.Is(err, errConfigOptionMissing) { + return "", err + } + // if 'userSecret' option is not specified, fetch encryptionPassphrase + // from storageclass secrets. + encryptionPassphrase, ok = kms.Secrets[encryptionPassphraseKey] + if !ok { + return "", fmt.Errorf( + "missing %q in storageclass secret", encryptionPassphraseKey) + } + } + + return encryptionPassphrase, nil +} + +// StoreDEK does nothing, as there is no passphrase per key (volume), so +// no need to store is anywhere. +func (kms secretsMetadataKMS) StoreDEK(ctx context.Context, key, value string) error { + return nil +} + +// RemoveDEK is doing nothing as no new passphrases are saved with +// secretsKMS. +func (kms secretsMetadataKMS) RemoveDEK(ctx context.Context, key string) error { + return nil } func (kms secretsMetadataKMS) RequiresDEKStore() DEKStoreType { @@ -208,7 +228,7 @@ type encryptedMetedataDEK struct { // nonce that was used for encrypting. func (kms secretsMetadataKMS) EncryptDEK(ctx context.Context, volumeID, plainDEK string) (string, error) { // use the passphrase from the secretKMS - passphrase, err := kms.secretsKMS.FetchDEK(ctx, volumeID) + passphrase, err := kms.FetchDEK(ctx, volumeID) if err != nil { return "", fmt.Errorf("failed to get passphrase: %w", err) } @@ -238,7 +258,7 @@ func (kms secretsMetadataKMS) EncryptDEK(ctx context.Context, volumeID, plainDEK // fetches secretKMS passphrase to decrypt the DEK. func (kms secretsMetadataKMS) DecryptDEK(ctx context.Context, volumeID, encryptedDEK string) (string, error) { // use the passphrase from the secretKMS - passphrase, err := kms.secretsKMS.FetchDEK(ctx, volumeID) + passphrase, err := kms.FetchDEK(ctx, volumeID) if err != nil { return "", fmt.Errorf("failed to get passphrase: %w", err) } @@ -265,7 +285,7 @@ func (kms secretsMetadataKMS) DecryptDEK(ctx context.Context, volumeID, encrypte func (kms secretsMetadataKMS) GetSecret(ctx context.Context, volumeID string) (string, error) { // use the passphrase from the secretKMS - return kms.secretsKMS.FetchDEK(ctx, volumeID) + return kms.FetchDEK(ctx, volumeID) } // generateCipher returns a AEAD cipher based on a passphrase and salt diff --git a/internal/kms/secretskms_test.go b/internal/kms/secretskms_test.go index 757504b5cfd..d30d1ba676a 100644 --- a/internal/kms/secretskms_test.go +++ b/internal/kms/secretskms_test.go @@ -70,15 +70,7 @@ func TestInitSecretsMetadataKMS(t *testing.T) { Secrets: map[string]string{}, } - // passphrase it not set, init should fail kms, err := initSecretsMetadataKMS(args) - require.Error(t, err) - require.Nil(t, kms) - - // set a passphrase to get a working KMS - args.Secrets[encryptionPassphraseKey] = "my-passphrase-from-kubernetes" - - kms, err = initSecretsMetadataKMS(args) require.NoError(t, err) require.NotNil(t, kms) require.Equal(t, DEKStoreMetadata, kms.RequiresDEKStore()) @@ -86,25 +78,40 @@ func TestInitSecretsMetadataKMS(t *testing.T) { func TestWorkflowSecretsMetadataKMS(t *testing.T) { t.Parallel() - secrets := map[string]string{ - encryptionPassphraseKey: "my-passphrase-from-kubernetes", - } args := ProviderInitArgs{ Tenant: "tenant", Config: nil, - Secrets: secrets, + Secrets: map[string]string{}, } volumeID := "csi-vol-1b00f5f8-b1c1-11e9-8421-9243c1f659f0" kms, err := initSecretsMetadataKMS(args) require.NoError(t, err) require.NotNil(t, kms) + require.Equal(t, DEKStoreMetadata, kms.RequiresDEKStore()) // plainDEK is the (LUKS) passphrase for the volume plainDEK := "usually created with generateNewEncryptionPassphrase()" ctx := context.TODO() + // with missing encryptionPassphraseKey, encrypting should fail + _, err = kms.EncryptDEK(ctx, volumeID, plainDEK) + require.Error(t, err) + + secrets := map[string]string{ + encryptionPassphraseKey: "my-passphrase-from-kubernetes", + } + args = ProviderInitArgs{ + Tenant: "tenant", + Config: nil, + Secrets: secrets, + } + + kms, err = initSecretsMetadataKMS(args) + require.NoError(t, err) + require.NotNil(t, kms) + encryptedDEK, err := kms.EncryptDEK(ctx, volumeID, plainDEK) require.NoError(t, err) require.NotEqual(t, "", encryptedDEK)