openpgp/packet: fix AES-192 encrypted session keys

The existing implementation checks to see if the session key size is a
multiple of the cipher blocksize. This fails for AES-192, which has a
keysize of 24 bytes and a 16 byte block size. Instead it should simply
check to ensure that the Session Key length is equal to the cipher
KeySize.

Fixes golang/go#17060

Change-Id: I1dc78129f7fb2ca5ec71b650a2adcb3752dca885
Reviewed-on: https://go-review.googlesource.com/35848
Reviewed-by: Adam Langley <agl@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

openpgp/packet/symmetric_key_encrypted.go

@@ -88,10 +88,10 @@ func (ske *SymmetricKeyEncrypted) Decrypt(passphrase []byte) ([]byte, CipherFunc
 		return nil, ske.CipherFunc, errors.UnsupportedError("unknown cipher: " + strconv.Itoa(int(cipherFunc)))
 	}
 	plaintextKey = plaintextKey[1:]
-	if l := len(plaintextKey); l == 0 || l%cipherFunc.blockSize() != 0 {
-		return nil, cipherFunc, errors.StructuralError("length of decrypted key not a multiple of block size")
+	if l, cipherKeySize := len(plaintextKey), cipherFunc.KeySize(); l != cipherFunc.KeySize() {
+		return nil, cipherFunc, errors.StructuralError("length of decrypted key (" + strconv.Itoa(l) + ") " +
+			"not equal to cipher keysize (" + strconv.Itoa(cipherKeySize) + ")")
 	}
-
 	return plaintextKey, cipherFunc, nil
 }
 

openpgp/packet/symmetric_key_encrypted_test.go

@@ -61,43 +61,57 @@ func TestSymmetricKeyEncrypted(t *testing.T) {
 const symmetricallyEncryptedHex = "8c0d04030302371a0b38d884f02060c91cf97c9973b8e58e028e9501708ccfe618fb92afef7fa2d80ddadd93cf"
 const symmetricallyEncryptedContentsHex = "cb1062004d14c4df636f6e74656e74732e0a"
 
-func TestSerializeSymmetricKeyEncrypted(t *testing.T) {
-	buf := bytes.NewBuffer(nil)
-	passphrase := []byte("testing")
-	const cipherFunc = CipherAES128
-	config := &Config{
-		DefaultCipher: cipherFunc,
+func TestSerializeSymmetricKeyEncryptedCiphers(t *testing.T) {
+	tests := [...]struct {
+		cipherFunc CipherFunction
+		name       string
+	}{
+		{Cipher3DES, "Cipher3DES"},
+		{CipherCAST5, "CipherCAST5"},
+		{CipherAES128, "CipherAES128"},
+		{CipherAES192, "CipherAES192"},
+		{CipherAES256, "CipherAES256"},
 	}
 
-	key, err := SerializeSymmetricKeyEncrypted(buf, passphrase, config)
-	if err != nil {
-		t.Errorf("failed to serialize: %s", err)
-		return
-	}
+	for _, test := range tests {
+		var buf bytes.Buffer
+		passphrase := []byte("testing")
+		config := &Config{
+			DefaultCipher: test.cipherFunc,
+		}
 
-	p, err := Read(buf)
-	if err != nil {
-		t.Errorf("failed to reparse: %s", err)
-		return
-	}
-	ske, ok := p.(*SymmetricKeyEncrypted)
-	if !ok {
-		t.Errorf("parsed a different packet type: %#v", p)
-		return
-	}
+		key, err := SerializeSymmetricKeyEncrypted(&buf, passphrase, config)
+		if err != nil {
+			t.Errorf("cipher(%s) failed to serialize: %s", test.name, err)
+			continue
+		}
 
-	if ske.CipherFunc != config.DefaultCipher {
-		t.Errorf("SKE cipher function is %d (expected %d)", ske.CipherFunc, config.DefaultCipher)
-	}
-	parsedKey, parsedCipherFunc, err := ske.Decrypt(passphrase)
-	if err != nil {
-		t.Errorf("failed to decrypt reparsed SKE: %s", err)
-		return
-	}
-	if !bytes.Equal(key, parsedKey) {
-		t.Errorf("keys don't match after Decrypt: %x (original) vs %x (parsed)", key, parsedKey)
-	}
-	if parsedCipherFunc != cipherFunc {
-		t.Errorf("cipher function doesn't match after Decrypt: %d (original) vs %d (parsed)", cipherFunc, parsedCipherFunc)
+		p, err := Read(&buf)
+		if err != nil {
+			t.Errorf("cipher(%s) failed to reparse: %s", test.name, err)
+			continue
+		}
+
+		ske, ok := p.(*SymmetricKeyEncrypted)
+		if !ok {
+			t.Errorf("cipher(%s) parsed a different packet type: %#v", test.name, p)
+			continue
+		}
+
+		if ske.CipherFunc != config.DefaultCipher {
+			t.Errorf("cipher(%s) SKE cipher function is %d (expected %d)", test.name, ske.CipherFunc, config.DefaultCipher)
+		}
+		parsedKey, parsedCipherFunc, err := ske.Decrypt(passphrase)
+		if err != nil {
+			t.Errorf("cipher(%s) failed to decrypt reparsed SKE: %s", test.name, err)
+			continue
+		}
+		if !bytes.Equal(key, parsedKey) {
+			t.Errorf("cipher(%s) keys don't match after Decrypt: %x (original) vs %x (parsed)", test.name, key, parsedKey)
+		}
+		if parsedCipherFunc != test.cipherFunc {
+			t.Errorf("cipher(%s) cipher function doesn't match after Decrypt: %d (original) vs %d (parsed)",
+				test.name, test.cipherFunc, parsedCipherFunc)
+		}
 	}
 }