From 3bd39b3ea5cdc84e43c18bb98adab6b009a7b3d7 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Fri, 3 Feb 2023 11:47:04 +0100 Subject: [PATCH] fix(GODT-1804): Only promote content headers if non-empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When attaching public key, we take the root mime part, create a new root, and put the old root alongside an additional public key mime part. But when moving the root, we would copy all content headers, even empty ones. So we’d be left with Content-Disposition: "" which would fail to parse. --- go.mod | 2 +- go.sum | 4 +- internal/bridge/send_test.go | 113 ++++++++++++++++++++++++++++ internal/bridge/testdata/invite.eml | 85 +++++++++++++++++++++ internal/user/smtp.go | 2 +- pkg/message/parser/part.go | 14 +++- 6 files changed, 213 insertions(+), 7 deletions(-) create mode 100644 internal/bridge/testdata/invite.eml diff --git a/go.mod b/go.mod index 00cc0a86..2ff81425 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Masterminds/semver/v3 v3.1.1 github.com/ProtonMail/gluon v0.14.2-0.20230127085305-bc2d818d9d13 github.com/ProtonMail/go-autostart v0.0.0-20210130080809-00ed301c8e9a - github.com/ProtonMail/go-proton-api v0.3.1-0.20230126112849-3c1ac277855e + github.com/ProtonMail/go-proton-api v0.3.1-0.20230203120457-1849bf7d578b github.com/ProtonMail/go-rfc5322 v0.11.0 github.com/ProtonMail/gopenpgp/v2 v2.4.10 github.com/PuerkitoBio/goquery v1.8.0 diff --git a/go.sum b/go.sum index 6456748f..36fd949a 100644 --- a/go.sum +++ b/go.sum @@ -41,8 +41,8 @@ github.com/ProtonMail/go-message v0.0.0-20210611055058-fabeff2ec753/go.mod h1:NB github.com/ProtonMail/go-mime v0.0.0-20220302105931-303f85f7fe0f/go.mod h1:NYt+V3/4rEeDuaev/zw1zCq8uqVEuPHzDPo3OZrlGJ4= github.com/ProtonMail/go-mime v0.0.0-20220429130430-2192574d760f h1:4IWzKjHzZxdrW9k4zl/qCwenOVHDbVDADPPHFLjs0Oc= github.com/ProtonMail/go-mime v0.0.0-20220429130430-2192574d760f/go.mod h1:qRZgbeASl2a9OwmsV85aWwRqic0NHPh+9ewGAzb4cgM= -github.com/ProtonMail/go-proton-api v0.3.1-0.20230126112849-3c1ac277855e h1:UkfLQc44UvknNCLoBEZb1qg7zfVWVLMvCE/LtdVEcAw= -github.com/ProtonMail/go-proton-api v0.3.1-0.20230126112849-3c1ac277855e/go.mod h1:JUo5IQG0hNuPRuDpOUsCOvtee6UjTEHHF1QN2i8RSos= +github.com/ProtonMail/go-proton-api v0.3.1-0.20230203120457-1849bf7d578b h1:3UX0j2WA3NsqmrjTK769sCIOX2KnISfo8tZdFl7+mRE= +github.com/ProtonMail/go-proton-api v0.3.1-0.20230203120457-1849bf7d578b/go.mod h1:JUo5IQG0hNuPRuDpOUsCOvtee6UjTEHHF1QN2i8RSos= github.com/ProtonMail/go-rfc5322 v0.11.0 h1:o5Obrm4DpmQEffvgsVqG6S4BKwC1Wat+hYwjIp2YcCY= github.com/ProtonMail/go-rfc5322 v0.11.0/go.mod h1:6oOKr0jXvpoE6pwTx/HukigQpX2J9WUf6h0auplrFTw= github.com/ProtonMail/go-srp v0.0.5 h1:xhUioxZgDbCnpo9JehyFhwwsn9JLWkUGfB0oiKXgiGg= diff --git a/internal/bridge/send_test.go b/internal/bridge/send_test.go index 7f009f61..b9e6e0ca 100644 --- a/internal/bridge/send_test.go +++ b/internal/bridge/send_test.go @@ -18,10 +18,12 @@ package bridge_test import ( + "bytes" "context" "crypto/tls" "fmt" "net" + "os" "strings" "testing" "time" @@ -217,3 +219,114 @@ func TestBridge_SendDraftFlags(t *testing.T) { }) }) } + +func TestBridge_SendInvite(t *testing.T) { + withEnv(t, func(ctx context.Context, s *server.Server, netCtl *proton.NetCtl, locator bridge.Locator, storeKey []byte) { + // Create a recipient user. + _, _, err := s.CreateUser("recipient", password) + require.NoError(t, err) + + // Set "attach public keys" to true for the user. + withClient(ctx, t, s, username, password, func(ctx context.Context, client *proton.Client) { + settings, err := client.SetAttachPublicKey(ctx, proton.SetAttachPublicKeyReq{AttachPublicKey: true}) + require.NoError(t, err) + require.Equal(t, proton.Bool(true), settings.AttachPublicKey) + }) + + // The sender should be fully synced. + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { + syncCh, done := chToType[events.Event, events.SyncFinished](bridge.GetEvents(events.SyncFinished{})) + defer done() + + userID, err := bridge.LoginFull(ctx, username, password, nil, nil) + require.NoError(t, err) + + require.Equal(t, userID, (<-syncCh).UserID) + }) + + // Start the bridge. + withBridge(ctx, t, s.GetHostURL(), netCtl, locator, storeKey, func(bridge *bridge.Bridge, _ *bridge.Mocks) { + // Get the sender user info. + userInfo, err := bridge.QueryUserInfo(username) + require.NoError(t, err) + + // Connect the sender IMAP client. + imapClient, err := client.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetIMAPPort()))) + require.NoError(t, err) + require.NoError(t, imapClient.Login(userInfo.Addresses[0], string(userInfo.BridgePass))) + defer imapClient.Logout() //nolint:errcheck + + // The message to send. + b, err := os.ReadFile("testdata/invite.eml") + require.NoError(t, err) + + // Save a draft. + require.NoError(t, imapClient.Append("Drafts", []string{imap.DraftFlag}, time.Now(), bytes.NewReader(b))) + + // Assert that the draft exists and is marked as a draft. + { + messages, err := clientFetch(imapClient, "Drafts") + require.NoError(t, err) + require.Len(t, messages, 1) + require.Contains(t, messages[0].Flags, imap.DraftFlag) + } + + // Connect the SMTP client. + smtpClient, err := smtp.Dial(net.JoinHostPort(constants.Host, fmt.Sprint(bridge.GetSMTPPort()))) + require.NoError(t, err) + defer smtpClient.Close() //nolint:errcheck + + // Upgrade to TLS. + require.NoError(t, smtpClient.StartTLS(&tls.Config{InsecureSkipVerify: true})) + + // Authorize with SASL PLAIN. + require.NoError(t, smtpClient.Auth(sasl.NewPlainClient( + userInfo.Addresses[0], + userInfo.Addresses[0], + string(userInfo.BridgePass)), + )) + + // Send the message. + require.NoError(t, smtpClient.SendMail( + userInfo.Addresses[0], + []string{"recipient@" + s.GetDomain()}, + bytes.NewReader(b), + )) + + // Delete the draft: add the \Deleted flag and expunge. + { + status, err := imapClient.Select("Drafts", false) + require.NoError(t, err) + require.Equal(t, uint32(1), status.Messages) + + // Add the \Deleted flag. + require.NoError(t, clientStore(imapClient, 1, 1, true, imap.FormatFlagsOp(imap.AddFlags, true), imap.DeletedFlag)) + + // Expunge. + require.NoError(t, imapClient.Expunge(nil)) + } + + // Assert that the draft is eventually gone. + require.Eventually(t, func() bool { + status, err := imapClient.Select("Drafts", false) + require.NoError(t, err) + return status.Messages == 0 + }, 10*time.Second, 100*time.Millisecond) + + // Assert that the message is eventually in the sent folder. + require.Eventually(t, func() bool { + messages, err := clientFetch(imapClient, "Sent") + require.NoError(t, err) + return len(messages) == 1 + }, 10*time.Second, 100*time.Millisecond) + + // Assert that the message is not marked as a draft. + { + messages, err := clientFetch(imapClient, "Sent") + require.NoError(t, err) + require.Len(t, messages, 1) + require.NotContains(t, messages[0].Flags, imap.DraftFlag) + } + }) + }) +} diff --git a/internal/bridge/testdata/invite.eml b/internal/bridge/testdata/invite.eml new file mode 100644 index 00000000..b9efbc92 --- /dev/null +++ b/internal/bridge/testdata/invite.eml @@ -0,0 +1,85 @@ +From: +To: +Subject: Testing calendar invite +Date: Fri, 3 Feb 2023 01:04:32 +0100 +Message-ID: <000001d93763$183b74e0$48b25ea0$@proton.local> +MIME-Version: 1.0 +Content-Type: text/calendar; method=REQUEST; + charset="utf-8" +Content-Transfer-Encoding: 7bit +X-Mailer: Microsoft Outlook 16.0 +Thread-Index: Adk3Yw5pLdgwsT46RviXb/nfvQlesQAAAmGA +Content-Language: en-gb + +BEGIN:VCALENDAR +PRODID:-//Microsoft Corporation//Outlook 16.0 MIMEDIR//EN +VERSION:2.0 +METHOD:REQUEST +X-MS-OLK-FORCEINSPECTOROPEN:TRUE +BEGIN:VTIMEZONE +TZID:Central European Standard Time +BEGIN:STANDARD +DTSTART:16011028T030000 +RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=10 +TZOFFSETFROM:+0200 +TZOFFSETTO:+0100 +END:STANDARD +BEGIN:DAYLIGHT +DTSTART:16010325T020000 +RRULE:FREQ=YEARLY;BYDAY=-1SU;BYMONTH=3 +TZOFFSETFROM:+0100 +TZOFFSETTO:+0200 +END:DAYLIGHT +END:VTIMEZONE +BEGIN:VEVENT +ATTENDEE;CN=recipient@proton.local;RSVP=TRUE:mailto:recipient@proton.local +CLASS:PUBLIC +CREATED:20230203T000432Z +DESCRIPTION:qweqweqweqweqweqwe/gn\\n +DTEND;TZID="Central European Standard Time":20230203T020000 +DTSTAMP:20230203T000432Z +DTSTART;TZID="Central European Standard Time":20230203T013000 +LAST-MODIFIED:20230203T000432Z +LOCATION:qweqwe +ORGANIZER;CN=username@proton.local:mailto:username@proton.local +PRIORITY:5 +SEQUENCE:0 +SUMMARY;LANGUAGE=en-gb:Testing calendar invite +TRANSP:OPAQUE +UID:040000008200E00074C5B7101A82E008000000003080B2796B37D901000000000000000 + 0100000001236CD1CD93CA9449C6FF1AC4DEAC44E +X-ALT-DESC;FMTTYPE=text/html:

qweqweqweqweqweqw + e

+X-MICROSOFT-CDO-BUSYSTATUS:TENTATIVE +X-MICROSOFT-CDO-IMPORTANCE:1 +X-MICROSOFT-CDO-INTENDEDSTATUS:BUSY +X-MICROSOFT-DISALLOW-COUNTER:FALSE +X-MS-OLK-AUTOSTARTCHECK:FALSE +X-MS-OLK-CONFTYPE:0 +BEGIN:VALARM +TRIGGER:-PT15M +ACTION:DISPLAY +DESCRIPTION:Reminder +END:VALARM +END:VEVENT +END:VCALENDAR + diff --git a/internal/user/smtp.go b/internal/user/smtp.go index 09080e54..c2699ce1 100644 --- a/internal/user/smtp.go +++ b/internal/user/smtp.go @@ -120,7 +120,7 @@ func (user *User) sendMail(authID string, from string, to []string, r io.Reader) } // If we have to attach the public key, do it now. - if settings.AttachPublicKey == proton.AttachPublicKeyEnabled { + if settings.AttachPublicKey { key, err := addrKR.GetKey(0) if err != nil { return fmt.Errorf("failed to get sending key: %w", err) diff --git a/pkg/message/parser/part.go b/pkg/message/parser/part.go index 1f1309d2..3248fa3d 100644 --- a/pkg/message/parser/part.go +++ b/pkg/message/parser/part.go @@ -186,9 +186,17 @@ func (p *Part) isMultipartMixed() bool { func getContentHeaders(header message.Header) message.Header { var res message.Header - res.Set("Content-Type", header.Get("Content-Type")) - res.Set("Content-Disposition", header.Get("Content-Disposition")) - res.Set("Content-Transfer-Encoding", header.Get("Content-Transfer-Encoding")) + if contentType := header.Get("Content-Type"); contentType != "" { + res.Set("Content-Type", contentType) + } + + if contentDisposition := header.Get("Content-Disposition"); contentDisposition != "" { + res.Set("Content-Disposition", contentDisposition) + } + + if contentTransferEncoding := header.Get("Content-Transfer-Encoding"); contentTransferEncoding != "" { + res.Set("Content-Transfer-Encoding", contentTransferEncoding) + } return res }