search

ProtonMail / proton-bridge

Proton Mail Bridge application
GNU General Public License v3.0
1.14k stars 152 forks source link

Failed to Parse To Address List in SMTP Send Request #389

Closed skweee closed 1 year ago

skweee commented 1 year ago

Expected Behavior

FritzBox (https://avm.de/produkte/fritzbox/) is a series of residential routers with a huge market share in Germany. It provides a so-called "Push service" that enables notifications from the device via e-mail (SMTP connector). Other mail server are able to process messages from Fritzbox. Also, the sample SMTP server from https://github.com/emersion/go-smtp is able to process the message without problems. Expected behavior is that Proton Bridge can also process and deliver these messages.

Current Behavior

Proton Bridge 3.1.3 (older versions not tested) is not accepting messages from Fritzbox (OS 7.50), reporting the following error: 554 5.0.0 Error: transaction failed, blame it on the weather: failed to parse message: failed to parse message header: failed to parse to: [Error offset=18]: expected atext char for atom

At the same time, a go-smtp sample test server is able to process the message, returnin the following: 250 2.0.0 OK: queued

Possible Solution

N/A

Steps to Reproduce

Attached log files show the SMTP communication (sensitive information such as AUTH data is replaced by dummy values):

In order to reproduce the issue, open a telnet connection to Proton Bridge SMTP and manually insert the client portion of the SMTP communication (correct e-mail address and AUTH data needs to be adjusted to Proton Bridge setup). Compare the results with the same procedure while replacing Proton Bridge SMTP with the sample go-smtp server.

Version Information

Bridge Quebec v3.1.3 (tested Linux and macOS versions) FritzBox OS 7.50

Context (Environment)

This currently blocks ProtonMail users from using the FritzBox push service. As messages sent by this service can contain sensitive information about the user's network environment, unencrypted SMTP transfer using external service should be avoided.

LBeernaertProton commented 1 year ago

Hey @skweee thank you for the report, but unfortunately to diagnose this error I need the full address list of To smtp field which I believe has been redacted.

The sending of the message is failing since we can't parse the address for the To header field of the message.

If you feel comfortable, you could mail me the values to my email address (see git commit logs).

LBeernaertProton commented 1 year ago

Alternatively, you can try produce a dummy address that triggers the same problem as your existing address. This would also help us to diagnose the issue.

skweee commented 1 year ago

Hey @LBeernaertProton - thanks for your quick feedback and analysis. Only the address in the To SMTP field has been changed.

You may be referring to the comma , at the end of the To header. This is not redacted, but actually the way FritzBox sends the message - even if it is simply addressed to one address.

I'm not sure what the relevant RFCs think about that, but other mail servers do not seem to have an issue with this. Is there a possibility to address this in the parser?

LBeernaertProton commented 1 year ago

@skweee the input <test@user.com>, does not cause issues.

There is clearly an issue in our parser, but I'm afraid that without the original address I can't diagnose this bug.

skweee commented 1 year ago

Interesting - in my tests I used several different address for the To: header, and I was able to reproduce the issue in all cases.

I will reach out to you via email to send you the information requested. By the way - I would also be happy to point my FritzBox to a public-facing instance of the bridge, if that helps analysis.

LBeernaertProton commented 1 year ago

@skweee I would be interested in the list of addresses you used for your testing as well :)

skweee commented 1 year ago

@LBeernaertProton - you've got mail!

LBeernaertProton commented 1 year ago

@skweee thanks for the mail. I confirm I can reproduce the issue.

LBeernaertProton commented 1 year ago

Internally tracked as GODT-2637.

skweee commented 1 year ago

I can confirm this is fixed in release 3.3.0. Thanks for the fix!