From 66b5b250fab04e7c83a94f36b99a3717a2323409 Mon Sep 17 00:00:00 2001 From: Seweryn Chlewicki Date: Thu, 5 Oct 2023 19:10:42 +0100 Subject: [PATCH] Make the nonce stable when generating delete records --- endpoint/crypto.go | 25 +++++++++++++++---------- endpoint/labels.go | 13 +++++++++++-- registry/txt.go | 2 +- registry/txt_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 13 deletions(-) diff --git a/endpoint/crypto.go b/endpoint/crypto.go index 1d6ebd1dd7..253cb227a5 100644 --- a/endpoint/crypto.go +++ b/endpoint/crypto.go @@ -29,6 +29,17 @@ import ( log "github.com/sirupsen/logrus" ) +const standardGcmNonceSize = 12 + +// GenerateNonce creates a random nonce of a fixed size +func GenerateNonce() ([]byte, error) { + nonce := make([]byte, standardGcmNonceSize) + if _, err := io.ReadFull(rand.Reader, nonce); err != nil { + return nil, err + } + return []byte(base64.StdEncoding.EncodeToString(nonce)), nil +} + // EncryptText gzip input data and encrypts it using the supplied AES key func EncryptText(text string, aesKey []byte, nonceEncoded []byte) (string, error) { block, err := aes.NewCipher(aesKey) @@ -36,20 +47,14 @@ func EncryptText(text string, aesKey []byte, nonceEncoded []byte) (string, error return "", err } - gcm, err := cipher.NewGCM(block) + gcm, err := cipher.NewGCMWithNonceSize(block, standardGcmNonceSize) if err != nil { return "", err } - nonce := make([]byte, gcm.NonceSize()) - if nonceEncoded == nil { - if _, err = io.ReadFull(rand.Reader, nonce); err != nil { - return "", err - } - } else { - if _, err = base64.StdEncoding.Decode(nonce, nonceEncoded); err != nil { - return "", err - } + nonce := make([]byte, standardGcmNonceSize) + if _, err = base64.StdEncoding.Decode(nonce, nonceEncoded); err != nil { + return "", err } data, err := compressData([]byte(text)) diff --git a/endpoint/labels.go b/endpoint/labels.go index c65333dc6e..8fbebc72f9 100644 --- a/endpoint/labels.go +++ b/endpoint/labels.go @@ -119,6 +119,9 @@ func (l Labels) SerializePlain(withQuotes bool) string { sort.Strings(keys) // sort for consistency for _, key := range keys { + if key == txtEncryptionNonce { + continue + } tokens = append(tokens, fmt.Sprintf("%s/%s=%s", heritage, key, l[key])) } if withQuotes { @@ -133,10 +136,16 @@ func (l Labels) Serialize(withQuotes bool, txtEncryptEnabled bool, aesKey []byte return l.SerializePlain(withQuotes) } - var encryptionNonce []byte = nil + var encryptionNonce []byte if extractedNonce, nonceExists := l[txtEncryptionNonce]; nonceExists { encryptionNonce = []byte(extractedNonce) - delete(l, txtEncryptionNonce) + } else { + var err error + encryptionNonce, err = GenerateNonce() + if err != nil { + log.Fatalf("Failed to generate cryptographic nonce %#v.", err) + } + l[txtEncryptionNonce] = string(encryptionNonce) } text := l.SerializePlain(false) diff --git a/registry/txt.go b/registry/txt.go index 9fae0fb603..6789e21306 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -220,7 +220,7 @@ func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpo endpoints := make([]*endpoint.Endpoint, 0) - if !im.mapper.recordTypeInAffix() && r.RecordType != endpoint.RecordTypeAAAA { + if !im.txtEncryptEnabled && !im.mapper.recordTypeInAffix() && r.RecordType != endpoint.RecordTypeAAAA { // old TXT record format txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true, im.txtEncryptEnabled, im.txtEncryptAESKey)) if txt != nil { diff --git a/registry/txt_test.go b/registry/txt_test.go index 9e2cc5e150..6ddebd6e5d 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -1527,6 +1527,47 @@ func TestFailGenerateTXT(t *testing.T) { assert.Equal(t, expectedTXT, gotTXT) } +func TestTXTRegistryApplyChangesEncrypt(t *testing.T) { + p := inmemory.NewInMemoryProvider() + p.CreateZone(testZone) + ctxEndpoints := []*endpoint.Endpoint{} + ctx := context.WithValue(context.Background(), provider.RecordsContextKey, ctxEndpoints) + + p.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{ + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), + newEndpointWithOwnerAndOwnedRecord("txt.cname-foobar.test-zone.example.org", "\"h8UQ6jelUFUsEIn7SbFktc2MYXPx/q8lySqI4VwfVtVaIbb2nkHWV/88KKbuLtu7fJNzMir8ELVeVnRSY01KdiIuj7ledqZe5ailEjQaU5Z6uEKd5pgs6sH8\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + }, + }) + + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "", []string{}, []string{}, true, []byte("12345678901234567890123456789012")) + records, _ := r.Records(ctx) + changes := &plan.Changes{ + Delete: records, + } + + // ensure that encryption nonce gets reused when deleting records + expected := &plan.Changes{ + Delete: []*endpoint.Endpoint{ + newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-foobar.test-zone.example.org", "\"h8UQ6jelUFUsEIn7SbFktc2MYXPx/q8lySqI4VwfVtVaIbb2nkHWV/88KKbuLtu7fJNzMir8ELVeVnRSY01KdiIuj7ledqZe5ailEjQaU5Z6uEKd5pgs6sH8\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + }, + } + + p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { + mExpected := map[string][]*endpoint.Endpoint{ + "Delete": expected.Delete, + } + mGot := map[string][]*endpoint.Endpoint{ + "Delete": got.Delete, + } + assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) + assert.Equal(t, nil, ctx.Value(provider.RecordsContextKey)) + } + err := r.ApplyChanges(ctx, changes) + require.NoError(t, err) +} + // TestMultiClusterDifferentRecordTypeOwnership validates the registry handles environments where the same zone is managed by // external-dns in different clusters and the ingress record type is different. For example one uses A records and the other // uses CNAME. In this environment the first cluster that establishes the owner record should maintain ownership even