diff --git a/Changes.md b/Changes.md index d52a7558e..522998def 100644 --- a/Changes.md +++ b/Changes.md @@ -1,9 +1,15 @@ ### Unreleased +#### Changed + +- outbound/mx_lookup: make it async/await + #### Fixed - fix(outbound): allow LHLO over insecure socket if TLS is forced #3278 +- outbound: emit log message when ignoring local MX +- fix(outbound): don't send SNI servername when connecting to an IP - fix(outbound): chown queue dir after creation #3291 ### [3.0.3] - 2024-02-07 diff --git a/outbound/hmail.js b/outbound/hmail.js index 381b63999..2694e9d7a 100644 --- a/outbound/hmail.js +++ b/outbound/hmail.js @@ -227,9 +227,7 @@ class HMailItem extends events.EventEmitter { } // if none of the above return codes, drop through to this... - mx_lookup.lookup_mx(this.todo.domain, (err, mxs) => { - this.found_mx(err, mxs); - }); + mx_lookup.lookup_mx(this.todo.domain, this.found_mx) } found_mx (err, mxs) { @@ -287,7 +285,7 @@ class HMailItem extends events.EventEmitter { } } - try_deliver () { + async try_deliver () { // check if there are any MXs left if (this.mxlist.length === 0) { @@ -300,6 +298,13 @@ class HMailItem extends events.EventEmitter { const mx = this.mxlist.shift(); const host = mx.exchange; + if (!obc.cfg.local_mx_ok) { + if (await net_utils.is_local_host(host)) { + this.loginfo(`MX ${host} is local, skipping since local_mx_ok=false`) + return this.try_deliver(); // try next MX + } + } + this.force_tls = this.todo.force_tls; if (!this.force_tls) { if (net_utils.ip_in_list(obtls.cfg.force_tls_hosts, host)) { diff --git a/outbound/mx_lookup.js b/outbound/mx_lookup.js index b221f5e05..29a14fd9d 100644 --- a/outbound/mx_lookup.js +++ b/outbound/mx_lookup.js @@ -1,11 +1,9 @@ "use strict"; -const dns = require('dns'); -const net_utils = require('haraka-net-utils') +const dns = require('node:dns').promises; +const net_utils = require('haraka-net-utils') -const obc = require('./config'); - -exports.lookup_mx = function lookup_mx (domain, cb) { +exports.lookup_mx = async function lookup_mx (domain, cb) { const mxs = []; // Possible DNS errors @@ -22,49 +20,41 @@ exports.lookup_mx = function lookup_mx (domain, cb) { // EREFUSED // SERVFAIL - // default wrap_mx just returns our object with "priority" and "exchange" keys - let wrap_mx = a => a; - async function process_dns (err, addresses) { - if (err) { - if (err.code === 'ENODATA' || err.code === 'ENOTFOUND') { - // Most likely this is a hostname with no MX record - // Drop through and we'll get the A record instead. - return 0; - } - cb(err); + try { + const addresses = await net_utils.get_mx(domain) + for (const a of addresses) { + mxs.push(a); } - else if (addresses?.length) { - for (let i=0,l=addresses.length; i < l; i++) { - if ( - obc.cfg.local_mx_ok || - await net_utils.is_local_host(addresses[i].exchange).catch(() => null) === false - ) { - const mx = wrap_mx(addresses[i]); - mxs.push(mx); - } - } - cb(null, mxs); + if (mxs.length) { + if (cb) return cb(null, mxs) + return mxs } - else { - // return zero if we need to keep trying next option - return 0; + } + catch (err) { + switch (err.code) { + case 'ENODATA': + case 'ENOTFOUND': + // likely a hostname with no MX record, drop through + break + default: + throw err(err) } - return 1; } - net_utils.get_mx(domain, async (err, addresses) => { - if (await process_dns(err, addresses)) return; + // No MX record, try resolving A record + + // wrap addresses with "priority" and "exchange" keys + const wrap_mx = a => ({priority:0,exchange:a}); + + const addresses = await dns.resolve(domain) + for (const a of addresses) { + mxs.push(wrap_mx(a)); + } - // if MX lookup failed, we lookup an A record. To do that we change - // wrap_mx() to return same thing as resolveMx() does. - wrap_mx = a => ({priority:0,exchange:a}); - // IS: IPv6 compatible - dns.resolve(domain, async (err2, addresses2) => { - if (await process_dns(err2, addresses2)) return; + if (mxs.length) return mxs - err2 = new Error("Found nowhere to deliver to"); - err2.code = 'NOMX'; - cb(err2); - }); - }); + const err = new Error("Found nowhere to deliver to"); + err.code = 'NOMX'; + if (cb) return cb(err) + throw err } diff --git a/outbound/tls.js b/outbound/tls.js index 93ad3715c..138e13b82 100644 --- a/outbound/tls.js +++ b/outbound/tls.js @@ -1,5 +1,7 @@ 'use strict'; +const net = require('net') + const logger = require('../logger'); const tls_socket = require('../tls_socket'); const config = require('haraka-config'); @@ -69,6 +71,7 @@ class OutboundTLS { } get_tls_options (mx) { + if (net.isIP(mx.exchange)) return this.cfg return Object.assign(this.cfg, {servername: mx.exchange}); } diff --git a/package.json b/package.json index 0feec2dc1..f4b949afd 100644 --- a/package.json +++ b/package.json @@ -17,11 +17,11 @@ }, "main": "haraka.js", "engines": { - "node": ">=16" + "node": ">=18" }, "dependencies": { - "address-rfc2821": "^2.1.1", - "address-rfc2822": "^2.1.0", + "address-rfc2821": "^2.1.2", + "address-rfc2822": "^2.2.0", "async": "^3.2.5", "daemon": "~1.1.0", "ipaddr.js": "~2.1.0", diff --git a/tests/outbound/index.js b/tests/outbound/index.js index 64e5d134e..f5e261040 100644 --- a/tests/outbound/index.js +++ b/tests/outbound/index.js @@ -166,10 +166,7 @@ exports.get_tls_options = { const testDir = path.resolve('tests'); this.outbound.config = this.outbound.config.module_config(testDir); this.obtls.test_config(tls_socket.config.module_config(testDir), this.outbound.config); - this.obtls.init(() => { - done(); - }) - + this.obtls.init(done) }, tearDown: done => { delete process.env.HARAKA_TEST_DIR; diff --git a/tests/outbound/mx_lookup.js b/tests/outbound/mx_lookup.js new file mode 100644 index 000000000..e2ac9d906 --- /dev/null +++ b/tests/outbound/mx_lookup.js @@ -0,0 +1,45 @@ + +const mx = require('../../outbound/mx_lookup'); + +exports.lookup_mx = { + 'MX records for example.com (await)': async test => { + test.expect(1) + const r = await mx.lookup_mx('example.com'); + test.deepEqual(r, [ { exchange: '', priority: 0 }]) + test.done() + }, + 'MX records for example.com (cb)': test => { + test.expect(1) + mx.lookup_mx('example.com', (err, r) => { + test.deepEqual(r, [ { exchange: '', priority: 0 }]) + test.done() + }); + }, + 'MX records for tnpi.net (await)': async test => { + test.expect(1) + const r = await mx.lookup_mx('tnpi.net'); + test.deepEqual(r, [ { exchange: 'mail.theartfarm.com', priority: 10 }]) + test.done() + }, + 'MX records for tnpi.net (cb)': test => { + test.expect(1) + mx.lookup_mx('tnpi.net', (err, r) => { + test.deepEqual(r, [ { exchange: 'mail.theartfarm.com', priority: 10 }]) + test.done() + }); + }, + 'MX records for gmail.com (await)': async test => { + test.expect(1) + const r = await mx.lookup_mx('gmail.com'); + // console.log(r) + test.ok(r.length) + test.done() + }, + 'MX records for gmail.com (callback)': test => { + test.expect(1) + mx.lookup_mx('gmail.com', (err, r) => { + test.ok(r.length) + test.done() + }); + }, +}