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

helo.check forward_dns is ignored #3040

Closed
brutus opened this issue Apr 7, 2022 · 2 comments · Fixed by #3041
Closed

helo.check forward_dns is ignored #3040

brutus opened this issue Apr 7, 2022 · 2 comments · Fixed by #3041

Comments

@brutus
Copy link

brutus commented Apr 7, 2022

Describe the bug

When using the helo.checks plugin with forward_dns=true I observed this behavior: Sending HELO google.de once fails with 550 HELO host has no forward DNS match as expected. Sending it again, let's me proceed and queue a mail.

Is this expected? Or am I missing something?

Expected behavior

Sending HELO with a host without a forward DNS match keeps being rejected with 550 HELO host has no forward DNS match.

Observed behavior

Sending HELO with a host without a forward DNS match is accepted.

Steps To Reproduce

I compiled some additional information in this gist.

[me@myshellhost ~]$ telnet stardust.uberspace.de 25
Trying 35.195.215.42...
Connected to stardust.uberspace.de.
Escape character is '^]'.
220 stardust.uberspace.de ESMTP Haraka/2.8.28 ready
HELO google.de
550 HELO host has no forward DNS match
HELO google.de
250 stardust.uberspace.de Hello [165.22.79.242]Haraka is at your service.

System Info:

Haraka Haraka.js — Version: 2.8.28
Node v16.14.2
OS Linux stardust.uberspace.de 3.10.0-1160.59.1.el7.x86_64 #1 SMP Wed Feb 23 16:47:03 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
openssl OpenSSL 1.0.2k-fips 26 Jan 2017
@msimerson msimerson mentioned this issue Apr 8, 2022
3 tasks
@msimerson
Copy link
Member

msimerson commented Apr 8, 2022

Short version: it's a bug. We are plumbing the depths of my memory, IIRC, a reason that multi test exists is for port 587 users. They would connect, EHLO, STARTTLS, and then EHLO again. Any DNS slowness penalized them twice. But as your example points out, it also opens a barn door. Removing the multi check (see #3041) fixes that, but there might other reasons we had that in there.

@brutus
Copy link
Author

brutus commented Apr 13, 2022

Hi @msimerson, thanks for the quick PR. I'm not fluent enough in the codebase to judge the impact but, I'm glad to see this tackled.

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 a pull request may close this issue.

2 participants