Skip to content

Commit

Permalink
ceph: fix kms auto-detection when full TLS
Browse files Browse the repository at this point in the history 
When TLS is used and includes a caert, client key/cert, we need to copy
the content of the secret to a file in the operator's container
filesystem so that we can build the TLS config and thus the HTTP Client,
which reads those files.
Also, removing the files after each API call so they don't persist on
the filesystem forever.

Signed-off-by: Sébastien Han <seb@redhat.com>
  • Loading branch information
@leseb
leseb committed Sep 30, 2021
1 parent 1f49b6f commit e3ca518
Show file tree
Hide file tree
Showing 8 changed files with 375 additions and 120 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/canary-integration-test.yml
View file
Expand Up @@ -140,6 +140,7 @@ jobs:
- name: deploy cluster
run: |
yq write -d1 -i cluster/examples/kubernetes/ceph/operator.yaml "spec.template.spec.containers[0].image" rook/ceph:local-build
kubectl create -f cluster/examples/kubernetes/ceph/operator.yaml
yq write -i tests/manifests/test-cluster-on-pvc-encrypted.yaml "spec.storage.storageClassDeviceSets[0].encrypted" false
yq write -i tests/manifests/test-cluster-on-pvc-encrypted.yaml "spec.storage.storageClassDeviceSets[0].count" 2
Expand Down Expand Up @@ -219,6 +220,7 @@ jobs:

- name: deploy cluster
run: |
yq write -d1 -i cluster/examples/kubernetes/ceph/operator.yaml "spec.template.spec.containers[0].image" rook/ceph:local-build
kubectl create -f cluster/examples/kubernetes/ceph/operator.yaml
yq write -i tests/manifests/test-cluster-on-pvc-encrypted.yaml "spec.storage.storageClassDeviceSets[0].encrypted" false
cat tests/manifests/test-on-pvc-db.yaml >> tests/manifests/test-cluster-on-pvc-encrypted.yaml
Expand Down Expand Up @@ -357,6 +359,7 @@ jobs:
- name: deploy cluster
run: |
yq write -d1 -i cluster/examples/kubernetes/ceph/operator.yaml "spec.template.spec.containers[0].image" rook/ceph:local-build
kubectl create -f cluster/examples/kubernetes/ceph/operator.yaml
yq write -i tests/manifests/test-cluster-on-pvc-encrypted.yaml "spec.storage.storageClassDeviceSets[0].count" 2
yq write -i tests/manifests/test-cluster-on-pvc-encrypted.yaml "spec.storage.storageClassDeviceSets[0].volumeClaimTemplates[0].spec.resources.requests.storage" 6Gi
Expand Down Expand Up @@ -428,6 +431,7 @@ jobs:

- name: deploy cluster
run: |
yq write -d1 -i cluster/examples/kubernetes/ceph/operator.yaml "spec.template.spec.containers[0].image" rook/ceph:local-build
kubectl create -f cluster/examples/kubernetes/ceph/operator.yaml
cat tests/manifests/test-on-pvc-db.yaml >> tests/manifests/test-cluster-on-pvc-encrypted.yaml
kubectl create -f tests/manifests/test-cluster-on-pvc-encrypted.yaml
Expand Down Expand Up @@ -571,6 +575,7 @@ jobs:

- name: deploy cluster
run: |
yq write -d1 -i cluster/examples/kubernetes/ceph/operator.yaml "spec.template.spec.containers[0].image" rook/ceph:local-build
kubectl create -f cluster/examples/kubernetes/ceph/operator.yaml
cat tests/manifests/test-kms-vault.yaml >> tests/manifests/test-cluster-on-pvc-encrypted.yaml
yq merge --inplace --arrays append tests/manifests/test-cluster-on-pvc-encrypted.yaml tests/manifests/test-kms-vault-spec.yaml
Expand Down Expand Up @@ -652,6 +657,7 @@ jobs:

- name: deploy cluster
run: |
yq write -d1 -i cluster/examples/kubernetes/ceph/operator.yaml "spec.template.spec.containers[0].image" rook/ceph:local-build
kubectl create -f cluster/examples/kubernetes/ceph/operator.yaml
yq write -i tests/manifests/test-cluster-on-pvc-encrypted.yaml "spec.storage.storageClassDeviceSets[0].encrypted" false
kubectl create -f tests/manifests/test-cluster-on-pvc-encrypted.yaml
Expand Down
29 changes: 25 additions & 4 deletions pkg/daemon/ceph/osd/kms/kms.go
View file
Expand Up @@ -83,7 +83,14 @@ func (c *Config) PutSecret(secretName, secretValue string) error {
}
if c.IsVault() {
// Store the secret in Vault
v, err := InitVault(c.context, c.clusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
removeCertFiles := make(chan bool, 1)
// Remove cert files from operator's filesystem
defer func() {
removeCertFiles <- true
close(removeCertFiles)
}()

v, err := InitVault(c.context, c.clusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails, removeCertFiles)
if err != nil {
return errors.Wrap(err, "failed to init vault kms")
}
Expand All @@ -102,7 +109,14 @@ func (c *Config) GetSecret(secretName string) (string, error) {
var value string
if c.IsVault() {
// Store the secret in Vault
v, err := InitVault(c.context, c.clusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
removeCertFiles := make(chan bool, 1)
// Remove cert files from operator's filesystem
defer func() {
removeCertFiles <- true
close(removeCertFiles)
}()

v, err := InitVault(c.context, c.clusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails, removeCertFiles)
if err != nil {
return "", errors.Wrap(err, "failed to get secret in vault")
}
Expand All @@ -121,7 +135,14 @@ func (c *Config) GetSecret(secretName string) (string, error) {
func (c *Config) DeleteSecret(secretName string) error {
if c.IsVault() {
// Store the secret in Vault
v, err := InitVault(c.context, c.clusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails)
removeCertFiles := make(chan bool, 1)
// Remove cert files from operator's filesystem
defer func() {
removeCertFiles <- true
close(removeCertFiles)
}()

v, err := InitVault(c.context, c.clusterInfo.Namespace, c.clusterSpec.Security.KeyManagementService.ConnectionDetails, removeCertFiles)
if err != nil {
return errors.Wrap(err, "failed to delete secret in vault")
}
Expand Down Expand Up @@ -202,7 +223,7 @@ func ValidateConnectionDetails(clusterdContext *clusterd.Context, securitySpec *
case VaultKVSecretEngineKey:
// Append Backend Version if not already present
if GetParam(securitySpec.KeyManagementService.ConnectionDetails, vault.VaultBackendKey) == "" {
backendVersion, err := BackendVersion(securitySpec.KeyManagementService.ConnectionDetails)
backendVersion, err := BackendVersion(clusterdContext, ns, securitySpec.KeyManagementService.ConnectionDetails)
if err != nil {
return errors.Wrap(err, "failed to get backend version")
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/daemon/ceph/osd/kms/kms_test.go
View file
Expand Up @@ -91,7 +91,7 @@ func TestValidateConnectionDetails(t *testing.T) {
securitySpec.KeyManagementService.ConnectionDetails["VAULT_CACERT"] = "vault-ca-secret"
err = ValidateConnectionDetails(context, securitySpec, ns)
assert.Error(t, err, "")
assert.EqualError(t, err, "failed to validate vault connection details: failed to find TLS connection details k8s secret \"VAULT_CACERT\"")
assert.EqualError(t, err, "failed to validate vault connection details: failed to find TLS connection details k8s secret \"vault-ca-secret\"")

// Error: TLS secret exists but empty key
tlsSecret := &v1.Secret{
Expand Down Expand Up @@ -122,7 +122,9 @@ func TestValidateConnectionDetails(t *testing.T) {
vault.TestWaitActive(t, core)
client := cluster.Cores[0].Client
// Mock the client here
vaultClient = func(secretConfig map[string]string) (*api.Client, error) { return client, nil }
vaultClient = func(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string, removeCertFiles chan bool) (*api.Client, error) {
return client, nil
}
if err := client.Sys().Mount("rook/", &api.MountInput{
Type: "kv-v2",
Options: map[string]string{"version": "2"},
Expand Down
50 changes: 45 additions & 5 deletions pkg/daemon/ceph/osd/kms/vault.go
View file
Expand Up @@ -19,7 +19,9 @@ package kms
import (
"context"
"io/ioutil"
"os"
"strings"
"time"

"github.com/hashicorp/vault/api"
"github.com/libopenstorage/secrets"
Expand Down Expand Up @@ -66,7 +68,7 @@ var (
*/

// InitVault inits the secret store
func InitVault(context *clusterd.Context, namespace string, config map[string]string) (secrets.Secrets, error) {
func InitVault(context *clusterd.Context, namespace string, config map[string]string, removeCertFiles chan bool) (secrets.Secrets, error) {
c := make(map[string]interface{})

// So that we don't alter the content of c.config for later iterations
Expand All @@ -77,7 +79,7 @@ func InitVault(context *clusterd.Context, namespace string, config map[string]st
}

// Populate TLS config
newConfigWithTLS, err := configTLS(context, namespace, oriConfig)
newConfigWithTLS, err := configTLS(context, namespace, oriConfig, removeCertFiles)
if err != nil {
return nil, errors.Wrap(err, "failed to initialize vault tls configuration")
}
Expand All @@ -96,8 +98,9 @@ func InitVault(context *clusterd.Context, namespace string, config map[string]st
return v, nil
}

func configTLS(clusterdContext *clusterd.Context, namespace string, config map[string]string) (map[string]string, error) {
func configTLS(clusterdContext *clusterd.Context, namespace string, config map[string]string, removeCertFiles chan bool) (map[string]string, error) {
ctx := context.TODO()
var filesToRemove []*os.File
for _, tlsOption := range cephv1.VaultTLSConnectionDetails {
tlsSecretName := GetParam(config, tlsOption)
if tlsSecretName == "" {
Expand All @@ -124,13 +127,50 @@ func configTLS(clusterdContext *clusterd.Context, namespace string, config map[s

logger.Debugf("replacing %q current content %q with %q", tlsOption, config[tlsOption], file.Name())

// update the env var with the path
// Update the env var with the path
config[tlsOption] = file.Name()

// Add the file to the list of files to remove
filesToRemove = append(filesToRemove, file)
} else {
logger.Debugf("value of tlsOption %q tlsSecretName is already correct %q", tlsOption, tlsSecretName)
}
}

// Run a goroutine that waits for a confirmation to remove the certificates from the filesystem
go func(filesToRemove []*os.File) {
logger.Debugf("files to remove: %+v", filesToRemove)
rmCertFiles := func() {
for _, file := range filesToRemove {
logger.Debugf("closing %q", file.Name())
err := file.Close()
if err != nil {
logger.Errorf("failed to close file %q. %v", file.Name(), err)
}
logger.Debugf("closed %q", file.Name())
logger.Debugf("removing %q", file.Name())
err = os.Remove(file.Name())
if err != nil {
logger.Errorf("failed to remove file %q. %v", file.Name(), err)
}
logger.Debugf("removed %q", file.Name())
}
}
for {
select {
case <-removeCertFiles:
logger.Debug("received confirmation from channel to remove vault certificates")
rmCertFiles()
logger.Info("successfully removed secret files")
return
case <-time.After(time.Second * 5):
logger.Info("never received anything from the channel removing vault cert files anyway")
rmCertFiles()
return
}
}
}(filesToRemove)

return config, nil
}

Expand Down Expand Up @@ -215,7 +255,7 @@ func validateVaultConnectionDetails(clusterdContext *clusterd.Context, ns string
// Fetch the secret
s, err := clusterdContext.Clientset.CoreV1().Secrets(ns).Get(ctx, tlsSecretName, v1.GetOptions{})
if err != nil {
return errors.Errorf("failed to find TLS connection details k8s secret %q", tlsOption)
return errors.Errorf("failed to find TLS connection details k8s secret %q", tlsSecretName)
}

// Check the Secret key and its content
Expand Down
36 changes: 31 additions & 5 deletions pkg/daemon/ceph/osd/kms/vault_api.go
View file
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/libopenstorage/secrets/vault"
"github.com/libopenstorage/secrets/vault/utils"
"github.com/pkg/errors"
"github.com/rook/rook/pkg/clusterd"

"github.com/hashicorp/vault/api"
)
Expand All @@ -38,16 +39,34 @@ var vaultClient = newVaultClient

// newVaultClient returns a vault client, there is no need for any secretConfig validation
// Since this is called after an already validated call InitVault()
func newVaultClient(secretConfig map[string]string) (*api.Client, error) {
func newVaultClient(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string, removeCertFiles chan bool) (*api.Client, error) {
// DefaultConfig uses the environment variables if present.
config := api.DefaultConfig()

// Always use a new map otherwise the map will mutate and subsequent calls will fail since the
// TLS content has been altered by the TLS config in vaultClient()
localSecretConfig := make(map[string]string)
for k, v := range secretConfig {
localSecretConfig[k] = v
}

// Convert map string to map interface
c := make(map[string]interface{})
for k, v := range secretConfig {
for k, v := range localSecretConfig {
c[k] = v
}

// Populate TLS config
newConfigWithTLS, err := configTLS(clusterdContext, namespace, localSecretConfig, removeCertFiles)
if err != nil {
return nil, errors.Wrap(err, "failed to initialize vault tls configuration")
}

// Populate TLS config
for key, value := range newConfigWithTLS {
c[key] = string(value)
}

// Configure TLS
if err := utils.ConfigureTLS(config, c); err != nil {
return nil, err
Expand All @@ -64,15 +83,15 @@ func newVaultClient(secretConfig map[string]string) (*api.Client, error) {
client.SetToken(strings.TrimSuffix(os.Getenv(api.EnvVaultToken), "\n"))

// Set Vault address, was validated by ValidateConnectionDetails()
err = client.SetAddress(strings.TrimSuffix(secretConfig[api.EnvVaultAddress], "\n"))
err = client.SetAddress(strings.TrimSuffix(localSecretConfig[api.EnvVaultAddress], "\n"))
if err != nil {
return nil, err
}

return client, nil
}

func BackendVersion(secretConfig map[string]string) (string, error) {
func BackendVersion(clusterdContext *clusterd.Context, namespace string, secretConfig map[string]string) (string, error) {
v1 := "v1"
v2 := "v2"

Expand All @@ -90,8 +109,15 @@ func BackendVersion(secretConfig map[string]string) (string, error) {
logger.Info("vault kv secret engine version set to v2")
return v2, nil
default:
removeCertFiles := make(chan bool, 1)
// Remove cert files from operator's filesystem
defer func() {
removeCertFiles <- true
close(removeCertFiles)
}()

// Initialize Vault client
vaultClient, err := vaultClient(secretConfig)
vaultClient, err := vaultClient(clusterdContext, namespace, secretConfig, removeCertFiles)
if err != nil {
return "", errors.Wrap(err, "failed to initialize vault client")
}
Expand Down

0 comments on commit e3ca518

Please sign in to comment.