Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return-Path and thus bounce handling not working #908

Closed
mzurborg-scopevisio opened this issue Aug 16, 2022 · 22 comments
Closed

Return-Path and thus bounce handling not working #908

mzurborg-scopevisio opened this issue Aug 16, 2022 · 22 comments
Labels
needs-investigation Potential bug. Needs investigation

Comments

@mzurborg-scopevisio
Copy link

mzurborg-scopevisio commented Aug 16, 2022

Version:

  • listmonk: 2.2.0
  • OS: Debian 11
  • Installation: Single binary started with custom systemd service

Some configuration infos:
Bounce handling is listmonk is on and listmonk expects bounces in a pop3 mailbox.

Description of the bug:
For all outgoing e-mails sent from listmonk, the "Return-Path" header is always exactly the same as the "From" header. Changing the "Return-Path" via custom headers in the campaign settings does not seem to do anything. Changing other headers such as "Errors-To" works.
For us, this means, that currently bounces are not sent back to the correct mailbox.
It has to be possible in listmonk to set "Return-Path" to a custom mail address, especially considering that our installation will use many different "From" addresses.

Steps to reproduce:

  1. Set global SMTP custom headers [{"Return-Path": "[email protected]"}] according to documentation on https://listmonk.app/docs/bounces/
  2. Create a campaign.
  3. Set From to [email protected].
  4. Send test mail or start campaign.
  5. Verify headers in our smtp gateway or in Thunderbird on the receiving end.
  6. Header Return-Path is [email protected], which is wrong.
@mzurborg-scopevisio mzurborg-scopevisio added the bug Something isn't working label Aug 16, 2022
@knadh
Copy link
Owner

knadh commented Aug 17, 2022

Are you able to check the Return-Path header on the SMTP side? Maybe the SMTP server is overriding the header you’re setting in listmonk?

@knadh knadh added needs-investigation Potential bug. Needs investigation and removed bug Something isn't working labels Aug 17, 2022
@andreafa
Copy link

I can confirm that the Return-Path custom header setting as described in the documentation does NOT work (see https://listmonk.app/docs/bounces/).

The workaround I found is to change it via mimedefang:

  • install mimedefang milter on Sendmail (it should work on Postfix too)
  • in /etc/mail/mimedefang-filter file, add within the sub filter_end subroutine:
    $return_path = '[email protected]';
    change_sender($return_path);

@knadh
Copy link
Owner

knadh commented Aug 26, 2022

That means the header set by listmonk isn't respected by the SMTP server. Since you mentioned sendmail, how exactly is listmonk connecting to your SMTP server?

@andreafa
Copy link

Actually Listmonk is configured to connect via smtp to localhost.

The Sendmail log shows:
Aug 25 07:43:15 ... sendmail[26582]: 27P5hF5c026582: from=[email protected], size=639, class=0, nrcpts=1, msgid=<1661406195158829191.25955.290365655482114464@...>, bodytype=8BITMIME, proto=ESMTP, daemon=MTA, relay=localhost [127.0.0.1]

The "from" address in Sendmail is the Mail-From address (also called "envelope sender" or "return-path") passed by the application. There's no way to change it unless using a milter like mimedefang.

@halms
Copy link

halms commented Sep 21, 2022

I just came about this issue as well.

My conclusion from some research is, that I think the receiving SMTP server is correct in ignoring the Return-Path header.

RFC2821 (SMTP) states on p. 51

   A message-originating SMTP system SHOULD NOT send a message that
   already contains a Return-path header.  SMTP servers performing a
   relay function MUST NOT inspect the message data, and especially not
   to the extent needed to determine if Return-path headers are present.
   SMTP servers making final delivery MAY remove Return-path headers
   before adding their own.

The RFC elaborates a bit on this on the page.

In my understanding, the correct way would be to not set a Return-Path header, but the SMTP envelope sender.

Looking at the source of smtppool.email, this could be done by setting the Sender field in the Email struct here: https://github.com/knadh/listmonk/blob/master/internal/messenger/email/email.go#L120

As this is a bit different than setting a header, but still a very desired functionality, I see two possible options to implement this:

  1. Parse the supplied headers (from campaign and from SMTP server config) and if there is a Return-Path header set, use this as the SMTP envelope sender.
    Less work adapting the settings, but it is kinda hacky. Also, the Return-Path header should probably then be removed and not added to the sent mail.
  2. Add a new setting to override the envelope sender.
    To do it right, this should probably be available as an overall default, per campaign, per SMTP server, and also for transactional messages. This seems cleaner, but requires creating additional settings, the corresponding UI changes, and possibly DB migrations.

On a side note:
A current workaround is just to set the From address to the desired Return-Path and then add a custom header From (either in the campaign or in the SMTP server settings), that sets the visible from address.

@knadh
Copy link
Owner

knadh commented Sep 22, 2022

Looking at the source of smtppool.email, this could be done by setting the Sender field in the Email struct here: https://github.com/knadh/listmonk/blob/master/internal/messenger/email/email.go#L120

Sender / From wouldn't make a difference. Sender overrides From and the final value is sent to the Go smtp lib as the From address.

It looks like the Return-Path will always be the From address, in which case, no change is warranted. Users can just set the desired return path as the From address. Maybe Reply-to helps here?

@knadh knadh closed this as completed Oct 17, 2022
@halms
Copy link

halms commented Oct 17, 2022

Sorry, I missed your reply before. You closed the issue, but I think this is not yet resolved.

I think there is a slight misunderstanding with the different Froms.
The Return-Path in the received message will always be what was originally the SMTP Envelope From address, but this can be distinct from the From: header within the message data (example of the SMPT communication).

Often, these two Froms are the same value, and there are some restrictions in order to pass DMARC, but for bulk email it is common to set the Envelope From to something like [email protected] in order to capture bounces (as is described in the docs) and the Header From to something nice like Example News <[email protected]>

The email library you forked handles these two possible Froms with the From and the Sender fields in the struct.
If a Sender is set, this is returned by the parseSender() function, if not, then the From field is used, resulting in the Envelope From and the Header From being the same.

The Envelope From is set when calling the Client.Mail in pool.go.
And I think the way this is supposed to be used by pool.go is

from, err := e.parseSender()

instead of

from, err := mail.ParseAddress(e.From)

Then the Sender field in the Email struct could actually serve it's purpose of setting the Envelope From and consequently the Return-Path header in the received email.

After this is changed, we would need a way to actually set the Sender field in the struct from the Frontend/API – for this I see the two possible options in my previous comment.

I want to also detail a bit the workaround I described:
One could set the desired Envelope From as the Default `from` email in settings or From address in the campaign, this will be the resulting Return-Path.
In order to edit the From: address that is shown in clients, one can then set a custom header From either in the SMTP server settings or in the per-campaign custom headers.

While this works (a comment describes that the From: header is only set if from the From field if there is no custom From header in the Headers field), I consider this bad usability for something that is a quite common use case (tracking of bounces using a custom bounce inbox – which is explicitly mentioned in the documentation).

(Reply-To serves a different use case. This informs a client, where it should direct replies to, but is of no use regarding the destination of bounces.)

@knadh
Copy link
Owner

knadh commented Oct 22, 2022

Ah, just realised, the parseSender() is a vestige from the fork. Sender there is simply a string that overrides the e-mail set in From. Irrespective of which variable is used, From or Sender, there is only one way to set the address, the Mail() command.

https://github.com/knadh/smtppool/blob/master/pool.go#L373

if err = c.conn.Mail(from.Address); err != nil {
	return true, err
}

There's no way to set a separate envelope sender and a from address. We can only pass one address via the Mail() command.

@halms
Copy link

halms commented Oct 22, 2022

The Mail() command (from the Client struct in net/mail) only sets the envelope sender (the MAIL FROM line in SMTP communcation).
The displayed From: is set as part of the email headers in the following lines using the Data() function.

The necessary headers are created by email.go before.
This is also the reason my described workaround works.

@knadh knadh reopened this Oct 22, 2022
@knadh
Copy link
Owner

knadh commented Oct 22, 2022

Duh! Understood, finally!

Will test this out and figure out a way to incorporate on the UI. Maybe it can just be picked up from the custom headers input rather than a new field.

@andreafa
Copy link

The Return-Path header should be good,
it's already described here: https://listmonk.app/docs/bounces/

@halms
Copy link

halms commented Oct 23, 2022

I also think Return-Path should work well.
Ideally, any Return-Path header set in the message should be stripped anyway since the RFC says a sent message should not contain it.
Possibly the easiest way is to set Sender field in the email struct to the value extracted from the header and then drop the header.
This and the one line change described in my comment should suffice.

I could try to create a PR if you want, even though my Go skills are still very limited.

@knadh
Copy link
Owner

knadh commented Nov 1, 2022

The return-path branch has this change. The Return-Path header set in campaign headers is picked up and used in the smtp.Mail() command as the envelop sender and the From set on the campaign is send as the header instead.

However, when I tried this with Gmail and SES and both, both still inject their ownReturn-Paths regardless. Would you be able to test this branch with your setups (Postfix?)? @halms @andreafa

@andreafa
Copy link

andreafa commented Nov 2, 2022

I would like to test it but I can only install/update Liskmonk from a Linux binary file (listmonk_2.X.X_linux_amd64.tar.gz).

I can provide the smtp servers. My experience is mostly on Sendmail but a temporary Postfix instance should not be a problem.
Tell me where to send the details and I'll do it.

@knadh
Copy link
Owner

knadh commented Nov 3, 2022

@andreafa here's the compiled binary for the return-path branch: listmonk-return-path.tar.gz

If you're unable to run this, please e-mail the test credentials to kailash [at] nadh.in

@andreafa
Copy link

andreafa commented Nov 3, 2022

The Return-Path header WORKS, well done!
But only if set in "New campaign -> Set custom headers"

It does NOT work in "Settings -> SMTP -> Custom headers box"
(as described here: https://listmonk.app/docs/bounces/)

@knadh
Copy link
Owner

knadh commented Nov 3, 2022

Could you please try this updated build? listmonk-return-path.tar.gz

  1. SMTP headers are applied first.
  2. Campaign level headers override SMTP level headers.

@andreafa
Copy link

andreafa commented Nov 3, 2022

Verified. Now it works fine with the Return-Path header
in "New campaign -> Set custom headers" OR in "Settings -> SMTP -> Custom headers box".

Thanks!

P.S. I don't think I need to check this with Postfix, they should behave the same way.
Let me know if you want me to try anyway

@knadh
Copy link
Owner

knadh commented Nov 3, 2022

Thanks for your patience! This is good to be merged. Needn't test with Postfix.

@halms
Copy link

halms commented Nov 4, 2022

Sorry for the delay.
I tested it with my setup (MXroute) and it works as expected.

But I would suggest a slightly different implementation:

  1. Change pool.go in smtppool module to use the parseSender() method from email.go
    – I created a PR: Use parseSender() to extract sender address smtppool#7
  2. Ensure usage of updated smptpool (new version, go dependency update, ...)
  3. Set the provided Sender field of the Email struct and do not manually mangle the From field in the struct and From: email header.

See my suggestin in PR #1008

@knadh knadh closed this as completed in 74322cd Nov 9, 2022
@knadh
Copy link
Owner

knadh commented Nov 9, 2022

This is merged. Thanks @halms. Replaced your PR with 74322cd as it had an unrelated HTML change.

@halms
Copy link

halms commented Nov 9, 2022

Great, thanks.
I think you can now also delete the return-path branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-investigation Potential bug. Needs investigation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants