Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat:(kms) encrypt data with DEK using AES-GCM instead of AES-CBC #111119

Merged
merged 1 commit into from
Aug 2, 2022
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,10 @@ func secretboxPrefixTransformer(config *apiserverconfig.SecretboxConfiguration)
func envelopePrefixTransformer(config *apiserverconfig.KMSConfiguration, envelopeService envelope.Service, prefix string) (value.PrefixTransformer, error) {
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
aramase marked this conversation as resolved.
Show resolved Hide resolved
return unionTransformers{aestransformer.NewCBCTransformer(block), aestransformer.NewGCMTransformer(block)}
// v1.25: write using AES-GCM only but support reads via AES-GCM and fallback to AES-CBC for backwards compatibility
aramase marked this conversation as resolved.
Show resolved Hide resolved
// TODO(aramase): Post v1.25: We cannot drop CBC read support until we automate storage migration.
// We could have a release note that hard requires users to perform storage migration.
return unionTransformers{aestransformer.NewGCMTransformer(block), aestransformer.NewCBCTransformer(block)}
}

envelopeTransformer, err := envelope.NewEnvelopeTransformer(envelopeService, int(*config.CacheSize), baseTransformerFunc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"crypto/aes"
"encoding/base64"
"encoding/binary"
"errors"
"fmt"
"net/http"
"strings"
Expand Down Expand Up @@ -87,10 +86,10 @@ func (r envelope) plainTextPayload(secretETCDPath string) ([]byte, error) {
// etcd path of the key is used as the authenticated context - need to pass it to decrypt
ctx := context.Background()
dataCtx := value.DefaultContext([]byte(secretETCDPath))
aescbcTransformer := aestransformer.NewCBCTransformer(block)
plainSecret, _, err := aescbcTransformer.TransformFromStorage(ctx, r.cipherTextPayload(), dataCtx)
aesgcmTransformer := aestransformer.NewGCMTransformer(block)
plainSecret, _, err := aesgcmTransformer.TransformFromStorage(ctx, r.cipherTextPayload(), dataCtx)
if err != nil {
return nil, fmt.Errorf("failed to transform from storage via AESCBC, err: %w", err)
return nil, fmt.Errorf("failed to transform from storage via AESGCM, err: %w", err)
}

return plainSecret, nil
Expand All @@ -103,10 +102,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
// 6. Direct AES GCM decryption of the cipherTextPayload written with AES CBC transform does not work
// 7. Existing AES CBC secrets should be un-enveloped on direct reads from Kube API Server
// 8. No-op updates to the secret should cause new AES GCM key to be used
// 9. Direct AES GCM decryption works after the new AES GCM key is used
func TestKMSProvider(t *testing.T) {
encryptionConfig := `
kind: EncryptionConfiguration
Expand Down Expand Up @@ -194,90 +193,89 @@ resources:
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)
// write data using AES CBC to simulate a downgrade
oldSecretBytes, err := base64.StdEncoding.DecodeString(oldSecret)
if err != nil {
t.Fatalf("failed to base64 decode future secret, err: %v", err)
t.Fatalf("failed to base64 decode old secret, err: %v", err)
}
futureKeyBytes, err := base64.StdEncoding.DecodeString(futureAESGCMKey)
oldKeyBytes, err := base64.StdEncoding.DecodeString(oldAESCBCKey)
if err != nil {
t.Fatalf("failed to base64 decode future key, err: %v", err)
t.Fatalf("failed to base64 decode old key, err: %v", err)
}
block, err := aes.NewCipher(futureKeyBytes)
block, err := aes.NewCipher(oldKeyBytes)
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))
oldEncryptedSecretBytes, err := aestransformer.NewCBCTransformer(block).TransformToStorage(ctx, oldSecretBytes, value.DefaultContext(secretETCDPath))
if err != nil {
t.Fatalf("failed to encrypt future secret, err: %v", err)
t.Fatalf("failed to encrypt old secret, err: %v", err)
}

futureEncryptedSecretBuf := cryptobyte.NewBuilder(nil)
futureEncryptedSecretBuf.AddBytes([]byte(wantPrefix))
futureEncryptedSecretBuf.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
b.AddBytes([]byte(futureAESGCMKey))
oldEncryptedSecretBuf := cryptobyte.NewBuilder(nil)
oldEncryptedSecretBuf.AddBytes([]byte(wantPrefix))
oldEncryptedSecretBuf.AddUint16LengthPrefixed(func(b *cryptobyte.Builder) {
b.AddBytes([]byte(oldAESCBCKey))
})
futureEncryptedSecretBuf.AddBytes(futureEncryptedSecretBytes)
oldEncryptedSecretBuf.AddBytes(oldEncryptedSecretBytes)

_, err = test.writeRawRecordToETCD(secretETCDPath, futureEncryptedSecretBuf.BytesOrPanic())
_, err = test.writeRawRecordToETCD(secretETCDPath, oldEncryptedSecretBuf.BytesOrPanic())
if err != nil {
t.Fatalf("failed to write future encrypted secret, err: %v", err)
t.Fatalf("failed to write old encrypted secret, err: %v", err)
}

// confirm that direct AES CBC decryption does not work
// confirm that direct AES GCM decryption does not work
failingRawEnvelope, err := test.getRawSecretFromETCD()
if err != nil {
t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err)
}
failingFutureEnvelope := envelope{
failingOldEnvelope := envelope{
providerName: providerName,
rawEnvelope: failingRawEnvelope,
plainTextDEK: futureKeyBytes,
plainTextDEK: oldKeyBytes,
}
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))
failingOldPlainSecret, err := failingOldEnvelope.plainTextPayload(secretETCDPath)
if err == nil {
t.Fatalf("AESGCM decryption failure not seen, data: %s", string(failingOldPlainSecret))
}

// AES GCM secrets should be un-enveloped on direct reads from Kube API Server.
futureSecretObj, err := secretClient.Get(ctx, testSecret, metav1.GetOptions{})
// Existing AES CBC secrets should be un-enveloped on direct reads from Kube API Server.
oldSecretObj, err := secretClient.Get(ctx, testSecret, metav1.GetOptions{})
if err != nil {
t.Fatalf("failed to read future secret via Kube API, err: %v", err)
t.Fatalf("failed to read old 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]))
if oldSecretVal != string(oldSecretObj.Data[secretKey]) {
t.Fatalf("expected %s from KubeAPI, but got %s", oldSecretVal, string(oldSecretObj.Data[secretKey]))
}

// no-op update should cause new AES CBC key to be used
futureSecretUpdated, err := secretClient.Update(ctx, futureSecretObj, metav1.UpdateOptions{})
// no-op update should cause new AES GCM key to be used
oldSecretUpdated, err := secretClient.Update(ctx, oldSecretObj, metav1.UpdateOptions{})
if err != nil {
t.Fatalf("failed to update future secret via Kube API, err: %v", err)
t.Fatalf("failed to update old 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)
if oldSecretObj.ResourceVersion == oldSecretUpdated.ResourceVersion {
t.Fatalf("old secret not updated on no-op write: %s", oldSecretObj.ResourceVersion)
}

// confirm that direct AES CBC decryption works
futureRawEnvelope, err := test.getRawSecretFromETCD()
// confirm that direct AES GCM decryption works
oldRawEnvelope, err := test.getRawSecretFromETCD()
if err != nil {
t.Fatalf("failed to read %s from etcd: %v", secretETCDPath, err)
}
futureEnvelope := envelope{
oldEnvelope := envelope{
providerName: providerName,
rawEnvelope: futureRawEnvelope,
rawEnvelope: oldRawEnvelope,
plainTextDEK: pluginMock.LastEncryptRequest(),
}
if !bytes.HasPrefix(futureRawEnvelope, []byte(wantPrefix)) {
t.Fatalf("expected secret to be prefixed with %s, but got %s", wantPrefix, futureRawEnvelope)
if !bytes.HasPrefix(oldRawEnvelope, []byte(wantPrefix)) {
t.Fatalf("expected secret to be prefixed with %s, but got %s", wantPrefix, oldRawEnvelope)
}
futurePlainSecret, err := futureEnvelope.plainTextPayload(secretETCDPath)
oldPlainSecret, err := oldEnvelope.plainTextPayload(secretETCDPath)
if err != nil {
t.Fatalf("failed to transform from storage via AESCBC, err: %v", err)
t.Fatalf("failed to transform from storage via AESGCM, err: %v", err)
}
if !strings.Contains(string(futurePlainSecret), futureSecretVal) {
t.Fatalf("expected %q after decryption, but got %q", futureSecretVal, string(futurePlainSecret))
if !strings.Contains(string(oldPlainSecret), oldSecretVal) {
t.Fatalf("expected %q after decryption, but got %q", oldSecretVal, string(oldPlainSecret))
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ const (
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"
// precomputed key and secret for use with AES CBC
// this looks exactly the same as the AES GCM secret but with a different value
oldAESCBCKey = "e0/+tts8FS254BZimFZWtUsOCOUDSkvzB72PyimMlkY="
oldSecret = "azhzAAoMCgJ2MRIGU2VjcmV0En4KXwoLdGVzdC1zZWNyZXQSABoWc2VjcmV0LWVuY3J5cHRpb24tdGVzdCIAKiQ3MmRmZTVjNC0xNDU2LTQyMzktYjFlZC1hZGZmYTJmMWY3YmEyADgAQggI5Jy/7wUQAHoAEhMKB2FwaV9rZXkSCPCfpJfwn5C8GgZPcGFxdWUaACIA"
oldSecretVal = "\xf0\x9f\xa4\x97\xf0\x9f\x90\xbc"
)

type unSealSecret func(ctx context.Context, cipherText []byte, dataCtx value.Context, config apiserverconfigv1.ProviderConfiguration) ([]byte, error)
Expand Down