Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add fernet key rotation #478

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions api/bases/keystone.openstack.org_keystoneapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions api/v1beta1/keystoneapi_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
xek marked this conversation as resolved.
Show resolved Hide resolved

// +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
Expand Down
10 changes: 10 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions config/crd/bases/keystone.openstack.org_keystoneapis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
127 changes: 116 additions & 11 deletions controllers/keystoneapi_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand All @@ -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
Copy link
Contributor

@olliewalsh olliewalsh Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: would move this (L1392-L1397) up so that err is checked right after setting

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())
olliewalsh marked this conversation as resolved.
Show resolved Hide resolved
}

//
// 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[:])
}

stuggi marked this conversation as resolved.
Show resolved Hide resolved
secret.Annotations[fernetAnnotation] = now.Format(time.RFC3339)
xek marked this conversation as resolved.
Show resolved Hide resolved

// 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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/keystone/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)...)
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/keystone/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,9 @@ const (
KeystonePublicPort int32 = 5000
// KeystoneInternalPort -
KeystoneInternalPort int32 = 5000

// DefaultFernetMaxActiveKeys -
DefaultFernetMaxActiveKeys = 5
// DefaultFernetRotationDays -
DefaultFernetRotationDays = 1
)
4 changes: 2 additions & 2 deletions pkg/keystone/cronjob.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)...)
}

Expand Down
5 changes: 3 additions & 2 deletions pkg/keystone/dbsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)...)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/keystone/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion pkg/keystone/fernet.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package keystone

import (
"encoding/base64"

"math/rand"
)

Expand Down
29 changes: 18 additions & 11 deletions pkg/keystone/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
},
},
},
Expand Down
Loading
Loading