Skip to content

Commit

Permalink
Merge pull request #1457 from iPraveenParihar/kms/vault-keyrotation
Browse files Browse the repository at this point in the history
kms: Root key rotation for Vault KMS
  • Loading branch information
jackyalbo authored Nov 11, 2024
2 parents 1fc5447 + 53d299b commit 87b1023
Show file tree
Hide file tree
Showing 6 changed files with 112 additions and 11 deletions.
7 changes: 7 additions & 0 deletions pkg/system/phase2_creating.go
Original file line number Diff line number Diff line change
Expand Up @@ -1030,6 +1030,13 @@ func (r *Reconciler) keyRotate() error {
return err
}

err = k.Get()
if err != nil {
r.Logger.Errorf("keyRotate, KMS Get error %v", err)
r.setKMSConditionStatus(nbv1.ConditionKMSErrorRead)
return err
}

// Generate new random root key and set it in the KMS
// Key - rotate begins
err = k.Set(util.RandomBase64(32))
Expand Down
10 changes: 6 additions & 4 deletions pkg/util/kms/kms_vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ const (

// Vault is a vault driver
type Vault struct {
UID string // NooBaa system UID
UID string // NooBaa system UID
name string // NooBaa system name
ns string // NooBaa system namespace
}

// NewVault is vault driver constructor
Expand All @@ -33,7 +35,7 @@ func NewVault(
namespace string,
uid string,
) Driver {
return &Vault{uid}
return &Vault{uid, name, namespace}
}

//
Expand Down Expand Up @@ -179,8 +181,8 @@ func writeCrtsToFile(secretName string, namespace string, secretValue []byte, en

// Version returns the current driver KMS version
// either single string or map, i.e. rotating key
func (*Vault) Version(kms *KMS) Version {
return &VersionSingleSecret{kms, nil}
func (k *Vault) Version(kms *KMS) Version {
return &VersionRotatingSecret{VersionBase{kms, nil}, k.name, k.ns}
}

// Register Vault driver with KMS layer
Expand Down
14 changes: 9 additions & 5 deletions pkg/util/kms/kms_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (v *VersionRotatingSecret) Reconcile(r SecretReconciler) error {

// Get implements SecretStorage interface for the secret map, i.e. rotating master root key
func (v *VersionRotatingSecret) Get() error {
s, _, err := v.k.GetSecret(v.backendSecretName(), v.k.driver.GetContext())
s, _, err := v.k.GetSecret(v.BackendSecretName(), v.k.driver.GetContext())
if err != nil {
// handle k8s get from non-existent secret
if strings.Contains(err.Error(), "not found") {
Expand All @@ -120,8 +120,8 @@ func (v *VersionRotatingSecret) Get() error {
return nil
}

// backendSecretName returns the rotating secret backend secret name
func (v *VersionRotatingSecret) backendSecretName() string {
// BackendSecretName returns the rotating secret backend secret name
func (v *VersionRotatingSecret) BackendSecretName() string {
return v.name + "-root-master-key-backend"
}

Expand All @@ -137,7 +137,7 @@ func (v *VersionRotatingSecret) Set(val string) error {
s[ActiveRootKey] = key
s[key] = val
v.data = s
_, err := v.k.PutSecret(v.backendSecretName(), toInterfaceMap(s), v.k.driver.SetContext())
_, err := v.k.PutSecret(v.BackendSecretName(), toInterfaceMap(s), v.k.driver.SetContext())
return err
}

Expand All @@ -154,11 +154,15 @@ func (v *VersionRotatingSecret) deleteSingleStringSecret() bool {
func (v *VersionRotatingSecret) Delete() error {
// Delete rotating secret backend
backendSecret := &corev1.Secret{}
backendSecret.Name = v.backendSecretName()
backendSecret.Name = v.BackendSecretName()
backendSecret.Namespace = v.ns
if !util.KubeDelete(backendSecret) {
return fmt.Errorf("KMS Delete error for the rotating master root secret backend")
}

err := v.k.DeleteSecret(v.BackendSecretName(), v.k.driver.DeleteContext())
if err != nil {
return err
}

return nil
Expand Down
8 changes: 6 additions & 2 deletions pkg/util/kms/test/dev/kms_dev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,12 @@ func simpleKmsSpec(token, apiAddress string) nbv1.KeyManagementServiceSpec {
func checkExternalSecret(noobaa *nbv1.NooBaa, expectedNil bool) {
k := noobaa.Spec.Security.KeyManagementService
uid := string(noobaa.UID)
driver := &kms.Vault{UID: uid}
path := k.ConnectionDetails[vault.VaultBackendPathKey] + driver.Path()
driver := kms.NewVault(noobaa.Name, noobaa.Namespace, uid)
secretPath := driver.Path()
if v, ok := (driver.Version(nil)).(*kms.VersionRotatingSecret); ok {
secretPath = v.BackendSecretName()
}
path := k.ConnectionDetails[vault.VaultBackendPathKey] + secretPath
cmd := exec.Command("kubectl", "exec", "vault-0", "--", "vault", "kv", "get", path)
logger.Printf("Running command: path %v args %v ", cmd.Path, cmd.Args)
err := cmd.Run()
Expand Down
40 changes: 40 additions & 0 deletions pkg/util/kms/test/tls-sa/kms_tls_sa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package kmstlstestsa
import (
"os"

"github.com/libopenstorage/secrets"
"github.com/libopenstorage/secrets/vault"
nbv1 "github.com/noobaa/noobaa-operator/v5/pkg/apis/noobaa/v1alpha1"
"github.com/noobaa/noobaa-operator/v5/pkg/options"
Expand Down Expand Up @@ -90,4 +91,43 @@ var _ = Describe("KMS - TLS Vault SA", func() {
})
})

Context("Verify Rotate", func() {
apiAddress, apiAddressFound := os.LookupEnv("API_ADDRESS")
noobaa := getMiniNooBaa()
noobaa.Spec.Security.KeyManagementService = tlsSAKMSSpec(apiAddress)
noobaa.Spec.Security.KeyManagementService.EnableKeyRotation = true
noobaa.Spec.Security.KeyManagementService.Schedule = "* * * * *" // every min

Specify("Verify API Address", func() {
Expect(apiAddressFound).To(BeTrue())
})
Specify("Create key rotate schedule system", func() {
Expect(util.KubeCreateFailExisting(noobaa)).To(BeTrue())
})
Specify("Verify KMS condition Type", func() {
Expect(util.NooBaaCondition(noobaa, nbv1.ConditionTypeKMSType, secrets.TypeVault)).To(BeTrue())
})
Specify("Verify KMS condition status Init", func() {
Expect(util.NooBaaCondStatus(noobaa, nbv1.ConditionKMSInit)).To(BeTrue())
})
Specify("Restart NooBaa operator", func() {
podList := &corev1.PodList{}
podSelector, _ := labels.Parse("noobaa-operator=deployment")
listOptions := client.ListOptions{Namespace: options.Namespace, LabelSelector: podSelector}

Expect(util.KubeList(podList, &listOptions)).To(BeTrue())
Expect(len(podList.Items)).To(BeEquivalentTo(1))
Expect(util.KubeDelete(&podList.Items[0])).To(BeTrue())
})
Specify("Verify KMS condition status Sync", func() {
Expect(util.NooBaaCondStatus(noobaa, nbv1.ConditionKMSSync)).To(BeTrue())
})
Specify("Verify KMS condition status Key Rotate", func() {
Expect(util.NooBaaCondStatus(noobaa, nbv1.ConditionKMSKeyRotate)).To(BeTrue())
})
Specify("Delete NooBaa", func() {
Expect(util.KubeDelete(noobaa)).To(BeTrue())
})
})

})
44 changes: 44 additions & 0 deletions pkg/util/kms/test/tls-token/kms_tls_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package kmstlstesttoken
import (
"os"

"github.com/libopenstorage/secrets"
"github.com/libopenstorage/secrets/vault"
nbv1 "github.com/noobaa/noobaa-operator/v5/pkg/apis/noobaa/v1alpha1"
"github.com/noobaa/noobaa-operator/v5/pkg/options"
Expand Down Expand Up @@ -77,4 +78,47 @@ var _ = Describe("KMS - TLS Vault Token", func() {
Expect(util.KubeDelete(noobaa)).To(BeTrue())
})
})

Context("Verify Rotate", func() {
noobaa := getMiniNooBaa()
noobaa.Spec.Security.KeyManagementService = tlsTokenKMSSpec(tokenSecretName, apiAddress)
noobaa.Spec.Security.KeyManagementService.EnableKeyRotation = true
noobaa.Spec.Security.KeyManagementService.Schedule = "* * * * *" // every min

Specify("Verify API Address", func() {
Expect(apiAddressFound).To(BeTrue())
})
Specify("Verify Token secret", func() {
Expect(tokenSecretNameFound).To(BeTrue())
logger.Printf("💬 Found TOKEN_SECRET_NAME=%v", tokenSecretName)
logger.Printf("💬 KMS Spec %v", noobaa.Spec.Security.KeyManagementService)
})
Specify("Create key rotate schedule system", func() {
Expect(util.KubeCreateFailExisting(noobaa)).To(BeTrue())
})
Specify("Verify KMS condition Type", func() {
Expect(util.NooBaaCondition(noobaa, nbv1.ConditionTypeKMSType, secrets.TypeVault)).To(BeTrue())
})
Specify("Verify KMS condition status Init", func() {
Expect(util.NooBaaCondStatus(noobaa, nbv1.ConditionKMSInit)).To(BeTrue())
})
Specify("Restart NooBaa operator", func() {
podList := &corev1.PodList{}
podSelector, _ := labels.Parse("noobaa-operator=deployment")
listOptions := client.ListOptions{Namespace: options.Namespace, LabelSelector: podSelector}

Expect(util.KubeList(podList, &listOptions)).To(BeTrue())
Expect(len(podList.Items)).To(BeEquivalentTo(1))
Expect(util.KubeDelete(&podList.Items[0])).To(BeTrue())
})
Specify("Verify KMS condition status Sync", func() {
Expect(util.NooBaaCondStatus(noobaa, nbv1.ConditionKMSSync)).To(BeTrue())
})
Specify("Verify KMS condition status Key Rotate", func() {
Expect(util.NooBaaCondStatus(noobaa, nbv1.ConditionKMSKeyRotate)).To(BeTrue())
})
Specify("Delete NooBaa", func() {
Expect(util.KubeDelete(noobaa)).To(BeTrue())
})
})
})

0 comments on commit 87b1023

Please sign in to comment.