chore(endpoint): harden crypto (#6197)

Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>

endpoint/crypto.go

@@ -25,23 +25,26 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io"
-
-	log "github.com/sirupsen/logrus"
 )
 
 const standardGcmNonceSize = 12
 
-// GenerateNonce creates a random nonce of a fixed size
-func GenerateNonce() ([]byte, error) {
+// GenerateNonce creates a random base64-encoded nonce of a fixed size.
+func GenerateNonce() (string, error) {
 	nonce := make([]byte, standardGcmNonceSize)
 	if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
-		return nil, err
+		return "", err
 	}
-	return []byte(base64.StdEncoding.EncodeToString(nonce)), nil
+	return 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) {
+// EncryptText gzips input data and encrypts it using the supplied AES key.
+// nonceEncoded must be a base64-encoded nonce of standardGcmNonceSize bytes.
+func EncryptText(text string, aesKey []byte, nonceEncoded string) (string, error) {
+	if len(nonceEncoded) == 0 {
+		return "", fmt.Errorf("nonce must be provided")
+	}
+
 	block, err := aes.NewCipher(aesKey)
 	if err != nil {
 		return "", err
@@ -53,7 +56,7 @@ func EncryptText(text string, aesKey []byte, nonceEncoded []byte) (string, error
 	}
 
 	nonce := make([]byte, standardGcmNonceSize)
-	if _, err = base64.StdEncoding.Decode(nonce, nonceEncoded); err != nil {
+	if _, err = base64.StdEncoding.Decode(nonce, []byte(nonceEncoded)); err != nil {
 		return "", err
 	}
 
@@ -66,40 +69,38 @@ func EncryptText(text string, aesKey []byte, nonceEncoded []byte) (string, error
 	return base64.StdEncoding.EncodeToString(cipherData), nil
 }
 
-// DecryptText decrypt gziped data using a supplied AES encryption key ang ungzip it
-// in case of decryption failed, will return original input and decryption error
+// DecryptText decrypts data using the supplied AES encryption key and decompresses it.
+// Returns the plaintext, the base64-encoded nonce, and any error.
 func DecryptText(text string, aesKey []byte) (string, string, error) {
 	block, err := aes.NewCipher(aesKey)
 	if err != nil {
 		return "", "", err
 	}
-	gcm, err := cipher.NewGCM(block)
+	gcm, err := cipher.NewGCMWithNonceSize(block, standardGcmNonceSize)
 	if err != nil {
 		return "", "", err
 	}
-	nonceSize := gcm.NonceSize()
 	data, err := base64.StdEncoding.DecodeString(text)
 	if err != nil {
 		return "", "", err
 	}
-	if len(data) <= nonceSize {
-		return "", "", fmt.Errorf("the encoded data from text %#v is shorter than %#v bytes and can't be decoded", text, nonceSize)
+	if len(data) <= standardGcmNonceSize {
+		return "", "", fmt.Errorf("encrypted data too short: got %d bytes, need more than %d", len(data), standardGcmNonceSize)
 	}
-	nonce, ciphertext := data[:nonceSize], data[nonceSize:]
+	nonce, ciphertext := data[:standardGcmNonceSize], data[standardGcmNonceSize:]
 	plaindata, err := gcm.Open(nil, nonce, ciphertext, nil)
 	if err != nil {
 		return "", "", err
 	}
 	plaindata, err = decompressData(plaindata)
 	if err != nil {
-		log.Debugf("Failed to decompress data based on the base64 encoded text %#v. Got error %#v.", text, err)
 		return "", "", err
 	}
 
 	return string(plaindata), base64.StdEncoding.EncodeToString(nonce), nil
 }
 
-// decompressData gzip compressed data
+// decompressData decompresses gzip-compressed data.
 func decompressData(data []byte) ([]byte, error) {
 	gz, err := gzip.NewReader(bytes.NewBuffer(data))
 	if err != nil {
@@ -114,7 +115,7 @@ func decompressData(data []byte) ([]byte, error) {
 	return b.Bytes(), nil
 }
 
-// compressData by gzip, for minify data stored in registry
+// compressData compresses data using gzip to minimize storage in the registry.
 func compressData(data []byte) ([]byte, error) {
 	var b bytes.Buffer
 	gz, err := gzip.NewWriterLevel(&b, gzip.BestCompression)
@@ -122,7 +123,6 @@ func compressData(data []byte) ([]byte, error) {
 		return nil, err
 	}
 
-	defer gz.Close()
 	if _, err = gz.Write(data); err != nil {
 		return nil, err
 	}

endpoint/crypto_test.go

@@ -27,10 +27,16 @@ import (
 )
 
 func TestEncrypt(t *testing.T) {
-	// Verify that text encryption and decryption works
+	// Verify that nil nonce is rejected
 	aesKey := []byte("s%zF`.*'5`9.AhI2!B,.~hmbs^.*TL?;")
 	plaintext := "Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, when an unknown printer took a galley of type and scrambled it to make a type specimen book. It has survived not only five centuries, but also the leap into electronic typesetting, remaining essentially unchanged. It was popularised in the 1960s with the release of Letraset sheets containing Lorem Ipsum passages, and more recently with desktop publishing software like Aldus PageMaker including versions of Lorem Ipsum."
-	encryptedtext, err := EncryptText(plaintext, aesKey, nil)
+	_, err := EncryptText(plaintext, aesKey, "")
+	require.EqualError(t, err, "nonce must be provided")
+
+	// Verify that text encryption and decryption works with a generated nonce
+	nonce, err := GenerateNonce()
+	require.NoError(t, err)
+	encryptedtext, err := EncryptText(plaintext, aesKey, nonce)
 	require.NoError(t, err)
 	decryptedtext, _, err := DecryptText(encryptedtext, aesKey)
 	require.NoError(t, err)
@@ -67,7 +73,7 @@ func TestGenerateNonceSuccess(t *testing.T) {
 	require.NotEmpty(t, nonce)
 
 	// Test nonce length
-	decodedNonce, err := base64.StdEncoding.DecodeString(string(nonce))
+	decodedNonce, err := base64.StdEncoding.DecodeString(nonce)
 	require.NoError(t, err)
 	require.Len(t, decodedNonce, standardGcmNonceSize)
 }
@@ -82,7 +88,7 @@ func TestGenerateNonceError(t *testing.T) {
 
 	nonce, err := GenerateNonce()
 	require.Error(t, err)
-	require.Nil(t, nonce)
+	require.Empty(t, nonce)
 }
 
 type faultyReader struct{}

endpoint/labels.go

@@ -133,16 +133,14 @@ func (l Labels) Serialize(withQuotes bool, txtEncryptEnabled bool, aesKey []byte
 		return l.SerializePlain(withQuotes)
 	}
 
-	var encryptionNonce []byte
-	if extractedNonce, nonceExists := l[txtEncryptionNonce]; nonceExists {
-		encryptionNonce = []byte(extractedNonce)
-	} else {
+	encryptionNonce, ok := l[txtEncryptionNonce]
+	if !ok {
 		var err error
 		encryptionNonce, err = GenerateNonce()
 		if err != nil {
-			log.Fatalf("Failed to generate cryptographic nonce %#v.", err)
+			log.Fatalf("Failed to generate cryptographic nonce: %v", err)
 		}
-		l[txtEncryptionNonce] = string(encryptionNonce)
+		l[txtEncryptionNonce] = encryptionNonce
 	}
 
 	text := l.SerializePlain(false)
@@ -150,8 +148,9 @@ func (l Labels) Serialize(withQuotes bool, txtEncryptEnabled bool, aesKey []byte
 	var err error
 	text, err = EncryptText(text, aesKey, encryptionNonce)
 	if err != nil {
+		// TODO: review if we could return error instead of crashing the external-dns
 		// if encryption failed, the external-dns will crash
-		log.Fatalf("Failed to encrypt the text %#v using the encryption key %#v. Got error %#v.", text, aesKey, err)
+		log.Fatalf("Failed to encrypt the text: %v", err)
 	}
 
 	if withQuotes {

endpoint/labels_test.go

@@ -105,7 +105,7 @@ func (suite *LabelsSuite) TestEncryptionFailed() {
 	_ = foo.Serialize(false, true, []byte("wrong-key"))
 
 	suite.True(fatalCrash, "should fail if encryption key is wrong")
-	suite.Contains(b.String(), "Failed to encrypt the text")
+	suite.Contains(b.String(), "Failed to encrypt the text:")
 }
 
 func (suite *LabelsSuite) TestEncryptionFailedFaultyReader() {