Bounce: non_local_msgid is broken

smfreegard commented 9 years ago

I was just looking at the bounce plugin as I had a new idea for a test and I noticed that non_local_msgid:

Is not documented
Doesn't handle multipart messages (e.g. it doesn't check body.children) for Message-ID headers.
Doesn't compare the Message-ID domain against rcpt_to.in_host_host OR the recipient domain against it to see if it is a local domain.
Never returns next() when a valid state is encountered, so it appears that it would always mark bounces as invalid.

Additionally - when I gave this a quick test using the plugin test mode; it blew up immediately because I didn't surround the Message-ID in <>'s:

DEBUG] [4B5A5B5B-C794-4711-AF9A-AB4DD387210E.1] [core] running data_post hook in bounce plugin
[INFO] [4B5A5B5B-C794-4711-AF9A-AB4DD387210E.1] [bounce] [ '\nMessage-ID: foo@foo\n' ]
[CRIT] [4B5A5B5B-C794-4711-AF9A-AB4DD387210E.1] [core] Plugin bounce failed: TypeError: Cannot read property '0' of null
    at Plugin.exports.non_local_msgid (/usr/lib/node_modules/Haraka/plugins/bounce.js:200:17)
    at Object.plugins.run_next_hook (/usr/lib/node_modules/Haraka/plugins.js:362:28)
    at callback (/usr/lib/node_modules/Haraka/plugins.js:337:21)
    at Plugin.exports.empty_return_path (/usr/lib/node_modules/Haraka/plugins/bounce.js:97:53)
    at Object.plugins.run_next_hook (/usr/lib/node_modules/Haraka/plugins.js:362:28)
    at Object.plugins.run_hooks (/usr/lib/node_modules/Haraka/plugins.js:250:13)
    at Connection.data_done (/usr/lib/node_modules/Haraka/connection.js:1453:13)
    at Connection.connection.data_respond (/usr/lib/node_modules/Haraka/bin/haraka:536:20)
    at callback (/usr/lib/node_modules/Haraka/plugins.js:333:39)
    at Plugin.exports.bad_rcpt (/usr/lib/node_modules/Haraka/plugins/bounce.js:136:53)

And when I did; the regexps pulled out the wrong values because:

domains.push(res[0].substring(1, (res[0].length -2))); is off-by-one. It would be better to use a regexp capture instead of the substring IMO anyway.

KingNoosh commented 7 years ago

Is this now closed due to https://github.com/haraka/Haraka/pull/1430? If not, what needs to be done?

msimerson commented 7 years ago

It's still open. It needs a mechanism within Haraka to ask, "is this domain name local?" Because "local" domains are typically defined in plugins, it's more than a trivial task. In my case, I have "local" domains defined in smtp_forward.ini and externally (via qmail-deliverable), so Haraka accepts mails for domains it knows nothing of. Hence, the stall.

For something like this to work reliably, we'd almost need a plugin like known-senders except as known-recipients and all it would do is listen on the rcpt_ok hooks and keep a redis DB populated with domains that we accept messages for. Or something like that...

smfreegard commented 7 years ago

For something like this to work reliably, we'd almost need a plugin like known-senders except as known-recipients and all it would do is listen on the rcpt_ok hooks and keep a redis DB populated with domains that we accept messages for. Or something like that...

There are a couple of plugins, that if loaded (e.g. rcpt_to.in_host_list and relay) that could probably do this without a Redis DB as the domains are defined in their configuration. It would just have to skip anything else.

haraka / haraka-plugin-bounce

Bounce: non_local_msgid is broken #1