From 7b112fc4486b94812e457a8ca4f8c68b33534354 Mon Sep 17 00:00:00 2001 From: Michal Horejsek Date: Wed, 16 Dec 2020 11:17:00 +0100 Subject: [PATCH] Prefer From header instead of MAIL FROM address --- internal/imap/mailbox_message.go | 2 +- internal/smtp/user.go | 87 ++++++++++++------- internal/transfer/provider_pmapi_target.go | 2 +- pkg/message/parser.go | 41 ++++++--- pkg/message/parser_test.go | 64 +++++++------- test/fakeapi/messages.go | 2 +- test/features/bridge/smtp/init.feature | 19 ++-- test/features/bridge/smtp/send/bcc.feature | 4 +- .../bridge/smtp/send/failures.feature | 13 ++- test/features/bridge/smtp/send/html.feature | 8 +- .../bridge/smtp/send/html_att.feature | 4 +- test/features/bridge/smtp/send/plain.feature | 14 +-- .../bridge/smtp/send/plain_att.feature | 6 +- .../bridge/smtp/send/same_message.feature | 6 +- .../bridge/smtp/send/two_messages.feature | 12 +-- test/mocks/smtp.go | 2 + unreleased.md | 3 + 17 files changed, 179 insertions(+), 110 deletions(-) diff --git a/internal/imap/mailbox_message.go b/internal/imap/mailbox_message.go index e127fa12..2893672a 100644 --- a/internal/imap/mailbox_message.go +++ b/internal/imap/mailbox_message.go @@ -70,7 +70,7 @@ func (im *imapMailbox) CreateMessage(flags []string, date time.Time, body imap.L // Called from go-imap in goroutines - we need to handle panics for each function. defer im.panicHandler.HandlePanic() - m, _, _, readers, err := message.Parse(body, "", "") + m, _, _, readers, err := message.Parse(body) if err != nil { return err } diff --git a/internal/smtp/user.go b/internal/smtp/user.go index 5442647c..b79209b5 100644 --- a/internal/smtp/user.go +++ b/internal/smtp/user.go @@ -31,7 +31,8 @@ import ( "github.com/ProtonMail/gopenpgp/v2/crypto" "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/pkg/listener" - "github.com/ProtonMail/proton-bridge/pkg/message" + pkgMessage "github.com/ProtonMail/proton-bridge/pkg/message" + "github.com/ProtonMail/proton-bridge/pkg/message/parser" "github.com/ProtonMail/proton-bridge/pkg/pmapi" goSMTPBackend "github.com/emersion/go-smtp" "github.com/pkg/errors" @@ -47,8 +48,8 @@ type smtpUser struct { username string addressID string - from string - to []string + returnPath string + to []string } // newSMTPUser returns struct implementing go-smtp/session interface. @@ -154,13 +155,13 @@ func (su *smtpUser) getAPIKeyData(recipient string) (apiKeys []pmapi.PublicKey, // Discard currently processed message. func (su *smtpUser) Reset() { log.Trace("Resetting the session") - su.from = "" + su.returnPath = "" su.to = []string{} } // Set return path for currently processed message. -func (su *smtpUser) Mail(from string, opts goSMTPBackend.MailOptions) error { - log.WithField("from", from).WithField("opts", opts).Trace("Setting mail from") +func (su *smtpUser) Mail(returnPath string, opts goSMTPBackend.MailOptions) error { + log.WithField("returnPath", returnPath).WithField("opts", opts).Trace("Setting mail from") // REQUIRETLS and SMTPUTF8 have to be announced to be used by client. // Bridge does not use those extensions so this should not happen. @@ -175,7 +176,14 @@ func (su *smtpUser) Mail(from string, opts goSMTPBackend.MailOptions) error { return errors.New("changing identity is not supported") } - su.from = from + if returnPath != "" { + addr := su.client().Addresses().ByEmail(returnPath) + if addr == nil { + return errors.New("backend: invalid return path: not owned by user") + } + } + + su.returnPath = returnPath return nil } @@ -191,17 +199,17 @@ func (su *smtpUser) Rcpt(to string) error { // Set currently processed message contents and send it. func (su *smtpUser) Data(r io.Reader) error { log.Trace("Sending the message") - if su.from == "" { - return errors.New("missing sender") + if su.returnPath == "" { + return errors.New("missing return path") } if len(su.to) == 0 { return errors.New("missing recipient") } - return su.Send(su.from, su.to, r) + return su.Send(su.returnPath, su.to, r) } // Send sends an email from the given address to the given addresses with the given body. -func (su *smtpUser) Send(from string, to []string, messageReader io.Reader) (err error) { //nolint[funlen] +func (su *smtpUser) Send(returnPath string, to []string, messageReader io.Reader) (err error) { //nolint[funlen] // Called from go-smtp in goroutines - we need to handle panics for each function. defer su.panicHandler.HandlePanic() @@ -210,7 +218,34 @@ func (su *smtpUser) Send(from string, to []string, messageReader io.Reader) (err return err } - var addr *pmapi.Address = su.client().Addresses().ByEmail(from) + returnPathAddr := su.client().Addresses().ByEmail(returnPath) + if returnPathAddr == nil { + err = errors.New("backend: invalid return path: not owned by user") + return + } + + parser, err := parser.New(messageReader) + if err != nil { + err = errors.Wrap(err, "failed to create new parser") + return + } + message, plainBody, attReaders, err := pkgMessage.ParserWithParser(parser) + if err != nil { + log.WithError(err).Error("Failed to parse message") + return + } + richBody := message.Body + + externalID := message.Header.Get("Message-Id") + externalID = strings.Trim(externalID, "<>") + + draftID, parentID := su.handleReferencesHeader(message) + + if err = su.handleSenderAndRecipients(message, returnPathAddr, returnPath, to); err != nil { + return err + } + + addr := su.client().Addresses().ByEmail(message.Sender.Address) if addr == nil { err = errors.New("backend: invalid email address: not owned by user") return @@ -237,20 +272,14 @@ func (su *smtpUser) Send(from string, to []string, messageReader io.Reader) (err attachedPublicKeyName = fmt.Sprintf("publickey - %v - %v", kr.GetIdentities()[0].Name, firstKey.GetFingerprint()[:8]) } - message, mimeBody, plainBody, attReaders, err := message.Parse(messageReader, attachedPublicKey, attachedPublicKeyName) - if err != nil { - log.WithError(err).Error("Failed to parse message") - return + if attachedPublicKey != "" { + pkgMessage.AttachPublicKey(parser, attachedPublicKey, attachedPublicKeyName) } - richBody := message.Body - externalID := message.Header.Get("Message-Id") - externalID = strings.Trim(externalID, "<>") - - draftID, parentID := su.handleReferencesHeader(message) - - if err = su.handleSenderAndRecipients(message, addr, from, to); err != nil { - return err + mimeBody, err := pkgMessage.BuildMIMEBody(parser) + if err != nil { + log.WithError(err).Error("Failed to build message") + return } message.AddressID = addr.ID @@ -415,14 +444,14 @@ func (su *smtpUser) handleReferencesHeader(m *pmapi.Message) (draftID, parentID return draftID, parentID } -func (su *smtpUser) handleSenderAndRecipients(m *pmapi.Message, addr *pmapi.Address, from string, to []string) (err error) { - from = pmapi.ConstructAddress(from, addr.Email) +func (su *smtpUser) handleSenderAndRecipients(m *pmapi.Message, returnPathAddr *pmapi.Address, returnPath string, to []string) (err error) { + returnPath = pmapi.ConstructAddress(returnPath, returnPathAddr.Email) // Check sender. if m.Sender == nil { - m.Sender = &mail.Address{Address: from} - } else { - m.Sender.Address = from + m.Sender = &mail.Address{Address: returnPath} + } else if m.Sender.Address == "" { + m.Sender.Address = returnPath } // Check recipients. diff --git a/internal/transfer/provider_pmapi_target.go b/internal/transfer/provider_pmapi_target.go index cd542568..b788bf1e 100644 --- a/internal/transfer/provider_pmapi_target.go +++ b/internal/transfer/provider_pmapi_target.go @@ -263,7 +263,7 @@ func (p *PMAPIProvider) parseMessage(msg Message) (m *pmapi.Message, r []io.Read } } }() - message, _, _, attachmentReaders, err := pkgMessage.Parse(bytes.NewBuffer(msg.Body), "", "") + message, _, _, attachmentReaders, err := pkgMessage.Parse(bytes.NewBuffer(msg.Body)) return message, attachmentReaders, err } diff --git a/pkg/message/parser.go b/pkg/message/parser.go index d06a0288..63e77625 100644 --- a/pkg/message/parser.go +++ b/pkg/message/parser.go @@ -36,15 +36,30 @@ import ( "github.com/sirupsen/logrus" ) -func Parse(r io.Reader, key, keyName string) (m *pmapi.Message, mimeBody, plainBody string, attReaders []io.Reader, err error) { - logrus.Trace("Parsing message") - +// Parse parses RAW message. +func Parse(r io.Reader) (m *pmapi.Message, mimeBody, plainBody string, attReaders []io.Reader, err error) { p, err := parser.New(r) if err != nil { - err = errors.Wrap(err, "failed to create new parser") - return + return nil, "", "", nil, errors.Wrap(err, "failed to create new parser") } + m, plainBody, attReaders, err = ParserWithParser(p) + if err != nil { + return nil, "", "", nil, errors.Wrap(err, "failed to parse the message") + } + + mimeBody, err = BuildMIMEBody(p) + if err != nil { + return nil, "", "", nil, errors.Wrap(err, "failed to build mime body") + } + + return m, mimeBody, plainBody, attReaders, nil +} + +// ParserWithParser parses message from Parser without building MIME body. +func ParserWithParser(p *parser.Parser) (m *pmapi.Message, plainBody string, attReaders []io.Reader, err error) { + logrus.Trace("Parsing message") + if err = convertEncodedTransferEncoding(p); err != nil { err = errors.Wrap(err, "failed to convert encoded transfer encodings") return @@ -77,13 +92,11 @@ func Parse(r io.Reader, key, keyName string) (m *pmapi.Message, mimeBody, plainB return } - // We only attach the public key manually to the MIME body for - // signed/encrypted external recipients. It's not important for it to be - // collected as an attachment; that's already done when we upload the draft. - if key != "" { - attachPublicKey(p.Root(), key, keyName) - } + return m, plainBody, attReaders, nil +} +// BuildMIMEBody builds mime body from the parser returned by NewParser. +func BuildMIMEBody(p *parser.Parser) (mimeBody string, err error) { mimeBodyBuffer := new(bytes.Buffer) if err = p.NewWriter().Write(mimeBodyBuffer); err != nil { @@ -91,7 +104,7 @@ func Parse(r io.Reader, key, keyName string) (m *pmapi.Message, mimeBody, plainB return } - return m, mimeBodyBuffer.String(), plainBody, attReaders, nil + return mimeBodyBuffer.String(), nil } // convertEncodedTransferEncoding decodes any RFC2047-encoded content transfer encodings. @@ -381,14 +394,14 @@ func getPlainBody(part *parser.Part) []byte { } } -func attachPublicKey(p *parser.Part, key, keyName string) { +func AttachPublicKey(p *parser.Parser, key, keyName string) { h := message.Header{} h.Set("Content-Type", fmt.Sprintf(`application/pgp-keys; name="%v.asc"; filename="%v.asc"`, keyName, keyName)) h.Set("Content-Disposition", fmt.Sprintf(`attachment; name="%v.asc"; filename="%v.asc"`, keyName, keyName)) h.Set("Content-Transfer-Encoding", "base64") - p.AddChild(&parser.Part{ + p.Root().AddChild(&parser.Part{ Header: h, Body: []byte(key), }) diff --git a/pkg/message/parser_test.go b/pkg/message/parser_test.go index 2918b984..ae812765 100644 --- a/pkg/message/parser_test.go +++ b/pkg/message/parser_test.go @@ -25,6 +25,7 @@ import ( "path/filepath" "testing" + "github.com/ProtonMail/proton-bridge/pkg/message/parser" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/text/encoding/charmap" @@ -33,7 +34,7 @@ import ( func TestParseTextPlain(t *testing.T) { f := getFileReader("text_plain.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -48,7 +49,7 @@ func TestParseTextPlain(t *testing.T) { func TestParseTextPlainUTF8(t *testing.T) { f := getFileReader("text_plain_utf8.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -63,7 +64,7 @@ func TestParseTextPlainUTF8(t *testing.T) { func TestParseTextPlainLatin1(t *testing.T) { f := getFileReader("text_plain_latin1.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -78,7 +79,7 @@ func TestParseTextPlainLatin1(t *testing.T) { func TestParseTextPlainUTF8Subject(t *testing.T) { f := getFileReader("text_plain_utf8_subject.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -94,7 +95,7 @@ func TestParseTextPlainUTF8Subject(t *testing.T) { func TestParseTextPlainLatin2Subject(t *testing.T) { f := getFileReader("text_plain_latin2_subject.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -110,7 +111,7 @@ func TestParseTextPlainLatin2Subject(t *testing.T) { func TestParseTextPlainUnknownCharsetIsActuallyLatin1(t *testing.T) { f := getFileReader("text_plain_unknown_latin1.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -125,7 +126,7 @@ func TestParseTextPlainUnknownCharsetIsActuallyLatin1(t *testing.T) { func TestParseTextPlainUnknownCharsetIsActuallyLatin2(t *testing.T) { f := getFileReader("text_plain_unknown_latin2.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -146,7 +147,7 @@ func TestParseTextPlainUnknownCharsetIsActuallyLatin2(t *testing.T) { func TestParseTextPlainAlready7Bit(t *testing.T) { f := getFileReader("text_plain_7bit.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -161,7 +162,7 @@ func TestParseTextPlainAlready7Bit(t *testing.T) { func TestParseTextPlainWithOctetAttachment(t *testing.T) { f := getFileReader("text_plain_octet_attachment.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -177,7 +178,7 @@ func TestParseTextPlainWithOctetAttachment(t *testing.T) { func TestParseTextPlainWithOctetAttachmentGoodFilename(t *testing.T) { f := getFileReader("text_plain_octet_attachment_good_2231_filename.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -194,7 +195,7 @@ func TestParseTextPlainWithOctetAttachmentGoodFilename(t *testing.T) { func TestParseTextPlainWithOctetAttachmentBadFilename(t *testing.T) { f := getFileReader("text_plain_octet_attachment_bad_2231_filename.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -211,7 +212,7 @@ func TestParseTextPlainWithOctetAttachmentBadFilename(t *testing.T) { func TestParseTextPlainWithPlainAttachment(t *testing.T) { f := getFileReader("text_plain_plain_attachment.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -227,7 +228,7 @@ func TestParseTextPlainWithPlainAttachment(t *testing.T) { func TestParseTextPlainEmptyAddresses(t *testing.T) { f := getFileReader("text_plain_empty_addresses.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -242,7 +243,7 @@ func TestParseTextPlainEmptyAddresses(t *testing.T) { func TestParseTextPlainWithImageInline(t *testing.T) { f := getFileReader("text_plain_image_inline.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -262,7 +263,7 @@ func TestParseTextPlainWithImageInline(t *testing.T) { func TestParseTextPlainWithDuplicateCharset(t *testing.T) { f := getFileReader("text_plain_duplicate_charset.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -277,7 +278,7 @@ func TestParseTextPlainWithDuplicateCharset(t *testing.T) { func TestParseWithMultipleTextParts(t *testing.T) { f := getFileReader("multiple_text_parts.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -292,7 +293,7 @@ func TestParseWithMultipleTextParts(t *testing.T) { func TestParseTextHTML(t *testing.T) { f := getFileReader("text_html.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -307,7 +308,7 @@ func TestParseTextHTML(t *testing.T) { func TestParseTextHTMLAlready7Bit(t *testing.T) { f := getFileReader("text_html_7bit.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) assert.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -322,7 +323,7 @@ func TestParseTextHTMLAlready7Bit(t *testing.T) { func TestParseTextHTMLWithOctetAttachment(t *testing.T) { f := getFileReader("text_html_octet_attachment.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -338,7 +339,7 @@ func TestParseTextHTMLWithOctetAttachment(t *testing.T) { func TestParseTextHTMLWithPlainAttachment(t *testing.T) { f := getFileReader("text_html_plain_attachment.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -355,7 +356,7 @@ func TestParseTextHTMLWithPlainAttachment(t *testing.T) { func TestParseTextHTMLWithImageInline(t *testing.T) { f := getFileReader("text_html_image_inline.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) assert.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -375,7 +376,10 @@ func TestParseTextHTMLWithImageInline(t *testing.T) { func TestParseWithAttachedPublicKey(t *testing.T) { f := getFileReader("text_plain.eml") - m, _, plainBody, attReaders, err := Parse(f, "publickey", "publickeyname") + p, err := parser.New(f) + require.NoError(t, err) + m, plainBody, attReaders, err := ParserWithParser(p) + AttachPublicKey(p, "publickey", "publickeyname") require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -392,7 +396,7 @@ func TestParseWithAttachedPublicKey(t *testing.T) { func TestParseTextHTMLWithEmbeddedForeignEncoding(t *testing.T) { f := getFileReader("text_html_embedded_foreign_encoding.eml") - m, _, plainBody, attReaders, err := Parse(f, "", "") + m, _, plainBody, attReaders, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -407,7 +411,7 @@ func TestParseTextHTMLWithEmbeddedForeignEncoding(t *testing.T) { func TestParseMultipartAlternative(t *testing.T) { f := getFileReader("multipart_alternative.eml") - m, _, plainBody, _, err := Parse(f, "", "") + m, _, plainBody, _, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"schizofrenic" `, m.Sender.String()) @@ -428,7 +432,7 @@ func TestParseMultipartAlternative(t *testing.T) { func TestParseMultipartAlternativeNested(t *testing.T) { f := getFileReader("multipart_alternative_nested.eml") - m, _, plainBody, _, err := Parse(f, "", "") + m, _, plainBody, _, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"schizofrenic" `, m.Sender.String()) @@ -449,7 +453,7 @@ func TestParseMultipartAlternativeNested(t *testing.T) { func TestParseMultipartAlternativeLatin1(t *testing.T) { f := getFileReader("multipart_alternative_latin1.eml") - m, _, plainBody, _, err := Parse(f, "", "") + m, _, plainBody, _, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"schizofrenic" `, m.Sender.String()) @@ -470,7 +474,7 @@ func TestParseMultipartAlternativeLatin1(t *testing.T) { func TestParseWithTrailingEndOfMailIndicator(t *testing.T) { f := getFileReader("text_html_trailing_end_of_mail.eml") - m, _, plainBody, _, err := Parse(f, "", "") + m, _, plainBody, _, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -483,7 +487,7 @@ func TestParseWithTrailingEndOfMailIndicator(t *testing.T) { func TestParseEncodedContentType(t *testing.T) { f := getFileReader("rfc2047-content-transfer-encoding.eml") - m, _, plainBody, _, err := Parse(f, "", "") + m, _, plainBody, _, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -495,7 +499,7 @@ func TestParseEncodedContentType(t *testing.T) { func TestParseNonEncodedContentType(t *testing.T) { f := getFileReader("non-encoded-content-transfer-encoding.eml") - m, _, plainBody, _, err := Parse(f, "", "") + m, _, plainBody, _, err := Parse(f) require.NoError(t, err) assert.Equal(t, `"Sender" `, m.Sender.String()) @@ -507,7 +511,7 @@ func TestParseNonEncodedContentType(t *testing.T) { func TestParseEncodedContentTypeBad(t *testing.T) { f := getFileReader("rfc2047-content-transfer-encoding-bad.eml") - _, _, _, _, err := Parse(f, "", "") // nolint[dogsled] + _, _, _, _, err := Parse(f) // nolint[dogsled] require.Error(t, err) } diff --git a/test/fakeapi/messages.go b/test/fakeapi/messages.go index 26227684..f558c497 100644 --- a/test/fakeapi/messages.go +++ b/test/fakeapi/messages.go @@ -211,7 +211,7 @@ func (api *FakePMAPI) Import(importMessageRequests []*pmapi.ImportMsgReq) ([]*pm } func (api *FakePMAPI) generateMessageFromImportRequest(msgReq *pmapi.ImportMsgReq) (*pmapi.Message, error) { - m, _, _, _, err := message.Parse(bytes.NewReader(msgReq.Body), "", "") // nolint[dogsled] + m, _, _, _, err := message.Parse(bytes.NewReader(msgReq.Body)) // nolint[dogsled] if err != nil { return nil, err } diff --git a/test/features/bridge/smtp/init.feature b/test/features/bridge/smtp/init.feature index 606fbe27..056a504a 100644 --- a/test/features/bridge/smtp/init.feature +++ b/test/features/bridge/smtp/init.feature @@ -17,7 +17,7 @@ Feature: SMTP initiation Given there is connected user "user" When SMTP client authenticates "user" Then SMTP response is "OK" - When SMTP client sends "MAIL FROM:" + When SMTP client sends "MAIL FROM:<[userAddress]>" Then SMTP response is "OK" When SMTP client sends "RCPT TO:" Then SMTP response is "OK" @@ -37,7 +37,7 @@ Feature: SMTP initiation Given there is connected user "user" When SMTP client authenticates "user" Then SMTP response is "OK" - When SMTP client sends "MAIL FROM:" + When SMTP client sends "MAIL FROM:<[userAddress]>" Then SMTP response is "OK" When SMTP client sends "DATA" Then SMTP response is "SMTP error: 502 5.5.1 Missing RCPT TO command." @@ -53,13 +53,13 @@ Feature: SMTP initiation When SMTP client sends "DATA" Then SMTP response is "OK" When SMTP client sends "hello\r\n." - Then SMTP response is "SMTP error: 554 5.0.0 Error: transaction failed, blame it on the weather: missing sender" + Then SMTP response is "SMTP error: 554 5.0.0 Error: transaction failed, blame it on the weather: missing return path" Scenario: Send with empty TO Given there is connected user "user" When SMTP client authenticates "user" Then SMTP response is "OK" - When SMTP client sends "MAIL FROM:" + When SMTP client sends "MAIL FROM:<[userAddress]>" Then SMTP response is "OK" When SMTP client sends "RCPT TO:<>" Then SMTP response is "OK" @@ -72,12 +72,19 @@ Feature: SMTP initiation Given there is connected user "user" When SMTP client authenticates "user" Then SMTP response is "OK" - When SMTP client sends "MAIL FROM: BODY=7BIT" + When SMTP client sends "MAIL FROM:<[userAddress]> BODY=7BIT" Then SMTP response is "OK" Scenario: Allow AUTH parameter of MAIL FROM command Given there is connected user "user" When SMTP client authenticates "user" Then SMTP response is "OK" - When SMTP client sends "MAIL FROM: AUTH=<>" + When SMTP client sends "MAIL FROM:<[userAddress]> AUTH=<>" Then SMTP response is "OK" + + Scenario: FROM not owned by user + Given there is connected user "user" + When SMTP client authenticates "user" + Then SMTP response is "OK" + When SMTP client sends "MAIL FROM:" + Then SMTP response is "SMTP error: 451 4.0.0 backend: invalid return path: not owned by user" diff --git a/test/features/bridge/smtp/send/bcc.feature b/test/features/bridge/smtp/send/bcc.feature index 683804f8..790f1dda 100644 --- a/test/features/bridge/smtp/send/bcc.feature +++ b/test/features/bridge/smtp/send/bcc.feature @@ -7,7 +7,7 @@ Feature: SMTP with bcc When SMTP client sends message with bcc "bridgetest2@protonmail.com" """ Subject: hello - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge hello @@ -43,7 +43,7 @@ Feature: SMTP with bcc When SMTP client sends message with bcc "bridgetest@protonmail.com" """ Subject: hello - From: Bridge Test + From: Bridge Test <[userAddress]> hello diff --git a/test/features/bridge/smtp/send/failures.feature b/test/features/bridge/smtp/send/failures.feature index e4b62421..c9a87652 100644 --- a/test/features/bridge/smtp/send/failures.feature +++ b/test/features/bridge/smtp/send/failures.feature @@ -6,7 +6,7 @@ Feature: SMTP wrong messages Scenario: Message with attachment and wrong boundaries When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: With attachment (wrong boundaries) Content-Type: multipart/related; boundary=bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 @@ -39,3 +39,14 @@ Feature: SMTP wrong messages """ Then SMTP response is "SMTP error: 554 5.0.0 Error: transaction failed, blame it on the weather: failed to create new parser: unexpected EOF" + + Scenario: Invalid from + When SMTP client sends message + """ + From: Bridge Test + To: Internal Bridge + + hello + + """ + Then SMTP response is "SMTP error: 554 5.0.0 Error: transaction failed, blame it on the weather: backend: invalid email address: not owned by user" diff --git a/test/features/bridge/smtp/send/html.feature b/test/features/bridge/smtp/send/html.feature index 4524a742..190008dc 100644 --- a/test/features/bridge/smtp/send/html.feature +++ b/test/features/bridge/smtp/send/html.feature @@ -6,7 +6,7 @@ Feature: SMTP sending of HTML messages Scenario: HTML message to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: HTML text external Content-Disposition: inline @@ -45,7 +45,7 @@ Feature: SMTP sending of HTML messages Scenario: HTML message with inline image to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Html Inline External Content-Disposition: inline @@ -120,7 +120,7 @@ Feature: SMTP sending of HTML messages Scenario: HTML message with alternative inline to internal account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: Html Inline Alternative Internal Content-Disposition: inline @@ -209,7 +209,7 @@ Feature: SMTP sending of HTML messages Scenario: HTML message with alternative inline to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Html Inline Alternative External Content-Disposition: inline diff --git a/test/features/bridge/smtp/send/html_att.feature b/test/features/bridge/smtp/send/html_att.feature index 005d0302..4beb0119 100644 --- a/test/features/bridge/smtp/send/html_att.feature +++ b/test/features/bridge/smtp/send/html_att.feature @@ -6,7 +6,7 @@ Feature: SMTP sending of HTML messages with attachments Scenario: HTML message with attachment to internal account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: HTML with attachment internal Content-Type: multipart/related; boundary=bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 @@ -65,7 +65,7 @@ Feature: SMTP sending of HTML messages with attachments Scenario: HTML message with attachment to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: HTML with attachment external PGP Content-Type: multipart/mixed; boundary=bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 diff --git a/test/features/bridge/smtp/send/plain.feature b/test/features/bridge/smtp/send/plain.feature index f34e4d8f..da0ccbb0 100644 --- a/test/features/bridge/smtp/send/plain.feature +++ b/test/features/bridge/smtp/send/plain.feature @@ -6,7 +6,7 @@ Feature: SMTP sending of plain messages Scenario: Only from and to headers to internal account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge hello @@ -40,7 +40,7 @@ Feature: SMTP sending of plain messages Scenario: Only from and to headers to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge hello @@ -74,7 +74,7 @@ Feature: SMTP sending of plain messages Scenario: Basic message to internal account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: Plain text internal Content-Disposition: inline @@ -111,7 +111,7 @@ Feature: SMTP sending of plain messages Scenario: Basic message to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Plain text external Content-Disposition: inline @@ -148,7 +148,7 @@ Feature: SMTP sending of plain messages Scenario: Message without charset is utf8 When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Plain text no charset external Content-Disposition: inline @@ -185,7 +185,7 @@ Feature: SMTP sending of plain messages Scenario: Message without charset is base64-encoded latin1 When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Plain text no charset external Content-Disposition: inline @@ -225,7 +225,7 @@ Feature: SMTP sending of plain messages Scenario: Message without charset and content is detected as HTML When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Plain, no charset, no content, external Content-Disposition: inline diff --git a/test/features/bridge/smtp/send/plain_att.feature b/test/features/bridge/smtp/send/plain_att.feature index 56462a2d..7690bc13 100644 --- a/test/features/bridge/smtp/send/plain_att.feature +++ b/test/features/bridge/smtp/send/plain_att.feature @@ -6,7 +6,7 @@ Feature: SMTP sending of plain messages with attachments Scenario: Basic message with attachment to internal account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: Plain with attachment Content-Type: multipart/related; boundary=bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 @@ -65,7 +65,7 @@ Feature: SMTP sending of plain messages with attachments Scenario: Plain message with attachment to external account When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge Subject: Plain with attachment external Content-Type: multipart/related; boundary=bc5bd30245232f31b6c976adcd59bb0069c9b13f986f9e40c2571bb80aa16606 @@ -124,7 +124,7 @@ Feature: SMTP sending of plain messages with attachments Scenario: Plain message with attachment to two external accounts When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: External Bridge 1 CC: External Bridge 2 Subject: Plain with attachment external PGP and external CC diff --git a/test/features/bridge/smtp/send/same_message.feature b/test/features/bridge/smtp/send/same_message.feature index 47f14fb2..db89d816 100644 --- a/test/features/bridge/smtp/send/same_message.feature +++ b/test/features/bridge/smtp/send/same_message.feature @@ -4,7 +4,7 @@ Feature: SMTP sending the same message twice And there is SMTP client logged in as "user" When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: Hello @@ -16,7 +16,7 @@ Feature: SMTP sending the same message twice Scenario: The exact same message is not sent twice When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: Hello @@ -31,7 +31,7 @@ Feature: SMTP sending the same message twice Scenario: Slight change means different message and is sent twice When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge Subject: Hello. diff --git a/test/features/bridge/smtp/send/two_messages.feature b/test/features/bridge/smtp/send/two_messages.feature index e1a98527..f8e652c2 100644 --- a/test/features/bridge/smtp/send/two_messages.feature +++ b/test/features/bridge/smtp/send/two_messages.feature @@ -4,7 +4,7 @@ Feature: SMTP sending two messages And there is SMTP client logged in as "user" When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge hello @@ -13,7 +13,7 @@ Feature: SMTP sending two messages Then SMTP response is "OK" When SMTP client sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge world @@ -28,7 +28,7 @@ Feature: SMTP sending two messages And there is SMTP client "smtp2" logged in as "userMoreAddresses" with address "secondary" When SMTP client "smtp1" sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge hello @@ -37,7 +37,7 @@ Feature: SMTP sending two messages Then SMTP response to "smtp1" is "OK" When SMTP client "smtp2" sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge world @@ -53,7 +53,7 @@ Feature: SMTP sending two messages When SMTP client "smtp1" sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge hello @@ -62,7 +62,7 @@ Feature: SMTP sending two messages Then SMTP response to "smtp1" is "OK" When SMTP client "smtp2" sends message """ - From: Bridge Test + From: Bridge Test <[userAddress]> To: Internal Bridge world diff --git a/test/mocks/smtp.go b/test/mocks/smtp.go index fb056e4f..a2e96d18 100644 --- a/test/mocks/smtp.go +++ b/test/mocks/smtp.go @@ -74,6 +74,8 @@ func (c *SMTPClient) SendCommands(commands ...string) *SMTPResponse { smtpResponse := &SMTPResponse{t: c.t} for _, command := range commands { + command = strings.ReplaceAll(command, "[userAddress]", c.address) + tstart := time.Now() c.debug.printReq(command) diff --git a/unreleased.md b/unreleased.md index a578d882..9f3b0c29 100644 --- a/unreleased.md +++ b/unreleased.md @@ -17,3 +17,6 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/) ### Changed * GODT-858 Bump go-rfc5322 dependency to v0.4.0 to handle some invalid RFC5322 groups. * GODT-923 Fix listener locking. + +### Changed +* GODT-389 Prefer `From` header instead of `MAIL FROM` address.