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

queue: retry with another IP when first attempt fails for dualstack remote servers #152

Merged
merged 1 commit into from
Apr 13, 2024
Merged

Conversation

lmeunier
Copy link
Contributor

@lmeunier lmeunier commented Apr 9, 2024

mox was already giving another try for received errors after the HELO/EHLO command. Now mox do the same for received errors when trying to deliver the message to the remote SMTP server.

This should help to deliver messages to SMTP server that rejects incoming messages because of bad ipv4 or ipv6 configuration (for example for servers checking reverse DNS records). mox will now try to deliver messages on both ip family instead before considering the error as permanent.

fix #149

@lmeunier
Copy link
Contributor Author

lmeunier commented Apr 9, 2024

@mjl- in the end, I reused the retry mechanism already present in the code for errors received at the start of the connexion. No changes in the configuration are needed. I don't know if it's better, or not, compared to https://github.com/mjl-/mox/issues/149#issuecomment-2041007381

I'm also wondering how to write a test for this commit. Any idea will be appreciated.

Copy link
Owner

@mjl- mjl- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change also seems useful, or did you run into problems with this? i can imagine that it's better to just not attempt delivering over ipv6 if it can cause trouble. but a mail server can connect dualstack, we can retry for these failures both during EHLO, and when it happens later.

I'm also wondering how to write a test for this commit. Any idea will be appreciated.

this isn't too easy indeed. i think the approach would be to write a test like those in TestQueue where you create a fake smtp server that expects two connections, that responds with a 5xx 7.x.x on first invocation, and expects another connection attempt which completes the smtp transaction successfully. the smtpclient.DialHook could store which address family was dialed, to check if both ipv4 and ipv6 was tried. a mock resolver that resolves to both ipv4 and ipv6 would be needed.
we currently also don't have a check if the retry. would be good to have. the TestQueue code is a bit read-only... if you want to give a try, it may be easier to copy some of that function into a new one: the various cases are connected, and they may require quite a refactor to fit another new case in...

queue/direct.go Outdated Show resolved Hide resolved
queue/direct.go Outdated Show resolved Hide resolved
…emote servers

mox was already giving another try for received errors after the
`HELO`/`EHLO` command. Now mox do the same for received errors when
trying to deliver the message to the remote SMTP server.

This should help to deliver messages to SMTP server that rejects
incoming messages because of bad ipv4 or ipv6 configuration (for example
for servers checking reverse DNS records). mox will now try to deliver
messages on both ip family instead before considering the error as
permanent.

fix #149
@lmeunier
Copy link
Contributor Author

this change also seems useful, or did you run into problems with this?

I didn't encounter any problems during my tests with this PR. But, I would prefer to use the other PR for these reasons:

  • mails refused over ipv6 are delayed like any failed delivery with transient error (and i don't like to wait ^^): mox first try with ipv6, RCPT command is rejected, then mox try agin a few minutes later with ipv4, and finaly the mail is delivered
  • I fear that repeated errors could result in my ipv6 being blacklisted in some RBL, and that my ipv6 will be rejected by other SMTP servers
  • this PR is a lot harder to test and is less predictable (TBH i'm not sure i haven't break something else with this PR...)

@mjl- mjl- merged commit 1ea851b into mjl-:main Apr 13, 2024
2 checks passed
@mjl-
Copy link
Owner

mjl- commented Apr 13, 2024

this change didn't look too dangerous to me wrt changing existing behaviour. and it will be useful to also detect this error code during smtp rcpt to in case of dns block listings of ipv6 vs ipv4, so i merged it, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remote SMTP server rejects messages because of mismatch between outgoing ipv6 address and reverse hostname
2 participants