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

get_ips_by_host 1.6 not working same as previous version #88

Closed
analogic opened this issue Apr 19, 2024 · 2 comments
Closed

get_ips_by_host 1.6 not working same as previous version #88

analogic opened this issue Apr 19, 2024 · 2 comments
Assignees

Comments

@analogic
Copy link

I've tracked down bug which hit me during dev

2024-04-19T13:12:58.833Z [DEBUG] [mail_from.is_resolvable] a.cz: MX 0 185.59.2.14 => queryAaaa ENOTFOUND 185.59.2.14
2024-04-19T13:12:58.833Z [INFO] [core] hook=mail plugin=mail_from.is_resolvable function=hook_mail params=<[email protected]> retval=DENY msg="MX without A/AAAA records"

Origin source is https://github.com/haraka/Haraka/blob/db8dbfa5cdce7fef3f6d39b1abf5c3a3f8aba60f/plugins/mail_from.is_resolvable.js#L107 - get_ips_by_host generate exactly two errors

  • queryA ENOTFOUND 185.59.2.14
  • queryAaaa ENOTFOUND 185.59.2.14

It seems that IP detection is missing somewhere

@msimerson
Copy link
Member

Hmmm, yes. I see the issue.

The issue is not with get_ips_by_host() (before -vs- after), which I'm confident returns exactly the same thing for any given input (test cases proving me wrong are welcome).

I'm pretty sure the issue is right here in the MF plugin. Which is kinda ironic, because it says right there in the plugin that some MXes return IPs, and there's even a config option to allow it. But that regex doesn't match a bare (normal) IP address. Why would an IP in this context be surrounded by brackets ([...])? 🤷🏼‍♂️ That looks like a useless use of regex to me, when a call to net.isIP() should do the job.

The relevant change in NU 1.6 is that get_mx now returns both explicit and implicit MX records. So now there might be IP addresses in the exchange property, and the MF plugin fails to detect them. In the three cases where I know they're being used (MF.is_resolvable, SPF, and outbound), those callers will get simplified as they don't need to query for implicit MX separately. I don't know of any cases where the implicit MX is unwanted but they can be easily detected and removed by filtering the exchanges through net.isIP.

The other difference is that get_mx now includes the from_dns property. Instead of being a boolean, now it's populated with the hostname from which the MX was resolved.

@analogic
Copy link
Author

I've also found another not reported outbound the lookup_mx bug (A resolve error not properly handled) and spotted your work on net_utils which makes more sense and also fix this. I agree, I don't need to differentiate between implicit/explicit mx except maybe for some spam measures. I've tested PR haraka/Haraka#3322 and it really fixes the problem!

2024-04-20T07:33:17.174Z [DEBUG] [mail_from.is_resolvable] resolving MX for domain a.cz
2024-04-20T07:33:17.203Z [DEBUG] [mail_from.is_resolvable] a.cz: MX => [{"priority":0,"exchange":"185.59.2.14","from_dns":"a.cz"}]
2024-04-20T07:33:17.203Z [INFO] [mail_from.is_resolvable] pass:implicit_mx
2024-04-20T07:33:17.203Z [DEBUG] [core] hook=mail plugin=mail_from.is_resolvable function=hook_mail params=<[email protected]> retval=CONT msg=""

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

No branches or pull requests

2 participants