diff --git a/api/bases/keystone.openstack.org_keystoneapis.yaml b/api/bases/keystone.openstack.org_keystoneapis.yaml index 1ce9da9e..f460b115 100644 --- a/api/bases/keystone.openstack.org_keystoneapis.yaml +++ b/api/bases/keystone.openstack.org_keystoneapis.yaml @@ -89,6 +89,20 @@ spec: description: EnableSecureRBAC - Enable Consistent and Secure RBAC policies type: boolean + fernetMaxActiveKeys: + default: 5 + description: FernetMaxActiveKeys - Maximum number of fernet token + keys after rotation + format: int32 + minimum: 3 + type: integer + fernetRotationDays: + default: 1 + description: FernetRotationDays - Rotate fernet token keys every X + days + format: int32 + minimum: 1 + type: integer memcachedInstance: default: memcached description: Memcached instance name. diff --git a/api/v1beta1/keystoneapi_types.go b/api/v1beta1/keystoneapi_types.go index a2064d7f..a4c9b3cc 100644 --- a/api/v1beta1/keystoneapi_types.go +++ b/api/v1beta1/keystoneapi_types.go @@ -119,6 +119,18 @@ type KeystoneAPISpecCore struct { // TrustFlushSuspend - Suspend the cron job to purge trusts TrustFlushSuspend bool `json:"trustFlushSuspend"` + // +kubebuilder:validation:Optional + // +kubebuilder:default=1 + // +kubebuilder:validation:Minimum=1 + // FernetRotationDays - Rotate fernet token keys every X days + FernetRotationDays *int32 `json:"fernetRotationDays"` + + // +kubebuilder:validation:Optional + // +kubebuilder:default=5 + // +kubebuilder:validation:Minimum=3 + // FernetMaxActiveKeys - Maximum number of fernet token keys after rotation + FernetMaxActiveKeys *int32 `json:"fernetMaxActiveKeys"` + // +kubebuilder:validation:Optional // +kubebuilder:default={admin: AdminPassword} // PasswordSelectors - Selectors to identify the AdminUser password from the Secret diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index f079bd32..7ee47492 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -147,6 +147,16 @@ func (in *KeystoneAPISpecCore) DeepCopyInto(out *KeystoneAPISpecCore) { *out = new(int32) **out = **in } + if in.FernetRotationDays != nil { + in, out := &in.FernetRotationDays, &out.FernetRotationDays + *out = new(int32) + **out = **in + } + if in.FernetMaxActiveKeys != nil { + in, out := &in.FernetMaxActiveKeys, &out.FernetMaxActiveKeys + *out = new(int32) + **out = **in + } out.PasswordSelectors = in.PasswordSelectors if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector diff --git a/config/crd/bases/keystone.openstack.org_keystoneapis.yaml b/config/crd/bases/keystone.openstack.org_keystoneapis.yaml index 1ce9da9e..f460b115 100644 --- a/config/crd/bases/keystone.openstack.org_keystoneapis.yaml +++ b/config/crd/bases/keystone.openstack.org_keystoneapis.yaml @@ -89,6 +89,20 @@ spec: description: EnableSecureRBAC - Enable Consistent and Secure RBAC policies type: boolean + fernetMaxActiveKeys: + default: 5 + description: FernetMaxActiveKeys - Maximum number of fernet token + keys after rotation + format: int32 + minimum: 3 + type: integer + fernetRotationDays: + default: 1 + description: FernetRotationDays - Rotate fernet token keys every X + days + format: int32 + minimum: 1 + type: integer memcachedInstance: default: memcached description: Memcached instance name. diff --git a/controllers/keystoneapi_controller.go b/controllers/keystoneapi_controller.go index f8db9a4b..e7f3e2da 100644 --- a/controllers/keystoneapi_controller.go +++ b/controllers/keystoneapi_controller.go @@ -857,7 +857,6 @@ func (r *KeystoneAPIReconciler) reconcileNormal( // // Create secret holding fernet keys (for token and credential) // - // TODO key rotation err = r.ensureFernetKeys(ctx, instance, helper, &configMapVars) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( @@ -1321,37 +1320,53 @@ func (r *KeystoneAPIReconciler) reconcileCloudConfig( return oko_secret.EnsureSecrets(ctx, h, instance, secrets, nil) } -// ensureFernetKeys - creates secret with fernet keys +// ensureFernetKeys - creates secret with fernet keys, rotates the keys func (r *KeystoneAPIReconciler) ensureFernetKeys( ctx context.Context, instance *keystonev1.KeystoneAPI, helper *helper.Helper, envVars *map[string]env.Setter, ) error { + fernetAnnotation := labels.GetGroupLabel(keystone.ServiceName) + "/rotatedat" labels := labels.GetLabels(instance, labels.GetGroupLabel(keystone.ServiceName), map[string]string{}) + now := time.Now().UTC() // // check if secret already exist // secretName := keystone.ServiceName + var numberKeys int + if instance.Spec.FernetMaxActiveKeys == nil { + numberKeys = keystone.DefaultFernetMaxActiveKeys + } else { + numberKeys = int(*instance.Spec.FernetMaxActiveKeys) + } + secret, hash, err := oko_secret.GetSecret(ctx, helper, secretName, instance.Namespace) + if err != nil && !k8s_errors.IsNotFound(err) { return err } else if k8s_errors.IsNotFound(err) { fernetKeys := map[string]string{ - "FernetKeys0": keystone.GenerateFernetKey(), - "FernetKeys1": keystone.GenerateFernetKey(), "CredentialKeys0": keystone.GenerateFernetKey(), "CredentialKeys1": keystone.GenerateFernetKey(), } + for i := 0; i < numberKeys; i++ { + fernetKeys[fmt.Sprintf("FernetKeys%d", i)] = keystone.GenerateFernetKey() + } + + annotations := map[string]string{ + fernetAnnotation: now.Format(time.RFC3339)} + tmpl := []util.Template{ { - Name: secretName, - Namespace: instance.Namespace, - Type: util.TemplateTypeNone, - CustomData: fernetKeys, - Labels: labels, + Name: secretName, + Namespace: instance.Namespace, + Type: util.TemplateTypeNone, + CustomData: fernetKeys, + Labels: labels, + Annotations: annotations, }, } err := oko_secret.EnsureSecrets(ctx, helper, instance, tmpl, envVars) @@ -1361,9 +1376,99 @@ func (r *KeystoneAPIReconciler) ensureFernetKeys( } else { // add hash to envVars (*envVars)[secret.Name] = env.SetValue(hash) - } - // TODO: fernet key rotation + changedKeys := false + + extraKey := fmt.Sprintf("FernetKeys%d", numberKeys) + + // + // Fernet Key rotation + // + if secret.Annotations == nil { + secret.Annotations = map[string]string{} + } + rotatedAt, err := time.Parse(time.RFC3339, secret.Annotations[fernetAnnotation]) + + var duration int + if instance.Spec.FernetRotationDays == nil { + duration = keystone.DefaultFernetRotationDays + } else { + duration = int(*instance.Spec.FernetRotationDays) + } + + if err != nil { + changedKeys = true + } else if rotatedAt.AddDate(0, 0, duration).Before(now) { + secret.Data[extraKey] = secret.Data["FernetKeys0"] + secret.Data["FernetKeys0"] = []byte(keystone.GenerateFernetKey()) + } + + // + // Remove extra keys when FernetMaxActiveKeys changes + // + for { + _, exists := secret.Data[extraKey] + if !exists { + break + } + changedKeys = true + i := 1 + for { + key := fmt.Sprintf("FernetKeys%d", i) + i++ + nextKey := fmt.Sprintf("FernetKeys%d", i) + _, exists = secret.Data[nextKey] + if !exists { + break + } + secret.Data[key] = secret.Data[nextKey] + delete(secret.Data, nextKey) + } + } + + // + // Add extra keys when FernetMaxActiveKeys changes + // + lastKey := fmt.Sprintf("FernetKeys%d", numberKeys-1) + for { + _, exists := secret.Data[lastKey] + if exists { + break + } + changedKeys = true + i := 1 + nextKeyValue := []byte(keystone.GenerateFernetKey()) + for { + key := fmt.Sprintf("FernetKeys%d", i) + i++ + keyValue, exists := secret.Data[key] + secret.Data[key] = nextKeyValue + nextKeyValue = keyValue + if !exists { + break + } + } + } + + if !changedKeys { + return nil + } + + fernetKeys := make(map[string]string, len(secret.Data)) + for k, v := range secret.Data { + fernetKeys[k] = string(v[:]) + } + + secret.Annotations[fernetAnnotation] = now.Format(time.RFC3339) + + // use update to apply changes to the secret, since EnsureSecrets + // does not handle annotation updates, also CreateOrPatchSecret would + // preserve the existing annotation + err = helper.GetClient().Update(ctx, secret, &client.UpdateOptions{}) + if err != nil { + return err + } + } return nil } diff --git a/pkg/keystone/bootstrap.go b/pkg/keystone/bootstrap.go index f03b1cdc..8748673d 100644 --- a/pkg/keystone/bootstrap.go +++ b/pkg/keystone/bootstrap.go @@ -60,12 +60,12 @@ func BootstrapJob( } // create Volume and VolumeMounts - volumes := getVolumes(instance.Name) + volumes := getVolumes(instance) volumeMounts := getVolumeMounts() // add CA cert if defined if instance.Spec.TLS.CaBundleSecretName != "" { - volumes = append(getVolumes(instance.Name), instance.Spec.TLS.CreateVolume()) + volumes = append(getVolumes(instance), instance.Spec.TLS.CreateVolume()) volumeMounts = append(getVolumeMounts(), instance.Spec.TLS.CreateVolumeMounts(nil)...) } diff --git a/pkg/keystone/const.go b/pkg/keystone/const.go index b2e112f6..205b68b5 100644 --- a/pkg/keystone/const.go +++ b/pkg/keystone/const.go @@ -28,4 +28,9 @@ const ( KeystonePublicPort int32 = 5000 // KeystoneInternalPort - KeystoneInternalPort int32 = 5000 + + // DefaultFernetMaxActiveKeys - + DefaultFernetMaxActiveKeys = 5 + // DefaultFernetRotationDays - + DefaultFernetRotationDays = 1 ) diff --git a/pkg/keystone/cronjob.go b/pkg/keystone/cronjob.go index 179ade74..38a1051b 100644 --- a/pkg/keystone/cronjob.go +++ b/pkg/keystone/cronjob.go @@ -46,12 +46,12 @@ func CronJob( completions := int32(1) // create Volume and VolumeMounts - volumes := getVolumes(instance.Name) + volumes := getVolumes(instance) volumeMounts := getVolumeMounts() // add CA cert if defined if instance.Spec.TLS.CaBundleSecretName != "" { - volumes = append(getVolumes(instance.Name), instance.Spec.TLS.CreateVolume()) + volumes = append(getVolumes(instance), instance.Spec.TLS.CreateVolume()) volumeMounts = append(getVolumeMounts(), instance.Spec.TLS.CreateVolumeMounts(nil)...) } diff --git a/pkg/keystone/dbsync.go b/pkg/keystone/dbsync.go index 652f1185..e760b4d6 100644 --- a/pkg/keystone/dbsync.go +++ b/pkg/keystone/dbsync.go @@ -45,12 +45,13 @@ func DbSyncJob( envVars["KOLLA_BOOTSTRAP"] = env.SetValue("true") // create Volume and VolumeMounts - volumes := getVolumes(instance.Name) + volumes := getVolumes(instance) volumeMounts := getVolumeMounts() // add CA cert if defined if instance.Spec.TLS.CaBundleSecretName != "" { - volumes = append(getVolumes(instance.Name), instance.Spec.TLS.CreateVolume()) + //TODO(afaranha): Why not reuse the 'volumes'? + volumes = append(getVolumes(instance), instance.Spec.TLS.CreateVolume()) volumeMounts = append(getVolumeMounts(), instance.Spec.TLS.CreateVolumeMounts(nil)...) } diff --git a/pkg/keystone/deployment.go b/pkg/keystone/deployment.go index 70cbb663..00c418f4 100644 --- a/pkg/keystone/deployment.go +++ b/pkg/keystone/deployment.go @@ -80,7 +80,7 @@ func Deployment( envVars["CONFIG_HASH"] = env.SetValue(configHash) // create Volume and VolumeMounts - volumes := getVolumes(instance.Name) + volumes := getVolumes(instance) volumeMounts := getVolumeMounts() // add CA cert if defined diff --git a/pkg/keystone/fernet.go b/pkg/keystone/fernet.go index 635b77db..10f9a08f 100644 --- a/pkg/keystone/fernet.go +++ b/pkg/keystone/fernet.go @@ -17,7 +17,6 @@ package keystone import ( "encoding/base64" - "math/rand" ) diff --git a/pkg/keystone/volumes.go b/pkg/keystone/volumes.go index 3055a62d..2f863ace 100644 --- a/pkg/keystone/volumes.go +++ b/pkg/keystone/volumes.go @@ -16,14 +16,30 @@ limitations under the License. package keystone import ( + "fmt" + keystonev1 "github.com/openstack-k8s-operators/keystone-operator/api/v1beta1" corev1 "k8s.io/api/core/v1" ) // getVolumes - service volumes -func getVolumes(name string) []corev1.Volume { +func getVolumes(instance *keystonev1.KeystoneAPI) []corev1.Volume { + name := instance.Name var scriptsVolumeDefaultMode int32 = 0755 var config0640AccessMode int32 = 0640 + fernetKeys := []corev1.KeyToPath{} + numberKeys := int(*instance.Spec.FernetMaxActiveKeys) + + for i := 0; i < numberKeys; i++ { + fernetKeys = append( + fernetKeys, + corev1.KeyToPath{ + Key: fmt.Sprintf("FernetKeys%d", i), + Path: fmt.Sprintf("%d", i), + }, + ) + } + return []corev1.Volume{ { Name: "scripts", @@ -48,16 +64,7 @@ func getVolumes(name string) []corev1.Volume { VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: ServiceName, - Items: []corev1.KeyToPath{ - { - Key: "FernetKeys0", - Path: "0", - }, - { - Key: "FernetKeys1", - Path: "1", - }, - }, + Items: fernetKeys, }, }, }, diff --git a/tests/functional/keystoneapi_controller_test.go b/tests/functional/keystoneapi_controller_test.go index 5633afb0..7dd98fa9 100644 --- a/tests/functional/keystoneapi_controller_test.go +++ b/tests/functional/keystoneapi_controller_test.go @@ -19,6 +19,8 @@ package functional_test import ( "fmt" "os" + "strconv" + "time" . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports @@ -33,6 +35,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" ) var _ = Describe("Keystone controller", func() { @@ -1106,6 +1109,71 @@ var _ = Describe("Keystone controller", func() { }) }) + // Set rotated at to past date, triggering rotation + When("When the fernet token rotate", func() { + BeforeEach(func() { + DeferCleanup( + k8sClient.Delete, ctx, CreateKeystoneMessageBusSecret(namespace, "rabbitmq-secret")) + DeferCleanup(th.DeleteInstance, CreateKeystoneAPI(keystoneAPIName, GetDefaultKeystoneAPISpec())) + DeferCleanup( + k8sClient.Delete, ctx, CreateKeystoneAPISecret(namespace, SecretName)) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, "memcached", memcachedSpec)) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + namespace, + GetKeystoneAPI(keystoneAPIName).Spec.DatabaseInstance, + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + mariadb.SimulateMariaDBAccountCompleted(keystoneAccountName) + mariadb.SimulateMariaDBDatabaseCompleted(keystoneDatabaseName) + infra.SimulateTransportURLReady(types.NamespacedName{ + Name: fmt.Sprintf("%s-keystone-transport", keystoneAPIName.Name), + Namespace: namespace, + }) + infra.SimulateMemcachedReady(types.NamespacedName{ + Name: "memcached", + Namespace: namespace, + }) + th.SimulateJobSuccess(dbSyncJobName) + th.SimulateJobSuccess(bootstrapJobName) + th.SimulateDeploymentReplicaReady(deploymentName) + }) + + It("rotates the fernet keys", func() { + keystone := GetKeystoneAPI(keystoneAPIName) + currentHash := keystone.Status.Hash["input"] + + currentSecret := th.GetSecret(types.NamespacedName{Namespace: keystoneAPIName.Namespace, Name: "keystone"}) + Expect(currentSecret).ToNot(BeNil()) + + rotatedAt, err := time.Parse(time.RFC3339, currentSecret.Annotations["keystone.openstack.org/rotatedat"]) + Expect(err).ToNot(HaveOccurred()) + + // set date to yesterday + currentSecret.Annotations["keystone.openstack.org/rotatedat"] = rotatedAt.Add(-25 * time.Hour).Format(time.RFC3339) + err = k8sClient.Update(ctx, ptr.To(currentSecret), &client.UpdateOptions{}) + Expect(err).ToNot(HaveOccurred()) + + Eventually(func(g Gomega) { + keystone = GetKeystoneAPI(keystoneAPIName) + g.Expect(keystone.Status.Hash["input"]).ToNot(Equal(currentHash)) + + updatedSecret := th.GetSecret(types.NamespacedName{Namespace: keystoneAPIName.Namespace, Name: "keystone"}) + g.Expect(updatedSecret).ToNot(BeNil()) + + oldKey := string(currentSecret.Data["FernetKeys"+strconv.Itoa(0)]) + newKey := string(updatedSecret.Data["FernetKeys"+strconv.Itoa((4))]) + + g.Expect(oldKey).To(Equal(newKey)) + }, timeout, interval).Should(Succeed()) + + }) + }) + // Run MariaDBAccount suite tests. these are pre-packaged ginkgo tests // that exercise standard account create / update patterns that should be // common to all controllers that ensure MariaDBAccount CRs. diff --git a/tests/kuttl/tests/keystone_tls/01-assert.yaml b/tests/kuttl/tests/keystone_tls/01-assert.yaml index fd413a48..0f44dce3 100644 --- a/tests/kuttl/tests/keystone_tls/01-assert.yaml +++ b/tests/kuttl/tests/keystone_tls/01-assert.yaml @@ -88,6 +88,12 @@ spec: path: "0" - key: FernetKeys1 path: "1" + - key: FernetKeys2 + path: "2" + - key: FernetKeys3 + path: "3" + - key: FernetKeys4 + path: "4" secretName: keystone - name: credential-keys secret: