Merge pull request #200 from ProtonMail/dont-trim-trailing-spaces

Don't trim trailing spaces from non-clearsigned text messages

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 : <20> 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

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()),

crypto/message_test.go

@@ -116,6 +116,38 @@ 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)
+	}
+
+	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)
+	}
+
+	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",

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)

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 {

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()))
 }

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