do not use broken key derivation for AES-GCM

This commit fixes a cryptographic security issue
w.r.t. wrapping data encryption keys.

The key derivation function (KDF) used when a
data encryption key is sealed is not a PRF.

One way to show that a function is not a PRF is
to proof that there is a relation between different
output values of the function.

In particular, the AES key derivation function used
before has the following property:
```
Let v0, v1, v2 and v3 be 128 bit values (HEX) as following:
 v0 = 00000000000000000000000000000000
 v1 = 01000000000000000000000000000000
 v2 = 00000000000000000000000000000001
 v3 = 01000000000000000000000000000001

Further let K be an arbitrary 256 bit key and f the AES key
derivation function. Then invoking f with K and the iv values
returns 4 output values
 k0 = f(K, v0)
 k1 = f(K, v1)
 k2 = f(K, v2)
 k3 = f(K, v3)

Now, the following statement is true:
 k0 ^ k1 ^ k2 == k3  // ^ is XOR

So, from deriving k0, k1, k2 you can infer k3. This clearly
violates the definition of a PRF.
```

This commit fixes this by replacing the KDF with `HMAC-SHA-256`.
Since, HMAC has been proven to a PRF if the used hash function is
collision-resistant this is an adequate fix.

However, switching to HMAC-SHA-256 is not ideal in the sense
that we now rely on 2 primitives (AES and SHA-256) instead of one.
However, this can be addressed in the future when more research has
been done.

internal/secret/secret.go

@@ -7,6 +7,8 @@ package secret
 import (
 	"crypto/aes"
 	"crypto/cipher"
+	"crypto/hmac"
+	"crypto/sha256"
 	"encoding/base64"
 	"encoding/binary"
 	"encoding/json"
@@ -114,21 +116,20 @@ func (s Secret) Wrap(plaintext, associatedData []byte) ([]byte, error) {
 
 	var algorithm string
 	if sioutil.NativeAES() {
-		algorithm = "AES-256-GCM"
+		algorithm = "AES-256-GCM-HMAC-SHA-256"
 	} else {
 		algorithm = "ChaCha20Poly1305"
 	}
 
 	var aead cipher.AEAD
 	switch algorithm {
-	case "AES-256-GCM":
-		var sealingKey []byte
+	case "AES-256-GCM-HMAC-SHA-256":
+		mac := hmac.New(sha256.New, s[:])
+		mac.Write(iv)
+		sealingKey := mac.Sum(nil)
+
 		var block cipher.Block
-		sealingKey, err = aesDeriveKey(s[:], iv)
-		if err != nil {
-			return nil, err
-		}
-		block, err = aes.NewCipher(sealingKey[:])
+		block, err = aes.NewCipher(sealingKey)
 		if err != nil {
 			return nil, err
 		}
@@ -182,8 +183,21 @@ func (s Secret) Unwrap(ciphertext []byte, associatedData []byte) ([]byte, error)
 
 	var aead cipher.AEAD
 	switch sealedKey.Algorithm {
+	case "AES-256-GCM-HMAC-SHA-256":
+		mac := hmac.New(sha256.New, s[:])
+		mac.Write(sealedKey.IV)
+		sealingKey := mac.Sum(nil)
+
+		block, err := aes.NewCipher(sealingKey[:])
+		if err != nil {
+			return nil, err
+		}
+		aead, err = cipher.NewGCM(block)
+		if err != nil {
+			return nil, err
+		}
 	case "AES-256-GCM":
-		sealingKey, err := aesDeriveKey(s[:], sealedKey.IV)
+		sealingKey, err := insecureAESDeriveKey(s[:], sealedKey.IV)
 		if err != nil {
 			return nil, err
 		}
@@ -218,7 +232,7 @@ func (s Secret) Unwrap(ciphertext []byte, associatedData []byte) ([]byte, error)
 	return plaintext, nil
 }
 
-// aesDeriveKey returns a new key derived from the
+// insecureAESDeriveKey returns a new key derived from the
 // provided key and iv using AES as pseudo-random-
 // permutation (PRP).
 // The provided key must be 16 (AES-128) or 32 (AES-256)
@@ -230,7 +244,24 @@ func (s Secret) Unwrap(ciphertext []byte, associatedData []byte) ([]byte, error)
 //
 // The main difference to RFC 8452 is that the iv
 // is 128 bit long while AES-GCM-SIV uses 96 bit nonces.
-func aesDeriveKey(key, iv []byte) ([]byte, error) {
+//
+// BUG(aead): The KDF implemented by insecureAESDeriveKey is
+// not a PRF. The following example shows that there
+// exists a relation between 4 generated keys:
+//   iv0 = 0¹⁶
+//   iv1 = 1 || 0¹⁵
+//   iv2 = 0¹⁵ || 1
+//   iv3 = 1 || 0¹⁴ || 1
+// Generating 4 derived keys with these 4 iv values using
+// the same key `K` gives:
+//   k0 = insecureAESDeriveKey(K, iv0)
+//   k1 = insecureAESDeriveKey(K, iv1)
+//   k2 = insecureAESDeriveKey(K, iv2)
+//   k3 = insecureAESDeriveKey(K, iv3)
+// Now, the following statement holds (^ is XOR):
+//   k0 ^ k1 ^ k2 == k3
+// This shows that insecureAESDeriveKey is not a PRF.
+func insecureAESDeriveKey(key, iv []byte) ([]byte, error) {
 	if n := len(iv); n != 16 {
 		return nil, errors.New("key: invalid iv size " + strconv.Itoa(n))
 	}

internal/secret/secret_test.go

@@ -190,7 +190,7 @@ func TestSecrectUnwrap(t *testing.T) {
 	}
 }
 
-var aesDeriveKeyTests = []struct {
+var insecureAESDeriveKeyTests = []struct {
 	Key        []byte
 	IV         []byte
 	DerivedKey []byte
@@ -228,9 +228,9 @@ var aesDeriveKeyTests = []struct {
 	},
 }
 
-func TestAESDeriveKey(t *testing.T) {
-	for i, test := range aesDeriveKeyTests {
-		key, err := aesDeriveKey(test.Key, test.IV)
+func TestInsecureAESDeriveKey(t *testing.T) {
+	for i, test := range insecureAESDeriveKeyTests {
+		key, err := insecureAESDeriveKey(test.Key, test.IV)
 		if err != nil && !test.ShouldFail {
 			t.Fatalf("Test %d: Failed to derive key: %v", i, err)
 		}