diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 8ff22b3..6318542 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -3,6 +3,9 @@ env: es6: true mocha: true +parserOptions: + ecmaVersion: 2020 + plugins: [ haraka ] extends: [ eslint:recommended, plugin:haraka/recommended ] diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index a6f1367..0000000 --- a/.travis.yml +++ /dev/null @@ -1,15 +0,0 @@ -language: node_js -node_js: - - "14" - -services: - -before_script: - -script: - - npm run lint - - npm test - -after_success: - -sudo: false diff --git a/Changes.md b/Changes.md index f91f1b3..17e8e9c 100644 --- a/Changes.md +++ b/Changes.md @@ -1,8 +1,17 @@ +## 1.0.5 - 2022-04-07 + +- fix: reduce log noise from same_ipv4_network #35 +- style: use dns promises, backtick strings, for..of +- style: ecmaVersion=2020 (node 14+) +- require haraka-net-utils > 1.3.4 +- ci: more updates +- ci: replace opendns test IP with 1.1.1.1 + + ## 1.0.4 - 2022-04-07 - ci: restore to working order -- fix: reduce log noise from same_ipv4_network #35 ## 1.0.3 - 2018-11-20 @@ -10,17 +19,20 @@ - Fix data hook #12 - Replace string concatenations with template literals #8 + ## 1.0.2 - 2017-09-19 - domains with fcrdns were being rejected #7 - eslint no-var #5 + ## 1.0.1 - 2017-09-14 - update name of fcrdns.ini (drop connect prefix) #2 - es6: replace var with const/let - style: remove ending semicolons + ## 1.0.0 - 2017-02-05 - initial release diff --git a/README.md b/README.md index cc97a80..fee124d 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,5 @@ [![Build Status][ci-img]][ci-url] [![Code Climate][clim-img]][clim-url] -[![Windows Build Status][ci-win-img]][ci-win-url] [![NPM][npm-img]][npm-url] @@ -139,10 +138,8 @@ The iprev results are added to the Authentication-Results header. -[ci-img]: https://travis-ci.org/haraka/haraka-plugin-fcrdns.svg -[ci-url]: https://travis-ci.org/haraka/haraka-plugin-fcrdns -[ci-win-img]: https://ci.appveyor.com/api/projects/status/xayl14cyhj8o834s?svg=true -[ci-win-url]: https://ci.appveyor.com/project/msimerson/haraka-plugin-fcrdns +[ci-img]: https://github.com/haraka/haraka-plugin-fcrdns/actions/workflows/ci-test.yml/badge.svg +[ci-url]: https://github.com/haraka/haraka-plugin-fcrdns/actions/workflows/ci-test.yml [cov-img]: https://codecov.io/github/haraka/haraka-plugin-fcrdns/coverage.svg [cov-url]: https://codecov.io/github/haraka/haraka-plugin-fcrdns [clim-img]: https://codeclimate.com/github/haraka/haraka-plugin-fcrdns/badges/gpa.svg diff --git a/appveyor.yml b/appveyor.yml deleted file mode 100644 index 23e656b..0000000 --- a/appveyor.yml +++ /dev/null @@ -1,20 +0,0 @@ -environment: - nodejs_version: "14" - -install: - - ps: Install-Product node $env:nodejs_version - - npm install - -before_build: -build: off -after_build: - -before_test: - - node --version - - npm --version - -test_script: - - npx mocha - -after_test: - diff --git a/index.js b/index.js index f4c47f8..8c7af57 100644 --- a/index.js +++ b/index.js @@ -1,22 +1,23 @@ 'use strict' -// build in node modules +// built in node modules const dns = require('dns') const net = require('net') +const { Resolver } = require('dns').promises; +const resolver = new Resolver(); + // NPM modules const constants = require('haraka-constants') const net_utils = require('haraka-net-utils') const tlds = require('haraka-tld') exports.register = function () { - const plugin = this - - plugin.load_fcrdns_ini() + this.load_fcrdns_ini() - plugin.register_hook('connect_init', 'initialize_fcrdns') - plugin.register_hook('lookup_rdns', 'do_dns_lookups') - plugin.register_hook('data', 'add_message_headers') + this.register_hook('connect_init', 'initialize_fcrdns') + this.register_hook('lookup_rdns', 'do_dns_lookups') + this.register_hook('data', 'add_message_headers') } exports.load_fcrdns_ini = function () { @@ -38,10 +39,8 @@ exports.load_fcrdns_ini = function () { } exports.initialize_fcrdns = function (next, connection) { - const plugin = this - // always init, so results.get is deterministic - connection.results.add(plugin, { + connection.results.add(this, { fcrdns: [], // PTR host names that resolve to this IP invalid_tlds: [], // rDNS names with invalid TLDs other_ips: [], // IPs from names that didn't match @@ -56,7 +55,6 @@ exports.initialize_fcrdns = function (next, connection) { } exports.resolve_ptr_names = function (ptr_names, connection, next) { - const plugin = this; // Fetch A & AAAA records for each PTR host name let pending_queries = 0 @@ -69,43 +67,44 @@ exports.resolve_ptr_names = function (ptr_names, connection, next) { // Make sure TLD is valid if (!tlds.get_organizational_domain(ptr_domain)) { - connection.results.add(plugin, {fail: `valid_tld(${ptr_domain})`}) - if (!plugin.cfg.reject.invalid_tld) continue - if (plugin.is_whitelisted(connection)) continue + connection.results.add(this, {fail: `valid_tld(${ptr_domain})`}) + if (!this.cfg.reject.invalid_tld) continue + if (this.is_whitelisted(connection)) continue if (net_utils.is_private_ip(connection.remote.ip)) continue return next(constants.DENY, `client [${connection.remote.ip}] rejected; invalid TLD in rDNS (${ptr_domain})`) } queries_run = true - connection.logdebug(plugin, `domain: ${ptr_domain}`) + connection.logdebug(this, `domain: ${ptr_domain}`) pending_queries++ - net_utils.get_ips_by_host(ptr_domain, function (err2, ips) { + + net_utils.get_ips_by_host(ptr_domain, (err, ips) => { pending_queries-- - if (err2) { - for (let e=0; e < err2.length; e++) { - switch (err2[e].code) { + if (err) { + for (const e of err) { + switch (e.code) { case dns.NODATA: case dns.NOTFOUND: break default: - connection.results.add(plugin, {err: err2[e].message}) + connection.results.add(this, {err: e.message}) } } } - connection.logdebug(plugin, `${ptr_domain} => ${ips}`) + connection.logdebug(this, `${ptr_domain} => ${ips}`) results[ptr_domain] = ips if (pending_queries > 0) return if (ips.length === 0) { - connection.results.add(plugin, { fail: `ptr_valid(${ptr_domain})` }) + connection.results.add(this, { fail: `ptr_valid(${ptr_domain})` }) } // Got all DNS results - connection.results.add(plugin, {ptr_name_to_ip: results}) - plugin.check_fcrdns(connection, results, next) + connection.results.add(this, {ptr_name_to_ip: results}) + this.check_fcrdns(connection, results, next) }) } @@ -114,22 +113,21 @@ exports.resolve_ptr_names = function (ptr_names, connection, next) { } exports.do_dns_lookups = function (next, connection) { - const plugin = this if (connection.remote.is_private) { - connection.results.add(plugin, {skip: 'private_ip'}) + connection.results.add(this, {skip: 'private_ip'}) return next() } const rip = connection.remote.ip // Set-up timer - const timer = setTimeout(function () { - connection.results.add(plugin, {err: 'timeout', emit: true}) - if (!plugin.cfg.reject.no_rdns) return nextOnce() - if (plugin.is_whitelisted(connection)) return nextOnce() + 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`) - }, plugin.cfg.main.timeout * 1000) + }, this.cfg.main.timeout * 1000) let called_next = 0 @@ -140,28 +138,31 @@ exports.do_dns_lookups = function (next, connection) { next(code, msg) } - dns.reverse(rip, (err, ptr_names) => { - connection.logdebug(plugin, `rdns.reverse(${rip})`) - if (err) return plugin.handle_ptr_error(connection, err, nextOnce) + try { + resolver.reverse(rip).then(ptr_names => { + connection.logdebug(this, `rdns.reverse(${rip})`) - connection.results.add(plugin, {ptr_names}) - connection.results.add(plugin, {has_rdns: true}) + connection.results.add(this, {ptr_names}) + connection.results.add(this, {has_rdns: true}) - plugin.resolve_ptr_names(ptr_names, connection, nextOnce); - }) + this.resolve_ptr_names(ptr_names, connection, nextOnce); + }) + } + catch (err) { + this.handle_ptr_error(connection, err, nextOnce) + } } exports.add_message_headers = function (next, connection) { - const plugin = this const txn = connection.transaction; ['rDNS', 'FCrDNS', 'rDNS-OtherIPs', 'HostID' ].forEach((h) => { - txn.remove_header('X-Haraka-' + h) + txn.remove_header(`X-Haraka-${h}`) }) const fcrdns = connection.results.get('fcrdns') if (!fcrdns) { - connection.results.add(plugin, {err: "no fcrdns results!?"}) + connection.results.add(this, {err: "no fcrdns results!?"}) return next() } @@ -178,29 +179,27 @@ exports.add_message_headers = function (next, connection) { } exports.handle_ptr_error = function (connection, err, next) { - const plugin = this const rip = connection.remote.ip switch (err.code) { case dns.NOTFOUND: case dns.NXDOMAIN: case dns.NODATA: - connection.results.add(plugin, {fail: 'has_rdns', emit: true}) - if (!plugin.cfg.reject.no_rdns) return next() - if (plugin.is_whitelisted((connection))) return next() + connection.results.add(this, {fail: 'has_rdns', emit: true}) + if (!this.cfg.reject.no_rdns) return next() + if (this.is_whitelisted((connection))) return next() return next(DENY, `client [${rip}] rejected; no rDNS`) } - connection.results.add(plugin, {err: err.code}) + connection.results.add(this, {err: err.code}) + + if (!this.cfg.reject.no_rdns) return next() + if (this.is_whitelisted(connection)) return next() - if (!plugin.cfg.reject.no_rdns) return next() - if (plugin.is_whitelisted(connection)) return next() next(DENYSOFT, `client [${rip}] rDNS lookup error (${err})`) } exports.check_fcrdns = function (connection, results, next) { - const plugin = this - let last_domain for (const fdom in results) { // mail.example.com if (!fdom) continue @@ -208,56 +207,56 @@ exports.check_fcrdns = function (connection, results, next) { // Multiple domains? if (last_domain && last_domain !== org_domain) { - connection.results.add(plugin, {ptr_multidomain: true}) + connection.results.add(this, {ptr_multidomain: true}) } else { last_domain = org_domain } // FCrDNS? PTR -> (A | AAAA) 3. PTR comparison - plugin.ptr_compare(results[fdom], connection, fdom) + this.ptr_compare(results[fdom], connection, fdom) - connection.results.add(plugin, {ptr_name_has_ips: true}) + connection.results.add(this, {ptr_name_has_ips: true}) - if (plugin.is_generic_rdns(connection, fdom) && - plugin.cfg.reject.generic_rdns && - !plugin.is_whitelisted(connection)) { + if (this.is_generic_rdns(connection, fdom) && + this.cfg.reject.generic_rdns && + !this.is_whitelisted(connection)) { return next(DENY, `client ${fdom} [${connection.remote.ip}] rejected;` + ` generic rDNS, please use your ISPs SMTP relay`) } } - plugin.log_summary(connection) - plugin.save_auth_results(connection) + this.log_summary(connection) + this.save_auth_results(connection) const r = connection.results.get('fcrdns') if (!r) return next() if (r.fcrdns && r.fcrdns.length) return next() - if (plugin.cfg.reject.no_fcrdns) { + if (this.cfg.reject.no_fcrdns) { return next(DENY, 'Sorry, no FCrDNS match found') } next() } exports.ptr_compare = function (ip_list, connection, domain) { - const plugin = this - if (!ip_list) return false - if (!ip_list.length) return false + if (!ip_list || !ip_list.length) return false - if (ip_list.indexOf(connection.remote.ip) !== -1) { - connection.results.add(plugin, {pass: 'fcrdns' }) - connection.results.push(plugin, {fcrdns: domain}) + if (ip_list.includes(connection.remote.ip)) { + connection.results.add(this, {pass: 'fcrdns' }) + connection.results.push(this, {fcrdns: domain}) return true } - const ip_list_ipv4 = ip_list.filter(net.isIPv4) - if (ip_list_ipv4.length && net_utils.same_ipv4_network(connection.remote.ip, ip_list_ipv4)) { - connection.results.add(plugin, {pass: 'fcrdns(net)' }) - connection.results.push(plugin, {fcrdns: domain}) + + const ip_list_v4 = ip_list.filter(net.isIPv4) + if (ip_list_v4.length && net_utils.same_ipv4_network(connection.remote.ip, ip_list_v4)) { + connection.results.add(this, {pass: 'fcrdns(net)' }) + connection.results.push(this, {fcrdns: domain}) return true } - for (let j=0; j 2) return list.slice(0,2).join(',') + '...' + if (list.length > 2) return `${list.slice(0,2).join(',')}...` return list.join(',') } exports.log_summary = function (connection) { if (!connection) return; // connection went away - const note = connection.results.get('fcrdns') - if (!note) return + const r = connection.results.get('fcrdns') + if (!r) return connection.loginfo(this, `ip=${connection.remote.ip} ` + - ` rdns="${hostNamesAsStr(note.ptr_names)}" rdns_len=${note.ptr_names.length}` + - ` fcrdns="${hostNamesAsStr(note.fcrdns)}" fcrdns_len=${note.fcrdns.length}` + - ` other_ips_len=${note.other_ips.length} invalid_tlds=${note.invalid_tlds.length}` + - ` generic_rdns=${((note.ptr_name_has_ips) ? 'true' : 'false')}` + ` rdns="${hostNamesAsStr(r.ptr_names)}" rdns_len=${r.ptr_names.length}` + + ` fcrdns="${hostNamesAsStr(r.fcrdns)}" fcrdns_len=${r.fcrdns.length}` + + ` other_ips_len=${r.other_ips.length} invalid_tlds=${r.invalid_tlds.length}` + + ` generic_rdns=${((r.ptr_name_has_ips) ? 'true' : 'false')}` ) } diff --git a/package.json b/package.json index c25d0f1..7c8ec3a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "haraka-plugin-fcrdns", - "version": "1.0.4", + "version": "1.0.5", "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": "*", + "haraka-net-utils": "^1.3.4", "haraka-tld": "*" } } diff --git a/test/index.js b/test/index.js index 9b7b9db..cce3834 100644 --- a/test/index.js +++ b/test/index.js @@ -6,37 +6,29 @@ const dns = require('dns') // npm modules const fixtures = require('haraka-test-fixtures') -// start of tests -// assert: https://nodejs.org/api/assert.html -// mocha: http://mochajs.org beforeEach(function (done) { this.plugin = new fixtures.plugin('fcrdns') this.plugin.register() this.connection = new fixtures.connection.createConnection() - this.plugin.initialize_fcrdns(() => { - done() - }, this.connection) + this.plugin.initialize_fcrdns(done, this.connection) }) describe('fcrdns', function () { - it('loads', function (done) { + it('loads', function () { assert.ok(this.plugin) - done() }) }) describe('load_fcrdns_ini', function () { - it('loads fcrdns.ini from config/fcrdns.ini', function (done) { + it('loads fcrdns.ini from config/fcrdns.ini', function () { this.plugin.load_fcrdns_ini() assert.ok(this.plugin.cfg) - done() }) - it('initializes enabled boolean', function (done) { + it('initializes enabled boolean', function () { this.plugin.load_fcrdns_ini() assert.equal(this.plugin.cfg.reject.no_rdns, false, this.plugin.cfg) - done() }) }) @@ -46,8 +38,8 @@ describe('handle_ptr_error', function () { err.code = dns.NOTFOUND this.plugin.handle_ptr_error(this.connection, err, function () { assert.equal(undefined, arguments[0]) + done() }) - done() }) it('ENOTFOUND reject.no_rdns=1', function (done) { @@ -56,8 +48,8 @@ describe('handle_ptr_error', function () { this.plugin.cfg.reject.no_rdns=1 this.plugin.handle_ptr_error(this.connection, err, function () { assert.equal(DENY, arguments[0]) + done() }) - done() }) it('dns.NOTFOUND reject.no_rdns=0', function (done) { @@ -76,8 +68,8 @@ describe('handle_ptr_error', function () { this.plugin.cfg.reject.no_rdns=1 this.plugin.handle_ptr_error(this.connection, err, function () { assert.equal(DENY, arguments[0]) + done() }) - done() }) it('dns.FAKE reject.no_rdns=0', function (done) { @@ -96,15 +88,15 @@ describe('handle_ptr_error', function () { this.plugin.cfg.reject.no_rdns=1 this.plugin.handle_ptr_error(this.connection, err, function () { assert.equal(DENYSOFT, arguments[0]) + done() }) - done() }) }) describe('is_generic_rdns', function () { it('mail.theartfarm.com', function (done) { - this.connection.remote.ip='208.75.177.101' + this.connection.remote.ip='66.128.51.165' assert.equal( false, this.plugin.is_generic_rdns(this.connection, 'mail.theartfarm.com') @@ -148,40 +140,35 @@ describe('is_generic_rdns', function () { describe('save_auth_results', function () { - it('fcrdns fail', function (done) { + it('fcrdns fail', function () { this.connection.results.add(this.plugin, { pass: 'fcrdns' }) assert.equal(false, this.plugin.save_auth_results(this.connection)) - done() }) - it('fcrdns pass', function (done) { + it('fcrdns pass', function () { this.connection.results.push(this.plugin, {fcrdns: 'example.com'}) assert.equal(true, this.plugin.save_auth_results(this.connection)) - done() }) }) describe('ptr_compare', function () { - it('fail', function (done) { + it('fail', function () { this.connection.remote.ip = '10.1.1.1' const iplist = ['10.0.1.1'] assert.equal(false, this.plugin.ptr_compare(iplist, this.connection, 'foo.example.com')) - done() }) - it('pass exact', function (done) { + it('pass exact', function () { this.connection.remote.ip = '10.1.1.1' const iplist = ['10.1.1.1'] assert.equal(true, this.plugin.ptr_compare(iplist, this.connection, 'foo.example.com')) - done() }) - it('pass net', function (done) { + it('pass net', function () { this.connection.remote.ip = '10.1.1.1' const iplist = ['10.1.1.2'] assert.equal(true, this.plugin.ptr_compare(iplist, this.connection, 'foo.example.com')) - done() }) }) @@ -213,29 +200,28 @@ describe('do_dns_lookups', function () { '8.8.4.4': 'dns.google', '2001:4860:4860::8844': 'dns.google', '4.2.2.2': 'level3.net', - '208.67.222.222': 'opendns.com', - // '2001:428::1': 'qwest.net', + // '208.67.222.222': 'opendns.com', + '1.1.1.1': 'one.one', } - Object.keys(testIps).forEach((ip) => { + for (const ip of Object.keys(testIps)) { it(`looks up ${ip}`, function (done) { - this.timeout(4000); + this.timeout(5000); - const conn = this.connection - conn.remote.ip = ip + this.connection.remote.ip = ip this.plugin.do_dns_lookups((rc, msg) => { - const res = conn.results.get('fcrdns') + const res = this.connection.results.get('fcrdns') assert.ok(new RegExp( testIps[ip] ).test(res.fcrdns[0])) // console.log(res); assert.equal(rc, undefined) assert.equal(msg, undefined) done() }, - conn) + this.connection) }) - }) + } }) describe('resolve_ptr_names', function () {