diff --git a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go index fd0c95c22ac..6b7857e3540 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go +++ b/staging/src/k8s.io/apiserver/pkg/server/options/encryptionconfig/config.go @@ -17,6 +17,7 @@ limitations under the License. package encryptionconfig import ( + "context" "crypto/aes" "crypto/cipher" "encoding/base64" @@ -32,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" + utilerrors "k8s.io/apimachinery/pkg/util/errors" apiserverconfig "k8s.io/apiserver/pkg/apis/config" apiserverconfigv1 "k8s.io/apiserver/pkg/apis/config/v1" "k8s.io/apiserver/pkg/apis/config/validation" @@ -365,7 +367,13 @@ func secretboxPrefixTransformer(config *apiserverconfig.SecretboxConfiguration) } func envelopePrefixTransformer(config *apiserverconfig.KMSConfiguration, envelopeService envelope.Service, prefix string) (value.PrefixTransformer, error) { - envelopeTransformer, err := envelope.NewEnvelopeTransformer(envelopeService, int(*config.CacheSize), aestransformer.NewCBCTransformer) + baseTransformerFunc := func(block cipher.Block) value.Transformer { + // v1.24: write using AES-CBC only but support reads via AES-CBC and AES-GCM (so we can move to AES-GCM) + // TODO(aramase): swap this ordering in v1.25 + return unionTransformers{aestransformer.NewCBCTransformer(block), aestransformer.NewGCMTransformer(block)} + } + + envelopeTransformer, err := envelope.NewEnvelopeTransformer(envelopeService, int(*config.CacheSize), baseTransformerFunc) if err != nil { return value.PrefixTransformer{}, err } @@ -374,3 +382,27 @@ func envelopePrefixTransformer(config *apiserverconfig.KMSConfiguration, envelop Prefix: []byte(prefix + config.Name + ":"), }, nil } + +type unionTransformers []value.Transformer + +func (u unionTransformers) TransformFromStorage(ctx context.Context, data []byte, dataCtx value.Context) (out []byte, stale bool, err error) { + var errs []error + for i, transformer := range u { + result, stale, err := transformer.TransformFromStorage(ctx, data, dataCtx) + if err != nil { + errs = append(errs, err) + continue + } + // when i != 0, we have transformed the data from storage using the new transformer, + // we want to issue a write to etcd even if the contents of the data haven't changed + return result, stale || i != 0, nil + } + if err := utilerrors.Reduce(utilerrors.NewAggregate(errs)); err != nil { + return nil, false, err + } + return nil, false, fmt.Errorf("unionTransformers: unable to transform from storage") +} + +func (u unionTransformers) TransformToStorage(ctx context.Context, data []byte, dataCtx value.Context) (out []byte, err error) { + return u[0].TransformToStorage(ctx, data, dataCtx) +} diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes/aes.go b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes/aes.go index 1e7047f8f62..69930c03908 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes/aes.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/encrypt/aes/aes.go @@ -96,7 +96,7 @@ func NewCBCTransformer(block cipher.Block) value.Transformer { } var ( - errInvalidBlockSize = fmt.Errorf("the stored data is not a multiple of the block size") + ErrInvalidBlockSize = fmt.Errorf("the stored data is not a multiple of the block size") errInvalidPKCS7Data = errors.New("invalid PKCS7 data (empty or not padded)") errInvalidPKCS7Padding = errors.New("invalid padding on input") ) @@ -110,7 +110,7 @@ func (t *cbc) TransformFromStorage(ctx context.Context, data []byte, dataCtx val data = data[blockSize:] if len(data)%blockSize != 0 { - return nil, false, errInvalidBlockSize + return nil, false, ErrInvalidBlockSize } result := make([]byte, len(data)) diff --git a/test/integration/controlplane/transformation/kms_transformation_test.go b/test/integration/controlplane/transformation/kms_transformation_test.go index 08f0f0a3da8..c86628f1ff6 100644 --- a/test/integration/controlplane/transformation/kms_transformation_test.go +++ b/test/integration/controlplane/transformation/kms_transformation_test.go @@ -23,13 +23,16 @@ import ( "bytes" "context" "crypto/aes" + "encoding/base64" "encoding/binary" + "errors" "fmt" "net/http" "strings" "testing" "time" + "golang.org/x/crypto/cryptobyte" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/storage/value" @@ -87,7 +90,7 @@ func (r envelope) plainTextPayload(secretETCDPath string) ([]byte, error) { aescbcTransformer := aestransformer.NewCBCTransformer(block) plainSecret, _, err := aescbcTransformer.TransformFromStorage(ctx, r.cipherTextPayload(), dataCtx) if err != nil { - return nil, fmt.Errorf("failed to transform from storage via AESCBC, err: %v", err) + return nil, fmt.Errorf("failed to transform from storage via AESCBC, err: %w", err) } return plainSecret, nil @@ -100,6 +103,10 @@ func (r envelope) plainTextPayload(secretETCDPath string) ([]byte, error) { // 3. KMS gRPC Plugin should encrypt the DEK with a Key Encryption Key (KEK) and pass it back to envelopeTransformer // 4. The cipherTextPayload (ex. Secret) should be encrypted via AES CBC transform // 5. Prefix-EncryptedDEK-EncryptedPayload structure should be deposited to ETCD +// 6. Direct AES CBC decryption of the cipherTextPayload written with AES GCM transform does not work +// 7. AES GCM secrets should be un-enveloped on direct reads from Kube API Server +// 8. No-op updates to the secret should cause new AES CBC key to be used +// 9. Direct AES CBC decryption works after the new AES CBC key is used func TestKMSProvider(t *testing.T) { encryptionConfig := ` kind: EncryptionConfiguration @@ -145,7 +152,7 @@ resources: if err != nil { t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err) } - envelope := envelope{ + envelopeData := envelope{ providerName: providerName, rawEnvelope: rawEnvelope, plainTextDEK: plainTextDEK, @@ -156,7 +163,9 @@ resources: t.Fatalf("expected secret to be prefixed with %s, but got %s", wantPrefix, rawEnvelope) } - decryptResponse, err := pluginMock.Decrypt(context.Background(), &kmsapi.DecryptRequest{Version: kmsAPIVersion, Cipher: envelope.cipherTextDEK()}) + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + decryptResponse, err := pluginMock.Decrypt(ctx, &kmsapi.DecryptRequest{Version: kmsAPIVersion, Cipher: envelopeData.cipherTextDEK()}) if err != nil { t.Fatalf("failed to decrypt DEK, %v", err) } @@ -167,7 +176,7 @@ resources: plainTextDEK, dekPlainAsWouldBeSeenByETCD) } - plainSecret, err := envelope.plainTextPayload(secretETCDPath) + plainSecret, err := envelopeData.plainTextPayload(secretETCDPath) if err != nil { t.Fatalf("failed to transform from storage via AESCBC, err: %v", err) } @@ -176,14 +185,101 @@ resources: t.Fatalf("expected %q after decryption, but got %q", secretVal, string(plainSecret)) } + secretClient := test.restClient.CoreV1().Secrets(testNamespace) // Secrets should be un-enveloped on direct reads from Kube API Server. - s, err := test.restClient.CoreV1().Secrets(testNamespace).Get(context.TODO(), testSecret, metav1.GetOptions{}) + s, err := secretClient.Get(ctx, testSecret, metav1.GetOptions{}) if err != nil { t.Fatalf("failed to get Secret from %s, err: %v", testNamespace, err) } if secretVal != string(s.Data[secretKey]) { t.Fatalf("expected %s from KubeAPI, but got %s", secretVal, string(s.Data[secretKey])) } + + // write data using AES GCM to simulate a downgrade + futureSecretBytes, err := base64.StdEncoding.DecodeString(futureSecret) + if err != nil { + t.Fatalf("failed to base64 decode future secret, err: %v", err) + } + futureKeyBytes, err := base64.StdEncoding.DecodeString(futureAESGCMKey) + if err != nil { + t.Fatalf("failed to base64 decode future key, err: %v", err) + } + block, err := aes.NewCipher(futureKeyBytes) + if err != nil { + t.Fatalf("invalid key, err: %v", err) + } + + // we cannot precompute this because the authenticated data changes per run + futureEncryptedSecretBytes, err := aestransformer.NewGCMTransformer(block).TransformToStorage(ctx, futureSecretBytes, value.DefaultContext(secretETCDPath)) + if err != nil { + t.Fatalf("failed to encrypt future secret, err: %v", err) + } + + futureEncryptedSecretBuf := cryptobyte.NewBuilder(nil) + futureEncryptedSecretBuf.AddBytes([]byte(wantPrefix)) + futureEncryptedSecretBuf.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) { + b.AddBytes([]byte(futureAESGCMKey)) + }) + futureEncryptedSecretBuf.AddBytes(futureEncryptedSecretBytes) + + _, err = test.writeRawRecordToETCD(secretETCDPath, futureEncryptedSecretBuf.BytesOrPanic()) + if err != nil { + t.Fatalf("failed to write future encrypted secret, err: %v", err) + } + + // confirm that direct AES CBC decryption does not work + failingRawEnvelope, err := test.getRawSecretFromETCD() + if err != nil { + t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err) + } + failingFutureEnvelope := envelope{ + providerName: providerName, + rawEnvelope: failingRawEnvelope, + plainTextDEK: futureKeyBytes, + } + failingFuturePlainSecret, err := failingFutureEnvelope.plainTextPayload(secretETCDPath) + if err == nil || !errors.Is(err, aestransformer.ErrInvalidBlockSize) { + t.Fatalf("AESCBC decryption failure not seen, err: %v, data: %s", err, string(failingFuturePlainSecret)) + } + + // AES GCM secrets should be un-enveloped on direct reads from Kube API Server. + futureSecretObj, err := secretClient.Get(ctx, testSecret, metav1.GetOptions{}) + if err != nil { + t.Fatalf("failed to read future secret via Kube API, err: %v", err) + } + if futureSecretVal != string(futureSecretObj.Data[secretKey]) { + t.Fatalf("expected %s from KubeAPI, but got %s", futureSecretVal, string(futureSecretObj.Data[secretKey])) + } + + // no-op update should cause new AES CBC key to be used + futureSecretUpdated, err := secretClient.Update(ctx, futureSecretObj, metav1.UpdateOptions{}) + if err != nil { + t.Fatalf("failed to update future secret via Kube API, err: %v", err) + } + if futureSecretObj.ResourceVersion == futureSecretUpdated.ResourceVersion { + t.Fatalf("future secret not updated on no-op write: %s", futureSecretObj.ResourceVersion) + } + + // confirm that direct AES CBC decryption works + futureRawEnvelope, err := test.getRawSecretFromETCD() + if err != nil { + t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err) + } + futureEnvelope := envelope{ + providerName: providerName, + rawEnvelope: futureRawEnvelope, + plainTextDEK: pluginMock.LastEncryptRequest(), + } + if !bytes.HasPrefix(futureRawEnvelope, []byte(wantPrefix)) { + t.Fatalf("expected secret to be prefixed with %s, but got %s", wantPrefix, futureRawEnvelope) + } + futurePlainSecret, err := futureEnvelope.plainTextPayload(secretETCDPath) + if err != nil { + t.Fatalf("failed to transform from storage via AESCBC, err: %v", err) + } + if !strings.Contains(string(futurePlainSecret), futureSecretVal) { + t.Fatalf("expected %q after decryption, but got %q", futureSecretVal, string(futurePlainSecret)) + } } func TestKMSHealthz(t *testing.T) { diff --git a/test/integration/controlplane/transformation/transformation_testcase.go b/test/integration/controlplane/transformation/transformation_testcase.go index 48af30bba43..a8028f26cf6 100644 --- a/test/integration/controlplane/transformation/transformation_testcase.go +++ b/test/integration/controlplane/transformation/transformation_testcase.go @@ -50,6 +50,12 @@ const ( testNamespace = "secret-encryption-test" testSecret = "test-secret" metricsPrefix = "apiserver_storage_" + + // precomputed key and secret for use with AES GCM + // this looks exactly the same as the AES CBC secret but with a different value + futureAESGCMKey = "e0/+tts8FS254BZimFZWtUsOCOUDSkvzB72PyimMlkY=" + futureSecret = "azhzAAoMCgJ2MRIGU2VjcmV0En4KXwoLdGVzdC1zZWNyZXQSABoWc2VjcmV0LWVuY3J5cHRpb24tdGVzdCIAKiQ3MmRmZTVjNC0xNDU2LTQyMzktYjFlZC1hZGZmYTJmMWY3YmEyADgAQggI5Jy/7wUQAHoAEhMKB2FwaV9rZXkSCPCfpJfwn5C8GgZPcGFxdWUaACIA" + futureSecretVal = "\xf0\x9f\xa4\x97\xf0\x9f\x90\xbc" ) type unSealSecret func(ctx context.Context, cipherText []byte, dataCtx value.Context, config apiserverconfigv1.ProviderConfiguration) ([]byte, error) @@ -242,6 +248,19 @@ func (e *transformTest) readRawRecordFromETCD(path string) (*clientv3.GetRespons return response, nil } +func (e *transformTest) writeRawRecordToETCD(path string, data []byte) (*clientv3.PutResponse, error) { + _, etcdClient, err := integration.GetEtcdClients(e.kubeAPIServer.ServerOpts.Etcd.StorageConfig.Transport) + if err != nil { + return nil, fmt.Errorf("failed to create etcd client: %v", err) + } + response, err := etcdClient.Put(context.Background(), path, string(data)) + if err != nil { + return nil, fmt.Errorf("failed to write secret to etcd %v", err) + } + + return response, nil +} + func (e *transformTest) printMetrics() error { e.logger.Logf("Transformation Metrics:") metrics, err := legacyregistry.DefaultGatherer.Gather()