Reduce encryption KMS configuration SC parameters

* moves KMS type from StorageClass into KMS configuration itself
 * updates omapval used to identify KMS to only it's ID without the type

why?

1. when using multiple KMS configurations (not currently supported)
automated parsing of kms configuration will be failing because some
entries in configs won't comply with the requested type
2. less options are needed in the StorageClass and less data used to
identify the KMS

Signed-off-by: Vasyl Purchel vasyl.purchel@workday.com
Signed-off-by: Andrea Baglioni andrea.baglioni@workday.com

charts/ceph-csi-rbd/values.yaml

@@ -31,9 +31,15 @@ csiConfig: []
 # Ref: https://github.com/ceph/ceph-csi/blob/master/docs/deploy-rbd.md
 # Example:
 # encryptionKMSConfig:
-#   - encryptionKMSID: "<kms-id>"
+#   vault-unique-id-1:
-#     <kms-specific-configs>
+#     encryptionKMSType: vault
-encryptionKMSConfig: []
+#     vaultAddress: https://vault.example.com
+#     vaultAuthPath: /v1/auth/kubernetes/login
+#     vaultRole: csi-kubernetes
+#     vaultPassphraseRoot: /v1/secret
+#     vaultPassphrasePath: ceph-csi/
+#     vaultCAVerify: "false"
+encryptionKMSConfig: {}
 
 nodeplugin:
   name: nodeplugin

docs/deploy-rbd.md

@@ -54,8 +54,7 @@ make image-cephcsi
 | `csi.storage.k8s.io/provisioner-secret-namespace`, `csi.storage.k8s.io/node-stage-secret-namespace` | yes (for Kubernetes) | namespaces of the above Secret objects                                                                                                                                                                                  |
 | `mounter`                                                                                           | no                   | if set to `rbd-nbd`, use `rbd-nbd` on nodes that have `rbd-nbd` and `nbd` kernel modules to map rbd images                                                                                                              |
 | `encrypted`                                                                                         | no                   | disabled by default, use `"true"` to enable LUKS encryption on pvc and `"false"` to disable it. **Do not change for existing storageclasses**                                                                           |
-| `encryptionKMS`                                                                                     | no                   | specifies key management system for encrypytion. Currently supports `vault`                                                                                                                                             |
+| `encryptionKMSID`                                                                                   | no                   | required if encryption is enabled and a kms is used to store passphrases                                                                                                                                                |
-| `encryptionKMSID`                                                                                   | no                   | required if `encryptionKMS` is set to `vault` to specify a unique identifier for vault configuration                                                                                                                    |
 
 **NOTE:** An accompanying CSI configuration file, needs to be provided to the
 running pods. Refer to [Creating CSI configuration](../examples/README.md#creating-csi-configuration)
@@ -223,14 +222,19 @@ To further improve security robustness it is possible to use unique passphrases
 generated for each volume and stored in a Key Management System (KMS). Currently
 HashiCorp Vault is the only KMS supported.
 
-To use Vault as KMS set `encryptionKMS` to `vault` and `encryptionKMSID` to a
+To use Vault as KMS set `encryptionKMSID` to a unique identifier for Vault
-unique identifier for Vault configuration. You will also need to create vault
+configuration. You will also need to create vault configuration similar to the
-configuration similar to the [example](../examples/rbd/kms-config.yaml)
+[example](../examples/rbd/kms-config.yaml) and use same `encryptionKMSID`.
-and use same `encryptionKMSID`. In order for ceph-csi to be able to access the
+Configuration must include `encryptionKMSType: "vault"`. In order for ceph-csi
-configuration you will need to have it mounted to csi-rbdplugin containers in
+to be able to access the configuration you will need to have it mounted to
-both daemonset (so kms client can be instantiated to encrypt/decrypt volumes)
+csi-rbdplugin containers in both daemonset (so kms client can be instantiated to
-and deployment pods (so kms client can be instantiated to delete passphrase on
+encrypt/decrypt volumes) and deployment pods (so kms client can be instantiated
-volume delete) `ceph-csi-encryption-kms-config` config map.
+to delete passphrase on volume delete) `ceph-csi-encryption-kms-config` config
+map.
+
+> Note: kms configuration must be a map of string values only
+> (`map[string]string`) so for numerical and boolean values make sure to put
+> quotes around.
 
 #### Configuring HashiCorp Vault
 

docs/design/proposals/encrypted-pvc.md

@@ -63,10 +63,9 @@ requirement by using dm-crypt module through cryptsetup cli interface.
 
 * StorageClass extended with following parameters:
   1. `encrypted` ("true" or "false")
-  1. `encryptionKMS` (string representing kms of choice)
+  1. `encryptionKMSID` (string representing kms configuration of choice)
   ceph-csi plugin may support different kms vendors with different type of
   authentication
-  1. `encryptionKMSID` (string representing kms configuration)
 
 * New KMS Configuration created.
 
@@ -103,10 +102,9 @@ parameters:
    # Encrypt volumes
    encrypted: "true"
 
-   # The type of kms we want to connect to: Barbican, aws kms or others can be
+   # Use external key management system for encryption passphrases by specifying
-   # supported
+   # a unique ID matching KMS ConfigMap. The ID is only used for correlation to
-   encryptionKMS: vault
+   # config map entry.
-   # String representing a KMS configuration
    encryptionKMSID: <kms-id>
 
 reclaimPolicy: Delete
@@ -120,12 +118,12 @@ apiVersion: v1
 kind: ConfigMap
 data:
   config.json: |-
-    [
+    {
-      {
+      "<kms-id>": {
-        "kmsID": "<kms-id>",
+        "encryptionKMSType": "kmsType",
         kms specific config...
       }
-    ]
+    }
 metadata:
   name: ceph-csi-encryption-kms-config
 ```

e2e/rbd.go

@@ -145,7 +145,6 @@ var _ = Describe("RBD", func() {
 				deleteResource(rbdExamplePath + "storageclass.yaml")
 				scOpts := map[string]string{
 					"encrypted":       "true",
-					"encryptionKMS":   "vault",
 					"encryptionKMSID": "vault-test",
 				}
 				createRBDStorageClass(f.ClientSet, f, scOpts)

examples/kms/vault/kms-config.yaml

@@ -3,16 +3,16 @@ apiVersion: v1
 kind: ConfigMap
 data:
   config.json: |-
-    [
+    {
-      {
+      "vault-test": {
-        "encryptionKMSID": "vault-test",
+        "encryptionKMSType": "vault",
         "vaultAddress": "http://vault.default.svc.cluster.local:8200",
         "vaultAuthPath": "/v1/auth/kubernetes/login",
         "vaultRole": "csi-kubernetes",
         "vaultPassphraseRoot": "/v1/secret",
         "vaultPassphrasePath": "ceph-csi/",
-        "vaultCAVerify": false
+        "vaultCAVerify": "false"
       }
-    ]
+    }
 metadata:
   name: ceph-csi-encryption-kms-config

examples/rbd/storageclass.yaml

@@ -44,11 +44,9 @@ parameters:
    # A string is expected here, i.e. “true”, not true.
    # encrypted: "true"
 
-   # Use external key management system for encryption passphrases
+   # Use external key management system for encryption passphrases by specifying
-   # encryptionKMS: vault
+   # a unique ID matching KMS ConfigMap. The ID is only used for correlation to
-
+   # config map entry.
-   # String representing KMS configuration. Should be unique and match ID in
-   # KMS ConfigMap. The ID is only used for correlation to config map entry.
    # encryptionKMSID: <kms-config-id>
 reclaimPolicy: Delete
 allowVolumeExpansion: true

pkg/rbd/rbd_journal.go

@@ -162,12 +162,12 @@ func checkVolExists(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials
 		return false, err
 	}
 
-	encryptionKmsConfig := ""
+	kmsID := ""
 	if rbdVol.Encrypted {
-		encryptionKmsConfig = rbdVol.KMS.KmsConfig()
+		kmsID = rbdVol.KMS.GetID()
 	}
 	imageUUID, err := volJournal.CheckReservation(ctx, rbdVol.Monitors, cr, rbdVol.Pool,
-		rbdVol.RequestName, "", encryptionKmsConfig)
+		rbdVol.RequestName, "", kmsID)
 	if err != nil {
 		return false, err
 	}
@@ -237,12 +237,12 @@ func reserveSnap(ctx context.Context, rbdSnap *rbdSnapshot, cr *util.Credentials
 // reserveVol is a helper routine to request a rbdVolume name reservation and generate the
 // volume ID for the generated name
 func reserveVol(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error {
-	encryptionKmsConfig := ""
+	kmsID := ""
 	if rbdVol.Encrypted {
-		encryptionKmsConfig = rbdVol.KMS.KmsConfig()
+		kmsID = rbdVol.KMS.GetID()
 	}
 	imageUUID, err := volJournal.ReserveName(ctx, rbdVol.Monitors, cr, rbdVol.Pool,
-		rbdVol.RequestName, "", encryptionKmsConfig)
+		rbdVol.RequestName, "", kmsID)
 	if err != nil {
 		return err
 	}

pkg/rbd/rbd_util.go

@@ -351,19 +351,15 @@ func genVolFromVolID(ctx context.Context, rbdVol *rbdVolume, volumeID string, cr
 		return err
 	}
 
-	kmsConfig := ""
+	kmsID := ""
-	rbdVol.RequestName, _, kmsConfig, err = volJournal.GetObjectUUIDData(
+	rbdVol.RequestName, _, kmsID, err = volJournal.GetObjectUUIDData(
 		ctx, rbdVol.Monitors, cr, rbdVol.Pool, vi.ObjectUUID, false)
 	if err != nil {
 		return err
 	}
-	if kmsConfig != "" {
+	if kmsID != "" {
 		rbdVol.Encrypted = true
-		kmsOpts, kmsConfigParseErr := util.GetKMSConfig(kmsConfig)
+		rbdVol.KMS, err = util.GetKMS(kmsID, secrets)
-		if kmsConfigParseErr != nil {
-			return kmsConfigParseErr
-		}
-		rbdVol.KMS, err = util.GetKMS(kmsOpts, secrets)
 		if err != nil {
 			return err
 		}
@@ -516,7 +512,10 @@ func genVolFromVolumeOptions(ctx context.Context, volOptions, credentials map[st
 		}
 
 		if rbdVol.Encrypted {
-			rbdVol.KMS, err = util.GetKMS(volOptions, credentials)
+			// deliberately ignore if parsing failed as GetKMS will return default
+			// implementation of kmsID is empty
+			kmsID := volOptions["encryptionKMSID"]
+			rbdVol.KMS, err = util.GetKMS(kmsID, credentials)
 			if err != nil {
 				return nil, fmt.Errorf("invalid encryption kms configuration: %s", err)
 			}

pkg/util/crypto.go

@@ -19,7 +19,9 @@ package util
 import (
 	"context"
 	"encoding/base64"
+	"encoding/json"
 	"fmt"
+	"io/ioutil"
 	"path"
 	"strings"
 
@@ -39,6 +41,10 @@ const (
 
 	// Encryption passphrase location in K8s secrets
 	encryptionPassphraseKey = "encryptionPassphrase"
+	kmsTypeKey              = "encryptionKMSType"
+
+	// Default KMS type
+	defaultKMSType = "default"
 
 	// kmsConfigPath is the location of the vault config file
 	kmsConfigPath = "/etc/ceph-csi-encryption-kms-config/config.json"
@@ -54,7 +60,7 @@ type EncryptionKMS interface {
 	GetPassphrase(key string) (string, error)
 	SavePassphrase(key, value string) error
 	DeletePassphrase(key string) error
-	KmsConfig() string
+	GetID() string
 }
 
 // MissingPassphrase is an error instructing to generate new passphrase
@@ -75,11 +81,6 @@ func initSecretsKMS(secrets map[string]string) (EncryptionKMS, error) {
 	return SecretsKMS{passphrase: passphraseValue}, nil
 }
 
-// KmsConfig returns KMS configuration: "<kms-type>|<kms-id>"
-func (kms SecretsKMS) KmsConfig() string {
-	return "secrets|kubernetes"
-}
-
 // GetPassphrase returns passphrase from Kubernetes secrets
 func (kms SecretsKMS) GetPassphrase(key string) (string, error) {
 	return kms.passphrase, nil
@@ -96,31 +97,52 @@ func (kms SecretsKMS) DeletePassphrase(key string) error {
 	return nil
 }
 
-// GetKMS returns an instance of Key Management System
+// GetID is returning ID representing default KMS `default`
-func GetKMS(opts, secrets map[string]string) (EncryptionKMS, error) {
+func (kms SecretsKMS) GetID() string {
-	kmsType, ok := opts["encryptionKMS"]
+	return defaultKMSType
-	if !ok || kmsType == "" || kmsType == "secrets" {
-		return initSecretsKMS(secrets)
-	}
-	if kmsType == "vault" {
-		return InitVaultKMS(opts, secrets)
-	}
-	return nil, fmt.Errorf("unknown encryption KMS type %s", kmsType)
 }
 
-// GetKMSConfig returns required keys for KMS to instantiate from it's config
+// GetKMS returns an instance of Key Management System
-// - map with kms type and ID keys
+func GetKMS(kmsID string, secrets map[string]string) (EncryptionKMS, error) {
-// - error if format is invalid
+	if kmsID == "" || kmsID == defaultKMSType {
-func GetKMSConfig(config string) (map[string]string, error) {
+		return initSecretsKMS(secrets)
-	kmsConfigParts := strings.Split(config, "|")
-	if len(kmsConfigParts) != 2 {
-		return make(map[string]string), fmt.Errorf("failed to parse encryption KMS "+
-			"configuration from config string, expected <type>|<id>, got: %s", config)
 	}
-	return map[string]string{
+
-		"encryptionKMS":   kmsConfigParts[0],
+	// #nosec
-		"encryptionKMSID": kmsConfigParts[1],
+	content, err := ioutil.ReadFile(kmsConfigPath)
-	}, nil
+	if err != nil {
+		return nil, fmt.Errorf("failed to read kms configuration from %s: %s",
+			kmsConfigPath, err)
+	}
+
+	var config map[string]interface{}
+	err = json.Unmarshal(content, &config)
+	if err != nil {
+		return nil, fmt.Errorf("failed to parse kms configuration: %s", err)
+	}
+
+	kmsConfigData, ok := config[kmsID].(map[string]interface{})
+	if !ok {
+		return nil, fmt.Errorf("missing encryption KMS configuration with %s", kmsID)
+	}
+	kmsConfig := make(map[string]string)
+	for key, value := range kmsConfigData {
+		kmsConfig[key], ok = value.(string)
+		if !ok {
+			return nil, fmt.Errorf("broken KMS config: '%s' for '%s' is not a string",
+				value, key)
+		}
+	}
+
+	kmsType, ok := kmsConfig[kmsTypeKey]
+	if !ok {
+		return nil, fmt.Errorf("encryption KMS configuration for %s is missing KMS type", kmsID)
+	}
+
+	if kmsType == "vault" {
+		return InitVaultKMS(kmsID, kmsConfig, secrets)
+	}
+	return nil, fmt.Errorf("unknown encryption KMS type %s", kmsType)
 }
 
 // GetCryptoPassphrase Retrieves passphrase to encrypt volume

pkg/util/vault.go

@@ -24,6 +24,7 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
+	"strconv"
 	"strings"
 )
 
@@ -48,9 +49,9 @@ const (
 kmsKMS represents a Hashicorp Vault KMS configuration
 
 Example JSON structure in the KMS config is,
-[
+{
-	{
+	"local_vault_unique_identifier": {
-		"encryptionKMSID": "local_vault_unique_identifier",
+		"encryptionKMSType": "vault",
 		"vaultAddress": "https://127.0.0.1:8500",
 		"vaultAuthPath": "/v1/auth/kubernetes/login",
 		"vaultRole": "csi-kubernetes",
@@ -61,88 +62,83 @@ Example JSON structure in the KMS config is,
 		"vaultCAFromSecret": "vault-ca"
 	},
 	...
-]
+}
 */
 type VaultKMS struct {
-	EncryptionKMSID     string `json:"encryptionKMSID"`
+	EncryptionKMSID     string
-	VaultAddress        string `json:"vaultAddress"`
+	VaultAddress        string
-	VaultAuthPath       string `json:"vaultAuthPath"`
+	VaultAuthPath       string
-	VaultRole           string `json:"vaultRole"`
+	VaultRole           string
-	VaultNamespace      string `json:"vaultNamespace"`
+	VaultNamespace      string
-	VaultPassphraseRoot string `json:"vaultPassphraseRoot"`
+	VaultPassphraseRoot string
-	VaultPassphrasePath string `json:"vaultPassphrasePath"`
+	VaultPassphrasePath string
-	VaultCAVerify       bool   `json:"vaultCAVerify"`
+	VaultCAVerify       bool
-	VaultCAFromSecret   string `json:"vaultCAFromSecret"`
 	vaultCA             *x509.CertPool
 }
 
 // InitVaultKMS returns an interface to HashiCorp Vault KMS
-func InitVaultKMS(opts, secrets map[string]string) (EncryptionKMS, error) {
+func InitVaultKMS(kmsID string, config, secrets map[string]string) (EncryptionKMS, error) {
-	var config []VaultKMS
+	var (
+		ok  bool
+		err error
+	)
+	kms := &VaultKMS{}
+	kms.EncryptionKMSID = kmsID
 
-	vaultID, ok := opts["encryptionKMSID"]
+	kms.VaultAddress, ok = config["vaultAddress"]
-	if !ok {
+	if !ok || kms.VaultAddress == "" {
-		return nil, fmt.Errorf("missing encryptionKMSID for vault as encryption KMS")
+		return nil, fmt.Errorf("missing 'vaultAddress' for vault KMS %s", kmsID)
+	}
+	kms.VaultAuthPath, ok = config["vaultAuthPath"]
+	if !ok || kms.VaultAuthPath == "" {
+		kms.VaultAuthPath = vaultDefaultAuthPath
+	}
+	kms.VaultRole, ok = config["vaultRole"]
+	if !ok || kms.VaultRole == "" {
+		kms.VaultRole = vaultDefaultRole
+	}
+	kms.VaultNamespace, ok = config["vaultNamespace"]
+	if !ok || kms.VaultNamespace == "" {
+		kms.VaultNamespace = vaultDefaultNamespace
+	}
+	kms.VaultPassphraseRoot, ok = config["vaultPassphraseRoot"]
+	if !ok || kms.VaultPassphraseRoot == "" {
+		kms.VaultPassphraseRoot = vaultDefaultPassphraseRoot
+	}
+	kms.VaultPassphrasePath, ok = config["vaultPassphrasePath"]
+	if !ok || kms.VaultPassphrasePath == "" {
+		kms.VaultPassphrasePath = vaultDefaultPassphrasePath
+	}
+	kms.VaultCAVerify = true
+	verifyCA, ok := config["vaultCAVerify"]
+	if ok {
+		kms.VaultCAVerify, err = strconv.ParseBool(verifyCA)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse 'vaultCAVerify' for vault <%s> kms config: %s",
+				kmsID, err)
+		}
+	}
+	vaultCAFromSecret, ok := config["vaultCAFromSecret"]
+	if ok && vaultCAFromSecret != "" {
+		caPEM, ok := secrets[vaultCAFromSecret]
+		if !ok {
+			return nil, fmt.Errorf("missing vault CA in secret %s", vaultCAFromSecret)
+		}
+		roots := x509.NewCertPool()
+		ok = roots.AppendCertsFromPEM([]byte(caPEM))
+		if !ok {
+			return nil, fmt.Errorf("failed loading CA bundle for vault from secret %s",
+				vaultCAFromSecret)
+		}
+		kms.vaultCA = roots
 	}
 
-	// #nosec
+	return kms, nil
-	content, err := ioutil.ReadFile(kmsConfigPath)
-	if err != nil {
-		return nil, fmt.Errorf("error fetching vault configuration for vault ID (%s): (%s)",
-			vaultID, err)
-	}
-
-	err = json.Unmarshal(content, &config)
-	if err != nil {
-		return nil, fmt.Errorf("unmarshal failed: %v. raw buffer response: %s",
-			err, string(content))
-	}
-
-	for i := range config {
-		vault := &config[i]
-		if vault.EncryptionKMSID != vaultID {
-			continue
-		}
-		if vault.VaultAddress == "" {
-			return nil, fmt.Errorf("missing vaultAddress for vault as encryption KMS")
-		}
-		if vault.VaultAuthPath == "" {
-			vault.VaultAuthPath = vaultDefaultAuthPath
-		}
-		if vault.VaultRole == "" {
-			vault.VaultRole = vaultDefaultRole
-		}
-		if vault.VaultNamespace == "" {
-			vault.VaultNamespace = vaultDefaultNamespace
-		}
-		if vault.VaultPassphraseRoot == "" {
-			vault.VaultPassphraseRoot = vaultDefaultPassphraseRoot
-		}
-		if vault.VaultPassphrasePath == "" {
-			vault.VaultPassphrasePath = vaultDefaultPassphrasePath
-		}
-		if vault.VaultCAFromSecret != "" {
-			caPEM, ok := secrets[vault.VaultCAFromSecret]
-			if !ok {
-				return nil, fmt.Errorf("missing vault CA in secret %s", vault.VaultCAFromSecret)
-			}
-			roots := x509.NewCertPool()
-			ok = roots.AppendCertsFromPEM([]byte(caPEM))
-			if !ok {
-				return nil, fmt.Errorf("failed loading CA bundle for vault from secret %s",
-					vault.VaultCAFromSecret)
-			}
-			vault.vaultCA = roots
-		}
-		return vault, nil
-	}
-
-	return nil, fmt.Errorf("missing configuration for vault ID (%s)", vaultID)
 }
 
-// KmsConfig returns KMS configuration: "<kms-type>|<kms-id>"
+// GetID is returning correlation ID to KMS configuration
-func (kms *VaultKMS) KmsConfig() string {
+func (kms *VaultKMS) GetID() string {
-	return fmt.Sprintf("vault|%s", kms.EncryptionKMSID)
+	return kms.EncryptionKMSID
 }
 
 // GetPassphrase returns passphrase from Vault

pkg/util/voljournal.go

@@ -416,8 +416,8 @@ func (cj *CSIJournal) GetObjectUUIDData(ctx context.Context, monitors string, cr
 		cj.cephUUIDDirectoryPrefix+objectUUID, cj.encryptKMSKey)
 	if err != nil {
 		if _, ok := err.(ErrKeyNotFound); !ok {
-			klog.Errorf(Log(ctx, "=> GetObjectUUIDData encryptedKMS failed: %s (%s)"), cj.cephUUIDDirectoryPrefix+objectUUID, err)
+			return "", "", "", fmt.Errorf("OMapVal for %s/%s failed to get encryption KMS value: %s",
-			return "", "", "", err
+				pool, cj.cephUUIDDirectoryPrefix+objectUUID, err)
 		}
 		// ErrKeyNotFound means no encryption KMS was used
 	}