search

haraka / haraka-net-utils

haraka network utilities
https://www.npmjs.com/package/haraka-net-utils
MIT License
2 stars 13 forks source link

get_ips_by_host 1.6 not working same as previous version #88

Closed analogic closed 5 months ago

analogic commented 5 months ago

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=<a@a.cz> 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

It seems that IP detection is missing somewhere

msimerson commented 5 months ago

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

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 https://github.com/haraka/Haraka/pull/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=<a@a.cz> retval=CONT msg=""