diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e0e0e6..ad295eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## Unreleased +## [2.4.3] 2022-02-24 + +### Security +- Fixed incorrect MDC parsing for session key decryption ### Changed - `SeparateKeyAndData` is now implemented in a more generic way, by checking for the location in the bytes of the last session key packet, then splitting the binary message after that point. @@ -13,7 +16,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `SeparateKeyAndData` now correctly parses AEAD packets. - `(ap *AttachmentProcessor) Finish()` now returns encryption errors correctly. - ## [2.4.2] 2022-01-13 ### Changed diff --git a/constants/armor.go b/constants/armor.go index 3ffa175..9ae380e 100644 --- a/constants/armor.go +++ b/constants/armor.go @@ -3,7 +3,7 @@ package constants // Constants for armored data. const ( - ArmorHeaderVersion = "GopenPGP 2.4.2" + ArmorHeaderVersion = "GopenPGP 2.4.3" ArmorHeaderComment = "https://gopenpgp.org" PGPMessageHeader = "PGP MESSAGE" PGPSignatureHeader = "PGP SIGNATURE" diff --git a/constants/version.go b/constants/version.go index 2a420d0..c900e29 100644 --- a/constants/version.go +++ b/constants/version.go @@ -1,3 +1,3 @@ package constants -const Version = "2.4.2" +const Version = "2.4.3" diff --git a/crypto/sessionkey.go b/crypto/sessionkey.go index 49c0c5a..481e505 100644 --- a/crypto/sessionkey.go +++ b/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 } diff --git a/crypto/sessionkey_test.go b/crypto/sessionkey_test.go index ce3c8e8..026461f 100644 --- a/crypto/sessionkey_test.go +++ b/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) diff --git a/crypto/testdata/message_badmdc b/crypto/testdata/message_badmdc new file mode 100644 index 0000000..ef24b2a --- /dev/null +++ b/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-----