From b1893091522a0b56475ca39013e3d952568a4620 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Wed, 16 Nov 2022 14:41:38 +0100 Subject: [PATCH 1/4] Don't trim trailing spaces from non-clearsigned text messages --- crypto/message.go | 2 +- crypto/signature_collector.go | 2 +- helper/cleartext.go | 5 +++-- helper/cleartext_test.go | 2 +- internal/common.go | 8 ++++++-- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/crypto/message.go b/crypto/message.go index d6072fe..4f34be4 100644 --- a/crypto/message.go +++ b/crypto/message.go @@ -92,7 +92,7 @@ func NewPlainMessageFromFile(data []byte, filename string, time uint32) *PlainMe // This allows seamless conversion to clear text signed messages (see RFC 4880 5.2.1 and 7.1). func NewPlainMessageFromString(text string) *PlainMessage { return &PlainMessage{ - Data: []byte(internal.CanonicalizeAndTrim(text)), + Data: []byte(internal.Canonicalize(text)), TextType: true, Filename: "", Time: uint32(GetUnixTime()), diff --git a/crypto/signature_collector.go b/crypto/signature_collector.go index b5aefa6..05f2ecb 100644 --- a/crypto/signature_collector.go +++ b/crypto/signature_collector.go @@ -99,7 +99,7 @@ func (sc *SignatureCollector) Accept( } sc.signature = string(buffer) str, _ := ioutil.ReadAll(rawBody) - canonicalizedBody := internal.CanonicalizeAndTrim(string(str)) + canonicalizedBody := internal.Canonicalize(internal.TrimEachLine(string(str))) rawBody = bytes.NewReader([]byte(canonicalizedBody)) if sc.keyring != nil { _, err = openpgp.CheckArmoredDetachedSignature(sc.keyring, rawBody, bytes.NewReader(buffer), sc.config) diff --git a/helper/cleartext.go b/helper/cleartext.go index c318739..d5517b1 100644 --- a/helper/cleartext.go +++ b/helper/cleartext.go @@ -2,6 +2,7 @@ package helper import ( "github.com/ProtonMail/gopenpgp/v2/crypto" + "github.com/ProtonMail/gopenpgp/v2/internal" "github.com/pkg/errors" ) @@ -48,7 +49,7 @@ func VerifyCleartextMessageArmored(publicKey, armored string, verifyTime int64) // SignCleartextMessage signs text given a private keyring, canonicalizes and // trims the newlines, and returns the PGP-compliant special armoring. func SignCleartextMessage(keyRing *crypto.KeyRing, text string) (string, error) { - message := crypto.NewPlainMessageFromString(text) + message := crypto.NewPlainMessageFromString(internal.TrimEachLine(text)) signature, err := keyRing.SignDetached(message) if err != nil { @@ -67,7 +68,7 @@ func VerifyCleartextMessage(keyRing *crypto.KeyRing, armored string, verifyTime return "", errors.Wrap(err, "gopengpp: unable to unarmor cleartext message") } - message := crypto.NewPlainMessageFromString(clearTextMessage.GetString()) + message := crypto.NewPlainMessageFromString(internal.TrimEachLine(clearTextMessage.GetString())) signature := crypto.NewPGPSignature(clearTextMessage.GetBinarySignature()) err = keyRing.VerifyDetached(message, signature, verifyTime) if err != nil { diff --git a/helper/cleartext_test.go b/helper/cleartext_test.go index 6cf25b6..1e39ff9 100644 --- a/helper/cleartext_test.go +++ b/helper/cleartext_test.go @@ -45,5 +45,5 @@ func TestSignClearText(t *testing.T) { if err != nil { t.Fatal("Cannot parse message:", err) } - assert.Exactly(t, internal.CanonicalizeAndTrim(inputPlainText), string(clearTextMessage.GetBinary())) + assert.Exactly(t, internal.Canonicalize(internal.TrimEachLine(inputPlainText)), string(clearTextMessage.GetBinary())) } diff --git a/internal/common.go b/internal/common.go index 9704cd0..534b33d 100644 --- a/internal/common.go +++ b/internal/common.go @@ -7,14 +7,18 @@ import ( "github.com/ProtonMail/gopenpgp/v2/constants" ) -func CanonicalizeAndTrim(text string) string { +func Canonicalize(text string) string { + return strings.ReplaceAll(strings.ReplaceAll(text, "\r\n", "\n"), "\n", "\r\n") +} + +func TrimEachLine(text string) string { lines := strings.Split(text, "\n") for i := range lines { lines[i] = strings.TrimRight(lines[i], " \t\r") } - return strings.Join(lines, "\r\n") + return strings.Join(lines, "\n") } // CreationTimeOffset stores the amount of seconds that a signature may be From 76b77258e37544d29ffe8ff5a366442ff464eda5 Mon Sep 17 00:00:00 2001 From: "M. Thiercelin" Date: Thu, 17 Nov 2022 15:47:13 +0100 Subject: [PATCH 2/4] Add tests for encrypring text with non canonical line ends --- crypto/message_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/crypto/message_test.go b/crypto/message_test.go index 17be0d6..439f6d9 100644 --- a/crypto/message_test.go +++ b/crypto/message_test.go @@ -116,6 +116,52 @@ func TestTextMessageEncryption(t *testing.T) { assert.Exactly(t, message.GetString(), decrypted.GetString()) } +func TestTextMessageEncryptionWithTrailingSpaces(t *testing.T) { + var original = "The secret code is... 1, 2, 3, 4, 5. I repeat: the secret code is... 1, 2, 3, 4, 5 " + var message = NewPlainMessageFromString(original) + + ciphertext, err := keyRingTestPublic.Encrypt(message, nil) + if err != nil { + t.Fatal("Expected no error when encrypting, got:", err) + } + + split, err := ciphertext.SplitMessage() + if err != nil { + t.Fatal("Expected no error when splitting, got:", err) + } + + assert.Len(t, split.GetBinaryDataPacket(), 133) // Assert uncompressed encrypted body length + + decrypted, err := keyRingTestPrivate.Decrypt(ciphertext, nil, 0) + if err != nil { + t.Fatal("Expected no error when decrypting, got:", err) + } + assert.Exactly(t, original, decrypted.GetString()) +} + +func TestTextMessageEncryptionWithNonCanonicalLinebreak(t *testing.T) { + var original = "The secret code is... 1, 2, 3, 4, 5. I repeat: the secret code is... 1, 2, 3, 4, 5 \n \n" + var message = NewPlainMessageFromString(original) + + ciphertext, err := keyRingTestPublic.Encrypt(message, nil) + if err != nil { + t.Fatal("Expected no error when encrypting, got:", err) + } + + split, err := ciphertext.SplitMessage() + if err != nil { + t.Fatal("Expected no error when splitting, got:", err) + } + + assert.Len(t, split.GetBinaryDataPacket(), 133) // Assert uncompressed encrypted body length + + decrypted, err := keyRingTestPrivate.Decrypt(ciphertext, nil, 0) + if err != nil { + t.Fatal("Expected no error when decrypting, got:", err) + } + assert.Exactly(t, original, decrypted.GetString()) +} + func TestTextMessageEncryptionWithCompression(t *testing.T) { var message = NewPlainMessageFromString( "The secret code is... 1, 2, 3, 4, 5. I repeat: the secret code is... 1, 2, 3, 4, 5", From 2b9d76708a0f6a31cf27e006a43d1315c86a4bcf Mon Sep 17 00:00:00 2001 From: "M. Thiercelin" Date: Thu, 17 Nov 2022 15:50:17 +0100 Subject: [PATCH 3/4] Size checks not needed in linebreaks unit tests --- crypto/message_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/crypto/message_test.go b/crypto/message_test.go index 439f6d9..906955a 100644 --- a/crypto/message_test.go +++ b/crypto/message_test.go @@ -125,13 +125,6 @@ func TestTextMessageEncryptionWithTrailingSpaces(t *testing.T) { t.Fatal("Expected no error when encrypting, got:", err) } - split, err := ciphertext.SplitMessage() - if err != nil { - t.Fatal("Expected no error when splitting, got:", err) - } - - assert.Len(t, split.GetBinaryDataPacket(), 133) // Assert uncompressed encrypted body length - decrypted, err := keyRingTestPrivate.Decrypt(ciphertext, nil, 0) if err != nil { t.Fatal("Expected no error when decrypting, got:", err) @@ -148,13 +141,6 @@ func TestTextMessageEncryptionWithNonCanonicalLinebreak(t *testing.T) { t.Fatal("Expected no error when encrypting, got:", err) } - split, err := ciphertext.SplitMessage() - if err != nil { - t.Fatal("Expected no error when splitting, got:", err) - } - - assert.Len(t, split.GetBinaryDataPacket(), 133) // Assert uncompressed encrypted body length - decrypted, err := keyRingTestPrivate.Decrypt(ciphertext, nil, 0) if err != nil { t.Fatal("Expected no error when decrypting, got:", err) From d357a230f0844bd4272e86fbef11e2f014c74596 Mon Sep 17 00:00:00 2001 From: Daniel Huigens Date: Thu, 17 Nov 2022 18:45:37 +0100 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0871e3d..b8bc66e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Updated `github.com/ProtonMail/go-mime` to latest versions, which cleans up uneeded dependencies. And fix an issue with PGP/MIME messages with non standard encodings. - Sanitize strings returned in `MIMECallbacks.OnBody()` and `PlainMessage.GetString()`. Strings that have non utf8 characters will be sanitized to have the "character unknown" character : � instead. - Detached sign text messages with signature type text. Similarly, clearsigned messages now also use signature type text. +- Leave trailing spaces of text messages intact (except for clearsigned messages, where the spec requires us to trim trailing spaces). Note that for backwards compatibility, when verifying detached signatures over text messages, the application will have to trim trailing spaces in order for the signature to verify, if it was created by a previous version of this library (using `crypto.NewPlainMessageFromString()`). ## [2.4.10] 2022-08-22 ### Changed