Merge pull request #167 from ProtonMail/feat/fix-mdc-session-key

Fix bad MDC messages parsing

crypto/sessionkey.go

@@ -11,6 +11,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/ProtonMail/go-crypto/openpgp"
+	pgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors"
 	"github.com/ProtonMail/go-crypto/openpgp/packet"
 )
 
@@ -31,6 +32,28 @@ var symKeyAlgos = map[string]packet.CipherFunction{
 	constants.AES256:    packet.CipherAES256,
 }
 
+type checkReader struct {
+	decrypted io.ReadCloser
+	body      io.Reader
+}
+
+func (cr checkReader) Read(buf []byte) (int, error) {
+	n, sensitiveParsingError := cr.body.Read(buf)
+	if sensitiveParsingError == io.EOF {
+		mdcErr := cr.decrypted.Close()
+		if mdcErr != nil {
+			return n, mdcErr
+		}
+		return n, io.EOF
+	}
+
+	if sensitiveParsingError != nil {
+		return n, pgpErrors.StructuralError("parsing error")
+	}
+
+	return n, nil
+}
+
 // GetCipherFunc returns the cipher function corresponding to the algorithm used
 // with this SessionKey.
 func (sk *SessionKey) GetCipherFunc() (packet.CipherFunction, error) {
@@ -335,6 +358,7 @@ func decryptStreamWithSessionKey(sk *SessionKey, messageReader io.Reader, verify
 		return nil, errors.Wrap(err, "gopenpgp: unable to decode symmetric packet")
 	}
 
+	md.UnverifiedBody = checkReader{decrypted, md.UnverifiedBody}
 	return md, nil
 }
 

crypto/sessionkey_test.go

@@ -2,6 +2,7 @@ package crypto
 
 import (
 	"encoding/base64"
+	"encoding/hex"
 	"errors"
 	"testing"
 
@@ -271,6 +272,25 @@ func TestDataPacketDecryption(t *testing.T) {
 	assert.Exactly(t, readTestFile("message_plaintext", true), decrypted.GetString())
 }
 
+func TestMDCFailDecryption(t *testing.T) {
+	pgpMessage, err := NewPGPMessageFromArmored(readTestFile("message_badmdc", false))
+	if err != nil {
+		t.Fatal("Expected no error when unarmoring, got:", err)
+	}
+
+	split, err := pgpMessage.SeparateKeyAndData()
+	if err != nil {
+		t.Fatal("Expected no error when splitting, got:", err)
+	}
+
+	sk, _ := hex.DecodeString("F76D3236E4F8A38785C50BDE7167475E95360BCE67A952710F6C16F18BB0655E")
+
+	sessionKey := NewSessionKeyFromToken(sk, "aes256")
+
+	_, err = sessionKey.Decrypt(split.GetBinaryDataPacket())
+	assert.NotNil(t, err)
+}
+
 func TestSessionKeyClear(t *testing.T) {
 	testSessionKey.Clear()
 	assertMemCleared(t, testSessionKey.Key)

crypto/testdata/message_badmdc

@@ -0,0 +1,14 @@
+-----BEGIN PGP MESSAGE-----
+
+wcDMA3wvqk35PDeyAQv/bxxV95z095aN6D+s6ayatoZc2xakyZenaCYiDIY26CmW
+iLmzNHasoPAmKa3dN3diiMuquKQLJQEoAeAd0L5xDwi7nTpan8E7F9p2krP+hGGy
+bNaWez5O0Eyw2D5VPokybYIJ+bLo0MEZ/DOzUQ8HijDt2pPQ37CJP6jhA3rcFgwb
+pEEjF2zWwI3PLrVdO8XcBI5y34ZCUO94FzQUN6/xVpeFQ3vlc9mOajv03l+wyisb
+4UeXbSJMEnZr915UqNiu8L9/y9BwVtZ2StRSiumMT4OPDjRAQMQQL7FcpogzVwPm
+oCmVw1ZcBltnBtpdH7F+M4+PUaaVZTTIYOxCBa1rfpnIcKefJpsAS7aaFZQUs+8a
+DxEygVzUF6rk2/WajmefwrZ+7EGWFQU0VfPHXqozDAuryzx7p9jMCn+QnyVvtYkv
+QqvO1SLa3RqLjqV67QnwCs2lo9WMQDcUchsAC34t1bOtzzqxZOlN12PHn4SyjRdo
+74vEv+SPViLjZPfcB3Kz0kwBQump/fZNQys1LvrAevWs+1UIaUhLzb5UmdELsCAY
+ebkFfoWZdskeR+1qSeui57C4ggOHu0Mm/JCbJxAFAHHn+RE8oQhnYiD6xTsA
+=cHtj
+-----END PGP MESSAGE-----