From 479fcf0b13f551517801b8677272c73f3f845565 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 22 Mar 2023 21:27:47 -0400 Subject: [PATCH 1/2] kmsv2: validate encrypt response at DEK generation time Prior to this change, we wait until the DEK is used to perform an encryption before validating the response. This means that the plugin could report healthy but all TransformToStorage calls would fail. Now we correctly cause the plugin to become unhealthy and do not attempt to use the newly generated DEK. Signed-off-by: Monis Khan --- .../options/encryptionconfig/config_test.go | 51 ++++++++++++++++--- .../value/encrypt/envelope/kmsv2/envelope.go | 9 ++++ 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go index 09d9a93e748..003441da329 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config_test.go @@ -81,9 +81,10 @@ func (t *testEnvelopeService) Encrypt(data []byte) ([]byte, error) { // testKMSv2EnvelopeService is a mock kmsv2 envelope service which can be used to simulate remote Envelope v2 services // for testing of the envelope transformer with other transformers. type testKMSv2EnvelopeService struct { - err error - keyID string - encryptCalls int + err error + keyID string + encryptCalls int + encryptAnnotations map[string][]byte } func (t *testKMSv2EnvelopeService) Decrypt(ctx context.Context, uid string, req *kmsservice.DecryptRequest) ([]byte, error) { @@ -99,8 +100,9 @@ func (t *testKMSv2EnvelopeService) Encrypt(ctx context.Context, uid string, data return nil, t.err } return &kmsservice.EncryptResponse{ - Ciphertext: []byte(base64.StdEncoding.EncodeToString(data)), - KeyID: t.keyID, + Ciphertext: []byte(base64.StdEncoding.EncodeToString(data)), + KeyID: t.keyID, + Annotations: t.encryptAnnotations, }, nil } @@ -123,17 +125,17 @@ func newMockErrorEnvelopeService(endpoint string, timeout time.Duration) (envelo // The factory method to create mock envelope kmsv2 service. func newMockEnvelopeKMSv2Service(ctx context.Context, endpoint, providerName string, timeout time.Duration) (kmsservice.Service, error) { - return &testKMSv2EnvelopeService{nil, "1", 0}, nil + return &testKMSv2EnvelopeService{nil, "1", 0, nil}, nil } // The factory method to create mock envelope kmsv2 service which always returns error. func newMockErrorEnvelopeKMSv2Service(endpoint string, timeout time.Duration) (kmsservice.Service, error) { - return &testKMSv2EnvelopeService{errors.New("test"), "1", 0}, nil + return &testKMSv2EnvelopeService{errors.New("test"), "1", 0, nil}, nil } // The factory method to create mock envelope kmsv2 service that always returns invalid keyID. func newMockInvalidKeyIDEnvelopeKMSv2Service(ctx context.Context, endpoint string, timeout time.Duration, keyID string) (kmsservice.Service, error) { - return &testKMSv2EnvelopeService{nil, keyID, 0}, nil + return &testKMSv2EnvelopeService{nil, keyID, 0, nil}, nil } func TestLegacyConfig(t *testing.T) { @@ -1758,6 +1760,39 @@ func Test_kmsv2PluginProbe_rotateDEKOnKeyIDChange(t *testing.T) { `errState=, errGen=failed to encrypt DEK, error: broken, statusKeyID="5", ` + `encryptKeyID="", stateKeyID="4", expirationTimestamp=` + now.Add(7*time.Minute).Format(time.RFC3339), }, + { + name: "invalid service response, no previous state", + service: &testKMSv2EnvelopeService{keyID: "1", encryptAnnotations: map[string][]byte{"panda": nil}}, + state: envelopekmsv2.State{}, + statusKeyID: "1", + wantState: envelopekmsv2.State{}, + wantEncryptCalls: 1, + wantLogs: []string{ + `"encrypting content using envelope service" uid="panda"`, + }, + wantErr: `failed to rotate DEK uid="panda", ` + + `errState=got unexpected nil transformer, errGen=failed to validate annotations: annotations: Invalid value: "panda": ` + + `should be a domain with at least two segments separated by dots, statusKeyID="1", ` + + `encryptKeyID="", stateKeyID="", expirationTimestamp=` + (time.Time{}).Format(time.RFC3339), + }, + { + name: "invalid service response, with previous state", + service: &testKMSv2EnvelopeService{keyID: "3", encryptAnnotations: map[string][]byte{"panda": nil}}, + state: validState("2", now), + statusKeyID: "3", + wantState: envelopekmsv2.State{ + KeyID: "2", + ExpirationTimestamp: now, + }, + wantEncryptCalls: 1, + wantLogs: []string{ + `"encrypting content using envelope service" uid="panda"`, + }, + wantErr: `failed to rotate DEK uid="panda", ` + + `errState=, errGen=failed to validate annotations: annotations: Invalid value: "panda": ` + + `should be a domain with at least two segments separated by dots, statusKeyID="3", ` + + `encryptKeyID="", stateKeyID="2", expirationTimestamp=` + now.Format(time.RFC3339), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go index 8d73647a852..43ba22d65e0 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go @@ -278,6 +278,15 @@ func GenerateTransformer(ctx context.Context, uid string, envelopeService kmsser return nil, nil, nil, fmt.Errorf("failed to encrypt DEK, error: %w", err) } + if err := validateEncryptedObject(&kmstypes.EncryptedObject{ + KeyID: resp.KeyID, + EncryptedDEK: resp.Ciphertext, + EncryptedData: []byte{0}, // any non-empty value to pass validation + Annotations: resp.Annotations, + }); err != nil { + return nil, nil, nil, err + } + cacheKey, err := generateCacheKey(resp.Ciphertext, resp.KeyID, resp.Annotations) if err != nil { return nil, nil, nil, err From f2fe1fff655968f2b5959b387683637c5a1f8e66 Mon Sep 17 00:00:00 2001 From: Anish Ramasekar Date: Thu, 23 Mar 2023 16:40:16 +0000 Subject: [PATCH 2/2] [KMSv2] add tests for generate transformer Signed-off-by: Anish Ramasekar --- .../value/encrypt/envelope/kmsv2/envelope.go | 2 + .../encrypt/envelope/kmsv2/envelope_test.go | 80 +++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go index 43ba22d65e0..e372e62de7c 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope.go @@ -265,6 +265,8 @@ func (t *envelopeTransformer) doDecode(originalData []byte) (*kmstypes.Encrypted return o, nil } +// GenerateTransformer generates a new transformer and encrypts the DEK using the envelope service. +// It returns the transformer, the encrypted DEK, cache key and error. func GenerateTransformer(ctx context.Context, uid string, envelopeService kmsservice.Service) (value.Transformer, *kmsservice.EncryptResponse, []byte, error) { transformer, newKey, err := aestransformer.NewGCMTransformerWithUniqueKeyUnsafe() if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope_test.go b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope_test.go index 0b16f8c133a..573cee6b58b 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/envelope/kmsv2/envelope_test.go @@ -990,6 +990,86 @@ func TestGenerateCacheKey(t *testing.T) { } } +func TestGenerateTransformer(t *testing.T) { + t.Parallel() + testCases := []struct { + name string + envelopeService func() kmsservice.Service + expectedErr string + }{ + { + name: "encrypt call fails", + envelopeService: func() kmsservice.Service { + envelopeService := newTestEnvelopeService() + envelopeService.SetDisabledStatus(true) + return envelopeService + }, + expectedErr: "Envelope service was disabled", + }, + { + name: "invalid key ID", + envelopeService: func() kmsservice.Service { + envelopeService := newTestEnvelopeService() + envelopeService.keyVersion = "" + return envelopeService + }, + expectedErr: "failed to validate key id: keyID is empty", + }, + { + name: "invalid encrypted DEK", + envelopeService: func() kmsservice.Service { + envelopeService := newTestEnvelopeService() + envelopeService.SetCiphertext([]byte{}) + return envelopeService + }, + expectedErr: "failed to validate encrypted DEK: encrypted DEK is empty", + }, + { + name: "invalid annotations", + envelopeService: func() kmsservice.Service { + envelopeService := newTestEnvelopeService() + envelopeService.SetAnnotations(map[string][]byte{"invalid": {}}) + return envelopeService + }, + expectedErr: "failed to validate annotations: annotations: Invalid value: \"invalid\": should be a domain with at least two segments separated by dots", + }, + { + name: "success", + envelopeService: func() kmsservice.Service { + return newTestEnvelopeService() + }, + expectedErr: "", + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + transformer, encryptResp, cacheKey, err := GenerateTransformer(testContext(t), "panda", tc.envelopeService()) + if tc.expectedErr == "" { + if err != nil { + t.Errorf("expected no error, got %q", errString(err)) + } + if transformer == nil { + t.Error("expected transformer, got nil") + } + if encryptResp == nil { + t.Error("expected encrypt response, got nil") + } + if cacheKey == nil { + t.Error("expected cache key, got nil") + } + } else { + if err == nil || !strings.Contains(err.Error(), tc.expectedErr) { + t.Errorf("expected error %q, got %q", tc.expectedErr, errString(err)) + } + } + }) + } +} + func errString(err error) string { if err == nil { return ""