GODT-1193: Don't doubly encode parts

pkg/message/build_framework_test.go

@@ -67,6 +67,10 @@ func newTestMessage(
 	arm, err := enc.GetArmored()
 	require.NoError(t, err)
 
+	return newRawTestMessage(messageID, addressID, mimeType, arm, date)
+}
+
+func newRawTestMessage(messageID, addressID, mimeType, body string, date time.Time) *pmapi.Message {
 	return &pmapi.Message{
 		ID:        messageID,
 		AddressID: addressID,
@@ -75,7 +79,7 @@ func newTestMessage(
 			"Content-Type": {mimeType},
 			"Date":         {date.In(time.UTC).Format(time.RFC1123Z)},
 		},
-		Body: arm,
+		Body: body,
 		Time: date.Unix(),
 	}
 }

pkg/message/build_rfc822.go

@@ -30,6 +30,7 @@ import (
 	"github.com/ProtonMail/gopenpgp/v2/crypto"
 	"github.com/ProtonMail/proton-bridge/pkg/pmapi"
 	"github.com/emersion/go-message"
+	"github.com/emersion/go-message/textproto"
 	"github.com/pkg/errors"
 )
 
@@ -279,13 +280,21 @@ func buildPGPMIMEFallbackRFC822(msg *pmapi.Message, opts JobOptions) ([]byte, er
 func writeMultipartSignedRFC822(header message.Header, body []byte, sig pmapi.Signature) ([]byte, error) { //nolint[funlen]
 	buf := new(bytes.Buffer)
 
+	boundary := newBoundary("").gen()
+
 	header.SetContentType("multipart/signed", map[string]string{
 		"micalg":   sig.Hash,
 		"protocol": "application/pgp-signature",
+		"boundary": boundary,
 	})
 
-	w, err := message.CreateWriter(buf, header)
+	if err := textproto.WriteHeader(buf, header.Header); err != nil {
-	if err != nil {
+		return nil, err
+	}
+
+	mw := textproto.NewMultipartWriter(buf)
+
+	if err := mw.SetBoundary(boundary); err != nil {
 		return nil, err
 	}
 
@@ -294,7 +303,7 @@ func writeMultipartSignedRFC822(header message.Header, body []byte, sig pmapi.Si
 		return nil, err
 	}
 
-	bodyPart, err := w.CreatePart(message.Header{Header: *bodyHeader})
+	bodyPart, err := mw.CreatePart(*bodyHeader)
 	if err != nil {
 		return nil, err
 	}
@@ -303,17 +312,13 @@ func writeMultipartSignedRFC822(header message.Header, body []byte, sig pmapi.Si
 		return nil, err
 	}
 
-	if err := bodyPart.Close(); err != nil {
-		return nil, err
-	}
-
 	var sigHeader message.Header
 
 	sigHeader.SetContentType("application/pgp-signature", map[string]string{"name": "OpenPGP_signature.asc"})
 	sigHeader.SetContentDisposition("attachment", map[string]string{"filename": "OpenPGP_signature"})
 	sigHeader.Set("Content-Description", "OpenPGP digital signature")
 
-	sigPart, err := w.CreatePart(sigHeader)
+	sigPart, err := mw.CreatePart(sigHeader.Header)
 	if err != nil {
 		return nil, err
 	}
@@ -327,11 +332,7 @@ func writeMultipartSignedRFC822(header message.Header, body []byte, sig pmapi.Si
 		return nil, err
 	}
 
-	if err := sigPart.Close(); err != nil {
+	if err := mw.Close(); err != nil {
-		return nil, err
-	}
-
-	if err := w.Close(); err != nil {
 		return nil, err
 	}
 
@@ -352,16 +353,11 @@ func writeMultipartEncryptedRFC822(header message.Header, body []byte) ([]byte,
 		header.Set(entFields.Key(), entFields.Value())
 	}
 
-	w, err := message.CreateWriter(buf, header)
+	if err := textproto.WriteHeader(buf, header.Header); err != nil {
-	if err != nil {
 		return nil, err
 	}
 
-	if _, err := w.Write(bodyData); err != nil {
+	if _, err := buf.Write(bodyData); err != nil {
-		return nil, err
-	}
-
-	if err := w.Close(); err != nil {
 		return nil, err
 	}
 

pkg/message/build_test.go

@@ -25,6 +25,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/ProtonMail/gopenpgp/v2/crypto"
 	"github.com/ProtonMail/proton-bridge/pkg/message/mocks"
 	tests "github.com/ProtonMail/proton-bridge/test"
 	"github.com/golang/mock/gomock"
@@ -152,6 +153,89 @@ func TestBuildHTMLEncryptedMessage(t *testing.T) {
 		expectBody(contains(`Where do boats go when they're sick`))
 }
 
+func TestBuildPlainSignedMessage(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	body := readerToString(getFileReader("text_plain.eml"))
+
+	kr := tests.MakeKeyRing(t)
+	sig := tests.MakeKeyRing(t)
+
+	enc, err := kr.Encrypt(crypto.NewPlainMessageFromString(body), sig)
+	require.NoError(t, err)
+
+	arm, err := enc.GetArmored()
+	require.NoError(t, err)
+
+	msg := newRawTestMessage("messageID", "addressID", "multipart/mixed", arm, time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
+
+	res, err := b.NewJob(context.Background(), newTestFetcher(m, kr, msg), msg.ID).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectContentType(is(`multipart/signed`)).
+		expectContentTypeParam(`micalg`, is(`SHA-256`)). // NOTE: Maybe this is bad... should probably be pgp-sha256
+		expectContentTypeParam(`protocol`, is(`application/pgp-signature`)).
+		expectDate(is(`Wed, 01 Jan 2020 00:00:00 +0000`))
+
+	section(t, res, 1).
+		expectContentType(is(`text/plain`)).
+		expectBody(is(`body`)).
+		expectSection(verifiesAgainst(sig, section(t, res, 2).signature()))
+
+	section(t, res, 2).
+		expectContentType(is(`application/pgp-signature`)).
+		expectContentTypeParam(`name`, is(`OpenPGP_signature.asc`)).
+		expectContentDisposition(is(`attachment`)).
+		expectContentDispositionParam(`filename`, is(`OpenPGP_signature`))
+}
+
+func TestBuildPlainSignedBase64Message(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	body := readerToString(getFileReader("text_plain_base64.eml"))
+
+	kr := tests.MakeKeyRing(t)
+	sig := tests.MakeKeyRing(t)
+
+	enc, err := kr.Encrypt(crypto.NewPlainMessageFromString(body), sig)
+	require.NoError(t, err)
+
+	arm, err := enc.GetArmored()
+	require.NoError(t, err)
+
+	msg := newRawTestMessage("messageID", "addressID", "multipart/mixed", arm, time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
+
+	res, err := b.NewJob(context.Background(), newTestFetcher(m, kr, msg), msg.ID).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectContentType(is(`multipart/signed`)).
+		expectContentTypeParam(`micalg`, is(`SHA-256`)). // NOTE: Maybe this is bad... should probably be pgp-sha256
+		expectContentTypeParam(`protocol`, is(`application/pgp-signature`)).
+		expectDate(is(`Wed, 01 Jan 2020 00:00:00 +0000`))
+
+	section(t, res, 1).
+		expectContentType(is(`text/plain`)).
+		expectTransferEncoding(is(`base64`)).
+		expectBody(is(`body`)).
+		expectSection(verifiesAgainst(sig, section(t, res, 2).signature()))
+
+	section(t, res, 2).
+		expectContentType(is(`application/pgp-signature`)).
+		expectContentTypeParam(`name`, is(`OpenPGP_signature.asc`)).
+		expectContentDisposition(is(`attachment`)).
+		expectContentDispositionParam(`filename`, is(`OpenPGP_signature`))
+}
+
 func TestBuildSignedPlainEncryptedMessage(t *testing.T) {
 	m := gomock.NewController(t)
 	defer m.Finish()

pkg/message/header.go

@@ -85,11 +85,14 @@ func readHeaderBody(b []byte) (*textproto.Header, []byte, error) {
 		return nil, nil, err
 	}
 
+	lines := HeaderLines(rawHeader)
+
 	var header textproto.Header
 
-	for _, line := range HeaderLines(rawHeader) {
+	// We add lines in reverse so that calling textproto.WriteHeader later writes with the correct order.
-		if len(bytes.TrimSpace(line)) > 0 {
+	for i := len(lines) - 1; i >= 0; i-- {
-			header.AddRaw(line)
+		if len(bytes.TrimSpace(lines[i])) > 0 {
+			header.AddRaw(lines[i])
 		}
 	}
 

pkg/message/testdata/text_plain_base64.eml

@@ -0,0 +1,5 @@
+From: Sender <sender@pm.me>
+To: Receiver <receiver@pm.me>
+Content-Transfer-Encoding: base64
+
+Ym9keQ==