feat: prepare KMS data encryption for migration to AES-GCM

Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>
Co-authored-by: Monis Khan <mok@vmware.com>
Signed-off-by: Anish Ramasekar <anish.ramasekar@gmail.com>

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)
+}

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))

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) {

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()