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.

Sign up for GitHub

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

Use the controller-runtime cache to get the decryption key #284

Merged
merged 1 commit into from
Aug 5, 2024
Merged

Use the controller-runtime cache to get the decryption key #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 1 addition & 34 deletions controllers/configurationpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,8 @@ func (r *ConfigurationPolicyReconciler) SetupWithManager(
// blank assignment to verify that ConfigurationPolicyReconciler implements reconcile.Reconciler
var _ reconcile.Reconciler = &ConfigurationPolicyReconciler{}

type cachedEncryptionKey struct {
key []byte
previousKey []byte
}

// ConfigurationPolicyReconciler reconciles a ConfigurationPolicy object
type ConfigurationPolicyReconciler struct {
cachedEncryptionKey *cachedEncryptionKey
// This client, initialized using mgr.Client() above, is a split client
// that reads objects from the cache and writes to the apiserver
client.Client
Expand Down Expand Up @@ -1090,13 +1084,12 @@ func (r *ConfigurationPolicyReconciler) handleTemplatization(
}

resolveOptions := templates.ResolveOptions{}
usedKeyCache := false

if usesEncryption(plc) {
var encryptionConfig templates.EncryptionConfig
var err error

encryptionConfig, usedKeyCache, err = r.getEncryptionConfig(plc, false)
encryptionConfig, err = r.getEncryptionConfig(context.TODO(), plc)
if err != nil {
addTemplateErrorViolation("", err.Error())

Expand Down Expand Up @@ -1170,32 +1163,6 @@ func (r *ConfigurationPolicyReconciler) handleTemplatization(

resolvedTemplate, tplErr := tmplResolver.ResolveTemplate(rawData, nil, &resolveOptions)
if tplErr != nil {
// If the error is because the padding is invalid, this either means the encrypted value was not
// generated by the "protect" template function or the AES key is incorrect. Control for a stale
// cached key.
isRefreshableError := errors.Is(tplErr, templates.ErrInvalidPKCS7Padding) ||
errors.Is(tplErr, templates.ErrInvalidAESKey) || errors.Is(tplErr, templates.ErrAESKeyNotSet)

if usedKeyCache && isRefreshableError {
log.V(2).Info(
"The template decryption failed likely due to an invalid encryption key, will refresh " +
"the encryption key cache and try the decryption again",
)

encryptionConfig, usedNewCache, err := r.getEncryptionConfig(plc, true)
if err != nil {
addTemplateErrorViolation("", err.Error())

return parentStatusUpdateNeeded, err
}

usedKeyCache = usedNewCache
resolveOptions.EncryptionConfig = encryptionConfig
resolvedTemplate, tplErr = tmplResolver.ResolveTemplate(rawData, nil, &resolveOptions)
}
}

if tplErr != nil { // this could be a *new* tplErr
if errors.Is(tplErr, templates.ErrInvalidAESKey) || errors.Is(tplErr, templates.ErrAESKeyNotSet) {
addTemplateErrorViolation("", `The "policy-encryption-key" Secret contains an invalid AES key`)

Expand Down
76 changes: 15 additions & 61 deletions controllers/encryption.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,92 +17,46 @@ import (

const IVAnnotation = "policy.open-cluster-management.io/encryption-iv"

// getEncryptionKey will get the encryption key in the managed cluster namespace used for policy template encryption.
func (r *ConfigurationPolicyReconciler) getEncryptionKey(namespace string) (*cachedEncryptionKey, error) {
// #nosec G101
const secretName = "policy-encryption-key"

ctx := context.TODO()
objectKey := types.NamespacedName{
Name: secretName,
Namespace: namespace,
}
encryptionSecret := &corev1.Secret{}

err := r.Get(ctx, objectKey, encryptionSecret)
if err != nil {
return nil, fmt.Errorf("failed to get the encryption key from Secret %s/%s: %w", namespace, secretName, err)
}

var key []byte
if len(encryptionSecret.Data["key"]) > 0 {
key = encryptionSecret.Data["key"]
}

var previousKey []byte
if len(encryptionSecret.Data["previousKey"]) > 0 {
previousKey = encryptionSecret.Data["previousKey"]
}

cachedKey := &cachedEncryptionKey{
key: key,
previousKey: previousKey,
}

return cachedKey, nil
}

// getEncryptionConfig returns the encryption config for decrypting values that were encrypted using Hub templates.
// The returned bool determines if the cache was used. This value can be helpful to know if the cache should be
// refreshed if the decryption failed. The forceRefresh argument will skip the cache and update it with what the
// API returns.
func (r *ConfigurationPolicyReconciler) getEncryptionConfig(policy *policyv1.ConfigurationPolicy, forceRefresh bool) (
templates.EncryptionConfig, bool, error,
func (r *ConfigurationPolicyReconciler) getEncryptionConfig(ctx context.Context, policy *policyv1.ConfigurationPolicy) (
templates.EncryptionConfig, error,
) {
log := log.WithValues("policy", policy.GetName())
usedCache := false

annotations := policy.GetAnnotations()
ivBase64 := annotations[IVAnnotation]

iv, err := base64.StdEncoding.DecodeString(ivBase64)
if err != nil {
err = fmt.Errorf(`the policy annotation of "%s" is not Base64: %w`, IVAnnotation, err)

return templates.EncryptionConfig{}, usedCache, err
return templates.EncryptionConfig{}, err
}

if r.cachedEncryptionKey == nil || forceRefresh {
r.cachedEncryptionKey = &cachedEncryptionKey{}
objectKey := types.NamespacedName{
Name: "policy-encryption-key",
Namespace: policy.GetNamespace(),
}

if r.cachedEncryptionKey.key == nil {
log.V(2).Info(
"The encryption key is not cached, getting the encryption key from the server for the EncryptionConfig " +
"object",
)

var err error
encryptionSecret := &corev1.Secret{}

// The managed cluster namespace will be the same namespace as the ConfigurationPolicy
r.cachedEncryptionKey, err = r.getEncryptionKey(policy.GetNamespace())
if err != nil {
return templates.EncryptionConfig{}, usedCache, err
}
} else {
log.V(2).Info("Using the cached encryption key for the EncryptionConfig object")
usedCache = true
err = r.Get(ctx, objectKey, encryptionSecret)
if err != nil {
return templates.EncryptionConfig{}, fmt.Errorf(
"failed to get the encryption key from Secret %s/policy-encryption-key: %w", policy.GetNamespace(), err,
)
}

encryptionConfig := templates.EncryptionConfig{
AESKey: r.cachedEncryptionKey.key,
AESKeyFallback: r.cachedEncryptionKey.previousKey,
AESKey: encryptionSecret.Data["key"],
AESKeyFallback: encryptionSecret.Data["previousKey"],
DecryptionConcurrency: r.DecryptionConcurrency,
DecryptionEnabled: true,
InitializationVector: iv,
}

return encryptionConfig, usedCache, nil
return encryptionConfig, nil
}

// usesEncryption detects if the initialization vector is set on the policy. If it is, then there
Expand Down
101 changes: 4 additions & 97 deletions controllers/encryption_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
package controllers

import (
"bytes"
"context"
"crypto/rand"
"testing"

Expand Down Expand Up @@ -67,81 +67,30 @@ func getEmptyPolicy() policyv1.ConfigurationPolicy {
}
}

func TestGetEncryptionKey(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

r := getReconciler(true)
cachedEncryptionKey, err := r.getEncryptionKey(clusterName)

Expect(err).ToNot(HaveOccurred())
Expect(cachedEncryptionKey.key).ToNot(BeNil())
Expect(cachedEncryptionKey.previousKey).ToNot(BeNil())
}

func TestGetEncryptionKeyFail(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

r := getReconciler(false)
cachedEncryptionKey, err := r.getEncryptionKey(clusterName)

Expect(err).To(HaveOccurred())
Expect(cachedEncryptionKey).To(BeNil())
}

func TestGetEncryptionConfig(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

r := getReconciler(true)
Expect(r.cachedEncryptionKey).To(BeNil())

policy := getEmptyPolicy()

config, usedCache, err := r.getEncryptionConfig(&policy, false)
config, err := r.getEncryptionConfig(context.TODO(), &policy)
Expect(err).ToNot(HaveOccurred())
Expect(usedCache).To(BeFalse())
Expect(config).ToNot(BeNil())
Expect(config.AESKey).ToNot(BeNil())
Expect(config.AESKeyFallback).ToNot(BeNil())
Expect(config.DecryptionEnabled).To(BeTrue())
Expect(config.DecryptionConcurrency).To(Equal(uint8(5)))
Expect(config.EncryptionEnabled).To(BeFalse())
Expect(config.InitializationVector).ToNot(BeNil())

Expect(r.cachedEncryptionKey).ToNot(BeNil())
Expect(r.cachedEncryptionKey.key).ToNot(BeNil())
Expect(r.cachedEncryptionKey.previousKey).ToNot(BeNil())
}

func TestGetEncryptionConfigCached(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

// Ensure there is no secret so that if the cache isn't used, getEncryptionConfig will fail.
r := getReconciler(false)

key := make([]byte, keySize/8)
_, err := rand.Read(key)
Expect(err).ToNot(HaveOccurred())

r.cachedEncryptionKey = &cachedEncryptionKey{key: key}
policy := getEmptyPolicy()

config, usedCache, err := r.getEncryptionConfig(&policy, false)
Expect(err).ToNot(HaveOccurred())
Expect(usedCache).To(BeTrue())
Expect(config).ToNot(BeNil())
Expect(config.AESKey).ToNot(BeNil())
}

func TestGetEncryptionConfigInvalidIV(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

r := getReconciler(true)
Expect(r.cachedEncryptionKey).To(BeNil())

policy := policyv1.ConfigurationPolicy{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -151,7 +100,7 @@ func TestGetEncryptionConfigInvalidIV(t *testing.T) {
},
}

_, _, err := r.getEncryptionConfig(&policy, false)
_, err := r.getEncryptionConfig(context.TODO(), &policy)
Expect(err.Error()).To(
Equal(
"the policy annotation of \"policy.open-cluster-management.io/encryption-iv\" is not Base64: illegal " +
Expand All @@ -165,11 +114,10 @@ func TestGetEncryptionConfigNoSecret(t *testing.T) {
RegisterFailHandler(Fail)

r := getReconciler(false)
Expect(r.cachedEncryptionKey).To(BeNil())

policy := getEmptyPolicy()

_, _, err := r.getEncryptionConfig(&policy, false)
_, err := r.getEncryptionConfig(context.TODO(), &policy)
Expect(err.Error()).To(
Equal(
`failed to get the encryption key from Secret local-cluster/policy-encryption-key: secrets ` +
Expand All @@ -178,47 +126,6 @@ func TestGetEncryptionConfigNoSecret(t *testing.T) {
)
}

func TestGetEncryptionConfigForceRefresh(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

r := getReconciler(true)
key := bytes.Repeat([]byte{byte('A')}, keySize/8)
r.cachedEncryptionKey = &cachedEncryptionKey{key: key}

policy := getEmptyPolicy()

config, usedCache, err := r.getEncryptionConfig(&policy, true)
Expect(err).ToNot(HaveOccurred())
Expect(usedCache).To(BeFalse())
Expect(config).ToNot(BeNil())
Expect(config.AESKey).ToNot(Equal(key))
}

func TestGetEncryptionKeyEmptySecret(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)

encryptionSecret := &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: secretName,
Namespace: clusterName,
},
Data: map[string][]byte{
"key": {},
"previousKey": {},
},
}
client := fake.NewClientBuilder().WithObjects(encryptionSecret).Build()

r := ConfigurationPolicyReconciler{Client: client, DecryptionConcurrency: 5}
cachedEncryptionKey, err := r.getEncryptionKey(clusterName)

Expect(err).ToNot(HaveOccurred())
Expect(cachedEncryptionKey.key).To(BeNil())
Expect(cachedEncryptionKey.previousKey).To(BeNil())
}

func TestUsesEncryption(t *testing.T) {
t.Parallel()
RegisterFailHandler(Fail)
Expand Down
22 changes: 14 additions & 8 deletions main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,13 @@ func main() {
}),
}

cacheByObject[&corev1.Secret{}] = cache.ByObject{
Field: fields.SelectorFromSet(fields.Set{
"metadata.namespace": watchNamespace,
"metadata.name": "policy-encryption-key",
}),
}

if opts.enableOperatorPolicy {
cacheByObject[&policyv1beta1.OperatorPolicy{}] = cache.ByObject{
Field: fields.SelectorFromSet(fields.Set{
Expand All @@ -241,7 +248,13 @@ func main() {
}
}
} else {
log.Info("Skipping restrictions on the ConfigurationPolicy cache because watchNamespace is empty")
log.Info("Skipping namespace restrictions on the cache because watchNamespace is empty")

cacheByObject[&corev1.Secret{}] = cache.ByObject{
Field: fields.SelectorFromSet(fields.Set{
"metadata.name": "policy-encryption-key",
}),
}
}

// No need to resync every 10 hours.
Expand Down Expand Up @@ -274,13 +287,6 @@ func main() {
ByObject: cacheByObject,
SyncPeriod: &disableResync,
},
// Disable the cache for Secrets to avoid a watch getting created when the `policy-encryption-key`
// Secret is retrieved. Special cache handling is done by the controller.
Client: client.Options{
Cache: &client.CacheOptions{
DisableFor: []client.Object{&corev1.Secret{}},
},
},
// Override the EventBroadcaster so that the spam filter will not ignore events for the policy but with
// different messages if a large amount of events for that policy are sent in a short time.
EventBroadcaster: record.NewBroadcasterWithCorrelatorOptions(
Expand Down