Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion pkg/operator/encryption/controllers/key_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"github.com/openshift/library-go/pkg/controller/factory"
"github.com/openshift/library-go/pkg/operator/encryption/crypto"
"github.com/openshift/library-go/pkg/operator/encryption/kms"
"github.com/openshift/library-go/pkg/operator/encryption/secrets"
"github.com/openshift/library-go/pkg/operator/encryption/state"
"github.com/openshift/library-go/pkg/operator/encryption/statemachine"
Expand Down Expand Up @@ -266,6 +267,14 @@ func (c *keyController) generateKeySecret(keyID uint64, currentMode state.Mode,
InternalReason: internalReason,
ExternalReason: externalReason,
}
if currentMode == state.KMS {
ks.KMSConfiguration = &apiserverv1.KMSConfiguration{
APIVersion: "v2",
Name: fmt.Sprintf("%d", keyID), // this will be updated by inserting resource name
Endpoint: kms.DefaultEndpoint,
Timeout: &metav1.Duration{Duration: kms.DefaultTimeout},
}
}
return secrets.FromKeyState(c.instanceName, ks)
}

Expand All @@ -287,7 +296,7 @@ func (c *keyController) getCurrentModeAndExternalReason(ctx context.Context) (st

reason := encryptionConfig.Encryption.Reason
switch currentMode := state.Mode(apiServer.Spec.Encryption.Type); currentMode {
case state.AESCBC, state.AESGCM, state.Identity: // secretbox is disabled for now
case state.AESCBC, state.AESGCM, state.KMS, state.Identity: // secretbox is disabled for now
return currentMode, reason, nil
case "": // unspecified means use the default (which can change over time)
return state.DefaultMode, reason, nil
Expand Down Expand Up @@ -341,6 +350,19 @@ func needsNewKey(grKeys state.GroupResourceState, currentMode state.Mode, extern
return 0, "", false
}

if currentMode == state.KMS {
// We are here because Encryption Mode is not changed

// For now in v1, we don't support configurational changes. Therefore,
// it is pointless comparing the secrets.

// For KMS mode, we don't do time-based rotation. Therefore, we shortcut here
// KMS keys are rotated externally by the KMS system.
// Moreover, we don't trigger new key when external reason is changed.
// Because it would lead to duplicate providers which is not allowed.
Comment on lines +361 to +362
Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC, even if we did roll out a new encryption config in response to the "reason" update, we're not even capable of triggering a KEK rotation unless a specific provider happened to expose that capability independently.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. That is correct.

return 0, "", false
}

// if the most recent secret has a different external reason than the current reason, we need to generate a new key
if latestKey.ExternalReason != externalReason && len(externalReason) != 0 {
return latestKeyID, "external-reason-changed", true
Expand Down
250 changes: 250 additions & 0 deletions pkg/operator/encryption/controllers/key_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ func TestKeyController(t *testing.T) {
apiServerWithAESGCM := simpleAPIServer.DeepCopy()
apiServerWithAESGCM.Spec.Encryption = configv1.APIServerEncryption{Type: "aesgcm"}

apiServerWithKMS := simpleAPIServer.DeepCopy()
apiServerWithKMS.Spec.Encryption = configv1.APIServerEncryption{Type: "KMS"}

scenarios := []struct {
name string
initialObjects []runtime.Object
Expand Down Expand Up @@ -324,6 +327,246 @@ func TestKeyController(t *testing.T) {
}
},
},

{
name: "checks if a KMS secret is created when KMS encryption is enabled",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
targetNamespace: "kms",
expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
},
apiServerObjects: []runtime.Object{apiServerWithKMS},
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 mode annotation is KMS
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"])
}

// Verify KMS config annotation exists
kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]
if kmsConfig == "" {
ts.Error("expected kms-config annotation to be present")
}
if kmsConfig != `{"apiVersion":"v2","name":"1","endpoint":"unix:///var/run/kmsplugin/kms.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-config: %s", kmsConfig)
}

// Verify internal reason
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] != "secrets-key-does-not-exist" {
ts.Errorf("unexpected internal reason: %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"])
}

wasSecretValidated = true
break
}
}
if !wasSecretValidated {
ts.Errorf("the secret wasn't created and validated")
}
},
},

{
name: "no-op when a valid KMS write key exists",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
encryptiontesting.CreateEncryptionKeySecretWithKMSConfig("kms", nil, 1),
},
apiServerObjects: []runtime.Object{apiServerWithKMS},
targetNamespace: "kms",
expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"},
},

{
name: "creates a new KMS key when switching from AESCBC to KMS",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("61def964fb967f5d7c44a2af8dab6865"), "aescbc"),
},
apiServerObjects: []runtime.Object{apiServerWithKMS},
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 mode changed to KMS
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"])
}

// Verify KMS config annotation exists
kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]
if kmsConfig != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-config: %s", kmsConfig)
}

// Verify internal reason is mode changed
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] != "secrets-encryption-mode-changed" {
ts.Errorf("unexpected internal reason: %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"])
}

wasSecretValidated = true
break
}
}
if !wasSecretValidated {
ts.Errorf("the secret wasn't created and validated")
}
},
},

{
name: "no-op when KMS key is migrated but not expired (no time-based rotation for KMS)",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
encryptiontesting.CreateExpiredMigratedEncryptionKeySecretWithKMSConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5),
encryptiontesting.CreateEncryptionKeySecretWithKMSConfig("kms", nil, 6),
},
apiServerObjects: []runtime.Object{apiServerWithKMS},
targetNamespace: "kms",
// Should be no-op because KMS keys don't have time-based rotation
expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"},
},
{
name: "no-op when latest KMS key is not migrated yet",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
encryptiontesting.CreateEncryptionKeySecretWithKMSConfig("kms", nil, 3),
},
apiServerObjects: []runtime.Object{apiServerWithKMS},
targetNamespace: "kms",
// Should be no-op because migration hasn't completed yet
expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed"},
},

{
name: "creates a new KMS key when switching from Identity to KMS",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
encryptiontesting.CreateEncryptionKeySecretWithRawKeyWithMode("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 5, []byte("identity-key"), "identity"),
},
apiServerObjects: []runtime.Object{apiServerWithKMS},
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 mode changed to KMS
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"])
}

// Verify KMS config annotation exists
kmsConfig := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]
if kmsConfig != `{"apiVersion":"v2","name":"6","endpoint":"unix:///var/run/kmsplugin/kms.sock","timeout":"10s"}` {
ts.Errorf("unexpected kms-config: %s", kmsConfig)
}

// Verify internal reason is mode changed
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] != "secrets-encryption-mode-changed" {
ts.Errorf("unexpected internal reason: %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"])
}

// Verify key ID incremented
if actualSecret.Name != "encryption-key-kms-6" {
ts.Errorf("expected key ID 6, got %s", actualSecret.Name)
}

wasSecretValidated = true
break
}
}
if !wasSecretValidated {
ts.Errorf("the secret wasn't created and validated")
}
},
},

{
name: "creates a new AESCBC key when switching from KMS to AESCBC",
targetGRs: []schema.GroupResource{
{Group: "", Resource: "secrets"},
},
initialObjects: []runtime.Object{
encryptiontesting.CreateDummyKubeAPIPod("kube-apiserver-1", "kms", "node-1"),
encryptiontesting.CreateEncryptionKeySecretWithKMSConfig("kms", []schema.GroupResource{{Group: "", Resource: "secrets"}}, 7),
},
apiServerObjects: []runtime.Object{apiServerWithAESCBC},
targetNamespace: "kms",
expectedActions: []string{"list:pods:kms", "get:secrets:kms", "list:secrets:openshift-config-managed", "create:secrets:openshift-config-managed", "create:events:kms"},
validateFunc: func(ts *testing.T, actions []clientgotesting.Action, targetNamespace string, targetGRs []schema.GroupResource) {
wasSecretValidated := false
for _, action := range actions {
if action.Matches("create", "secrets") {
createAction := action.(clientgotesting.CreateAction)
actualSecret := createAction.GetObject().(*corev1.Secret)

// Verify mode changed to aescbc
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"] != "aescbc" {
ts.Errorf("expected mode to be aescbc, got %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/mode"])
}

// Verify KMS config annotation is removed (not present for AESCBC)
if kmsConfig, exists := actualSecret.Annotations["encryption.apiserver.operator.openshift.io/kms-config"]; exists {
ts.Errorf("expected kms-config annotation to be absent, got: %s", kmsConfig)
}

// Verify internal reason is mode changed
if actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"] != "secrets-encryption-mode-changed" {
ts.Errorf("unexpected internal reason: %s", actualSecret.Annotations["encryption.apiserver.operator.openshift.io/internal-reason"])
}

// Verify key ID incremented to 8
if actualSecret.Name != "encryption-key-kms-8" {
ts.Errorf("expected key ID 8, got %s", actualSecret.Name)
}

// Verify it's a valid 32-byte AES key
if err := encryptiontesting.ValidateEncryptionKey(actualSecret); err != nil {
ts.Error(err)
}

wasSecretValidated = true
break
}
}
if !wasSecretValidated {
ts.Errorf("the secret wasn't created and validated")
}
},
},
}

for _, scenario := range scenarios {
Expand Down Expand Up @@ -444,6 +687,7 @@ func TestGetCurrentModeAndExternalReason(t *testing.T) {
prefix []string
apiServerObjects []runtime.Object
expectedReasonFromCfg string
expectedKMSConfig []byte
}{
{
name: "no prefix provided, flat observed config",
Expand Down Expand Up @@ -478,6 +722,12 @@ func TestGetCurrentModeAndExternalReason(t *testing.T) {
name: "reading empty config works",
apiServerObjects: []runtime.Object{&configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}}},
},

{
name: "kms encryption mode",
apiServerObjects: []runtime.Object{&configv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, Spec: configv1.APIServerSpec{Encryption: configv1.APIServerEncryption{Type: "KMS"}}}},
expectedKMSConfig: []byte("unix:///var/run/kmsplugin/kms.sock"),
},
}

for _, scenario := range scenarios {
Expand Down
Loading