From 15729b4f461944c9a5eeb86083bf198eee0b4bde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Sat, 25 Oct 2025 17:32:18 +0300 Subject: [PATCH 01/10] Introduce KMS encryption mode in encryption library --- go.mod | 4 +- .../encryption/controllers/key_controller.go | 79 +++- .../controllers/key_controller_test.go | 2 +- .../controllers/migration_controller_test.go | 220 +++++++++++ .../controllers/state_controller_test.go | 72 ++++ pkg/operator/encryption/crypto/keys.go | 1 + .../encryption/encryptionconfig/config.go | 22 ++ pkg/operator/encryption/kms/kms.go | 139 +++++++ pkg/operator/encryption/kms/kms_client.go | 85 +++++ pkg/operator/encryption/kms/kms_test.go | 102 ++++++ pkg/operator/encryption/secrets/secrets.go | 18 +- .../encryption/secrets/secrets_test.go | 341 ++++++++++++++++++ pkg/operator/encryption/secrets/types.go | 8 + pkg/operator/encryption/state/helpers.go | 15 +- pkg/operator/encryption/state/types.go | 7 +- 15 files changed, 1096 insertions(+), 19 deletions(-) create mode 100644 pkg/operator/encryption/kms/kms.go create mode 100644 pkg/operator/encryption/kms/kms_client.go create mode 100644 pkg/operator/encryption/kms/kms_test.go diff --git a/go.mod b/go.mod index 227c0d87f6..a2474eb197 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( golang.org/x/net v0.43.0 golang.org/x/sys v0.36.0 golang.org/x/time v0.9.0 + google.golang.org/grpc v1.72.1 gopkg.in/evanphx/json-patch.v4 v4.12.0 gopkg.in/natefinch/lumberjack.v2 v2.2.1 k8s.io/api v0.34.1 @@ -43,6 +44,7 @@ require ( k8s.io/client-go v0.34.1 k8s.io/component-base v0.34.1 k8s.io/klog/v2 v2.130.1 + k8s.io/kms v0.34.1 k8s.io/kube-aggregator v0.34.1 k8s.io/utils v0.0.0-20250604170112-4c0f3b243397 sigs.k8s.io/kube-storage-version-migrator v0.0.6-0.20230721195810-5c8923c5ff96 @@ -125,11 +127,9 @@ require ( golang.org/x/tools v0.36.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20250303144028-a0af3efb3deb // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20250303144028-a0af3efb3deb // indirect - google.golang.org/grpc v1.72.1 // indirect google.golang.org/protobuf v1.36.5 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/kms v0.34.1 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.31.2 // indirect sigs.k8s.io/json v0.0.0-20241014173422-cfa47c3a1cc8 // indirect diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index c999f140f0..a9eacbb9a1 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/openshift/library-go/pkg/operator/encryption/kms" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -20,6 +21,7 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/ptr" + configv1 "github.com/openshift/api/config/v1" operatorv1 "github.com/openshift/api/operator/v1" configv1client "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1" @@ -159,11 +161,20 @@ func (c *keyController) sync(ctx context.Context, syncCtx factory.SyncContext) ( } func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext factory.SyncContext, encryptedGRs []schema.GroupResource) error { - currentMode, externalReason, err := c.getCurrentModeAndExternalReason(ctx) + currentMode, externalReason, kmsConfig, err := c.getCurrentModeAndExternalReason(ctx) if err != nil { return err } + // Compute KMS hashes if using KMS mode + var kmsConfigHash, kmsKeyIDHash string + if currentMode == state.KMS && kmsConfig != nil { + kmsConfigHash, kmsKeyIDHash, err = c.getKMSHashes(ctx, kmsConfig) + if err != nil { + return err + } + } + currentConfig, desiredEncryptionState, secrets, isProgressingReason, err := statemachine.GetEncryptionConfigAndState(ctx, c.deployer, c.secretClient, c.encryptionSecretSelector, encryptedGRs) if err != nil { return err @@ -191,7 +202,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact var commonReason *string for gr, grKeys := range desiredEncryptionState { - latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs) + latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, kmsConfigHash, kmsKeyIDHash) if !needed { continue } @@ -218,7 +229,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact sort.Sort(sort.StringSlice(reasons)) internalReason := strings.Join(reasons, ", ") - keySecret, err := c.generateKeySecret(newKeyID, currentMode, internalReason, externalReason) + keySecret, err := c.generateKeySecret(newKeyID, currentMode, internalReason, externalReason, kmsConfigHash, kmsKeyIDHash) if err != nil { return fmt.Errorf("failed to create key: %v", err) } @@ -255,7 +266,7 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c return nil // we made this key earlier } -func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, internalReason, externalReason string) (*corev1.Secret, error) { +func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, internalReason, externalReason, kmsConfigHash, kmsKeyIDHash string) (*corev1.Secret, error) { bs := crypto.ModeToNewKeyFunc[currentMode]() ks := state.KeyState{ Key: apiserverv1.Key{ @@ -265,40 +276,69 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, Mode: currentMode, InternalReason: internalReason, ExternalReason: externalReason, + KMSConfigHash: kmsConfigHash, + KMSKeyIDHash: kmsKeyIDHash, } return secrets.FromKeyState(c.instanceName, ks) } -func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (state.Mode, string, error) { +func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (state.Mode, string, *configv1.KMSConfig, error) { apiServer, err := c.apiServerClient.Get(ctx, "cluster", metav1.GetOptions{}) if err != nil { - return "", "", err + return "", "", nil, err } operatorSpec, _, _, err := c.operatorClient.GetOperatorState() if err != nil { - return "", "", err + return "", "", nil, err } encryptionConfig, err := structuredUnsupportedConfigFrom(operatorSpec.UnsupportedConfigOverrides.Raw, c.unsupportedConfigPrefix) if err != nil { - return "", "", err + return "", "", nil, err } reason := encryptionConfig.Encryption.Reason switch currentMode := state.Mode(apiServer.Spec.Encryption.Type); currentMode { case state.AESCBC, state.AESGCM, state.Identity: // secretbox is disabled for now - return currentMode, reason, nil + return currentMode, reason, nil, nil + case state.KMS: + return currentMode, reason, apiServer.Spec.Encryption.KMS, nil case "": // unspecified means use the default (which can change over time) - return state.DefaultMode, reason, nil + return state.DefaultMode, reason, nil, nil default: - return "", "", fmt.Errorf("unknown encryption mode configured: %s", currentMode) + return "", "", nil, fmt.Errorf("unknown encryption mode configured: %s", currentMode) + } +} + +func (c *keyController) getKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) { + // Generate unix socket path from KMS config and get the hash + socketPath, configHash, err := kms.GenerateUnixSocketPath(kmsConfig) + if err != nil { + return "", "", fmt.Errorf("failed to generate KMS unix socket path: %w", err) + } + + kmsClient, err := kms.NewKMSClient(socketPath) + if err != nil { + return "", "", fmt.Errorf("failed to create KMS client: %w", err) + } + defer kmsClient.Close() + + statusResp, err := kmsClient.Status(ctx) + if err != nil { + return "", "", fmt.Errorf("failed to call KMS Status endpoint: %w", err) + } + + if statusResp.Healthz != "ok" { + return "", "", fmt.Errorf("KMS plugin is unhealthy: %s", statusResp.Healthz) } + + return configHash, kms.ComputeKMSKeyIDHash(statusResp.KeyID), nil } // needsNewKey checks whether a new key must be created for the given resource. If true, it also returns the latest // used key ID and a reason string. -func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource) (uint64, string, bool) { +func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource, kmsConfigHash, kmsKeyIDHash string) (uint64, string, bool) { // we always need to have some encryption keys unless we are turned off if len(grKeys.ReadKeys) == 0 { return 0, "key-does-not-exist", currentMode != state.Identity @@ -346,6 +386,21 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern return latestKeyID, "external-reason-changed", true } + // if we are using KMS, check if the KMS configuration or key ID hash has changed + if currentMode == state.KMS { + if latestKey.KMSConfigHash != kmsConfigHash && len(kmsConfigHash) != 0 { + return latestKeyID, "kms-config-changed", true + } + + if latestKey.KMSKeyIDHash != kmsKeyIDHash && len(kmsKeyIDHash) != 0 { + return latestKeyID, "kms-key-id-changed", true + } + + // For KMS mode, we don't do time-based rotation + // KMS keys are rotated externally by the KMS system + return 0, "", false + } + // we check for encryptionSecretMigratedTimestamp set by migration controller to determine when migration completed // this also generates back pressure for key rotation when migration takes a long time or was recently completed return latestKeyID, "rotation-interval-has-passed", time.Since(latestKey.Migrated.Timestamp) > encryptionSecretMigrationInterval diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 03ba8c45e4..f5f90721b7 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -495,7 +495,7 @@ func TestGetCurrentModeAndExternalReason(t *testing.T) { // act target := keyController{unsupportedConfigPrefix: scenario.prefix, operatorClient: fakeOperatorClient, apiServerClient: fakeApiServerClient} - _, externalReason, err := target.getCurrentModeAndExternalReason(context.TODO()) + _, externalReason, _, err := target.getCurrentModeAndExternalReason(context.TODO()) // validate if err != nil { diff --git a/pkg/operator/encryption/controllers/migration_controller_test.go b/pkg/operator/encryption/controllers/migration_controller_test.go index eda0d0225f..5a28ae671a 100644 --- a/pkg/operator/encryption/controllers/migration_controller_test.go +++ b/pkg/operator/encryption/controllers/migration_controller_test.go @@ -566,6 +566,226 @@ func TestMigrationController(t *testing.T) { }, }, + // KMS mode test case - validates that migration completes when KMSKeyIDHash changes + { + name: "KMS key rotation: all migrations finished when KMSKeyIDHash changes", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + targetAPIResources: []metav1.APIResource{ + { + Name: "secrets", + Namespaced: true, + Group: "", + Version: "v1", + }, + }, + initialResources: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + }, + initialSecrets: []*corev1.Secret{ + // Old KMS key with old KMSKeyIDHash + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 1, "kms") + s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" + s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "0123456789abcdef" // 16 hex chars + s.Kind = "Secret" + s.APIVersion = corev1.SchemeGroupVersion.String() + return s + }(), + // New KMS key with new KMSKeyIDHash (simulating KMS key rotation) + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 2, "kms") + s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" // same config + s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) + s.Kind = "Secret" + s.APIVersion = corev1.SchemeGroupVersion.String() + return s + }(), + // Encryption config with new key as write key (KMS mode) + func() *corev1.Secret { + ec := &apiserverconfigv1.EncryptionConfiguration{ + Resources: []apiserverconfigv1.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + Providers: []apiserverconfigv1.ProviderConfiguration{ + { + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "kms-provider-fedcba9876543210-2", // new key ID hash (16 hex chars) + Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", + Timeout: &metav1.Duration{ + Duration: 10 * time.Second, + }, + }, + }, + { + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "kms-provider-0123456789abcdef-1", // old key ID hash (16 hex chars) + Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", + Timeout: &metav1.Duration{ + Duration: 10 * time.Second, + }, + }, + }, + { + Identity: &apiserverconfigv1.IdentityConfiguration{}, + }, + }, + }, + }, + } + ecs := createEncryptionCfgSecret(t, "kms", "1", ec) + ecs.APIVersion = corev1.SchemeGroupVersion.String() + return ecs + }(), + }, + migratorEnsureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {"2": {finished: true}}, + }, + expectedActions: []string{ + "list:pods:kms", + "get:secrets:kms", + "list:secrets:openshift-config-managed", + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "update:secrets:openshift-config-managed", + "create:events:operator", + }, + expectedMigratorCalls: []string{ + "ensure:secrets:2", + }, + validateFunc: func(ts *testing.T, actionsKube []clientgotesting.Action, initialSecrets []*corev1.Secret, targetGRs []schema.GroupResource, unstructuredObjs []runtime.Object) { + // Verify that the new key (key 2) was migrated + validateSecretsWereAnnotated(ts, targetGRs, actionsKube, []*corev1.Secret{initialSecrets[1]}, nil) + }, + validateOperatorClientFunc: func(ts *testing.T, operatorClient v1helpers.OperatorClient) { + expectedConditions := []operatorv1.OperatorCondition{ + { + Type: "EncryptionMigrationControllerDegraded", + Status: "False", + }, + { + Type: "EncryptionMigrationControllerProgressing", + Status: "False", + }, + } + encryptiontesting.ValidateOperatorClientConditions(ts, operatorClient, expectedConditions) + }, + }, + + // KMS mode test case - validates that migration is in progress (not finished) + { + name: "KMS key rotation: migration in progress when KMSKeyIDHash changes", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + targetAPIResources: []metav1.APIResource{ + { + Name: "secrets", + Namespaced: true, + Group: "", + Version: "v1", + }, + }, + initialResources: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + }, + initialSecrets: []*corev1.Secret{ + // Old KMS key with old KMSKeyIDHash + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 1, "kms") + s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" + s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "0123456789abcdef" // 16 hex chars + s.Kind = "Secret" + s.APIVersion = corev1.SchemeGroupVersion.String() + return s + }(), + // New KMS key with new KMSKeyIDHash (simulating KMS key rotation) + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 2, "kms") + s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" // same config + s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) + s.Kind = "Secret" + s.APIVersion = corev1.SchemeGroupVersion.String() + return s + }(), + // Encryption config with new key as write key (KMS mode) + func() *corev1.Secret { + ec := &apiserverconfigv1.EncryptionConfiguration{ + Resources: []apiserverconfigv1.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + Providers: []apiserverconfigv1.ProviderConfiguration{ + { + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "kms-provider-fedcba9876543210-2", // new key ID hash (16 hex chars) + Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", + Timeout: &metav1.Duration{ + Duration: 10 * time.Second, + }, + }, + }, + { + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "kms-provider-0123456789abcdef-1", // old key ID hash (16 hex chars) + Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", + Timeout: &metav1.Duration{ + Duration: 10 * time.Second, + }, + }, + }, + { + Identity: &apiserverconfigv1.IdentityConfiguration{}, + }, + }, + }, + }, + } + ecs := createEncryptionCfgSecret(t, "kms", "1", ec) + ecs.APIVersion = corev1.SchemeGroupVersion.String() + return ecs + }(), + }, + migratorEnsureReplies: map[schema.GroupResource]map[string]finishedResultErr{ + {Group: "", Resource: "secrets"}: {"2": {finished: false}}, // Migration NOT finished + }, + expectedActions: []string{ + "list:pods:kms", + "get:secrets:kms", + "list:secrets:openshift-config-managed", + "list:secrets:openshift-config-managed", + }, + expectedMigratorCalls: []string{ + "ensure:secrets:2", + }, + validateFunc: func(ts *testing.T, actionsKube []clientgotesting.Action, initialSecrets []*corev1.Secret, targetGRs []schema.GroupResource, unstructuredObjs []runtime.Object) { + // Verify that the secret was NOT annotated as migrated (since migration is in progress) + validateSecretsWereAnnotated(ts, []schema.GroupResource{}, actionsKube, nil, []*corev1.Secret{initialSecrets[1]}) + }, + validateOperatorClientFunc: func(ts *testing.T, operatorClient v1helpers.OperatorClient) { + expectedConditions := []operatorv1.OperatorCondition{ + { + Type: "EncryptionMigrationControllerDegraded", + Status: "False", + }, + { + Type: "EncryptionMigrationControllerProgressing", + Reason: "Migrating", + Message: "migrating resources to a new write key: [core/secrets]", + Status: "True", + }, + } + encryptiontesting.ValidateOperatorClientConditions(ts, operatorClient, expectedConditions) + }, + }, + // TODO: add more tests for not so happy paths } diff --git a/pkg/operator/encryption/controllers/state_controller_test.go b/pkg/operator/encryption/controllers/state_controller_test.go index 46fc507c5c..c6e84ca61e 100644 --- a/pkg/operator/encryption/controllers/state_controller_test.go +++ b/pkg/operator/encryption/controllers/state_controller_test.go @@ -707,6 +707,78 @@ func TestStateController(t *testing.T) { encryptiontesting.ValidateOperatorClientConditions(ts, operatorClient, []operatorv1.OperatorCondition{expectedCondition}) }, }, + { + name: "secret with EncryptionConfig is created for KMS mode with proper KMS provider configuration", + targetNamespace: "kms", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + initialResources: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + // Set KMS-specific annotations with config hash and key ID hash + s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "1234567890abcdef" + s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "fedcba0987654321" + return s + }(), + }, + expectedActions: []string{ + "list:pods:kms", + "get:secrets:kms", + "list:secrets:openshift-config-managed", + "get:secrets:openshift-config-managed", + "create:secrets:openshift-config-managed", + "create:events:kms", + "create:events:kms", + }, + expectedEncryptionCfg: func() *apiserverconfigv1.EncryptionConfiguration { + return &apiserverconfigv1.EncryptionConfiguration{ + TypeMeta: metav1.TypeMeta{ + Kind: "EncryptionConfiguration", + APIVersion: "apiserver.config.k8s.io/v1", + }, + Resources: []apiserverconfigv1.ResourceConfiguration{ + { + Resources: []string{"secrets"}, + Providers: []apiserverconfigv1.ProviderConfiguration{ + { + Identity: &apiserverconfigv1.IdentityConfiguration{}, + }, + { + KMS: &apiserverconfigv1.KMSConfiguration{ + APIVersion: "v2", + Name: "kms-provider-fedcba0987654321-1", // kms-provider-{keyIDHash16}-{key ID} + Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", + Timeout: &metav1.Duration{ + Duration: 10 * time.Second, + }, + }, + }, + }, + }, + }, + } + }(), + validateFunc: func(ts *testing.T, actions []clientgotesting.Action, destName string, expectedEncryptionCfg *apiserverconfigv1.EncryptionConfiguration) { + wasSecretValidated := false + for _, action := range actions { + if action.Matches("create", "secrets") { + createAction := action.(clientgotesting.CreateAction) + actualSecret := createAction.GetObject().(*corev1.Secret) + err := validateSecretWithEncryptionConfig(actualSecret, expectedEncryptionCfg, destName) + if err != nil { + ts.Fatalf("failed to verify the encryption config, due to %v", 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 a623d30f79..b9393ece66 100644 --- a/pkg/operator/encryption/crypto/keys.go +++ b/pkg/operator/encryption/crypto/keys.go @@ -12,6 +12,7 @@ var ( state.AESGCM: NewAES256Key, state.SecretBox: NewAES256Key, // secretbox requires a 32 byte key so we can reuse the same function here state.Identity: NewIdentityKey, + state.KMS: NewIdentityKey, // this is not used in KMS } ) diff --git a/pkg/operator/encryption/encryptionconfig/config.go b/pkg/operator/encryption/encryptionconfig/config.go index 3082aa653f..1a8b130b9d 100644 --- a/pkg/operator/encryption/encryptionconfig/config.go +++ b/pkg/operator/encryption/encryptionconfig/config.go @@ -4,6 +4,7 @@ import ( "encoding/base64" "sort" + "github.com/openshift/library-go/pkg/operator/encryption/kms" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime/schema" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/apiserver/v1" @@ -106,6 +107,25 @@ func ToEncryptionState(encryptionConfig *apiserverconfigv1.EncryptionConfigurati Mode: s, } + case provider.KMS != nil: + configHash, keyIDHash, keyName, err := kms.ExtractKMSHashAndKeyName(provider) + if err != nil { + klog.Warningf("skipping invalid encryption KMS config for resource %v", provider) + continue // should never happen + } + + ks = state.KeyState{ + Key: apiserverconfigv1.Key{ + Name: keyName, + // We set this unused secret just to align with what we set initially. + // This is unused. + Secret: base64.StdEncoding.EncodeToString(crypto.ModeToNewKeyFunc[state.KMS]()), + }, + Mode: state.KMS, + KMSConfigHash: configHash, + KMSKeyIDHash: keyIDHash, + } + default: klog.Infof("skipping invalid provider index %d for resource %s", i, resourceConfig.Resources[0]) continue // should never happen @@ -192,6 +212,8 @@ func stateToProviders(desired state.GroupResourceState) []apiserverconfigv1.Prov Keys: []apiserverconfigv1.Key{key.Key}, }, }) + case state.KMS: + providers = append(providers, kms.GenerateKMSProviderConfigurationFromKey(key)) default: // this should never happen because our input should always be valid klog.Infof("skipping key %s as it has invalid mode %s", key.Key.Name, key.Mode) diff --git a/pkg/operator/encryption/kms/kms.go b/pkg/operator/encryption/kms/kms.go new file mode 100644 index 0000000000..4f73f9bb6b --- /dev/null +++ b/pkg/operator/encryption/kms/kms.go @@ -0,0 +1,139 @@ +package kms + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "regexp" + "time" + + configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/library-go/pkg/operator/encryption/state" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + v1 "k8s.io/apiserver/pkg/apis/apiserver/v1" +) + +const ( + // unixSocketBaseDir is the base directory for KMS unix sockets + unixSocketBaseDir = "unix://var/run/kms" +) + +// GenerateUnixSocketPath generates a unique unix socket path from KMS configuration +// by hashing the provider-specific configuration. +// Returns the socket path and the hash value (first 16 characters). +func GenerateUnixSocketPath(kmsConfig *configv1.KMSConfig) (socketPath string, hash string, err error) { + if kmsConfig == nil { + return "", "", fmt.Errorf("kmsConfig cannot be nil") + } + + // Determine KMS type and generate path accordingly + switch kmsConfig.Type { + case configv1.AWSKMSProvider: + if kmsConfig.AWS == nil { + return "", "", fmt.Errorf("AWS KMS config cannot be nil for AWS provider type") + } + return generateAWSUnixSocketPath(kmsConfig.AWS) + default: + return "", "", fmt.Errorf("unsupported KMS provider type: %s", kmsConfig.Type) + } +} + +// generateAWSUnixSocketPath generates a unique unix socket path from AWS KMS configuration +// by hashing the ARN and region. Returns the socket path and the hash (first 16 characters). +func generateAWSUnixSocketPath(awsConfig *configv1.AWSKMSConfig) (string, string, error) { + if awsConfig.KeyARN == "" { + return "", "", fmt.Errorf("AWS KMS KeyARN cannot be empty") + } + + if awsConfig.Region == "" { + return "", "", fmt.Errorf("AWS region cannot be empty") + } + + // Combine KeyARN and region for hashing + combined := awsConfig.KeyARN + ":" + awsConfig.Region + + // Compute SHA256 hash + hash := sha256.Sum256([]byte(combined)) + hashStr := hex.EncodeToString(hash[:]) + + // Take first 16 characters of hash for shorter path + shortHash := hashStr[:16] + + socketPath := fmt.Sprintf("%s/kms-%s.sock", unixSocketBaseDir, shortHash) + + return socketPath, shortHash, nil +} + +// ComputeKMSKeyIDHash computes a hash of the KMS key ID returned from the Status endpoint. +// Returns the first 16 characters of the SHA256 hash. +func ComputeKMSKeyIDHash(keyID string) string { + if keyID == "" { + return "" + } + + // Compute SHA256 hash + hash := sha256.Sum256([]byte(keyID)) + hashStr := hex.EncodeToString(hash[:]) + + return hashStr[:16] +} + +var ( + // endpointHashRegex matches the config hash in endpoint path: unix://var/run/kms/kms-{configHash16}.sock + endpointHashRegex = regexp.MustCompile(`kms-([a-f0-9]{16})\.sock$`) + // providerNameRegex matches the key ID hash and key ID in provider name: kms-provider-{keyIDHash16}-{keyID} + // Example: kms-provider-abcdef1234567890-1 + providerNameRegex = regexp.MustCompile(`^kms-provider-([a-f0-9]{16})-(.+)$`) +) + +// ExtractKMSHashAndKeyName extracts the KMSConfigHash, KMSKeyIDHash, and key.Name embedded into provider +// name and socket path. Returns (configHash, keyIDHash, keyName, error) +func ExtractKMSHashAndKeyName(provider v1.ProviderConfiguration) (string, string, string, error) { + // Extract the config hash from the endpoint path: unix://var/run/kms/kms-{configHash}.sock + endpoint := provider.KMS.Endpoint + var configHash string + if matches := endpointHashRegex.FindStringSubmatch(endpoint); len(matches) == 2 { + configHash = matches[1] + } else { + return "", "", "", fmt.Errorf("invalid KMS endpoint format: %s", endpoint) + } + + // Extract the key ID hash and key ID from the provider name: kms-provider-{keyIDHash16}-{keyID} + // Example: kms-provider-abcdef1234567890-1 + var keyIDHash, keyName string + providerName := provider.KMS.Name + if matches := providerNameRegex.FindStringSubmatch(providerName); len(matches) == 3 { + keyIDHash = matches[1] + keyName = matches[2] + } else { + return "", "", "", fmt.Errorf("invalid KMS provider name format: %s", providerName) + } + + return configHash, keyIDHash, keyName, nil +} + +// GenerateKMSProviderConfigurationFromKey generates the compatible ProviderConfiguration with +// opinionated and extractable fields. We embed: +// - KMSConfigHash in the socket path (endpoint) +// - KMSKeyIDHash and key.Name in the provider name +// This allows us to extract all three values and detect both config changes and key rotations. +func GenerateKMSProviderConfigurationFromKey(key state.KeyState) v1.ProviderConfiguration { + // Embed KMSConfigHash in the endpoint so we can extract it + // This must generate the same format as GenerateUnixSocketPath + socketPath := fmt.Sprintf("%s/kms-%s.sock", unixSocketBaseDir, key.KMSConfigHash) + // Embed KMSKeyIDHash and key ID in the provider name so we can extract them when reading back + // Format: kms-provider-{keyIDHash16}-{keyID} + // This must match the providerNameRegex + providerName := fmt.Sprintf("kms-provider-%s-%s", key.KMSKeyIDHash, key.Key.Name) + + return v1.ProviderConfiguration{ + KMS: &v1.KMSConfiguration{ + APIVersion: "v2", + Name: providerName, + Endpoint: socketPath, + Timeout: &metav1.Duration{ + Duration: 10 * time.Second, + }, + }, + } +} diff --git a/pkg/operator/encryption/kms/kms_client.go b/pkg/operator/encryption/kms/kms_client.go new file mode 100644 index 0000000000..c0aa86e1b0 --- /dev/null +++ b/pkg/operator/encryption/kms/kms_client.go @@ -0,0 +1,85 @@ +package kms + +import ( + "context" + "fmt" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + kmsv2 "k8s.io/kms/apis/v2" +) + +const ( + // defaultTimeout is the default timeout for KMS operations + defaultTimeout = 30 * time.Second +) + +// KMSClient is an interface for interacting with KMS plugins +type KMSClient interface { + // Status calls the KMS plugin's Status endpoint and returns the response + Status(ctx context.Context) (*StatusResponse, error) + // Close closes the connection to the KMS plugin + Close() error +} + +// StatusResponse represents the response from a KMS Status call +type StatusResponse struct { + Version string + Healthz string + KeyID string +} + +// kmsClient implements the KMSClient interface +type kmsClient struct { + conn *grpc.ClientConn + kmsClient kmsv2.KeyManagementServiceClient + endpoint string +} + +func NewKMSClient(endpoint string) (KMSClient, error) { + if endpoint == "" { + return nil, fmt.Errorf("kms endpoint cannot be empty") + } + + options := []grpc.DialOption{ + grpc.WithTransportCredentials(insecure.NewCredentials()), + } + + conn, err := grpc.NewClient(endpoint, options...) + if err != nil { + return nil, fmt.Errorf("failed to connect to KMS plugin at %s: %w", endpoint, err) + } + + return &kmsClient{ + conn: conn, + kmsClient: kmsv2.NewKeyManagementServiceClient(conn), + endpoint: endpoint, + }, nil +} + +// Status calls the KMS plugin's Status endpoint +func (c *kmsClient) Status(ctx context.Context) (*StatusResponse, error) { + timeoutCtx, cancel := context.WithTimeout(ctx, defaultTimeout) + defer cancel() + + // Call the Status endpoint + resp, err := c.kmsClient.Status(timeoutCtx, &kmsv2.StatusRequest{}) + if err != nil { + return nil, fmt.Errorf("failed to call KMS Status endpoint at %s: %w", c.endpoint, err) + } + + return &StatusResponse{ + Version: resp.GetVersion(), + Healthz: resp.GetHealthz(), + KeyID: resp.GetKeyId(), + }, nil +} + +// Close closes the connection to the KMS plugin +func (c *kmsClient) Close() error { + if c.conn != nil { + return c.conn.Close() + } + return nil +} diff --git a/pkg/operator/encryption/kms/kms_test.go b/pkg/operator/encryption/kms/kms_test.go new file mode 100644 index 0000000000..1cb19539e9 --- /dev/null +++ b/pkg/operator/encryption/kms/kms_test.go @@ -0,0 +1,102 @@ +package kms + +import ( + "testing" + + configv1 "github.com/openshift/api/config/v1" +) + +func TestGenerateUnixSocketPath(t *testing.T) { + tests := []struct { + name string + kmsConfig *configv1.KMSConfig + wantPath string + wantHash string + wantErr bool + }{ + { + name: "valid AWS KMS config generates socket path", + kmsConfig: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012", + Region: "us-east-1", + }, + }, + wantPath: "unix://var/run/kms/kms-2e55e11c0b187f2d.sock", + wantHash: "2e55e11c0b187f2d", + wantErr: false, + }, + { + name: "missing ARN returns error", + kmsConfig: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "", + Region: "us-east-1", + }, + }, + wantErr: true, + }, + { + name: "missing region returns error", + kmsConfig: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012", + Region: "", + }, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotPath, gotHash, err := GenerateUnixSocketPath(tt.kmsConfig) + if (err != nil) != tt.wantErr { + t.Errorf("GenerateUnixSocketPath() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + + if gotPath != tt.wantPath { + t.Fatalf("GenerateUnixSocketPath() gotPath = %v, want %v", gotPath, tt.wantPath) + } + if gotHash != tt.wantHash { + t.Fatalf("GenerateUnixSocketPath() gotHash = %v, want %v", gotHash, tt.wantHash) + } + }) + } + + // Test determinism - same config should always generate same path + t.Run("deterministic generation", func(t *testing.T) { + kmsConfig := &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key", + Region: "us-east-1", + }, + } + + path1, hash1, err := GenerateUnixSocketPath(kmsConfig) + if err != nil { + t.Fatalf("first call failed: %v", err) + } + + path2, hash2, err := GenerateUnixSocketPath(kmsConfig) + if err != nil { + t.Fatalf("second call failed: %v", err) + } + + if path1 != path2 { + t.Errorf("paths not deterministic: %v != %v", path1, path2) + } + + if hash1 != hash2 { + t.Errorf("hashes not deterministic: %v != %v", hash1, hash2) + } + }) +} diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index 4e54317c7d..e33f9fc6af 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -58,14 +58,21 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { key.ExternalReason = v } + if v, ok := s.Annotations[EncryptionSecretKMSConfigHash]; ok && len(v) > 0 { + key.KMSConfigHash = v + } + if v, ok := s.Annotations[EncryptionSecretKMSKeyIDHash]; ok && len(v) > 0 { + key.KMSKeyIDHash = v + } + keyMode := state.Mode(s.Annotations[encryptionSecretMode]) switch keyMode { - case state.AESCBC, state.AESGCM, state.SecretBox, state.Identity: + case state.AESCBC, state.AESGCM, state.SecretBox, state.Identity, state.KMS: key.Mode = keyMode default: return state.KeyState{}, fmt.Errorf("secret %s/%s has invalid mode: %s", s.Namespace, s.Name, keyMode) } - if keyMode != state.Identity && len(data) == 0 { + if keyMode != state.Identity && keyMode != state.KMS && len(data) == 0 { return state.KeyState{}, fmt.Errorf("secret %s/%s of mode %q must have non-empty key", s.Namespace, s.Name, keyMode) } @@ -113,6 +120,13 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) { s.Annotations[EncryptionSecretMigratedResources] = string(bs) } + if len(ks.KMSConfigHash) > 0 { + s.Annotations[EncryptionSecretKMSConfigHash] = ks.KMSConfigHash + } + if len(ks.KMSKeyIDHash) > 0 { + s.Annotations[EncryptionSecretKMSKeyIDHash] = ks.KMSKeyIDHash + } + return s, nil } diff --git a/pkg/operator/encryption/secrets/secrets_test.go b/pkg/operator/encryption/secrets/secrets_test.go index 81b81b017b..e4788f02c1 100644 --- a/pkg/operator/encryption/secrets/secrets_test.go +++ b/pkg/operator/encryption/secrets/secrets_test.go @@ -6,6 +6,8 @@ import ( "testing" "time" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" v1 "k8s.io/apiserver/pkg/apis/apiserver/v1" "k8s.io/utils/diff" @@ -111,6 +113,57 @@ func TestRoundtrip(t *testing.T) { ExternalReason: "external", }, }, + { + name: "kms with empty secret data", + component: "apiserver", + ks: state.KeyState{ + Key: v1.Key{ + Name: "1", + Secret: "", + }, + Backed: true, + Mode: "kms", + KMSConfigHash: "2e55e11c0b187f2d", + KMSKeyIDHash: "abcdef1234567890", + InternalReason: "kms-config-changed", + ExternalReason: "kms-key-rotated", + }, + }, + { + name: "kms with full metadata", + component: "apiserver", + ks: state.KeyState{ + Key: v1.Key{ + Name: "2", + Secret: "", + }, + Backed: true, + Mode: "kms", + KMSConfigHash: "3f66f22d1c298e3e", + KMSKeyIDHash: "fedcba0987654321", + Migrated: state.MigrationState{ + Timestamp: now, + Resources: []schema.GroupResource{ + {Resource: "secrets"}, + }, + }, + InternalReason: "kms-provider-changed", + ExternalReason: "user-initiated-rotation", + }, + }, + { + name: "kms minimal - only config hash", + component: "apiserver", + ks: state.KeyState{ + Key: v1.Key{ + Name: "3", + Secret: "", + }, + Backed: true, + Mode: "kms", + KMSConfigHash: "1a2b3c4d5e6f7890", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -128,3 +181,291 @@ func TestRoundtrip(t *testing.T) { }) } } + +func TestToKeyState_KMS(t *testing.T) { + tests := []struct { + name string + secretName string + secretData []byte + annotations map[string]string + wantKeyState state.KeyState + wantErr bool + errorContains string + }{ + { + name: "valid KMS secret with empty data", + secretName: "encryption-key-apiserver-1", + secretData: []byte{}, + annotations: map[string]string{ + encryptionSecretMode: "kms", + EncryptionSecretKMSConfigHash: "2e55e11c0b187f2d", + EncryptionSecretKMSKeyIDHash: "abcdef1234567890", + encryptionSecretInternalReason: "kms-config-changed", + encryptionSecretExternalReason: "kms-key-rotated", + }, + wantKeyState: state.KeyState{ + Key: v1.Key{ + Name: "1", + Secret: "", + }, + Backed: true, + Mode: "kms", + KMSConfigHash: "2e55e11c0b187f2d", + KMSKeyIDHash: "abcdef1234567890", + InternalReason: "kms-config-changed", + ExternalReason: "kms-key-rotated", + }, + wantErr: false, + }, + { + name: "KMS secret without KMS key ID hash (only config hash)", + secretName: "encryption-key-apiserver-2", + secretData: []byte{}, + annotations: map[string]string{ + encryptionSecretMode: "kms", + EncryptionSecretKMSConfigHash: "3f66f22d1c298e3e", + }, + wantKeyState: state.KeyState{ + Key: v1.Key{ + Name: "2", + Secret: "", + }, + Backed: true, + Mode: "kms", + KMSConfigHash: "3f66f22d1c298e3e", + KMSKeyIDHash: "", + }, + wantErr: false, + }, + { + name: "KMS secret with both hashes missing (should still work)", + secretName: "encryption-key-apiserver-3", + secretData: []byte{}, + annotations: map[string]string{ + encryptionSecretMode: "kms", + }, + wantKeyState: state.KeyState{ + Key: v1.Key{ + Name: "3", + Secret: "", + }, + Backed: true, + Mode: "kms", + KMSConfigHash: "", + KMSKeyIDHash: "", + }, + wantErr: false, + }, + { + name: "invalid mode should fail", + secretName: "encryption-key-apiserver-4", + secretData: []byte("some-key"), + annotations: map[string]string{ + encryptionSecretMode: "invalid-mode", + }, + wantErr: true, + errorContains: "has invalid mode", + }, + { + name: "aescbc with empty data should fail", + secretName: "encryption-key-apiserver-5", + secretData: []byte{}, + annotations: map[string]string{ + encryptionSecretMode: "aescbc", + }, + wantErr: true, + errorContains: "must have non-empty key", + }, + { + name: "aesgcm with empty data should fail", + secretName: "encryption-key-apiserver-6", + secretData: []byte{}, + annotations: map[string]string{ + encryptionSecretMode: "aesgcm", + }, + wantErr: true, + errorContains: "must have non-empty key", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: tt.secretName, + Namespace: "openshift-config-managed", + Annotations: tt.annotations, + }, + Data: map[string][]byte{ + EncryptionSecretKeyDataKey: tt.secretData, + }, + } + + got, err := ToKeyState(secret) + if (err != nil) != tt.wantErr { + t.Errorf("ToKeyState() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + if tt.errorContains != "" && err != nil { + if !contains(err.Error(), tt.errorContains) { + t.Errorf("ToKeyState() error = %v, should contain %q", err, tt.errorContains) + } + } + return + } + if !reflect.DeepEqual(got, tt.wantKeyState) { + t.Errorf("ToKeyState() mismatch:\n%s", diff.ObjectDiff(tt.wantKeyState, got)) + } + }) + } +} + +func TestFromKeyState_KMS(t *testing.T) { + tests := []struct { + name string + component string + keyState state.KeyState + wantSecretName string + wantDataEmpty bool + wantAnnotations map[string]string + wantErr bool + }{ + { + name: "KMS key state creates secret with annotations", + component: "apiserver", + keyState: state.KeyState{ + Key: v1.Key{ + Name: "1", + Secret: "", + }, + Mode: "kms", + KMSConfigHash: "2e55e11c0b187f2d", + KMSKeyIDHash: "abcdef1234567890", + }, + wantSecretName: "encryption-key-apiserver-1", + wantDataEmpty: true, + wantAnnotations: map[string]string{ + encryptionSecretMode: "kms", + EncryptionSecretKMSConfigHash: "2e55e11c0b187f2d", + EncryptionSecretKMSKeyIDHash: "abcdef1234567890", + }, + wantErr: false, + }, + { + name: "KMS key state with only config hash", + component: "apiserver", + keyState: state.KeyState{ + Key: v1.Key{ + Name: "2", + Secret: "", + }, + Mode: "kms", + KMSConfigHash: "3f66f22d1c298e3e", + }, + wantSecretName: "encryption-key-apiserver-2", + wantDataEmpty: true, + wantAnnotations: map[string]string{ + encryptionSecretMode: "kms", + EncryptionSecretKMSConfigHash: "3f66f22d1c298e3e", + }, + wantErr: false, + }, + { + name: "KMS key state without any hashes", + component: "apiserver", + keyState: state.KeyState{ + Key: v1.Key{ + Name: "3", + Secret: "", + }, + Mode: "kms", + }, + wantSecretName: "encryption-key-apiserver-3", + wantDataEmpty: true, + wantAnnotations: map[string]string{ + encryptionSecretMode: "kms", + }, + wantErr: false, + }, + { + name: "KMS with reasons", + component: "apiserver", + keyState: state.KeyState{ + Key: v1.Key{ + Name: "4", + Secret: "", + }, + Mode: "kms", + KMSConfigHash: "1a2b3c4d5e6f7890", + InternalReason: "kms-provider-changed", + ExternalReason: "user-rotation", + }, + wantSecretName: "encryption-key-apiserver-4", + wantDataEmpty: true, + wantAnnotations: map[string]string{ + encryptionSecretMode: "kms", + EncryptionSecretKMSConfigHash: "1a2b3c4d5e6f7890", + encryptionSecretInternalReason: "kms-provider-changed", + encryptionSecretExternalReason: "user-rotation", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := FromKeyState(tt.component, tt.keyState) + if (err != nil) != tt.wantErr { + t.Errorf("FromKeyState() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr { + return + } + + // Verify secret name + if got.Name != tt.wantSecretName { + t.Errorf("FromKeyState() secret name = %v, want %v", got.Name, tt.wantSecretName) + } + + // Verify namespace + if got.Namespace != "openshift-config-managed" { + t.Errorf("FromKeyState() namespace = %v, want openshift-config-managed", got.Namespace) + } + + // Verify data is empty for KMS + dataLen := len(got.Data[EncryptionSecretKeyDataKey]) + if tt.wantDataEmpty && dataLen != 0 { + t.Errorf("FromKeyState() expected empty data for KMS, got %d bytes", dataLen) + } + + // Verify annotations + for k, v := range tt.wantAnnotations { + if gotV, ok := got.Annotations[k]; !ok { + t.Errorf("FromKeyState() missing annotation %q", k) + } else if gotV != v { + t.Errorf("FromKeyState() annotation %q = %v, want %v", k, gotV, v) + } + } + + // Verify mode annotation specifically + if got.Annotations[encryptionSecretMode] != "kms" { + t.Errorf("FromKeyState() mode annotation = %v, want kms", got.Annotations[encryptionSecretMode]) + } + }) + } +} + +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && containsAt(s, substr)) +} + +func containsAt(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/pkg/operator/encryption/secrets/types.go b/pkg/operator/encryption/secrets/types.go index 7161e4a124..5b6be9574f 100644 --- a/pkg/operator/encryption/secrets/types.go +++ b/pkg/operator/encryption/secrets/types.go @@ -41,6 +41,14 @@ const ( // determine if a new key should be created even if encryptionSecretMigrationInterval has not been reached. encryptionSecretExternalReason = "encryption.apiserver.operator.openshift.io/external-reason" + // EncryptionSecretKMSConfigHash is the annotation that stores the hash of the KMS configuration. + // This is used to detect changes in the KMS configuration that would require a new key. + EncryptionSecretKMSConfigHash = "encryption.apiserver.operator.openshift.io/kms-config-hash" + + // EncryptionSecretKMSKeyIDHash is the annotation that stores the hash of the KMS key ID. + // This is used to detect changes in the KMS key ID (e.g., after key rotation in the KMS system). + EncryptionSecretKMSKeyIDHash = "encryption.apiserver.operator.openshift.io/kms-key-id-hash" + // In the data field of the secret API object, this (map) key is used to hold the actual encryption key // (i.e. for AES-CBC mode the value associated with this map key is 32 bytes of random noise). EncryptionSecretKeyDataKey = "encryption.apiserver.operator.openshift.io-key" diff --git a/pkg/operator/encryption/state/helpers.go b/pkg/operator/encryption/state/helpers.go index 656aeafef6..6dde64f8e6 100644 --- a/pkg/operator/encryption/state/helpers.go +++ b/pkg/operator/encryption/state/helpers.go @@ -74,10 +74,23 @@ func NameToKeyID(name string) (uint64, bool) { } func EqualKeyAndEqualID(s1, s2 *KeyState) bool { - if s1.Mode != s2.Mode || s1.Key.Secret != s2.Key.Secret { + if s1.Mode != s2.Mode { return false } + // For KMS mode, compare both KMSConfigHash and KMSKeyIDHash to detect config or key rotation + if s1.Mode == KMS { + if s1.KMSConfigHash != s2.KMSConfigHash { + return false + } + if s1.KMSKeyIDHash != s2.KMSKeyIDHash { + return false + } + } else if s1.Key.Secret != s2.Key.Secret { + return false + } + + // Verify key IDs match (for both KMS and non-KMS modes) id1, valid1 := NameToKeyID(s1.Key.Name) id2, valid2 := NameToKeyID(s2.Key.Name) return valid1 && valid2 && id1 == id2 diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index 460c21bfa2..9ec37aa118 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -26,7 +26,7 @@ type GroupResourceState struct { } func (k GroupResourceState) HasWriteKey() bool { - return len(k.WriteKey.Key.Name) > 0 && len(k.WriteKey.Key.Secret) > 0 + return (len(k.WriteKey.Key.Name) > 0 && len(k.WriteKey.Key.Secret) > 0) || k.WriteKey.KMSConfigHash != "" } type KeyState struct { @@ -40,6 +40,10 @@ type KeyState struct { InternalReason string // the user via unsupportConfigOverrides.encryption.reason triggered this key. ExternalReason string + // hash of the KMS configuration to detect changes + KMSConfigHash string + // hash of the KMS key ID to detect changes + KMSKeyIDHash string } type MigrationState struct { @@ -60,6 +64,7 @@ const ( 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 + KMS Mode = "kms" // Changing this value requires caution to not break downgrades. // Specifically, if some new Mode is released in version X, that new Mode cannot From 196d70f9eaa644abfd78c246f4e6c5f11f901d6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 27 Oct 2025 11:16:05 +0300 Subject: [PATCH 02/10] Mock KMS status call and add extensive KMS tests --- .../encryption/controllers/key_controller.go | 11 +- .../controllers/key_controller_test.go | 255 ++++++++++++++++++ .../controllers/migration_controller_test.go | 4 +- 3 files changed, 266 insertions(+), 4 deletions(-) diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index a9eacbb9a1..229ec75d14 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -41,6 +41,9 @@ import ( // greater than the last key's ID (the first key has a key ID of 1). const encryptionSecretMigrationInterval = time.Hour * 24 * 7 // one week +// kmsHashesGetter is a function type for getting KMS config and key ID hashes +var kmsHashesGetterFunc func(ctx context.Context, kmsConfig *configv1.KMSConfig) (configHash string, keyIDHash string, err error) + // keyController creates new keys if necessary. It // * watches // - secrets in openshift-config-managed @@ -106,6 +109,8 @@ func NewKeyController( secretClient: secretClient, } + kmsHashesGetterFunc = defaultGetKMSHashes + return factory.New(). WithSync(c.sync). WithControllerInstanceName(c.controllerInstanceName). @@ -169,7 +174,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact // Compute KMS hashes if using KMS mode var kmsConfigHash, kmsKeyIDHash string if currentMode == state.KMS && kmsConfig != nil { - kmsConfigHash, kmsKeyIDHash, err = c.getKMSHashes(ctx, kmsConfig) + kmsConfigHash, kmsKeyIDHash, err = kmsHashesGetterFunc(ctx, kmsConfig) if err != nil { return err } @@ -311,7 +316,9 @@ func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (st } } -func (c *keyController) getKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) { +// defaultGetKMSHashes is the default implementation of getting KMS hashes +// It calls the real KMS client to get the status and compute hashes +func defaultGetKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) { // Generate unix socket path from KMS config and get the hash socketPath, configHash, err := kms.GenerateUnixSocketPath(kmsConfig) if err != nil { diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index f5f90721b7..1528d6356a 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -53,6 +53,8 @@ func TestKeyController(t *testing.T) { validateFunc func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) validateOperatorClientFunc func(ts *testing.T, operatorClient v1helpers.OperatorClient) expectedError error + kmsConfigHash string + kmsKeyIDHash string }{ { name: "no apiservers config", @@ -324,6 +326,253 @@ func TestKeyController(t *testing.T) { } }, }, + + // KMS mode test cases + { + name: "KMS: creates first encryption key when none exists", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + kmsConfigHash: "config-hash-12345678", + kmsKeyIDHash: "key-id-hash-abcdefgh", + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + }, + apiServerObjects: []runtime.Object{ + func() *configv1.APIServer { + apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + apiServer.Spec.Encryption = configv1.APIServerEncryption{ + Type: "kms", + KMS: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key", + Region: "us-east-1", + }, + }, + } + return apiServer + }(), + }, + 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) + + // Verify KMS annotations are set + if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] == "" { + ts.Error("expected KMS config hash annotation to be set") + } + if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] == "" { + ts.Error("expected KMS key ID hash annotation to be set") + } + if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"] != "kms" { + ts.Errorf("expected mode to be 'kms', got '%s'", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"]) + } + + wasSecretValidated = true + break + } + } + if !wasSecretValidated { + ts.Errorf("the secret wasn't created and validated") + } + }, + }, + + { + name: "KMS: creates new key when KMS config hash changes", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + kmsConfigHash: "new-config-hash-xyz", // Different hash for new key ARN + kmsKeyIDHash: "key-id-hash-abcd1234", + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "old-config-hash-1234" + s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "key-id-hash-abcd1234" + return s + }(), + }, + apiServerObjects: []runtime.Object{ + func() *configv1.APIServer { + apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + apiServer.Spec.Encryption = configv1.APIServerEncryption{ + Type: "kms", + KMS: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/new-key", // Different key + Region: "us-east-1", + }, + }, + } + return apiServer + }(), + }, + 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) + + // Verify new KMS config hash is set and different from old + newConfigHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] + if newConfigHash == "" || newConfigHash == "old-config-hash-1234" { + ts.Errorf("expected new KMS config hash, got '%s'", newConfigHash) + } + + // Verify internal reason mentions config change + internalReason := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] + if internalReason != "secrets-kms-config-changed" { + ts.Errorf("expected internal reason 'secrets-kms-config-changed', got '%s'", internalReason) + } + + wasSecretValidated = true + break + } + } + if !wasSecretValidated { + ts.Errorf("the secret wasn't created and validated") + } + }, + }, + + { + name: "KMS: creates new key when KMS key ID hash changes (key rotation)", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + kmsConfigHash: "config-hash-12345678", + kmsKeyIDHash: "new-key-id-hash-xyz", + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "config-hash-12345678" + s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "old-key-id-hash-abc" + return s + }(), + }, + apiServerObjects: []runtime.Object{ + func() *configv1.APIServer { + apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + apiServer.Spec.Encryption = configv1.APIServerEncryption{ + Type: "kms", + KMS: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key", + Region: "us-east-1", + }, + }, + } + return apiServer + }(), + }, + 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) + + // Verify new KMS key ID hash is set and different from old + newKeyIDHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] + if newKeyIDHash == "" || newKeyIDHash == "old-key-id-hash-abc" { + ts.Errorf("expected new KMS key ID hash, got '%s'", newKeyIDHash) + } + + // Verify config hash stays the same + configHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] + if configHash != "config-hash-12345678" { + ts.Errorf("expected config hash to remain 'config-hash-12345678', got '%s'", configHash) + } + + // Verify internal reason mentions key ID change + internalReason := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] + if internalReason != "secrets-kms-key-id-changed" { + ts.Errorf("expected internal reason 'secrets-kms-key-id-changed', got '%s'", internalReason) + } + + wasSecretValidated = true + break + } + } + if !wasSecretValidated { + ts.Errorf("the secret wasn't created and validated") + } + }, + }, + + { + name: "KMS: no-op when hashes match and key is migrated", + targetGRs: []schema.GroupResource{ + {Group: "", Resource: "secrets"}, + }, + kmsConfigHash: "config-hash-12345678", + kmsKeyIDHash: "key-id-hash-abcdefgh", + initialObjects: []runtime.Object{ + encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), + func() *corev1.Secret { + s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "config-hash-12345678" + s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "key-id-hash-abcdefgh" + return s + }(), + }, + apiServerObjects: []runtime.Object{ + func() *configv1.APIServer { + apiServer := &configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}} + apiServer.Spec.Encryption = configv1.APIServerEncryption{ + Type: "kms", + KMS: &configv1.KMSConfig{ + Type: configv1.AWSKMSProvider, + AWS: &configv1.AWSKMSConfig{ + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/test-key", + Region: "us-east-1", + }, + }, + } + return apiServer + }(), + }, + targetNamespace: "kms", + expectedActions: []string{ + "list:pods:kms", + "get:secrets:kms", + "list:secrets:openshift-config-managed", + }, + }, } for _, scenario := range scenarios { @@ -375,6 +624,12 @@ func TestKeyController(t *testing.T) { target := NewKeyController(scenario.targetNamespace, nil, provider, deployer, alwaysFulfilledPreconditions, fakeOperatorClient, fakeApiServerClient, fakeApiServerInformer, kubeInformers, fakeSecretClient, scenario.encryptionSecretSelector, eventRecorder) + if scenario.kmsConfigHash != "" || scenario.kmsKeyIDHash != "" { + kmsHashesGetterFunc = func(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) { + return scenario.kmsConfigHash, scenario.kmsKeyIDHash, nil + } + } + // act err = target.Sync(context.TODO(), factory.NewSyncContext("test", eventRecorder)) diff --git a/pkg/operator/encryption/controllers/migration_controller_test.go b/pkg/operator/encryption/controllers/migration_controller_test.go index 5a28ae671a..b37e5fff91 100644 --- a/pkg/operator/encryption/controllers/migration_controller_test.go +++ b/pkg/operator/encryption/controllers/migration_controller_test.go @@ -598,7 +598,7 @@ func TestMigrationController(t *testing.T) { func() *corev1.Secret { s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 2, "kms") s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" // same config - s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) + s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) s.Kind = "Secret" s.APIVersion = corev1.SchemeGroupVersion.String() return s @@ -709,7 +709,7 @@ func TestMigrationController(t *testing.T) { func() *corev1.Secret { s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 2, "kms") s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" // same config - s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) + s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) s.Kind = "Secret" s.APIVersion = corev1.SchemeGroupVersion.String() return s From 8b6ff7a542ab6a914eda2c6918420c132067d1ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 27 Oct 2025 18:03:09 +0300 Subject: [PATCH 03/10] Use secret for storage of kms hashes --- .../encryption/controllers/key_controller.go | 38 +++++------ .../controllers/key_controller_test.go | 67 ++++--------------- .../controllers/migration_controller_test.go | 59 +++++++++++----- .../controllers/state_controller_test.go | 17 +++-- pkg/operator/encryption/crypto/keys.go | 12 ++-- .../encryption/encryptionconfig/config.go | 11 ++- pkg/operator/encryption/kms/kms.go | 35 +++++----- pkg/operator/encryption/kms/kms_test.go | 26 +++++++ pkg/operator/encryption/secrets/secrets.go | 6 -- .../encryption/secrets/secrets_test.go | 8 --- pkg/operator/encryption/secrets/types.go | 4 -- pkg/operator/encryption/state/helpers.go | 15 +---- pkg/operator/encryption/state/types.go | 4 +- 13 files changed, 144 insertions(+), 158 deletions(-) diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 229ec75d14..4dfcef0080 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -42,7 +42,7 @@ import ( const encryptionSecretMigrationInterval = time.Hour * 24 * 7 // one week // kmsHashesGetter is a function type for getting KMS config and key ID hashes -var kmsHashesGetterFunc func(ctx context.Context, kmsConfig *configv1.KMSConfig) (configHash string, keyIDHash string, err error) +var kmsHashesGetterFunc func(ctx context.Context, kmsConfig *configv1.KMSConfig) (configHash string, keyIDHash []byte, err error) // keyController creates new keys if necessary. It // * watches @@ -172,9 +172,10 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact } // Compute KMS hashes if using KMS mode - var kmsConfigHash, kmsKeyIDHash string + var kmsConfigHash string + var kmsKeyHash []byte if currentMode == state.KMS && kmsConfig != nil { - kmsConfigHash, kmsKeyIDHash, err = kmsHashesGetterFunc(ctx, kmsConfig) + kmsConfigHash, kmsKeyHash, err = kmsHashesGetterFunc(ctx, kmsConfig) if err != nil { return err } @@ -207,7 +208,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact var commonReason *string for gr, grKeys := range desiredEncryptionState { - latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, kmsConfigHash, kmsKeyIDHash) + latestKeyID, internalReason, needed := needsNewKey(grKeys, currentMode, externalReason, encryptedGRs, kmsKeyHash) if !needed { continue } @@ -234,7 +235,7 @@ func (c *keyController) checkAndCreateKeys(ctx context.Context, syncContext fact sort.Sort(sort.StringSlice(reasons)) internalReason := strings.Join(reasons, ", ") - keySecret, err := c.generateKeySecret(newKeyID, currentMode, internalReason, externalReason, kmsConfigHash, kmsKeyIDHash) + keySecret, err := c.generateKeySecret(newKeyID, currentMode, internalReason, externalReason, kmsConfigHash, kmsKeyHash) if err != nil { return fmt.Errorf("failed to create key: %v", err) } @@ -271,8 +272,8 @@ func (c *keyController) validateExistingSecret(ctx context.Context, keySecret *c return nil // we made this key earlier } -func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, internalReason, externalReason, kmsConfigHash, kmsKeyIDHash string) (*corev1.Secret, error) { - bs := crypto.ModeToNewKeyFunc[currentMode]() +func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, internalReason, externalReason, kmsConfigHash string, kmsKeyIDHash []byte) (*corev1.Secret, error) { + bs := crypto.ModeToNewKeyFunc[currentMode](kmsKeyIDHash) ks := state.KeyState{ Key: apiserverv1.Key{ Name: fmt.Sprintf("%d", keyID), @@ -282,7 +283,6 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode, InternalReason: internalReason, ExternalReason: externalReason, KMSConfigHash: kmsConfigHash, - KMSKeyIDHash: kmsKeyIDHash, } return secrets.FromKeyState(c.instanceName, ks) } @@ -318,34 +318,34 @@ func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (st // defaultGetKMSHashes is the default implementation of getting KMS hashes // It calls the real KMS client to get the status and compute hashes -func defaultGetKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) { +func defaultGetKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, []byte, error) { // Generate unix socket path from KMS config and get the hash socketPath, configHash, err := kms.GenerateUnixSocketPath(kmsConfig) if err != nil { - return "", "", fmt.Errorf("failed to generate KMS unix socket path: %w", err) + return "", nil, fmt.Errorf("failed to generate KMS unix socket path: %w", err) } kmsClient, err := kms.NewKMSClient(socketPath) if err != nil { - return "", "", fmt.Errorf("failed to create KMS client: %w", err) + return "", nil, fmt.Errorf("failed to create KMS client: %w", err) } defer kmsClient.Close() statusResp, err := kmsClient.Status(ctx) if err != nil { - return "", "", fmt.Errorf("failed to call KMS Status endpoint: %w", err) + return "", nil, fmt.Errorf("failed to call KMS Status endpoint: %w", err) } if statusResp.Healthz != "ok" { - return "", "", fmt.Errorf("KMS plugin is unhealthy: %s", statusResp.Healthz) + return "", nil, fmt.Errorf("KMS plugin is unhealthy: %s", statusResp.Healthz) } - return configHash, kms.ComputeKMSKeyIDHash(statusResp.KeyID), nil + return configHash, kms.ComputeKMSKeyHash(configHash, statusResp.KeyID), nil } // needsNewKey checks whether a new key must be created for the given resource. If true, it also returns the latest // used key ID and a reason string. -func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource, kmsConfigHash, kmsKeyIDHash string) (uint64, string, bool) { +func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, externalReason string, encryptedGRs []schema.GroupResource, kmsKeyHash []byte) (uint64, string, bool) { // we always need to have some encryption keys unless we are turned off if len(grKeys.ReadKeys) == 0 { return 0, "key-does-not-exist", currentMode != state.Identity @@ -395,12 +395,8 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern // if we are using KMS, check if the KMS configuration or key ID hash has changed if currentMode == state.KMS { - if latestKey.KMSConfigHash != kmsConfigHash && len(kmsConfigHash) != 0 { - return latestKeyID, "kms-config-changed", true - } - - if latestKey.KMSKeyIDHash != kmsKeyIDHash && len(kmsKeyIDHash) != 0 { - return latestKeyID, "kms-key-id-changed", true + if latestKey.Key.Secret != base64.StdEncoding.EncodeToString(kmsKeyHash) { + return latestKeyID, "kms-key-changed", true } // For KMS mode, we don't do time-based rotation diff --git a/pkg/operator/encryption/controllers/key_controller_test.go b/pkg/operator/encryption/controllers/key_controller_test.go index 1528d6356a..80b8973bcf 100644 --- a/pkg/operator/encryption/controllers/key_controller_test.go +++ b/pkg/operator/encryption/controllers/key_controller_test.go @@ -373,9 +373,6 @@ func TestKeyController(t *testing.T) { if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] == "" { ts.Error("expected KMS config hash annotation to be set") } - if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] == "" { - ts.Error("expected KMS key ID hash annotation to be set") - } if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"] != "kms" { ts.Errorf("expected mode to be 'kms', got '%s'", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"]) } @@ -391,18 +388,18 @@ func TestKeyController(t *testing.T) { }, { - name: "KMS: creates new key when KMS config hash changes", + name: "KMS: no-op when only KMS config hash changes but key ID hash is the same", targetGRs: []schema.GroupResource{ {Group: "", Resource: "secrets"}, }, - kmsConfigHash: "new-config-hash-xyz", // Different hash for new key ARN - kmsKeyIDHash: "key-id-hash-abcd1234", + kmsConfigHash: "new-config-hash-xyz", // Different hash for new key ARN + kmsKeyIDHash: "key-id-hash-abcd1234", // Same key ID hash initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + // Secret with the same key ID hash (stored in Data) + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, []byte("key-id-hash-abcd1234"), "kms") s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "old-config-hash-1234" - s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "key-id-hash-abcd1234" return s }(), }, @@ -414,7 +411,7 @@ func TestKeyController(t *testing.T) { KMS: &configv1.KMSConfig{ Type: configv1.AWSKMSProvider, AWS: &configv1.AWSKMSConfig{ - KeyARN: "arn:aws:kms:us-east-1:123456789012:key/new-key", // Different key + KeyARN: "arn:aws:kms:us-east-1:123456789012:key/new-key", // Different key ARN Region: "us-east-1", }, }, @@ -427,35 +424,6 @@ func TestKeyController(t *testing.T) { "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) - - // Verify new KMS config hash is set and different from old - newConfigHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] - if newConfigHash == "" || newConfigHash == "old-config-hash-1234" { - ts.Errorf("expected new KMS config hash, got '%s'", newConfigHash) - } - - // Verify internal reason mentions config change - internalReason := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] - if internalReason != "secrets-kms-config-changed" { - ts.Errorf("expected internal reason 'secrets-kms-config-changed', got '%s'", internalReason) - } - - wasSecretValidated = true - break - } - } - if !wasSecretValidated { - ts.Errorf("the secret wasn't created and validated") - } }, }, @@ -469,9 +437,9 @@ func TestKeyController(t *testing.T) { initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + // Secret with old key ID hash stored in Data + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, []byte("old-key-id-hash-abc"), "kms") s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "config-hash-12345678" - s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "old-key-id-hash-abc" return s }(), }, @@ -506,22 +474,16 @@ func TestKeyController(t *testing.T) { createAction := action.(clientgotesting.CreateAction) actualSecret := createAction.GetObject().(*corev1.Secret) - // Verify new KMS key ID hash is set and different from old - newKeyIDHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] - if newKeyIDHash == "" || newKeyIDHash == "old-key-id-hash-abc" { - ts.Errorf("expected new KMS key ID hash, got '%s'", newKeyIDHash) - } - // Verify config hash stays the same configHash := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] if configHash != "config-hash-12345678" { ts.Errorf("expected config hash to remain 'config-hash-12345678', got '%s'", configHash) } - // Verify internal reason mentions key ID change + // Verify internal reason mentions KMS key change internalReason := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] - if internalReason != "secrets-kms-key-id-changed" { - ts.Errorf("expected internal reason 'secrets-kms-key-id-changed', got '%s'", internalReason) + if internalReason != "secrets-kms-key-changed" { + ts.Errorf("expected internal reason 'secrets-kms-key-changed', got '%s'", internalReason) } wasSecretValidated = true @@ -544,9 +506,8 @@ func TestKeyController(t *testing.T) { initialObjects: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, []byte("key-id-hash-abcdefgh"), "kms") s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "config-hash-12345678" - s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "key-id-hash-abcdefgh" return s }(), }, @@ -625,8 +586,8 @@ func TestKeyController(t *testing.T) { target := NewKeyController(scenario.targetNamespace, nil, provider, deployer, alwaysFulfilledPreconditions, fakeOperatorClient, fakeApiServerClient, fakeApiServerInformer, kubeInformers, fakeSecretClient, scenario.encryptionSecretSelector, eventRecorder) if scenario.kmsConfigHash != "" || scenario.kmsKeyIDHash != "" { - kmsHashesGetterFunc = func(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, string, error) { - return scenario.kmsConfigHash, scenario.kmsKeyIDHash, nil + kmsHashesGetterFunc = func(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, []byte, error) { + return scenario.kmsConfigHash, []byte(scenario.kmsKeyIDHash), nil } } diff --git a/pkg/operator/encryption/controllers/migration_controller_test.go b/pkg/operator/encryption/controllers/migration_controller_test.go index b37e5fff91..481ea7ffca 100644 --- a/pkg/operator/encryption/controllers/migration_controller_test.go +++ b/pkg/operator/encryption/controllers/migration_controller_test.go @@ -5,12 +5,13 @@ import ( "encoding/json" "errors" "fmt" - clocktesting "k8s.io/utils/clock/testing" "reflect" "strings" "testing" "time" + clocktesting "k8s.io/utils/clock/testing" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -585,20 +586,32 @@ func TestMigrationController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), }, initialSecrets: []*corev1.Secret{ - // Old KMS key with old KMSKeyIDHash + // Old KMS key with old keyHash func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 1, "kms") + // For KMS mode, Secret field contains the hex-encoded combined hash (32 chars) + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode( + "kms", + nil, + 1, + []byte("0123456789abcdef0123456789abcdef"), // 32-char hex hash + "kms", + ) s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" - s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "0123456789abcdef" // 16 hex chars s.Kind = "Secret" s.APIVersion = corev1.SchemeGroupVersion.String() return s }(), - // New KMS key with new KMSKeyIDHash (simulating KMS key rotation) + // New KMS key with new keyHash (simulating KMS key rotation) func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 2, "kms") + // For KMS mode, Secret field contains the hex-encoded combined hash (32 chars) + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode( + "kms", + nil, + 2, + []byte("fedcba9876543210fedcba9876543210"), // 32-char hex hash + "kms", + ) s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" // same config - s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) s.Kind = "Secret" s.APIVersion = corev1.SchemeGroupVersion.String() return s @@ -613,7 +626,7 @@ func TestMigrationController(t *testing.T) { { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", - Name: "kms-provider-fedcba9876543210-2", // new key ID hash (16 hex chars) + Name: "kms-provider-fedcba9876543210fedcba9876543210-2", // hex key hash (32 chars) Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", Timeout: &metav1.Duration{ Duration: 10 * time.Second, @@ -623,7 +636,7 @@ func TestMigrationController(t *testing.T) { { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", - Name: "kms-provider-0123456789abcdef-1", // old key ID hash (16 hex chars) + Name: "kms-provider-0123456789abcdef0123456789abcdef-1", // hex key hash (32 chars) Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", Timeout: &metav1.Duration{ Duration: 10 * time.Second, @@ -696,20 +709,32 @@ func TestMigrationController(t *testing.T) { encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), }, initialSecrets: []*corev1.Secret{ - // Old KMS key with old KMSKeyIDHash + // Old KMS key with old keyHash func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 1, "kms") + // For KMS mode, Secret field contains the hex-encoded combined hash (32 chars) + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode( + "kms", + nil, + 1, + []byte("0123456789abcdef0123456789abcdef"), // 32-char hex hash + "kms", + ) s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" - s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "0123456789abcdef" // 16 hex chars s.Kind = "Secret" s.APIVersion = corev1.SchemeGroupVersion.String() return s }(), - // New KMS key with new KMSKeyIDHash (simulating KMS key rotation) + // New KMS key with new keyHash (simulating KMS key rotation) func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", nil, 2, "kms") + // For KMS mode, Secret field contains the hex-encoded combined hash (32 chars) + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode( + "kms", + nil, + 2, + []byte("fedcba9876543210fedcba9876543210"), // 32-char hex hash + "kms", + ) s.Annotations[secrets.EncryptionSecretKMSConfigHash] = "1234567890abcdef" // same config - s.Annotations[secrets.EncryptionSecretKMSKeyIDHash] = "fedcba9876543210" // different key ID hash (16 hex chars) s.Kind = "Secret" s.APIVersion = corev1.SchemeGroupVersion.String() return s @@ -724,7 +749,7 @@ func TestMigrationController(t *testing.T) { { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", - Name: "kms-provider-fedcba9876543210-2", // new key ID hash (16 hex chars) + Name: "kms-provider-fedcba9876543210fedcba9876543210-2", // hex key hash (32 chars) Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", Timeout: &metav1.Duration{ Duration: 10 * time.Second, @@ -734,7 +759,7 @@ func TestMigrationController(t *testing.T) { { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", - Name: "kms-provider-0123456789abcdef-1", // old key ID hash (16 hex chars) + Name: "kms-provider-0123456789abcdef0123456789abcdef-1", // hex key hash (32 chars) Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", Timeout: &metav1.Duration{ Duration: 10 * time.Second, diff --git a/pkg/operator/encryption/controllers/state_controller_test.go b/pkg/operator/encryption/controllers/state_controller_test.go index c6e84ca61e..57fc254d94 100644 --- a/pkg/operator/encryption/controllers/state_controller_test.go +++ b/pkg/operator/encryption/controllers/state_controller_test.go @@ -5,10 +5,11 @@ import ( "encoding/base64" "errors" "fmt" - clocktesting "k8s.io/utils/clock/testing" "testing" "time" + clocktesting "k8s.io/utils/clock/testing" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -716,10 +717,16 @@ func TestStateController(t *testing.T) { initialResources: []runtime.Object{ encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"), func() *corev1.Secret { - s := encryptiontesting.CreateEncryptionKeySecretNoDataWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 1, "kms") - // Set KMS-specific annotations with config hash and key ID hash + // For KMS mode, Secret field contains the hex-encoded combined hash (32 chars) + s := encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode( + "kms", + []schema.GroupResource{{Group: "", Resource: "secrets"}}, + 1, + []byte("1234567890abcdef1234567890abcdef"), // 32-char hex hash + "kms", + ) + // Set KMS-specific annotations with config hash s.Annotations["encryption.apiserver.operator.openshift.io/kms-config-hash"] = "1234567890abcdef" - s.Annotations["encryption.apiserver.operator.openshift.io/kms-key-id-hash"] = "fedcba0987654321" return s }(), }, @@ -748,7 +755,7 @@ func TestStateController(t *testing.T) { { KMS: &apiserverconfigv1.KMSConfiguration{ APIVersion: "v2", - Name: "kms-provider-fedcba0987654321-1", // kms-provider-{keyIDHash16}-{key ID} + Name: "kms-provider-1234567890abcdef1234567890abcdef-1", // kms-provider-{keyHash32}-{key ID} Endpoint: "unix://var/run/kms/kms-1234567890abcdef.sock", Timeout: &metav1.Duration{ Duration: 10 * time.Second, diff --git a/pkg/operator/encryption/crypto/keys.go b/pkg/operator/encryption/crypto/keys.go index b9393ece66..76c67763bf 100644 --- a/pkg/operator/encryption/crypto/keys.go +++ b/pkg/operator/encryption/crypto/keys.go @@ -7,16 +7,16 @@ import ( ) var ( - ModeToNewKeyFunc = map[state.Mode]func() []byte{ + ModeToNewKeyFunc = map[state.Mode]func(externalKey []byte) []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, - state.KMS: NewIdentityKey, // this is not used in KMS + state.KMS: NewKMSKey, } ) -func NewAES256Key() []byte { +func NewAES256Key(_ []byte) []byte { b := make([]byte, 32) // AES-256 == 32 byte key if _, err := rand.Read(b); err != nil { panic(err) // rand should never fail @@ -24,6 +24,10 @@ func NewAES256Key() []byte { return b } -func NewIdentityKey() []byte { +func NewIdentityKey(_ []byte) []byte { return make([]byte, 16) // the key is not used to perform encryption but must be a valid AES key } + +func NewKMSKey(externalKey []byte) []byte { + return externalKey +} diff --git a/pkg/operator/encryption/encryptionconfig/config.go b/pkg/operator/encryption/encryptionconfig/config.go index 1a8b130b9d..a39968650d 100644 --- a/pkg/operator/encryption/encryptionconfig/config.go +++ b/pkg/operator/encryption/encryptionconfig/config.go @@ -16,7 +16,7 @@ import ( ) var ( - emptyStaticIdentityKey = base64.StdEncoding.EncodeToString(crypto.NewIdentityKey()) + emptyStaticIdentityKey = base64.StdEncoding.EncodeToString(crypto.NewIdentityKey(nil)) ) // FromEncryptionState converts state to config. @@ -108,7 +108,7 @@ func ToEncryptionState(encryptionConfig *apiserverconfigv1.EncryptionConfigurati } case provider.KMS != nil: - configHash, keyIDHash, keyName, err := kms.ExtractKMSHashAndKeyName(provider) + configHash, keyHash, keyName, err := kms.ExtractKMSHashAndKeyName(provider) if err != nil { klog.Warningf("skipping invalid encryption KMS config for resource %v", provider) continue // should never happen @@ -116,14 +116,11 @@ func ToEncryptionState(encryptionConfig *apiserverconfigv1.EncryptionConfigurati ks = state.KeyState{ Key: apiserverconfigv1.Key{ - Name: keyName, - // We set this unused secret just to align with what we set initially. - // This is unused. - Secret: base64.StdEncoding.EncodeToString(crypto.ModeToNewKeyFunc[state.KMS]()), + Name: keyName, + Secret: keyHash, }, Mode: state.KMS, KMSConfigHash: configHash, - KMSKeyIDHash: keyIDHash, } default: diff --git a/pkg/operator/encryption/kms/kms.go b/pkg/operator/encryption/kms/kms.go index 4f73f9bb6b..8560cade3b 100644 --- a/pkg/operator/encryption/kms/kms.go +++ b/pkg/operator/encryption/kms/kms.go @@ -2,6 +2,7 @@ package kms import ( "crypto/sha256" + "encoding/base64" "encoding/hex" "fmt" "regexp" @@ -21,7 +22,7 @@ const ( // GenerateUnixSocketPath generates a unique unix socket path from KMS configuration // by hashing the provider-specific configuration. // Returns the socket path and the hash value (first 16 characters). -func GenerateUnixSocketPath(kmsConfig *configv1.KMSConfig) (socketPath string, hash string, err error) { +func GenerateUnixSocketPath(kmsConfig *configv1.KMSConfig) (string, string, error) { if kmsConfig == nil { return "", "", fmt.Errorf("kmsConfig cannot be nil") } @@ -64,26 +65,27 @@ func generateAWSUnixSocketPath(awsConfig *configv1.AWSKMSConfig) (string, string return socketPath, shortHash, nil } -// ComputeKMSKeyIDHash computes a hash of the KMS key ID returned from the Status endpoint. -// Returns the first 16 characters of the SHA256 hash. -func ComputeKMSKeyIDHash(keyID string) string { +// ComputeKMSKeyHash computes a hash of the KMS key ID returned from the Status endpoint. +// Returns the first 32 characters of the SHA256 hash. +func ComputeKMSKeyHash(configHash, keyID string) []byte { if keyID == "" { - return "" + return nil } + combined := configHash + ":" + keyID // Compute SHA256 hash - hash := sha256.Sum256([]byte(keyID)) + hash := sha256.Sum256([]byte(combined)) hashStr := hex.EncodeToString(hash[:]) - return hashStr[:16] + return []byte(hashStr[:32]) } var ( // endpointHashRegex matches the config hash in endpoint path: unix://var/run/kms/kms-{configHash16}.sock endpointHashRegex = regexp.MustCompile(`kms-([a-f0-9]{16})\.sock$`) - // providerNameRegex matches the key ID hash and key ID in provider name: kms-provider-{keyIDHash16}-{keyID} - // Example: kms-provider-abcdef1234567890-1 - providerNameRegex = regexp.MustCompile(`^kms-provider-([a-f0-9]{16})-(.+)$`) + // providerNameRegex matches the key ID hash and key ID in provider name: kms-provider-{keyIDHash32}-{keyID} + // Example: kms-provider-abcdef1234567890abcdef1234567890-1 + providerNameRegex = regexp.MustCompile(`^kms-provider-([a-f0-9]{32})-(.+)$`) ) // ExtractKMSHashAndKeyName extracts the KMSConfigHash, KMSKeyIDHash, and key.Name embedded into provider @@ -99,17 +101,17 @@ func ExtractKMSHashAndKeyName(provider v1.ProviderConfiguration) (string, string } // Extract the key ID hash and key ID from the provider name: kms-provider-{keyIDHash16}-{keyID} - // Example: kms-provider-abcdef1234567890-1 - var keyIDHash, keyName string + // Example: kms-provider-abcdef1234567890abcdef1234567890-1 + var keyHash, keyName string providerName := provider.KMS.Name if matches := providerNameRegex.FindStringSubmatch(providerName); len(matches) == 3 { - keyIDHash = matches[1] + keyHash = matches[1] keyName = matches[2] } else { return "", "", "", fmt.Errorf("invalid KMS provider name format: %s", providerName) } - return configHash, keyIDHash, keyName, nil + return configHash, base64.StdEncoding.EncodeToString([]byte(keyHash)), keyName, nil } // GenerateKMSProviderConfigurationFromKey generates the compatible ProviderConfiguration with @@ -122,9 +124,10 @@ func GenerateKMSProviderConfigurationFromKey(key state.KeyState) v1.ProviderConf // This must generate the same format as GenerateUnixSocketPath socketPath := fmt.Sprintf("%s/kms-%s.sock", unixSocketBaseDir, key.KMSConfigHash) // Embed KMSKeyIDHash and key ID in the provider name so we can extract them when reading back - // Format: kms-provider-{keyIDHash16}-{keyID} + // Format: kms-provider-{keyIDHash32}-{keyID} // This must match the providerNameRegex - providerName := fmt.Sprintf("kms-provider-%s-%s", key.KMSKeyIDHash, key.Key.Name) + decoded, _ := base64.StdEncoding.DecodeString(key.Key.Secret) + providerName := fmt.Sprintf("kms-provider-%s-%s", decoded, key.Key.Name) return v1.ProviderConfiguration{ KMS: &v1.KMSConfiguration{ diff --git a/pkg/operator/encryption/kms/kms_test.go b/pkg/operator/encryption/kms/kms_test.go index 1cb19539e9..180b394c2c 100644 --- a/pkg/operator/encryption/kms/kms_test.go +++ b/pkg/operator/encryption/kms/kms_test.go @@ -100,3 +100,29 @@ func TestGenerateUnixSocketPath(t *testing.T) { } }) } + +func TestComputeKMSKeyHash(t *testing.T) { + tests := []struct { + name string + configHash string + keyID string + wantHash string + }{ + { + name: "valid config hash and key ID", + configHash: "2e55e11c0b187f2d", + keyID: "arn:aws:kms:us-east-1:123456789012:key/12345678-1234-1234-1234-123456789012", + wantHash: "26f87255a78a26fb93d8da5f7b1ada0c", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotHash := ComputeKMSKeyHash(tt.configHash, tt.keyID) + + if string(gotHash) != tt.wantHash { + t.Fatalf("ComputeKMSKeyHash() gotHash = %v, want %v", string(gotHash), tt.wantHash) + } + }) + } +} diff --git a/pkg/operator/encryption/secrets/secrets.go b/pkg/operator/encryption/secrets/secrets.go index e33f9fc6af..f814d153d9 100644 --- a/pkg/operator/encryption/secrets/secrets.go +++ b/pkg/operator/encryption/secrets/secrets.go @@ -61,9 +61,6 @@ func ToKeyState(s *corev1.Secret) (state.KeyState, error) { if v, ok := s.Annotations[EncryptionSecretKMSConfigHash]; ok && len(v) > 0 { key.KMSConfigHash = v } - if v, ok := s.Annotations[EncryptionSecretKMSKeyIDHash]; ok && len(v) > 0 { - key.KMSKeyIDHash = v - } keyMode := state.Mode(s.Annotations[encryptionSecretMode]) switch keyMode { @@ -123,9 +120,6 @@ func FromKeyState(component string, ks state.KeyState) (*corev1.Secret, error) { if len(ks.KMSConfigHash) > 0 { s.Annotations[EncryptionSecretKMSConfigHash] = ks.KMSConfigHash } - if len(ks.KMSKeyIDHash) > 0 { - s.Annotations[EncryptionSecretKMSKeyIDHash] = ks.KMSKeyIDHash - } return s, nil } diff --git a/pkg/operator/encryption/secrets/secrets_test.go b/pkg/operator/encryption/secrets/secrets_test.go index e4788f02c1..0c3595426a 100644 --- a/pkg/operator/encryption/secrets/secrets_test.go +++ b/pkg/operator/encryption/secrets/secrets_test.go @@ -124,7 +124,6 @@ func TestRoundtrip(t *testing.T) { Backed: true, Mode: "kms", KMSConfigHash: "2e55e11c0b187f2d", - KMSKeyIDHash: "abcdef1234567890", InternalReason: "kms-config-changed", ExternalReason: "kms-key-rotated", }, @@ -140,7 +139,6 @@ func TestRoundtrip(t *testing.T) { Backed: true, Mode: "kms", KMSConfigHash: "3f66f22d1c298e3e", - KMSKeyIDHash: "fedcba0987654321", Migrated: state.MigrationState{ Timestamp: now, Resources: []schema.GroupResource{ @@ -199,7 +197,6 @@ func TestToKeyState_KMS(t *testing.T) { annotations: map[string]string{ encryptionSecretMode: "kms", EncryptionSecretKMSConfigHash: "2e55e11c0b187f2d", - EncryptionSecretKMSKeyIDHash: "abcdef1234567890", encryptionSecretInternalReason: "kms-config-changed", encryptionSecretExternalReason: "kms-key-rotated", }, @@ -211,7 +208,6 @@ func TestToKeyState_KMS(t *testing.T) { Backed: true, Mode: "kms", KMSConfigHash: "2e55e11c0b187f2d", - KMSKeyIDHash: "abcdef1234567890", InternalReason: "kms-config-changed", ExternalReason: "kms-key-rotated", }, @@ -233,7 +229,6 @@ func TestToKeyState_KMS(t *testing.T) { Backed: true, Mode: "kms", KMSConfigHash: "3f66f22d1c298e3e", - KMSKeyIDHash: "", }, wantErr: false, }, @@ -252,7 +247,6 @@ func TestToKeyState_KMS(t *testing.T) { Backed: true, Mode: "kms", KMSConfigHash: "", - KMSKeyIDHash: "", }, wantErr: false, }, @@ -341,14 +335,12 @@ func TestFromKeyState_KMS(t *testing.T) { }, Mode: "kms", KMSConfigHash: "2e55e11c0b187f2d", - KMSKeyIDHash: "abcdef1234567890", }, wantSecretName: "encryption-key-apiserver-1", wantDataEmpty: true, wantAnnotations: map[string]string{ encryptionSecretMode: "kms", EncryptionSecretKMSConfigHash: "2e55e11c0b187f2d", - EncryptionSecretKMSKeyIDHash: "abcdef1234567890", }, wantErr: false, }, diff --git a/pkg/operator/encryption/secrets/types.go b/pkg/operator/encryption/secrets/types.go index 5b6be9574f..32a576dea2 100644 --- a/pkg/operator/encryption/secrets/types.go +++ b/pkg/operator/encryption/secrets/types.go @@ -45,10 +45,6 @@ const ( // This is used to detect changes in the KMS configuration that would require a new key. EncryptionSecretKMSConfigHash = "encryption.apiserver.operator.openshift.io/kms-config-hash" - // EncryptionSecretKMSKeyIDHash is the annotation that stores the hash of the KMS key ID. - // This is used to detect changes in the KMS key ID (e.g., after key rotation in the KMS system). - EncryptionSecretKMSKeyIDHash = "encryption.apiserver.operator.openshift.io/kms-key-id-hash" - // In the data field of the secret API object, this (map) key is used to hold the actual encryption key // (i.e. for AES-CBC mode the value associated with this map key is 32 bytes of random noise). EncryptionSecretKeyDataKey = "encryption.apiserver.operator.openshift.io-key" diff --git a/pkg/operator/encryption/state/helpers.go b/pkg/operator/encryption/state/helpers.go index 6dde64f8e6..656aeafef6 100644 --- a/pkg/operator/encryption/state/helpers.go +++ b/pkg/operator/encryption/state/helpers.go @@ -74,23 +74,10 @@ func NameToKeyID(name string) (uint64, bool) { } func EqualKeyAndEqualID(s1, s2 *KeyState) bool { - if s1.Mode != s2.Mode { + if s1.Mode != s2.Mode || s1.Key.Secret != s2.Key.Secret { return false } - // For KMS mode, compare both KMSConfigHash and KMSKeyIDHash to detect config or key rotation - if s1.Mode == KMS { - if s1.KMSConfigHash != s2.KMSConfigHash { - return false - } - if s1.KMSKeyIDHash != s2.KMSKeyIDHash { - return false - } - } else if s1.Key.Secret != s2.Key.Secret { - return false - } - - // Verify key IDs match (for both KMS and non-KMS modes) id1, valid1 := NameToKeyID(s1.Key.Name) id2, valid2 := NameToKeyID(s2.Key.Name) return valid1 && valid2 && id1 == id2 diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index 9ec37aa118..5ec7ae35f9 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -26,7 +26,7 @@ type GroupResourceState struct { } func (k GroupResourceState) HasWriteKey() bool { - return (len(k.WriteKey.Key.Name) > 0 && len(k.WriteKey.Key.Secret) > 0) || k.WriteKey.KMSConfigHash != "" + return len(k.WriteKey.Key.Name) > 0 && len(k.WriteKey.Key.Secret) > 0 } type KeyState struct { @@ -42,8 +42,6 @@ type KeyState struct { ExternalReason string // hash of the KMS configuration to detect changes KMSConfigHash string - // hash of the KMS key ID to detect changes - KMSKeyIDHash string } type MigrationState struct { From 71699d34e967ddd84a5f04a7b9bc59344f55cc6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 30 Oct 2025 11:23:39 +0300 Subject: [PATCH 04/10] fix encryption type --- pkg/operator/encryption/state/types.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/operator/encryption/state/types.go b/pkg/operator/encryption/state/types.go index 5ec7ae35f9..8d2c621f5e 100644 --- a/pkg/operator/encryption/state/types.go +++ b/pkg/operator/encryption/state/types.go @@ -62,7 +62,7 @@ const ( 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 - KMS Mode = "kms" + KMS Mode = "KMS" // Changing this value requires caution to not break downgrades. // Specifically, if some new Mode is released in version X, that new Mode cannot From ea8e6aa29848954896421d946cd5cd1fb357bba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 30 Oct 2025 14:56:26 +0300 Subject: [PATCH 05/10] Temporarily disable kms plugin access --- pkg/operator/encryption/controllers/key_controller.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/operator/encryption/controllers/key_controller.go b/pkg/operator/encryption/controllers/key_controller.go index 4dfcef0080..9e6d947b75 100644 --- a/pkg/operator/encryption/controllers/key_controller.go +++ b/pkg/operator/encryption/controllers/key_controller.go @@ -320,12 +320,12 @@ func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (st // It calls the real KMS client to get the status and compute hashes func defaultGetKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (string, []byte, error) { // Generate unix socket path from KMS config and get the hash - socketPath, configHash, err := kms.GenerateUnixSocketPath(kmsConfig) + _, configHash, err := kms.GenerateUnixSocketPath(kmsConfig) if err != nil { return "", nil, fmt.Errorf("failed to generate KMS unix socket path: %w", err) } - kmsClient, err := kms.NewKMSClient(socketPath) + /*kmsClient, err := kms.NewKMSClient(socketPath) if err != nil { return "", nil, fmt.Errorf("failed to create KMS client: %w", err) } @@ -338,9 +338,10 @@ func defaultGetKMSHashes(ctx context.Context, kmsConfig *configv1.KMSConfig) (st if statusResp.Healthz != "ok" { return "", nil, fmt.Errorf("KMS plugin is unhealthy: %s", statusResp.Healthz) - } + }*/ - return configHash, kms.ComputeKMSKeyHash(configHash, statusResp.KeyID), nil + keyId := "kms" + return configHash, kms.ComputeKMSKeyHash(configHash, keyId), nil } // needsNewKey checks whether a new key must be created for the given resource. If true, it also returns the latest From 2da49addafa41d3286605aebd78f4c5565e1e864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 31 Oct 2025 10:30:00 +0300 Subject: [PATCH 06/10] update unix path --- pkg/operator/encryption/kms/kms.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/operator/encryption/kms/kms.go b/pkg/operator/encryption/kms/kms.go index 8560cade3b..becec7bec6 100644 --- a/pkg/operator/encryption/kms/kms.go +++ b/pkg/operator/encryption/kms/kms.go @@ -16,7 +16,7 @@ import ( const ( // unixSocketBaseDir is the base directory for KMS unix sockets - unixSocketBaseDir = "unix://var/run/kms" + unixSocketBaseDir = "unix:///var/run/kms" ) // GenerateUnixSocketPath generates a unique unix socket path from KMS configuration From 4a45d508098c1890657d817a6f893d9ed0b6f5eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Mon, 3 Nov 2025 17:11:16 +0300 Subject: [PATCH 07/10] Append resource name in kms provider to make it unique --- .../encryption/encryptionconfig/config.go | 6 ++--- pkg/operator/encryption/kms/kms.go | 24 ++++++++++--------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/pkg/operator/encryption/encryptionconfig/config.go b/pkg/operator/encryption/encryptionconfig/config.go index a39968650d..467cac1488 100644 --- a/pkg/operator/encryption/encryptionconfig/config.go +++ b/pkg/operator/encryption/encryptionconfig/config.go @@ -26,7 +26,7 @@ func FromEncryptionState(encryptionState map[schema.GroupResource]state.GroupRes for gr, grKeys := range encryptionState { resourceConfigs = append(resourceConfigs, apiserverconfigv1.ResourceConfiguration{ Resources: []string{gr.String()}, // we are forced to lose data here because this API is broken - Providers: stateToProviders(grKeys), + Providers: stateToProviders(gr.Resource, grKeys), }) } @@ -156,7 +156,7 @@ func ToEncryptionState(encryptionConfig *apiserverconfigv1.EncryptionConfigurati // it primarily handles the conversion of KeyState to the appropriate provider config. // the identity mode is transformed into a custom aesgcm provider that simply exists to // curry the associated null key secret through the encryption state machine. -func stateToProviders(desired state.GroupResourceState) []apiserverconfigv1.ProviderConfiguration { +func stateToProviders(resource string, desired state.GroupResourceState) []apiserverconfigv1.ProviderConfiguration { allKeys := desired.ReadKeys providers := make([]apiserverconfigv1.ProviderConfiguration, 0, len(allKeys)+1) // one extra for identity @@ -210,7 +210,7 @@ func stateToProviders(desired state.GroupResourceState) []apiserverconfigv1.Prov }, }) case state.KMS: - providers = append(providers, kms.GenerateKMSProviderConfigurationFromKey(key)) + providers = append(providers, kms.GenerateKMSProviderConfigurationFromKey(resource, key)) default: // this should never happen because our input should always be valid klog.Infof("skipping key %s as it has invalid mode %s", key.Key.Name, key.Mode) diff --git a/pkg/operator/encryption/kms/kms.go b/pkg/operator/encryption/kms/kms.go index becec7bec6..1c9339cd44 100644 --- a/pkg/operator/encryption/kms/kms.go +++ b/pkg/operator/encryption/kms/kms.go @@ -83,9 +83,9 @@ func ComputeKMSKeyHash(configHash, keyID string) []byte { var ( // endpointHashRegex matches the config hash in endpoint path: unix://var/run/kms/kms-{configHash16}.sock endpointHashRegex = regexp.MustCompile(`kms-([a-f0-9]{16})\.sock$`) - // providerNameRegex matches the key ID hash and key ID in provider name: kms-provider-{keyIDHash32}-{keyID} - // Example: kms-provider-abcdef1234567890abcdef1234567890-1 - providerNameRegex = regexp.MustCompile(`^kms-provider-([a-f0-9]{32})-(.+)$`) + // providerNameRegex matches the key ID hash, key ID, and resource in provider name: kms-provider-{keyIDHash32}-{keyID}-{resource} + // Example: kms-provider-abcdef1234567890abcdef1234567890-1-secrets + providerNameRegex = regexp.MustCompile(`^kms-provider-([a-f0-9]{32})-([^-]+)-(.+)$`) ) // ExtractKMSHashAndKeyName extracts the KMSConfigHash, KMSKeyIDHash, and key.Name embedded into provider @@ -100,13 +100,14 @@ func ExtractKMSHashAndKeyName(provider v1.ProviderConfiguration) (string, string return "", "", "", fmt.Errorf("invalid KMS endpoint format: %s", endpoint) } - // Extract the key ID hash and key ID from the provider name: kms-provider-{keyIDHash16}-{keyID} - // Example: kms-provider-abcdef1234567890abcdef1234567890-1 + // Extract the key ID hash, key ID, and resource from the provider name: kms-provider-{keyIDHash32}-{keyID}-{resource} + // Example: kms-provider-abcdef1234567890abcdef1234567890-1-secrets var keyHash, keyName string providerName := provider.KMS.Name - if matches := providerNameRegex.FindStringSubmatch(providerName); len(matches) == 3 { + if matches := providerNameRegex.FindStringSubmatch(providerName); len(matches) == 4 { keyHash = matches[1] keyName = matches[2] + // matches[3] is the resource, but we don't need to return it } else { return "", "", "", fmt.Errorf("invalid KMS provider name format: %s", providerName) } @@ -117,17 +118,18 @@ func ExtractKMSHashAndKeyName(provider v1.ProviderConfiguration) (string, string // GenerateKMSProviderConfigurationFromKey generates the compatible ProviderConfiguration with // opinionated and extractable fields. We embed: // - KMSConfigHash in the socket path (endpoint) -// - KMSKeyIDHash and key.Name in the provider name +// - KMSKeyIDHash, key.Name, and resource in the provider name // This allows us to extract all three values and detect both config changes and key rotations. -func GenerateKMSProviderConfigurationFromKey(key state.KeyState) v1.ProviderConfiguration { +// The resource parameter ensures uniqueness when the same KMS config is used for multiple resources. +func GenerateKMSProviderConfigurationFromKey(resource string, key state.KeyState) v1.ProviderConfiguration { // Embed KMSConfigHash in the endpoint so we can extract it // This must generate the same format as GenerateUnixSocketPath socketPath := fmt.Sprintf("%s/kms-%s.sock", unixSocketBaseDir, key.KMSConfigHash) - // Embed KMSKeyIDHash and key ID in the provider name so we can extract them when reading back - // Format: kms-provider-{keyIDHash32}-{keyID} + // Embed KMSKeyIDHash, key ID, and resource in the provider name so we can extract them when reading back + // Format: kms-provider-{keyIDHash32}-{keyID}-{resource} // This must match the providerNameRegex decoded, _ := base64.StdEncoding.DecodeString(key.Key.Secret) - providerName := fmt.Sprintf("kms-provider-%s-%s", decoded, key.Key.Name) + providerName := fmt.Sprintf("kms-provider-%s-%s-%s", decoded, key.Key.Name, resource) return v1.ProviderConfiguration{ KMS: &v1.KMSConfiguration{ From e256f7d259ea1d56252f12e9ec1df2d4083babbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Tue, 4 Nov 2025 12:35:05 +0300 Subject: [PATCH 08/10] Add PoC plugin lifecycle --- pkg/operator/encryption/kms/plugin.go | 292 ++++++++++++++++++++++++++ 1 file changed, 292 insertions(+) create mode 100644 pkg/operator/encryption/kms/plugin.go diff --git a/pkg/operator/encryption/kms/plugin.go b/pkg/operator/encryption/kms/plugin.go new file mode 100644 index 0000000000..cbb1b5c20d --- /dev/null +++ b/pkg/operator/encryption/kms/plugin.go @@ -0,0 +1,292 @@ +package kms + +import ( + "fmt" + + configv1 "github.com/openshift/api/config/v1" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" +) + +const ( + // defaultKMSPluginImage is the default AWS KMS plugin image + // This should be overridden by the operator with the actual image reference + defaultKMSPluginImage = "registry.k8s.io/kms-plugin-aws:latest" + + // defaultHealthPort is the default port for the KMS plugin health endpoint + defaultHealthPort = 8080 + + // KMSContainerName is the standard name for the KMS plugin sidecar container + KMSContainerName = "kms-plugin" + + // KMSSocketVolumeName is the standard name for the socket volume + KMSSocketVolumeName = "kms-socket" + + // KMSCredentialsVolumeName is the standard name for the credentials volume + KMSCredentialsVolumeName = "aws-credentials" +) + +// ContainerConfig holds additional configuration beyond what's in configv1.KMSConfig +// for building the KMS plugin sidecar container +type ContainerConfig struct { + // KMSConfig is the desired configv1.KMSConfig + // Required + KMSConfig *configv1.KMSConfig + + // Image is the container image for the KMS plugin + // Required + Image string + + // UseHostNetwork indicates if the pod uses hostNetwork: true + // If true, the container will access AWS credentials via EC2 IMDS + // If false, credentials must be provided via CredentialsSecretName + UseHostNetwork bool + + // CredentialsSecretName is the name of the secret containing AWS credentials + // Only required when UseHostNetwork is false + // The secret should contain a key "credentials" in AWS shared credentials file format + CredentialsSecretName string + + // SocketPath is the Unix socket path where the KMS plugin listens + // Optional - defaults to defaultSocketPath + SocketPath string + + // HealthPort is the port for the KMS plugin health endpoint + // Optional - defaults to defaultHealthPort + HealthPort int32 + + // CPURequest is the CPU request for the container + // Optional - defaults to "10m" + CPURequest string + + // MemoryRequest is the memory request for the container + // Optional - defaults to "50Mi" + MemoryRequest string +} + +// Validate ensures the ContainerConfig is valid +func (c *ContainerConfig) Validate() error { + if c.Image == "" { + return fmt.Errorf("Image is required") + } + if !c.UseHostNetwork && c.CredentialsSecretName == "" { + return fmt.Errorf("CredentialsSecretName is required when UseHostNetwork is false") + } + return nil +} + +// setDefaults sets default values for unspecified fields +func (c *ContainerConfig) setDefaults() { + if c.SocketPath == "" { + socket, _, err := GenerateUnixSocketPath(c.KMSConfig) + if err != nil { + panic(err) + } + c.SocketPath = socket + } + if c.HealthPort == 0 { + c.HealthPort = defaultHealthPort + } + if c.CPURequest == "" { + c.CPURequest = "10m" + } + if c.MemoryRequest == "" { + c.MemoryRequest = "50Mi" + } +} + +// buildPluginContainer creates a corev1.Container spec for the KMS plugin sidecar +// based on the KMS configuration from openshift/api and container-specific config +func buildPluginContainer(kmsConfig *configv1.KMSConfig, containerConfig *ContainerConfig) (*corev1.Container, error) { + if kmsConfig == nil { + return nil, fmt.Errorf("kmsConfig cannot be nil") + } + if containerConfig == nil { + return nil, fmt.Errorf("containerConfig cannot be nil") + } + + // Validate inputs + if err := containerConfig.Validate(); err != nil { + return nil, fmt.Errorf("invalid container config: %w", err) + } + + // Set defaults + containerConfig.setDefaults() + + // Currently only AWS is supported + if kmsConfig.Type != configv1.AWSKMSProvider { + return nil, fmt.Errorf("unsupported KMS provider type: %s (only %s is supported)", kmsConfig.Type, configv1.AWSKMSProvider) + } + if kmsConfig.AWS == nil { + return nil, fmt.Errorf("AWS KMS config is required when type is AWS") + } + + container := &corev1.Container{ + Name: KMSContainerName, + Image: containerConfig.Image, + Command: []string{ + "/aws-encryption-provider", + }, + Args: []string{ + fmt.Sprintf("--key=%s", kmsConfig.AWS.KeyARN), + fmt.Sprintf("--region=%s", kmsConfig.AWS.Region), + fmt.Sprintf("--listen=%s", containerConfig.SocketPath), + }, + Ports: []corev1.ContainerPort{ + { + Name: "healthz", + ContainerPort: containerConfig.HealthPort, + Protocol: corev1.ProtocolTCP, + }, + }, + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.To(false), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + }, + ReadOnlyRootFilesystem: ptr.To(true), + RunAsUser: ptr.To(int64(0)), // Required for AWS SDK credential chain + }, + VolumeMounts: []corev1.VolumeMount{ + { + Name: KMSSocketVolumeName, + MountPath: "/var/run/kms", + }, + }, + LivenessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(containerConfig.HealthPort), + }, + }, + InitialDelaySeconds: 10, + PeriodSeconds: 10, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, + ReadinessProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz", + Port: intstr.FromInt32(containerConfig.HealthPort), + }, + }, + InitialDelaySeconds: 5, + PeriodSeconds: 5, + TimeoutSeconds: 3, + FailureThreshold: 3, + }, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse(containerConfig.CPURequest), + corev1.ResourceMemory: resource.MustParse(containerConfig.MemoryRequest), + }, + }, + TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError, + } + + // Add credentials mount if not using hostNetwork + if !containerConfig.UseHostNetwork { + container.Env = append(container.Env, corev1.EnvVar{ + Name: "AWS_SHARED_CREDENTIALS_FILE", + Value: "/var/run/secrets/aws/credentials", + }) + container.VolumeMounts = append(container.VolumeMounts, corev1.VolumeMount{ + Name: KMSCredentialsVolumeName, + MountPath: "/var/run/secrets/aws", + ReadOnly: true, + }) + } + + return container, nil +} + +// buildPluginVolumes creates the required volumes for the KMS plugin +func buildPluginVolumes(useHostNetwork bool, credentialsSecretName string, hostPath bool) []corev1.Volume { + volumes := []corev1.Volume{ + { + Name: KMSSocketVolumeName, + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + } + + // For static pods using hostPath, override the socket volume + if hostPath { + volumes[0].VolumeSource = corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/var/run/kms", + Type: ptr.To(corev1.HostPathDirectoryOrCreate), + }, + } + } + + // Add credentials volume if not using hostNetwork + if !useHostNetwork && credentialsSecretName != "" { + volumes = append(volumes, corev1.Volume{ + Name: KMSCredentialsVolumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: credentialsSecretName, + Items: []corev1.KeyToPath{ + { + Key: "credentials", + Path: "credentials", + }, + }, + }, + }, + }) + } + + return volumes +} + +// AddKMSPluginToPodSpec injects the KMS plugin container and volumes into a PodSpec +// This is a convenience function that combines buildPluginContainer() and buildPluginVolumes() +func AddKMSPluginToPodSpec( + podSpec *corev1.PodSpec, + kmsConfig *configv1.KMSConfig, + containerConfig *ContainerConfig, + useHostPathForSocket bool, +) error { + if podSpec == nil { + return fmt.Errorf("podSpec cannot be nil") + } + + // Create the KMS plugin container + kmsContainer, err := buildPluginContainer(kmsConfig, containerConfig) + if err != nil { + return fmt.Errorf("failed to create KMS plugin container: %w", err) + } + + // Add the container to the pod spec + podSpec.Containers = append(podSpec.Containers, *kmsContainer) + + // Add required volumes + volumes := buildPluginVolumes(containerConfig.UseHostNetwork, containerConfig.CredentialsSecretName, useHostPathForSocket) + podSpec.Volumes = append(podSpec.Volumes, volumes...) + + // Mount the KMS socket in the API server container + // Find the main API server container and add the socket mount + for i := range podSpec.Containers { + // Look for common API server container names + if podSpec.Containers[i].Name == "kube-apiserver" || + podSpec.Containers[i].Name == "openshift-apiserver" || + podSpec.Containers[i].Name == "oauth-apiserver" { + podSpec.Containers[i].VolumeMounts = append(podSpec.Containers[i].VolumeMounts, corev1.VolumeMount{ + Name: KMSSocketVolumeName, + MountPath: "/var/run/kms", + ReadOnly: true, + }) + break + } + } + + return nil +} From 31496363b7d75658848b3822daef2174b4e63c8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Tue, 4 Nov 2025 12:35:49 +0300 Subject: [PATCH 09/10] Link the original PR --- pkg/operator/encryption/kms/plugin.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/operator/encryption/kms/plugin.go b/pkg/operator/encryption/kms/plugin.go index cbb1b5c20d..ce777cf0af 100644 --- a/pkg/operator/encryption/kms/plugin.go +++ b/pkg/operator/encryption/kms/plugin.go @@ -11,6 +11,8 @@ import ( "k8s.io/utils/ptr" ) +// from https://github.com/flavianmissi/library-go/tree/kms-plugin-sidecars + const ( // defaultKMSPluginImage is the default AWS KMS plugin image // This should be overridden by the operator with the actual image reference From 519a2c27c78dd908e67bfd7b15a159fa722d34ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Wed, 5 Nov 2025 09:19:10 +0300 Subject: [PATCH 10/10] Remove incorrect unix socket prefix --- pkg/operator/encryption/kms/plugin.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/operator/encryption/kms/plugin.go b/pkg/operator/encryption/kms/plugin.go index ce777cf0af..8b85a0cc7f 100644 --- a/pkg/operator/encryption/kms/plugin.go +++ b/pkg/operator/encryption/kms/plugin.go @@ -2,6 +2,7 @@ package kms import ( "fmt" + "strings" configv1 "github.com/openshift/api/config/v1" @@ -87,7 +88,7 @@ func (c *ContainerConfig) setDefaults() { if err != nil { panic(err) } - c.SocketPath = socket + c.SocketPath = strings.Replace(socket, "unix://", "", 1) } if c.HealthPort == 0 { c.HealthPort = defaultHealthPort