search

vert-x3 / vertx-mail-client

Apache License 2.0
34 stars 33 forks source link

bare <LF> received after DATA #219

Closed cs8898 closed 3 months ago

cs8898 commented 7 months ago

When sending Mails with longer Header Values, a "newer" Postfix Server will Reject Mail Submission due to a bare <LF> received after DATA Error.

This happens because the Recipient List will be splitted after >=75 chars by just adding a single \n instead of \r\n.

I Plan to add a PR to correct this behavior. In the io.vertx.ext.mail.mailencoder.Utils class are some more places where just a single LineFeed is added.

Version

Which version(s) did you encounter this bug ? 3.5.3+

Context

SMTP Smuggling

Do you have a reproducer?

Send Mails with a long Subject/Recepient List.

The Gist contains Two Dumps of the RAW SNMP Messages, one from vertx and the other one from Thunderbird.
Thunderbird encodes all as \r\n.

Relevant class

io.vertx.ext.mail.mailencoder.Utils

Legion2 commented 7 months ago

We had an production incident because all emails were rejected by our email provider (STRATO) after they rolled out an update to their email server. The new email server now enforces the spec compliment encoding of headers and therefore returned SMTP protocol violation: A header line must be terminated by CRLF for the headers sent by vertx which are not correctly encoded according to the spec.

Legion2 commented 7 months ago

 @cs8898 the PR you created does not directly fix the bug here https://github.com/vert-x3/vertx-mail-client/blob/dea280c0ef0e0c3a1ac329c607f2fa5ff2c6fd2a/src/main/java/io/vertx/ext/mail/mailencoder/Utils.java#L155 which should use \r\n instead of \n according to spec. Should I create a PR for this?

cs8898 commented 7 months ago

Yes thats correct, the encoder only uses \n thats why i only added \r during sending to the SMTP Server. With adding while encoding all the tests musst bei changed. And it kind of differs the way the MSG Body is handeled.

Just saying that \r\r\n can also lead to problems. But Sendmail does not tackle these.

Could be an aspect to discuss.

Legion2 commented 7 months ago

Adding /r afterwards to all headers seems like a workaround. Which is not necessary for this particular problem I'm facing, because we can just fix the header encoding to be according to the specification.

cs8898 commented 7 months ago

Can you reference the specs and relevant chapter here, what specs are you refering to? Jakarta Mail?

I can tackle the necessary changes next week.

Legion2 commented 7 months ago

I refer to this RFC https://datatracker.ietf.org/doc/html/rfc2047#section-2

An 'encoded-word' may not be more than 75 characters long, including 'charset', 'encoding', 'encoded-text', and delimiters. If it is desirable to encode more text than will fit in an 'encoded-word' of 75 characters, multiple 'encoded-word's (separated by CRLF SPACE) may be used.

Legion2 commented 4 months ago

@cs8898 had you time to look at this?

cs8898 commented 4 months ago

Hi sorry, was lacking of time for that. I try to schedule it for that weekend.

I'm currently using the exclude for my server https://www.postfix.org/postconf.5.html#smtpd_forbid_bare_newline_exclusions

cs8898 commented 4 months ago

@Legion2 have a look at #221

Legion2 commented 4 months ago

@cs8898 Thanks for your work.

cs8898 commented 4 months ago

@vietj is @alexlehm still the maintainer of the smtp client? Will anyone from the "core" maintainers give us some feedback on that issue.

At least for users not having the possibility to modify their postfix configs, as its the case when you use hosted services...

gaol commented 4 months ago

@cs8898 sorry for late response. I will check the pr this week.