diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d768329..77b3c5e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,28 +14,10 @@ jobs: # uses: haraka/.github/.github/workflows/coverage.yml@master # secrets: inherit - test: - needs: [ lint, get-lts ] - runs-on: ${{ matrix.os }} - strategy: - matrix: - os: [ ubuntu-latest, windows-latest ] - node-version: ${{ fromJson(needs.get-lts.outputs.active) }} - fail-fast: false - steps: - - uses: actions/checkout@v3 - - uses: actions/setup-node@v3 - name: Node ${{ matrix.node-version }} on ${{ matrix.os }} - with: - node-version: ${{ matrix.node-version }} - - run: npm install - - run: npm test + ubuntu: + needs: [ lint ] + uses: haraka/.github/.github/workflows/ubuntu.yml@master - get-lts: - runs-on: ubuntu-latest - steps: - - id: get - uses: msimerson/node-lts-versions@v1 - outputs: - active: ${{ steps.get.outputs.active }} - lts: ${{ steps.get.outputs.lts }} + windows: + needs: [ lint ] + uses: haraka/.github/.github/workflows/windows.yml@master \ No newline at end of file diff --git a/Changes.md b/Changes.md index 72c6877..17015b3 100644 --- a/Changes.md +++ b/Changes.md @@ -1,6 +1,12 @@ ### Unreleased +### [1.1.0] - 2022-07-20 + +- fix: timeout DNS 1 second before plugin timeout +- resolve_ptr_names: simplify with async/await & Promise.all + + ### [1.0.6] - 2022-06-05 - ci: update GHA shared workflows @@ -45,4 +51,7 @@ ### 1.0.0 - 2017-02-05 - initial release + + [1.0.6]: https://github.com/haraka/haraka-plugin-fcrdns/releases/tag/1.0.6 +[1.1.0]: https://github.com/haraka/haraka-plugin-fcrdns/releases/tag/1.1.0 diff --git a/index.js b/index.js index 8c7af57..ae9701d 100644 --- a/index.js +++ b/index.js @@ -34,7 +34,7 @@ exports.load_fcrdns_ini = function () { }) if (isNaN(plugin.cfg.main.timeout)) { - plugin.cfg.main.timeout = (plugin.timeout || 30) - 1; + plugin.cfg.main.timeout = plugin.timeout || 30; } } @@ -54,19 +54,16 @@ exports.initialize_fcrdns = function (next, connection) { next() } -exports.resolve_ptr_names = function (ptr_names, connection, next) { +exports.resolve_ptr_names = async function (ptr_names, connection, next) { // Fetch A & AAAA records for each PTR host name - let pending_queries = 0 - let queries_run = false + const promises = [] + const forwardNames = [] - const results = {} - for (let i=0; i { - pending_queries-- - - if (err) { - for (const e of err) { - switch (e.code) { - case dns.NODATA: - case dns.NOTFOUND: - break - default: - connection.results.add(this, {err: e.message}) - } - } - } - - connection.logdebug(this, `${ptr_domain} => ${ips}`) - results[ptr_domain] = ips - - if (pending_queries > 0) return - - if (ips.length === 0) { - connection.results.add(this, { fail: `ptr_valid(${ptr_domain})` }) - } + connection.logdebug(this, `PTRdomain: ${ptr_domain}`) - // Got all DNS results - connection.results.add(this, {ptr_name_to_ip: results}) - this.check_fcrdns(connection, results, next) - }) + forwardNames.push(ptr_domain.toLowerCase()) + promises.push(net_utils.get_ips_by_host(ptr_domain.toLowerCase())) } - // No valid PTR - if (!queries_run || (queries_run && pending_queries === 0)) next() + Promise.all(promises).then(ipsPerHost => { + const resultsByForwardName = {} + for (let i = 0; i < ipsPerHost.length; i++) { + resultsByForwardName[forwardNames[i]] = ipsPerHost[i] + if (ipsPerHost[i].length === 0) { + connection.results.add(this, { fail: `ptr_valid(${forwardNames[i]})` }) + } + } + connection.results.add(this, { ptr_name_to_ip: resultsByForwardName }) + this.check_fcrdns(connection, resultsByForwardName, next) + }) } -exports.do_dns_lookups = function (next, connection) { +exports.do_dns_lookups = async function (next, connection) { if (connection.remote.is_private) { connection.results.add(this, {skip: 'private_ip'}) @@ -122,12 +100,13 @@ exports.do_dns_lookups = function (next, connection) { const rip = connection.remote.ip // Set-up timer + const timeoutMs = (this.cfg.main.timeout - 1) * 1000 const timer = setTimeout(() => { connection.results.add(this, {err: 'timeout', emit: true}) if (!this.cfg.reject.no_rdns) return nextOnce() if (this.is_whitelisted(connection)) return nextOnce() - return nextOnce(DENYSOFT, `client [${rip}] rDNS lookup timeout`) - }, this.cfg.main.timeout * 1000) + nextOnce(DENYSOFT, `client [${rip}] rDNS lookup timeout`) + }, timeoutMs) let called_next = 0 @@ -139,14 +118,15 @@ exports.do_dns_lookups = function (next, connection) { } try { - resolver.reverse(rip).then(ptr_names => { - connection.logdebug(this, `rdns.reverse(${rip})`) + const ptr_names = await resolver.reverse(rip) + if (called_next) return // timed out + + connection.logdebug(this, `rdns.reverse(${rip})`) - connection.results.add(this, {ptr_names}) - connection.results.add(this, {has_rdns: true}) + connection.results.add(this, {ptr_names}) + connection.results.add(this, {has_rdns: true}) - this.resolve_ptr_names(ptr_names, connection, nextOnce); - }) + this.resolve_ptr_names(ptr_names, connection, nextOnce); } catch (err) { this.handle_ptr_error(connection, err, nextOnce) diff --git a/package.json b/package.json index 1668070..c448a58 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "haraka-plugin-fcrdns", - "version": "1.0.6", + "version": "1.1.0", "description": "Haraka plugin that checks a remote for Forward Confirmed reverse DNS", "main": "index.js", "scripts": { @@ -31,7 +31,7 @@ }, "dependencies": { "haraka-constants": "*", - "haraka-net-utils": "^1.3.4", + "haraka-net-utils": "^1.3.5", "haraka-tld": "*" } } diff --git a/test/index.js b/test/index.js index cce3834..3f9b9ef 100644 --- a/test/index.js +++ b/test/index.js @@ -194,6 +194,36 @@ describe('check_fcrdns', function () { }) }) +describe('resolve_ptr_names', function () { + this.timeout(5000); + + const validCases = { + 'mail.theartfarm.com': [], + 'smtp.gmail.com': [], + } + + for (const c in validCases) { + it(`gets IPs for ${c}`, function (done) { + const ptr_names = [ c ] + this.plugin.resolve_ptr_names(ptr_names, this.connection, () => { + // console.log(this.connection.results.store.fcrdns) + assert.ok(this.connection.results.store.fcrdns.ptr_name_to_ip[c].length) + assert.equal(this.connection.results.store.fcrdns.ptr_name_has_ips, true) + done() + }) + }) + } + + it('ignores invalid host names', function (done) { + const ptr_names = [ 'mail.invalid' ] + this.plugin.resolve_ptr_names(ptr_names, this.connection, () => { + // console.log(this.connection.results.store.fcrdns) + assert.ok(this.connection.results.store.fcrdns.other_ips.length === 0) + done() + }) + }) +}) + describe('do_dns_lookups', function () { const testIps = { @@ -223,26 +253,3 @@ describe('do_dns_lookups', function () { }) } }) - -describe('resolve_ptr_names', function () { - this.timeout(5000); - - it('gets IPs from valid host names', function (done) { - const ptr_names = [ 'mail.theartfarm.com' ] - this.plugin.resolve_ptr_names(ptr_names, this.connection, () => { - // console.log(this.connection.results.store.fcrdns) - assert.ok(this.connection.results.store.fcrdns.other_ips.length) - done() - }) - }) - - it('ignores invalid host names', function (done) { - const ptr_names = [ 'mail.invalid' ] - this.plugin.resolve_ptr_names(ptr_names, this.connection, () => { - // console.log(this.connection.results.store.fcrdns) - assert.ok(this.connection.results.store.fcrdns.other_ips.length === 0) - done() - }) - }) - -})