Skip to content

Commit 0863c1b

Browse files
zerotensRakshith-R
authored andcommitted
rbd: fix encrypted PVC with metadata KMS cannot be deleted
Signed-off-by: Zerotens <12968743+zerotens@users.noreply.github.com> (cherry picked from commit 5b587c9) # Conflicts: # PendingReleaseNotes.md
1 parent 63d64a8 commit 0863c1b

File tree

3 files changed

+69
-40
lines changed

3 files changed

+69
-40
lines changed

internal/kms/dummy.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,11 @@ func newDefaultTestDummy() EncryptionKMS {
5454

5555
func newSecretsMetadataTestDummy() EncryptionKMS {
5656
smKMS := secretsMetadataKMS{}
57-
smKMS.secretsKMS = secretsKMS{passphrase: base64.URLEncoding.EncodeToString(
58-
[]byte("test dummy passphrase"))}
57+
smKMS.Secrets = map[string]string{
58+
encryptionPassphraseKey: "test dummy passphrase",
59+
}
60+
smKMS.Tenant = "tenant"
61+
smKMS.Config = nil
5962

6063
return smKMS
6164
}

internal/kms/secretskms.go

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func (kms secretsKMS) RemoveDEK(ctx context.Context, key string) error {
9797
// secretsMetadataKMS is a KMS based on the secretKMS, but stores the
9898
// Data-Encryption-Key (DEK) in the metadata of the volume.
9999
type secretsMetadataKMS struct {
100-
secretsKMS
100+
ProviderInitArgs
101101
}
102102

103103
var _ = RegisterProvider(Provider{
@@ -109,28 +109,9 @@ var _ = RegisterProvider(Provider{
109109
// so that the passphrase from the user provided or StorageClass secrets can be used
110110
// for encrypting/decrypting DEKs that are stored in a detached DEKStore.
111111
func initSecretsMetadataKMS(args ProviderInitArgs) (EncryptionKMS, error) {
112-
var (
113-
smKMS secretsMetadataKMS
114-
encryptionPassphrase string
115-
ok bool
116-
err error
117-
)
112+
var smKMS secretsMetadataKMS
118113

119-
encryptionPassphrase, err = smKMS.fetchEncryptionPassphrase(
120-
args.Config, args.Tenant)
121-
if err != nil {
122-
if !errors.Is(err, errConfigOptionMissing) {
123-
return nil, err
124-
}
125-
// if 'userSecret' option is not specified, fetch encryptionPassphrase
126-
// from storageclass secrets.
127-
encryptionPassphrase, ok = args.Secrets[encryptionPassphraseKey]
128-
if !ok {
129-
return nil, fmt.Errorf(
130-
"missing %q in storageclass secret", encryptionPassphraseKey)
131-
}
132-
}
133-
smKMS.secretsKMS = secretsKMS{passphrase: encryptionPassphrase}
114+
smKMS.ProviderInitArgs = args
134115

135116
return smKMS, nil
136117
}
@@ -184,7 +165,45 @@ func (kms secretsMetadataKMS) fetchEncryptionPassphrase(
184165

185166
// Destroy frees all used resources.
186167
func (kms secretsMetadataKMS) Destroy() {
187-
kms.secretsKMS.Destroy()
168+
// nothing to do
169+
}
170+
171+
// FetchDEK returns passphrase from Kubernetes secrets.
172+
func (kms secretsMetadataKMS) FetchDEK(ctx context.Context, key string) (string, error) {
173+
var (
174+
encryptionPassphrase string
175+
ok bool
176+
err error
177+
)
178+
179+
encryptionPassphrase, err = kms.fetchEncryptionPassphrase(
180+
kms.Config, kms.Tenant)
181+
if err != nil {
182+
if !errors.Is(err, errConfigOptionMissing) {
183+
return "", err
184+
}
185+
// if 'userSecret' option is not specified, fetch encryptionPassphrase
186+
// from storageclass secrets.
187+
encryptionPassphrase, ok = kms.Secrets[encryptionPassphraseKey]
188+
if !ok {
189+
return "", fmt.Errorf(
190+
"missing %q in storageclass secret", encryptionPassphraseKey)
191+
}
192+
}
193+
194+
return encryptionPassphrase, nil
195+
}
196+
197+
// StoreDEK does nothing, as there is no passphrase per key (volume), so
198+
// no need to store is anywhere.
199+
func (kms secretsMetadataKMS) StoreDEK(ctx context.Context, key, value string) error {
200+
return nil
201+
}
202+
203+
// RemoveDEK is doing nothing as no new passphrases are saved with
204+
// secretsKMS.
205+
func (kms secretsMetadataKMS) RemoveDEK(ctx context.Context, key string) error {
206+
return nil
188207
}
189208

190209
func (kms secretsMetadataKMS) RequiresDEKStore() DEKStoreType {
@@ -208,7 +227,7 @@ type encryptedMetedataDEK struct {
208227
// nonce that was used for encrypting.
209228
func (kms secretsMetadataKMS) EncryptDEK(ctx context.Context, volumeID, plainDEK string) (string, error) {
210229
// use the passphrase from the secretKMS
211-
passphrase, err := kms.secretsKMS.FetchDEK(ctx, volumeID)
230+
passphrase, err := kms.FetchDEK(ctx, volumeID)
212231
if err != nil {
213232
return "", fmt.Errorf("failed to get passphrase: %w", err)
214233
}
@@ -238,7 +257,7 @@ func (kms secretsMetadataKMS) EncryptDEK(ctx context.Context, volumeID, plainDEK
238257
// fetches secretKMS passphrase to decrypt the DEK.
239258
func (kms secretsMetadataKMS) DecryptDEK(ctx context.Context, volumeID, encryptedDEK string) (string, error) {
240259
// use the passphrase from the secretKMS
241-
passphrase, err := kms.secretsKMS.FetchDEK(ctx, volumeID)
260+
passphrase, err := kms.FetchDEK(ctx, volumeID)
242261
if err != nil {
243262
return "", fmt.Errorf("failed to get passphrase: %w", err)
244263
}
@@ -265,7 +284,7 @@ func (kms secretsMetadataKMS) DecryptDEK(ctx context.Context, volumeID, encrypte
265284

266285
func (kms secretsMetadataKMS) GetSecret(ctx context.Context, volumeID string) (string, error) {
267286
// use the passphrase from the secretKMS
268-
return kms.secretsKMS.FetchDEK(ctx, volumeID)
287+
return kms.FetchDEK(ctx, volumeID)
269288
}
270289

271290
// generateCipher returns a AEAD cipher based on a passphrase and salt

internal/kms/secretskms_test.go

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,41 +70,48 @@ func TestInitSecretsMetadataKMS(t *testing.T) {
7070
Secrets: map[string]string{},
7171
}
7272

73-
// passphrase it not set, init should fail
7473
kms, err := initSecretsMetadataKMS(args)
75-
require.Error(t, err)
76-
require.Nil(t, kms)
77-
78-
// set a passphrase to get a working KMS
79-
args.Secrets[encryptionPassphraseKey] = "my-passphrase-from-kubernetes"
80-
81-
kms, err = initSecretsMetadataKMS(args)
8274
require.NoError(t, err)
8375
require.NotNil(t, kms)
8476
require.Equal(t, DEKStoreMetadata, kms.RequiresDEKStore())
8577
}
8678

8779
func TestWorkflowSecretsMetadataKMS(t *testing.T) {
8880
t.Parallel()
89-
secrets := map[string]string{
90-
encryptionPassphraseKey: "my-passphrase-from-kubernetes",
91-
}
9281
args := ProviderInitArgs{
9382
Tenant: "tenant",
9483
Config: nil,
95-
Secrets: secrets,
84+
Secrets: map[string]string{},
9685
}
9786
volumeID := "csi-vol-1b00f5f8-b1c1-11e9-8421-9243c1f659f0"
9887

9988
kms, err := initSecretsMetadataKMS(args)
10089
require.NoError(t, err)
10190
require.NotNil(t, kms)
91+
require.Equal(t, DEKStoreMetadata, kms.RequiresDEKStore())
10292

10393
// plainDEK is the (LUKS) passphrase for the volume
10494
plainDEK := "usually created with generateNewEncryptionPassphrase()"
10595

10696
ctx := context.TODO()
10797

98+
// with missing encryptionPassphraseKey, encrypting should fail
99+
_, err = kms.EncryptDEK(ctx, volumeID, plainDEK)
100+
require.Error(t, err)
101+
102+
secrets := map[string]string{
103+
encryptionPassphraseKey: "my-passphrase-from-kubernetes",
104+
}
105+
args = ProviderInitArgs{
106+
Tenant: "tenant",
107+
Config: nil,
108+
Secrets: secrets,
109+
}
110+
111+
kms, err = initSecretsMetadataKMS(args)
112+
require.NoError(t, err)
113+
require.NotNil(t, kms)
114+
108115
encryptedDEK, err := kms.EncryptDEK(ctx, volumeID, plainDEK)
109116
require.NoError(t, err)
110117
require.NotEqual(t, "", encryptedDEK)

0 commit comments

Comments
 (0)