Fix SKESK V5 packet implementation to comply with the spec (#179)

This removes components introduced by the crypto-refresh from v5 SKESK packets (Octet counts in packet encoding and HKDF based key derivation) reverting it to the original gpg v5 implementation
ProtonMail · Jul 17, 2023 · 5aa5874 · 5aa5874
2 parents e01326f + 1d61fc9
commit 5aa5874
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 977 deletions.
diff --git a/openpgp/packet/symmetric_key_encrypted.go b/openpgp/packet/symmetric_key_encrypted.go
@@ -7,13 +7,11 @@ package packet
 import (
 	"bytes"
 	"crypto/cipher"
-	"crypto/sha256"
 	"io"
 	"strconv"
 
 	"github.com/ProtonMail/go-crypto/openpgp/errors"
 	"github.com/ProtonMail/go-crypto/openpgp/s2k"
-	"golang.org/x/crypto/hkdf"
 )
 
 // This is the largest session key that we'll support. Since at most 256-bit cipher
@@ -45,13 +43,6 @@ func (ske *SymmetricKeyEncrypted) parse(r io.Reader) error {
 		return errors.UnsupportedError("unknown SymmetricKeyEncrypted version")
 	}
 
-	if ske.Version == 5 {
-		// Scalar octet count
-		if _, err := readFull(r, buf[:]); err != nil {
-			return err
-		}
-	}
-
 	// Cipher function
 	if _, err := readFull(r, buf[:]); err != nil {
 		return err
@@ -67,11 +58,6 @@ func (ske *SymmetricKeyEncrypted) parse(r io.Reader) error {
 			return errors.StructuralError("cannot read AEAD octet from packet")
 		}
 		ske.Mode = AEADMode(buf[0])
-
-		// Scalar octet count
-		if _, err := readFull(r, buf[:]); err != nil {
-			return err
-		}
 	}
 
 	var err error
@@ -220,7 +206,7 @@ func SerializeSymmetricKeyEncryptedReuseKey(w io.Writer, sessionKey []byte, pass
 	case 5:
 		ivLen := config.AEAD().Mode().IvLength()
 		tagLen := config.AEAD().Mode().TagLength()
-		packetLength = 5 + len(s2kBytes) + ivLen + keySize + tagLen
+		packetLength = 3 + len(s2kBytes) + ivLen + keySize + tagLen
 	}
 	err = serializeHeader(w, packetTypeSymmetricKeyEncrypted, packetLength)
 	if err != nil {
@@ -230,20 +216,12 @@ func SerializeSymmetricKeyEncryptedReuseKey(w io.Writer, sessionKey []byte, pass
 	// Symmetric Key Encrypted Version
 	buf := []byte{byte(version)}
 
-	if version == 5 {
-		// Scalar octet count
-		buf = append(buf, byte(3+len(s2kBytes)+config.AEAD().Mode().IvLength()))
-	}
-
 	// Cipher function
 	buf = append(buf, byte(cipherFunc))
 
 	if version == 5 {
 		// AEAD mode
 		buf = append(buf, byte(config.AEAD().Mode()))
-
-		// Scalar octet count
-		buf = append(buf, byte(len(s2kBytes)))
 	}
 	_, err = w.Write(buf)
 	if err != nil {
@@ -293,11 +271,6 @@ func SerializeSymmetricKeyEncryptedReuseKey(w io.Writer, sessionKey []byte, pass
 }
 
 func getEncryptedKeyAeadInstance(c CipherFunction, mode AEADMode, inputKey, associatedData []byte) (aead cipher.AEAD) {
-	hkdfReader := hkdf.New(sha256.New, inputKey, []byte{}, associatedData)
-
-	encryptionKey := make([]byte, c.KeySize())
-	_, _ = readFull(hkdfReader, encryptionKey)
-
-	blockCipher := c.new(encryptionKey)
+	blockCipher := c.new(inputKey)
 	return mode.new(blockCipher)
 }
diff --git a/openpgp/packet/symmetric_key_encrypted_data_test.go b/openpgp/packet/symmetric_key_encrypted_data_test.go
@@ -9,28 +9,18 @@ type packetSequence struct {
 	contents string
 }
 
-var keyAndIpePackets = []*packetSequence{symEncTest, aeadEaxRFC, aeadOcbRFC, aeadGcmRFC}
+var keyAndIpePackets = []*packetSequence{aeadEaxRFC, aeadOcbRFC}
 
+// https://www.ietf.org/archive/id/draft-koch-openpgp-2015-rfc4880bis-00.html#name-complete-aead-eax-encrypted-
 var aeadEaxRFC = &packetSequence{
 	password: "password",
-	packets:  "c340051e07010b0308a5ae579d1fc5d82bff69224f919993b3506fa3b59a6a73cff8da746b88e357e8ae54eb87e1d70575d72f60232990523e9a59094922406be1c3d269020701069ff90e3b321964f3a42913c8dcc6619325015227efb7eaeaa49f04c2e674175d4a3d226ed6afcb9ca9ac122c1470e11c63d4c0ab241c6a938ad48bf99a5a99b90bba8325de61047540258ab7959a95ad051dda96eb15431dfef5f5e2255ca78261546e339a",
-	contents: "cb1362000000000048656c6c6f2c20776f726c6421d50eae5bf0cd6705500355816cb0c8ff",
+	packets:  "c33e0507010308cd5a9f70fbe0bc6590bc669e34e500dcaedc5b32aa2dab02359dee19d07c3446c4312a34ae1967a2fb7e928ea5b4fa8012bd456d1738c63c36d44a0107010eb732379f73c4928de25facfe6517ec105dc11a81dc0cb8a2f6f3d90016384a56fc821ae11ae8dbcb49862655dea88d06a81486801b0ff387bd2eab013de1259586906eab2476",
+	contents: "cb1462000000000048656c6c6f2c20776f726c64210a",
 }
 
+// https://www.ietf.org/archive/id/draft-koch-openpgp-2015-rfc4880bis-00.html#name-complete-aead-ocb-encrypted-
 var aeadOcbRFC = &packetSequence{
 	password: "password",
-	packets:  "c33f051d07020b030856a298d2f5e36453ffcfcc5c11664edb9db42590d7dc46b078c5c0419cc51b3a4687cb32e5b7031ce7c66975765b5c21d92aef4cc05c3fead2690207020620a661f731fc9a3032b5623326027e3a5d8db5748ebeff0b0c5910d09ecdd641ff9fd38562758035bc49754ce1bf3fffa7dad0a3b8104f5133cf42a4100a83eef4ca1b4801a8846bf42bcda7c8ce9d65e212f301cbcd98fdcade694a877ad4247323f6e857",
-	contents: "cb1362000000000048656c6c6f2c20776f726c6421d50eae6aa1649b56aa835b2613902bd2",
-}
-
-var aeadGcmRFC = &packetSequence{
-	password: "password",
-	packets:  "c33c051a07030b0308e9d39785b2070008ffb42e7c483ef4884457cb37260c0c4bf3f2cd6cb7b6e38b5bf33467c1c71944dd590346662f5ade61ff84bce0d26902070306fcb94490bcb98bbdc9d106c6090266940f72e89edc21b5596b1576b101ed0f9ffc6fc6d65bbfd24dcd0790966e6d1e85a30053784cb1d8b6a0699ef12155a7b2ad6258531b57651fd7777912fa95e35d9b40216f69a4c248db28ff4331f1632907399e6ff9",
-	contents: "cb1362000000000048656c6c6f2c20776f726c6421d50e1ce2269a9eddef81032172b7ed7c",
-}
-
-var symEncTest = &packetSequence{
-	password: "password",
-	packets:  "c32e04090308f9f479ee0862ee8700a86d5cce4c166b5a7d664dcbe0f0eb2696a3e8a815fe8913251605ad79cc865f15d24301c3da8f5003383b9bd62c673589e2292d990902227311905ff4a7f694727578468e15d9f1aadb41572c4b2a789d7f93896661249200b64af9fbf6abf001f5498d036a",
-	contents: "cb1875076d73672e7478745cafc23e636f6e74656e74732e0d0a",
+	packets:  "c33d05070203089f0b7da3e5ea64779099e326e5400a90936cefb4e8eba08c6773716d1f2714540a38fcac529949dac529d3de31e15b4aeb729e330033dbedd4490107020e5ed2bc1e470abe8f1d644c7a6c8a567b0f7701196611a154ba9c2574cd056284a8ef68035c623d93cc708a43211bb6eaf2b27f7c18d571bcd83b20add3a08b73af15b9a098",
+	contents: "cb1462000000000048656c6c6f2c20776f726c64210a",
 }
diff --git a/openpgp/read_test.go b/openpgp/read_test.go
@@ -588,48 +588,6 @@ func TestSignatureV3Message(t *testing.T) {
 	return
 }
 
-func TestSymmetricAeadGcmOpenPGPJsMessage(t *testing.T) {
-	passphrase := []byte("test")
-	file, err := os.Open("test_data/aead-sym-message.asc")
-	if err != nil {
-		t.Fatal(err)
-	}
-	armoredEncryptedMessage, err := ioutil.ReadAll(file)
-	if err != nil {
-		t.Fatal(err)
-	}
-	// Unarmor string
-	raw, err := armor.Decode(strings.NewReader(string(armoredEncryptedMessage)))
-	if err != nil {
-		t.Error(err)
-		return
-	}
-	// Mock passphrase prompt
-	promptFunc := func(keys []Key, symmetric bool) ([]byte, error) {
-		return passphrase, nil
-	}
-	// Decrypt message
-	md, err := ReadMessage(raw.Body, nil, promptFunc, nil)
-	if err != nil {
-		t.Error(err)
-		return
-	}
-	contents, err := ioutil.ReadAll(md.UnverifiedBody)
-	if err != nil {
-		t.Errorf("error reading UnverifiedBody: %s", err)
-	}
-
-	// The plaintext is https://www.gutenberg.org/cache/epub/1080/pg1080.txt
-	// We compare the SHA512 hashes.
-	wantHash := modestProposalSha512
-	gotHashRaw := sha512.Sum512(contents)
-	gotHash := base64.StdEncoding.EncodeToString(gotHashRaw[:])
-
-	if wantHash != gotHash {
-		t.Fatal("Did not decrypt OpenPGPjs message correctly")
-	}
-}
-
 func TestSymmetricDecryptionArgon2(t *testing.T) {
 	// Appendix IETF OpenPGP crypto refresh draft v08 A.8.1
 	passphrase := []byte("password")