From abaa55e36ec42bcade37586fbeabfb5bf48cd5b8 Mon Sep 17 00:00:00 2001 From: Damien Grisonnet Date: Thu, 16 Feb 2023 17:39:17 +0100 Subject: [PATCH] encryption: enable aes-gcm provider Signed-off-by: Damien Grisonnet --- .../encryption/controllers/key_controller.go | 2 +- .../controllers/key_controller_test.go | 78 +++++++++++++------ pkg/operator/encryption/crypto/keys.go | 1 + .../encryption/encryptionconfig/config.go | 15 +++- .../encryptionconfig/config_test.go | 34 ++++++++ pkg/operator/encryption/secrets/secrets.go | 2 +- .../encryption/secrets/secrets_test.go | 34 ++++++++ pkg/operator/encryption/state/types.go | 3 +- pkg/operator/encryption/testing/helpers.go | 60 +++++++++----- 9 files changed, 178 insertions(+), 51 deletions(-) diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 03c1a088c5..7970c83ff1 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -270,7 +270,7 @@ func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (st reason := encryptionConfig.Encryption.Reason switch currentMode := state.Mode(apiServer.Spec.Encryption.Type); currentMode { - case state.AESCBC, state.Identity: // secretbox is disabled for now + case state.AESCBC, state.AESGCM, state.Identity: // secretbox is disabled for now return currentMode, reason, nil case "": // unspecified means use the default (which can change over time) return state.DefaultMode, reason, nil diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index a6d432fd2a..92d8890891 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -31,14 +31,13 @@ import ( ) func TestKeyController(t *testing.T) { - apiServerAesCBC := []runtime.Object{&configv1.APIServer{ - ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, - Spec: configv1.APIServerSpec{ - Encryption: configv1.APIServerEncryption{ - Type: "aescbc", - }, - }, - }} + simpleAPIServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + + apiServerWithAESCBC := simpleAPIServer.DeepCopy() + apiServerWithAESCBC.Spec.Encryption = configv1.APIServerEncryption{Type: "aescbc"} + + apiServerWithAESGCM := simpleAPIServer.DeepCopy() + apiServerWithAESGCM.Spec.Encryption = configv1.APIServerEncryption{Type: "aesgcm"} scenarios := []struct { name string @@ -73,7 +72,7 @@ func TestKeyController(t *testing.T) { }, targetNamespace: "kms", initialObjects: []runtime.Object{}, - apiServerObjects: []runtime.Object{&configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}}, + apiServerObjects: []runtime.Object{simpleAPIServer}, validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { }, expectedActions: []string{"list:pods:kms"}, @@ -86,7 +85,7 @@ func TestKeyController(t *testing.T) { }, targetNamespace: "kms", initialObjects: []runtime.Object{encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1")}, - apiServerObjects: []runtime.Object{&configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}}, + apiServerObjects: []runtime.Object{simpleAPIServer}, validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { }, expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, @@ -104,14 +103,7 @@ func TestKeyController(t *testing.T) { initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), }, - apiServerObjects: []runtime.Object{&configv1.APIServer{ - ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, - Spec: configv1.APIServerSpec{ - Encryption: configv1.APIServerEncryption{ - Type: "aescbc", - }, - }, - }}, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { wasSecretValidated := false for _, action := range actions { @@ -145,7 +137,7 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateEncryptionKeySecretWithRawKey("kms", nil, 7, []byte("61def964fb967f5d7c44a2af8dab6865")), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, @@ -159,7 +151,7 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateMigratedEncryptionKeySecretWithRawKey("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 7, []byte("61def964fb967f5d7c44a2af8dab6865"), time.Now()), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, @@ -173,7 +165,7 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateEncryptionKeySecretWithRawKey("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 7, []byte("61def964fb967f5d7c44a2af8dab6865")), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"}, }, @@ -187,7 +179,7 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateExpiredMigratedEncryptionKeySecretWithRawKey("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("61def964fb967f5d7c44a2af8dab6865")), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"}, validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { @@ -245,7 +237,7 @@ func TestKeyController(t *testing.T) { return ecs }(), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{ "list:pods:kms", @@ -266,7 +258,7 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateExpiredMigratedEncryptionKeySecretWithRawKey("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("61def964fb967f5d7c44a2af8dab6865")), encryptiontesting.CreateEncryptionKeySecretWithRawKey("kms", nil, 6, []byte("61def964fb967f5d7c44a2af8dab6865")), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"}, }, @@ -280,7 +272,7 @@ func TestKeyController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), encryptiontesting.CreateEncryptionKeySecretWithRawKey("kms", nil, 1, []byte("")), }, - apiServerObjects: apiServerAesCBC, + apiServerObjects: []runtime.Object{apiServerWithAESCBC}, targetNamespace: "kms", expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "get:secrets:openshift-config-managed"}, validateOperatorClientFunc: func(ts *testing.T, operatorClient v1helpers.OperatorClient) { @@ -294,6 +286,42 @@ func TestKeyController(t *testing.T) { }, expectedError: errors.New("secret encryption-key-kms-1 is invalid, new keys cannot be created for encryption target"), }, + + { + name: "creates a new write key because the encryption mode changed", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("61def964fb967f5d7c44a2af8dab6865"), "aescbc"), + }, + apiServerObjects: []runtime.Object{apiServerWithAESGCM}, + targetNamespace: "kms", + expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"}, + validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) { + wasSecretValidated := false + for _, action := range actions { + if action.Matches("create", "secrets") { + createAction := action.(clientgotesting.CreateAction) + actualSecret := createAction.GetObject().(*corev1.Secret) + expectedSecret := encryptiontesting.CreateEncryptionKeySecretWithKeyFromExistingSecretWithMode(targetNamespace, []schema.GroupResource{}, 6, actualSecret, "aesgcm") + expectedSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] = "secrets-encryption-mode-changed" + if !equality.Semantic.DeepEqual(actualSecret, expectedSecret) { + ts.Errorf(diff.ObjectDiff(expectedSecret, actualSecret)) + } + if err := encryptiontesting.ValidateEncryptionKey(actualSecret); err != nil { + ts.Error(err) + } + wasSecretValidated = true + break + } + } + if !wasSecretValidated { + ts.Errorf("the secret wasn't created and validated") + } + }, + }, } for _, scenario := range scenarios { diff --git a/pkg/operator/encryption/crypto/keys.go b/pkg/operator/encryption/crypto/keys.go index 2d660ce1d5..a623d30f79 100644 --- a/pkg/operator/encryption/crypto/keys.go +++ b/pkg/operator/encryption/crypto/keys.go @@ -9,6 +9,7 @@ import ( var ( ModeToNewKeyFunc = map[state.Mode]func() []byte{ state.AESCBC: NewAES256Key, + state.AESGCM: NewAES256Key, state.SecretBox: NewAES256Key, // secretbox requires a 32 byte key so we can reuse the same function here state.Identity: NewIdentityKey, } diff --git a/pkg/operator/encryption/encryptionconfig/config.go b/pkg/operator/encryption/encryptionconfig/config.go index e17cf55ad0..52dc997f88 100644 --- a/pkg/operator/encryption/encryptionconfig/config.go +++ b/pkg/operator/encryption/encryptionconfig/config.go @@ -95,10 +95,15 @@ func ToEncryptionState(encryptionConfig *apiserverconfigv1.EncryptionConfigurati // skip fake provider. If this is write-key, wait for first aesgcm provider providing the write key. continue - case provider.AESGCM != nil && len(provider.AESGCM.Keys) == 1 && provider.AESGCM.Keys[0].Secret == emptyStaticIdentityKey: + case provider.AESGCM != nil && len(provider.AESGCM.Keys) == 1: + s := state.AESGCM + if provider.AESGCM.Keys[0].Secret == emptyStaticIdentityKey { + s = state.Identity + } + ks = state.KeyState{ Key: provider.AESGCM.Keys[0], - Mode: state.Identity, + Mode: s, } default: @@ -164,6 +169,12 @@ func stateToProviders(desired state.GroupResourceState) []apiserverconfigv1.Prov Keys: []apiserverconfigv1.Key{key.Key}, }, }) + case state.AESGCM: + providers = append(providers, apiserverconfigv1.ProviderConfiguration{ + AESGCM: &apiserverconfigv1.AESConfiguration{ + Keys: []apiserverconfigv1.Key{key.Key}, + }, + }) case state.SecretBox: providers = append(providers, apiserverconfigv1.ProviderConfiguration{ Secretbox: &apiserverconfigv1.SecretboxConfiguration{ diff --git a/pkg/operator/encryption/encryptionconfig/config_test.go b/pkg/operator/encryption/encryptionconfig/config_test.go index d18933a6e9..9ef6fbf7fe 100644 --- a/pkg/operator/encryption/encryptionconfig/config_test.go +++ b/pkg/operator/encryption/encryptionconfig/config_test.go @@ -327,6 +327,40 @@ func TestToEncryptionState(t *testing.T) { // scenario 9 // TODO: encryption on after being off + + // scenario 10 + { + name: "aes-gcm write key and aes-cbc read key", + input: func() *apiserverconfigv1.EncryptionConfiguration { + keysRes := encryptiontesting.EncryptionKeysResourceTuple{ + Resource: "secrets", + Keys: []apiserverconfigv1.Key{ + { + Name: "35", + Secret: "MTcxNTgyYTBmY2Q2YzVmZGI2NWNiZjVhM2U5MjQ5ZDc=", + }, + { + Name: "34", + Secret: "MTcxNTgyYTBmY2Q2YzVmZGI2NWNiZjVhM2U5MjQ5ZDc=", + }, + }, + Modes: []string{"aesgcm", "aescbc"}, + } + ec := encryptiontesting.CreateEncryptionCfgWithWriteKey([]encryptiontesting.EncryptionKeysResourceTuple{keysRes}) + return ec + }(), + output: map[schema.GroupResource]state.GroupResourceState{ + {Group: "", Resource: "secrets"}: { + WriteKey: state.KeyState{ + Key: apiserverconfigv1.Key{Name: "35", Secret: "MTcxNTgyYTBmY2Q2YzVmZGI2NWNiZjVhM2U5MjQ5ZDc="}, Mode: "aesgcm", + }, + ReadKeys: []state.KeyState{ + {Key: apiserverconfigv1.Key{Name: "35", Secret: "MTcxNTgyYTBmY2Q2YzVmZGI2NWNiZjVhM2U5MjQ5ZDc="}, Mode: "aesgcm"}, + {Key: apiserverconfigv1.Key{Name: "34", Secret: "MTcxNTgyYTBmY2Q2YzVmZGI2NWNiZjVhM2U5MjQ5ZDc="}, Mode: "aescbc"}, + }, + }, + }, + }, } for _, scenario := range scenarios { diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index 6bcda7997a..e01dec338d 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -60,7 +60,7 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { keyMode := state.Mode(s.Annotations[encryptionSecretMode]) switch keyMode { - case state.AESCBC, state.SecretBox, state.Identity: + case state.AESCBC, state.AESGCM, state.SecretBox, state.Identity: key.Mode = keyMode default: return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid mode: %s", s.Namespace, s.Name, keyMode) diff --git a/pkg/operator/encryption/secrets/secrets_test.go b/pkg/operator/encryption/secrets/secrets_test.go index 9eb97cd83a..942fa44e9f 100644 --- a/pkg/operator/encryption/secrets/secrets_test.go +++ b/pkg/operator/encryption/secrets/secrets_test.go @@ -55,6 +55,40 @@ func TestRoundtrip(t *testing.T) { Mode: "aescbc", }, }, + { + name: "full aesgcm", + component: "kms", + ks: state.KeyState{ + Key: v1.Key{ + Name: "54", + Secret: base64.StdEncoding.EncodeToString([]byte("abcdef")), + }, + Backed: true, // this will be set by ToKeyState() + Mode: "aesgcm", + Migrated: state.MigrationState{ + Timestamp: now, + Resources: []schema.GroupResource{ + {Resource: "secrets"}, + {Resource: "configmaps"}, + {Group: "networking.openshift.io", Resource: "routes"}, + }, + }, + InternalReason: "internal", + ExternalReason: "external", + }, + }, + { + name: "sparse aesgcm", + component: "kms", + ks: state.KeyState{ + Key: v1.Key{ + Name: "54", + Secret: base64.StdEncoding.EncodeToString([]byte("abcdef")), + }, + Backed: true, // this will be set by ToKeyState() + Mode: "aesgcm", + }, + }, { name: "identity", component: "kms", diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index 9a7174e43c..51b4e53699 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -56,7 +56,8 @@ type Mode string // These values are encoded into the secret and thus must not be changed. // Strings are used over iota because they are easier for a human to understand. const ( - AESCBC Mode = "aescbc" // available from the first release, see defaultMode below + AESCBC Mode = "aescbc" // available from the first release, see defaultMode below + AESGCM Mode = "aesgcm" SecretBox Mode = "secretbox" // available from the first release, see defaultMode below Identity Mode = "identity" // available from the first release, see defaultMode below diff --git a/pkg/operator/encryption/testing/helpers.go b/pkg/operator/encryption/testing/helpers.go index 90e3149909..45754c6421 100644 --- a/pkg/operator/encryption/testing/helpers.go +++ b/pkg/operator/encryption/testing/helpers.go @@ -73,7 +73,11 @@ func CreateEncryptionKeySecretWithRawKeyWithMode(targetNS string, grs []schema.G } func CreateEncryptionKeySecretWithKeyFromExistingSecret(targetNS string, grs []schema.GroupResource, keyID uint64, existingSecret *corev1.Secret) *corev1.Secret { - secret := CreateEncryptionKeySecretNoData(targetNS, grs, keyID) + return CreateEncryptionKeySecretWithKeyFromExistingSecretWithMode(targetNS, grs, keyID, existingSecret, "aescbc") +} + +func CreateEncryptionKeySecretWithKeyFromExistingSecretWithMode(targetNS string, grs []schema.GroupResource, keyID uint64, existingSecret *corev1.Secret, mode string) *corev1.Secret { + secret := CreateEncryptionKeySecretNoDataWithMode(targetNS, grs, keyID, mode) if rawKey, exist := existingSecret.Data[encryptionSecretKeyDataForTest]; exist { secret.Data[encryptionSecretKeyDataForTest] = rawKey } @@ -182,20 +186,7 @@ func CreateEncryptionCfgNoWriteKeyMultipleReadKeys(keysResources []EncryptionKey if len(keysResource.Modes) == len(keysResource.Keys) { desiredMode = keysResource.Modes[i] } - switch desiredMode { - case "aesgcm": - rc.Providers = append(rc.Providers, apiserverconfigv1.ProviderConfiguration{ - AESGCM: &apiserverconfigv1.AESConfiguration{ - Keys: []apiserverconfigv1.Key{key}, - }, - }) - default: - rc.Providers = append(rc.Providers, apiserverconfigv1.ProviderConfiguration{ - AESCBC: &apiserverconfigv1.AESConfiguration{ - Keys: []apiserverconfigv1.Key{key}, - }, - }) - } + rc.Providers = append(rc.Providers, *createProviderCfg(desiredMode, key)) } ec.Resources = append(ec.Resources, rc) } @@ -208,12 +199,12 @@ func CreateEncryptionCfgWithWriteKey(keysResources []EncryptionKeysResourceTuple for _, keysResource := range keysResources { // TODO allow secretbox -> not sure if EncryptionKeysResourceTuple makes sense providers := []apiserverconfigv1.ProviderConfiguration{} - for _, key := range keysResource.Keys { - providers = append(providers, apiserverconfigv1.ProviderConfiguration{ - AESCBC: &apiserverconfigv1.AESConfiguration{ - Keys: []apiserverconfigv1.Key{key}, - }, - }) + for i, key := range keysResource.Keys { + desiredMode := "" + if len(keysResource.Modes) == len(keysResource.Keys) { + desiredMode = keysResource.Modes[i] + } + providers = append(providers, *createProviderCfg(desiredMode, key)) } providers = append(providers, apiserverconfigv1.ProviderConfiguration{ Identity: &apiserverconfigv1.IdentityConfiguration{}, @@ -234,6 +225,33 @@ func CreateEncryptionCfgWithWriteKey(keysResources []EncryptionKeysResourceTuple } } +func createProviderCfg(mode string, key apiserverconfigv1.Key) *apiserverconfigv1.ProviderConfiguration { + switch mode { + case "aesgcm": + return &apiserverconfigv1.ProviderConfiguration{ + AESGCM: &apiserverconfigv1.AESConfiguration{ + Keys: []apiserverconfigv1.Key{key}, + }, + } + case "secretbox": + return &apiserverconfigv1.ProviderConfiguration{ + Secretbox: &apiserverconfigv1.SecretboxConfiguration{ + Keys: []apiserverconfigv1.Key{key}, + }, + } + case "identity": + return &apiserverconfigv1.ProviderConfiguration{ + Identity: &apiserverconfigv1.IdentityConfiguration{}, + } + default: + return &apiserverconfigv1.ProviderConfiguration{ + AESCBC: &apiserverconfigv1.AESConfiguration{ + Keys: []apiserverconfigv1.Key{key}, + }, + } + } +} + type EncryptionKeysResourceTuple struct { Resource string Keys []apiserverconfigv1.Key