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

mjl- / mox

modern full-featured open source secure mail server for low-maintenance self-hosted email

https://www.xmox.nl

MIT License

3.36k stars 89 forks source link

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

Closed lmeunier closed 2 months ago

lmeunier commented 2 months ago

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 commented 2 months ago

@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.

lmeunier commented 2 months ago

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- commented 2 months ago

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!