util: add vaultDestroyKeys option to destroy Vault kv-v2 secrets

Hashicorp Vault does not completely remove the secrets in a kv-v2
backend when the keys are deleted. The metadata of the keys will be
kept, and it is possible to recover the contents of the keys afterwards.

With the new `vaultDestroyKeys` configuration parameter, this behaviour
can now be selected. By default the parameter will be set to `true`,
indicating that the keys and contents should completely be destroyed.
Setting it to any other value will make it possible to recover the
deleted keys.

Signed-off-by: Niels de Vos <ndevos@redhat.com>

examples/kms/vault/csi-kms-connection-details.yaml

@@ -29,6 +29,7 @@ data:
       "KMS_PROVIDER": "vaulttokens",
       "VAULT_ADDR": "http://vault.default.svc.cluster.local:8200",
       "VAULT_BACKEND_PATH": "secret",
+      "VAULT_DESTROY_KEYS": "true",
       "VAULT_SKIP_VERIFY": "true"
     }
   vault-tenant-sa-test: |-

examples/kms/vault/kms-config.yaml

@@ -10,6 +10,7 @@ data:
         "vaultAuthPath": "/v1/auth/kubernetes/login",
         "vaultRole": "csi-kubernetes",
         "vaultBackend": "kv-v2",
+        "vaultDestroyKeys": "true",
         "vaultPassphraseRoot": "/v1/secret",
         "vaultPassphrasePath": "ceph-csi/",
         "vaultCAVerify": "false"
@@ -29,7 +30,8 @@ data:
                   "vaultCAVerify": "true"
               },
               "an-other-app": {
-                  "tenantTokenName": "storage-encryption-token"
+                  "tenantTokenName": "storage-encryption-token",
+                  "vaultDestroyKeys": "false"
               }
           }
       },

internal/util/vault.go

@@ -44,6 +44,7 @@ const (
 	vaultDefaultNamespace      = ""
 	vaultDefaultPassphrasePath = ""
 	vaultDefaultCAVerify       = "true"
+	vaultDefaultDestroyKeys    = "true"
 )
 
 var (
@@ -75,6 +76,15 @@ type vaultConnection struct {
 	secrets     loss.Secrets
 	vaultConfig map[string]interface{}
 	keyContext  map[string]string
+
+	// vaultDestroyKeys will by default set to `true`, and causes secrets
+	// to be deleted from Hashicorp Vault to be completely removed. Usually
+	// secrets in a kv-v2 store will be soft-deleted, and recovering the
+	// contents is still possible.
+	//
+	// This option is only valid during deletion of keys, see
+	// getDeleteKeyContext() for more details.
+	vaultDestroyKeys bool
 }
 
 type VaultKMS struct {
@@ -153,6 +163,20 @@ func (vc *vaultConnection) initConnection(config map[string]interface{}) error {
 		vaultConfig[vault.VaultBackendPathKey] = vaultBackendPath
 	}
 
+	// always set the default to prevent recovering kv-v2 keys
+	vaultDestroyKeys := vaultDefaultDestroyKeys
+	err = setConfigString(&vaultDestroyKeys, config, "vaultDestroyKeys")
+	if errors.Is(err, errConfigOptionInvalid) {
+		return err
+	}
+	if firstInit || !errors.Is(err, errConfigOptionMissing) {
+		if vaultDestroyKeys == vaultDefaultDestroyKeys {
+			vc.vaultDestroyKeys = true
+		} else {
+			vc.vaultDestroyKeys = false
+		}
+	}
+
 	vaultTLSServerName := "" // optional
 	err = setConfigString(&vaultTLSServerName, config, "vaultTLSServerName")
 	if errors.Is(err, errConfigOptionInvalid) {
@@ -274,6 +298,25 @@ func (vc *vaultConnection) Destroy() {
 	}
 }
 
+// getDeleteKeyContext creates a new KeyContext that has an optional value set
+// to destroy the contents of secrets. This is configurable with the
+// `vaultDestroyKeys` configuration parameter.
+//
+// Setting the option `DestroySecret` option in the KeyContext while creating
+// new keys, causes failures, so the option only needs to be set in the
+// RemoveDEK() calls.
+func (vc *vaultConnection) getDeleteKeyContext() map[string]string {
+	keyContext := map[string]string{}
+	for k, v := range vc.keyContext {
+		keyContext[k] = v
+	}
+	if vc.vaultDestroyKeys {
+		keyContext[loss.DestroySecret] = vaultDefaultDestroyKeys
+	}
+
+	return keyContext
+}
+
 var _ = RegisterKMSProvider(KMSProvider{
 	UniqueID:    kmsTypeVault,
 	Initializer: initVaultKMS,
@@ -382,7 +425,7 @@ func (kms *VaultKMS) StoreDEK(key, value string) error {
 // RemoveDEK deletes passphrase from Vault.
 func (kms *VaultKMS) RemoveDEK(key string) error {
 	pathKey := filepath.Join(kms.vaultPassphrasePath, key)
-	err := kms.secrets.DeleteSecret(pathKey, kms.keyContext)
+	err := kms.secrets.DeleteSecret(pathKey, kms.getDeleteKeyContext())
 	if err != nil {
 		return fmt.Errorf("delete passphrase at %s request to vault failed: %w", pathKey, err)
 	}

internal/util/vault_test.go

@@ -21,7 +21,9 @@ import (
 	"os"
 	"testing"
 
+	loss "github.com/libopenstorage/secrets"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDetectAuthMountPath(t *testing.T) {
@@ -101,6 +103,28 @@ func TestSetConfigString(t *testing.T) {
 	}
 }
 
+func TestDefaultVaultDestroyKeys(t *testing.T) {
+	t.Parallel()
+
+	vc := &vaultConnection{}
+	config := make(map[string]interface{})
+	config["vaultAddress"] = "https://vault.test.example.com"
+	err := vc.initConnection(config)
+	require.NoError(t, err)
+	keyContext := vc.getDeleteKeyContext()
+	destroySecret, ok := keyContext[loss.DestroySecret]
+	assert.NotEqual(t, destroySecret, "")
+	assert.True(t, ok)
+
+	// setting vaultDestroyKeys to !true should remove the loss.DestroySecret entry
+	config["vaultDestroyKeys"] = "false"
+	err = vc.initConnection(config)
+	require.NoError(t, err)
+	keyContext = vc.getDeleteKeyContext()
+	_, ok = keyContext[loss.DestroySecret]
+	assert.False(t, ok)
+}
+
 func TestVaultKMSRegistered(t *testing.T) {
 	t.Parallel()
 	_, ok := kmsManager.providers[kmsTypeVault]

internal/util/vault_tokens.go

@@ -59,6 +59,7 @@ type standardVault struct {
 	VaultADDR          string `json:"VAULT_ADDR"`
 	VaultBackend       string `json:"VAULT_BACKEND"`
 	VaultBackendPath   string `json:"VAULT_BACKEND_PATH"`
+	VaultDestroyKeys   string `json:"VAULT_DESTROY_KEYS"`
 	VaultCACert        string `json:"VAULT_CACERT"`
 	VaultTLSServerName string `json:"VAULT_TLS_SERVER_NAME"`
 	VaultClientCert    string `json:"VAULT_CLIENT_CERT"`
@@ -73,6 +74,7 @@ type vaultTokenConf struct {
 	VaultAddress                 string `json:"vaultAddress"`
 	VaultBackend                 string `json:"vaultBackend"`
 	VaultBackendPath             string `json:"vaultBackendPath"`
+	VaultDestroyKeys             string `json:"vaultDestroyKeys"`
 	VaultCAFromSecret            string `json:"vaultCAFromSecret"`
 	VaultTLSServerName           string `json:"vaultTLSServerName"`
 	VaultClientCertFromSecret    string `json:"vaultClientCertFromSecret"`
@@ -87,6 +89,7 @@ func (v *vaultTokenConf) convertStdVaultToCSIConfig(s *standardVault) {
 	v.VaultAddress = s.VaultADDR
 	v.VaultBackend = s.VaultBackend
 	v.VaultBackendPath = s.VaultBackendPath
+	v.VaultDestroyKeys = s.VaultDestroyKeys
 	v.VaultCAFromSecret = s.VaultCACert
 	v.VaultClientCertFromSecret = s.VaultClientCert
 	v.VaultClientCertKeyFromSecret = s.VaultClientKey
@@ -479,7 +482,7 @@ func (vtc *vaultTenantConnection) StoreDEK(key, value string) error {
 
 // RemoveDEK deletes passphrase from Vault.
 func (vtc *vaultTenantConnection) RemoveDEK(key string) error {
-	err := vtc.secrets.DeleteSecret(key, vtc.keyContext)
+	err := vtc.secrets.DeleteSecret(key, vtc.getDeleteKeyContext())
 	if err != nil {
 		return fmt.Errorf("delete passphrase at %s request to vault failed: %w", key, err)
 	}
@@ -526,6 +529,7 @@ func isTenantConfigOption(opt string) bool {
 	case "vaultBackendPath":
 	case "vaultAuthNamespace":
 	case "vaultNamespace":
+	case "vaultDestroyKeys":
 	case "vaultTLSServerName":
 	case "vaultCAFromSecret":
 	case "vaultCAVerify":

internal/util/vault_tokens_test.go

@@ -127,6 +127,7 @@ func TestStdVaultToCSIConfig(t *testing.T) {
 		"VAULT_ADDR":"https://vault.example.com",
 		"VAULT_BACKEND":"kv-v2",
 		"VAULT_BACKEND_PATH":"/secret",
+		"VAULT_DESTROY_KEYS":"true",
 		"VAULT_CACERT":"",
 		"VAULT_TLS_SERVER_NAME":"vault.example.com",
 		"VAULT_CLIENT_CERT":"",
@@ -156,6 +157,8 @@ func TestStdVaultToCSIConfig(t *testing.T) {
 		t.Errorf("unexpected value for VaultBackend: %s", v.VaultBackend)
 	case v.VaultBackendPath != "/secret":
 		t.Errorf("unexpected value for VaultBackendPath: %s", v.VaultBackendPath)
+	case v.VaultDestroyKeys != vaultDefaultDestroyKeys:
+		t.Errorf("unexpected value for VaultDestroyKeys: %s", v.VaultDestroyKeys)
 	case v.VaultCAFromSecret != "":
 		t.Errorf("unexpected value for VaultCAFromSecret: %s", v.VaultCAFromSecret)
 	case v.VaultClientCertFromSecret != "":
@@ -180,6 +183,7 @@ func TestTransformConfig(t *testing.T) {
 	cm["VAULT_ADDR"] = "https://vault.example.com"
 	cm["VAULT_BACKEND"] = "kv-v2"
 	cm["VAULT_BACKEND_PATH"] = "/secret"
+	cm["VAULT_DESTROY_KEYS"] = "true"
 	cm["VAULT_CACERT"] = ""
 	cm["VAULT_TLS_SERVER_NAME"] = "vault.example.com"
 	cm["VAULT_CLIENT_CERT"] = ""
@@ -194,6 +198,7 @@ func TestTransformConfig(t *testing.T) {
 	assert.Equal(t, config["vaultAddress"], cm["VAULT_ADDR"])
 	assert.Equal(t, config["vaultBackend"], cm["VAULT_BACKEND"])
 	assert.Equal(t, config["vaultBackendPath"], cm["VAULT_BACKEND_PATH"])
+	assert.Equal(t, config["vaultDestroyKeys"], cm["VAULT_DESTROY_KEYS"])
 	assert.Equal(t, config["vaultCAFromSecret"], cm["VAULT_CACERT"])
 	assert.Equal(t, config["vaultTLSServerName"], cm["VAULT_TLS_SERVER_NAME"])
 	assert.Equal(t, config["vaultClientCertFromSecret"], cm["VAULT_CLIENT_CERT"])