From 0eb56025e52d2d1d4f20c9801853bf94ea147787 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Wed, 27 Dec 2023 13:55:45 -0800 Subject: [PATCH] improvements --- config/dns-list.ini | 20 ++++++++++---- index.js | 64 ++++++++++++--------------------------------- package.json | 1 - test/dns-list.js | 38 +++++++++++++++------------ 4 files changed, 53 insertions(+), 70 deletions(-) diff --git a/config/dns-list.ini b/config/dns-list.ini index 2c2a772..b6b8eba 100644 --- a/config/dns-list.ini +++ b/config/dns-list.ini @@ -4,14 +4,24 @@ ; periodically check each DNS list, disabling ones that fail checks periodic_checks = 30 -; zones: a comma separated list of DNSBL zones +; zones: an array or a comma separated list of DNS zones ; -zones=b.barracudacentral.org, truncate.gbudb.net, psbl.surriel.com, bl.spamcop.net, dnsbl-1.uceprotect.net, zen.spamhaus.org, dnsbl.sorbs.net, dnsbl.justspam.org, list.dnswl.org, hostkarma.junkemailfilter.com +zones[]=b.barracudacentral.org +zones[]=truncate.gbudb.net +zones[]=psbl.surriel.com +zones[]=bl.spamcop.net +zones[]=dnsbl-1.uceprotect.net +zones[]=zen.spamhaus.org +zones[]=dnsbl.sorbs.net +zones[]=dnsbl.justspam.org +zones[]=list.dnswl.org +zones[]=hostkarma.junkemailfilter.com + ; search: Default (first) -; first: consider first DNSBL response conclusive. End processing. -; all: process all DNSBL results -search=first +; first: consider first response conclusive. End processing. +; all: process all list results +search=all [stats] diff --git a/index.js b/index.js index 1933aa4..051146c 100644 --- a/index.js +++ b/index.js @@ -3,7 +3,6 @@ const dns = require('dns').promises; const net = require('net'); const net_utils = require('haraka-net-utils'); -const async = require('async'); exports.disable_allowed = false; let redis_client; @@ -18,13 +17,10 @@ exports.register = function () { this.register_hook('connect', 'onConnect'); - // DNS allow // IMPORTANT: don't run this on hook_rcpt otherwise we're an open relay... - /* ['ehlo','helo','mail'].forEach(hook => { this.register_hook(hook, 'check_dnswl'); }); - */ } exports.load_config = function () { @@ -40,7 +36,12 @@ exports.load_config = function () { this.load_config(); }); - this.cfg.main.zones = new Set(this.cfg.main.zones.split(/[\s,;]+/)); + if (Array.isArray(this.cfg.main.zones)) { + this.cfg.main.zones = new Set(this.cfg.main.zones); + } + else { + this.cfg.main.zones = new Set(this.cfg.main.zones.split(/[\s,;]+/)); + } // Compatibility with old-plugin for (const z in this.config.get('dnsbl.zones', 'list')) { @@ -93,9 +94,7 @@ exports.onConnect = function (next, connection) { const plugin = this async function eachActiveDnsList (ip, zone) { - // console.log(`eachActiveDnsList ip ${ip} zone ${zone}`) const type = plugin.getListType(zone) - // console.log(`eachActiveDnsList ip ${ip} zone ${zone} type ${type}`) const ips = await plugin.lookup(ip, zone) // console.log(`eachActiveDnsList ip ${ip} zone ${zone} type ${type} ips ${ips}`) @@ -106,12 +105,15 @@ exports.onConnect = function (next, connection) { } for (const i of ips) { - // console.log(`zone: ${zone} i: ${plugin.cfg[zone][i]}`) - if (plugin.cfg[zone][i]) connection.results.add(plugin, { msg: plugin.cfg[zone][i] }) + if (plugin.cfg[zone] && plugin.cfg[zone][i]) { + // console.log(`zone: ${zone} i: ${plugin.cfg[zone][i]}`) + connection.results.add(plugin, { msg: plugin.cfg[zone][i] }) + } } switch (type) { case 'allow': + connection.notes.dnswl = true connection.results.add(plugin, { pass: zone }) return nextOnce(OK, [zone]) case 'karma': @@ -132,7 +134,7 @@ exports.onConnect = function (next, connection) { Promise.all(promises).then(() => { // console.log(`Promise.all`) - if (connection.results.get(plugin).fail.length) { + if (connection.results.get(plugin).fail?.length) { nextOnce(DENY, connection.results.get(plugin).fail) return } @@ -178,11 +180,10 @@ exports.lookup = async function (ip, zone) { try { const query = ipQuery(ip, zone) - // console.log(`lookup query ${query}`) const a = await dns.resolve4(query, 'A') // console.log(`lookup ${query} -> a: ${a}`) - // this.stats_incr_zone(err, zone, start); // Statistics + this.stats_incr_zone(null, zone, start); // Statistics // Check for a result outside 127/8 // This should *never* happen on a proper DNS list @@ -208,7 +209,8 @@ exports.lookup = async function (ip, zone) { return a } catch (err) { - // console.log(`lookup err ${err}`) + this.stats_incr_zone(err, zone, start); // Statistics + if (err.code === dns.NOTFOUND) return; // unlisted, not an error if (err.code === dns.TIMEOUT) { // list timed out @@ -230,14 +232,12 @@ exports.stats_incr_zone = function (err, zone, start) { const foo = (err) ? err.code : 'LISTED'; redis_client.hIncrBy(rkey, foo, 1); redis_client.hGet(rkey, 'AVG_RT').then(rt => { - const avg = parseInt(rt) ? (parseInt(elapsed) + parseInt(rt))/2 - : parseInt(elapsed); + const avg = parseInt(rt) ? (parseInt(elapsed) + parseInt(rt))/2 : parseInt(elapsed); redis_client.hSet(rkey, 'AVG_RT', avg); }); } exports.init_redis = function () { - console.log(`init_redis`) if (redis_client) return; const redis = require('redis'); @@ -256,38 +256,6 @@ exports.init_redis = function () { }) } -exports.multi = function (lookup, zones, cb) { - if (!lookup || !zones) return cb(); - if (typeof zones === 'string') zones = [ `${zones}` ]; - const self = this; - const listed = []; - - function redis_incr (zone) { - if (!self.cfg.stats.enable) return; - - // Statistics: check hit overlap - for (const element of listed) { - const foo = (element === zone) ? 'TOTAL' : element; - redis_client.hIncrBy(`dns-list-overlap:${zone}`, foo, 1); - } - } - - function zoneIter (zone, done) { - self.lookup(lookup, zone, (err, a) => { - if (a) { - listed.push(zone); - redis_incr(zone); - } - cb(err, zone, a, true); - done(); - }) - } - function zonesDone (err) { - cb(err, null, null, false); - } - async.each(zones, zoneIter, zonesDone); -} - exports.getListType = function (zone) { if (this.cfg[zone] === undefined) return 'block' return this.cfg[zone]?.type || 'block' // default: block diff --git a/package.json b/package.json index 3927645..2ecbc2f 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,6 @@ "mocha": "^10.2.0" }, "dependencies": { - "async": "^3.2.5", "haraka-net-utils": "^1.5.3" } } diff --git a/test/dns-list.js b/test/dns-list.js index 2520fc5..d5675dc 100644 --- a/test/dns-list.js +++ b/test/dns-list.js @@ -45,8 +45,7 @@ describe('lookup', function () { it('Spamcop, test IPv4', async function () { const a = await this.plugin.lookup('127.0.0.2', 'bl.spamcop.net') - // assert.ok(a); - assert.deepEqual(['127.0.0.2'], a); + assert.deepStrictEqual(['127.0.0.2'], a); }) it('Spamcop, unlisted IPv6', async function () { @@ -66,7 +65,6 @@ describe('lookup', function () { it('CBL', async function () { const a = await this.plugin.lookup('127.0.0.2', 'xbl.spamhaus.org') - // console.log(a) assert.deepStrictEqual(a, [ '127.0.0.4' ]); }) }) @@ -89,6 +87,8 @@ describe('check_zone', function () { }) describe('check_zones', function () { + this.timeout(7000) + it('tests each block list', async function () { await this.plugin.check_zones(6000); }) @@ -97,14 +97,12 @@ describe('check_zones', function () { describe('onConnect', function () { beforeEach(function () { - // this.plugin = new fixtures.plugin('index') - // this.plugin.load_config() this.connection = fixtures.connection.createConnection() }) it('onConnect 127.0.0.1', function (done) { this.connection.set('remote.ip', '127.0.0.1') - this.plugin.zones=[ 'bl.spamcop.net', 'list.dnswl.org' ] + this.plugin.zones = new Set([ 'bl.spamcop.net', 'list.dnswl.org' ]) this.plugin.onConnect((code, msg) => { assert.strictEqual(code, undefined); assert.strictEqual(msg, undefined); @@ -115,10 +113,17 @@ describe('onConnect', function () { it('onConnect 127.0.0.2', function (done) { this.connection.set('remote.ip', '127.0.0.2') - this.plugin.zones=[ 'bl.spamcop.net', 'list.dnswl.org' ] + this.plugin.zones = new Set([ 'bl.spamcop.net', 'list.dnswl.org' ]) this.plugin.onConnect((code, msg) => { - assert.strictEqual(code, DENY); - assert.strictEqual(msg, 'host [127.0.0.2] is listed on bl.spamcop.net'); + // console.log(`code: ${code}, ${msg}`) + if (code === OK) { + assert.strictEqual(code, OK); + assert.strictEqual(msg, 'host [127.0.0.2] is listed on list.dnswl.org'); + } + else { + assert.strictEqual(code, DENY); + assert.strictEqual(msg, 'host [127.0.0.2] is listed on bl.spamcop.net'); + } done() }, this.connection) @@ -126,10 +131,11 @@ describe('onConnect', function () { it('Spamcop + CBL', function (done) { this.connection.set('remote.ip', '127.0.0.2') - this.plugin.zones = [ 'bl.spamcop.net', 'xbl.spamhaus.org' ] + this.plugin.zones = new Set([ 'bl.spamcop.net', 'xbl.spamhaus.org' ]) this.plugin.onConnect((code, msg) => { + // console.log(`code: ${code}, ${msg}`) assert.strictEqual(code, DENY); - assert.strictEqual(msg, 'host [127.0.0.2] is listed on bl.spamcop.net'); + assert.ok(/is listed on/.test(msg)); done() }, this.connection) @@ -137,7 +143,7 @@ describe('onConnect', function () { it('Spamcop + CBL + negative result', function (done) { this.connection.set('remote.ip', '127.0.0.1') - this.plugin.zones = [ 'bl.spamcop.net', 'xbl.spamhaus.org' ] + this.plugin.zones = new Set([ 'bl.spamcop.net', 'xbl.spamhaus.org' ]) this.plugin.onConnect((code, msg) => { // console.log(`test return ${code} ${msg}`) assert.strictEqual(code, undefined); @@ -148,7 +154,7 @@ describe('onConnect', function () { it('IPv6 addresses supported', function (done) { this.connection.set('remote.ip', '::1') - this.plugin.zones = ['bl.spamcop.net','xbl.spamhaus.org']; + this.plugin.zones = new Set(['bl.spamcop.net','xbl.spamhaus.org']) this.plugin.onConnect((code, msg) => { assert.strictEqual(code, undefined); assert.strictEqual(msg, undefined); @@ -161,16 +167,16 @@ describe('first', function () { beforeEach(function () { this.plugin.cfg.main.search='first' - this.plugin.zones = [ 'xbl.spamhaus.org', 'bl.spamcop.net' ]; + this.plugin.zones = new Set([ 'xbl.spamhaus.org', 'bl.spamcop.net' ]); this.connection = fixtures.connection.createConnection() }) it('positive result', function (done) { this.connection.set('remote.ip', '127.0.0.2') this.plugin.onConnect((code, msg) => { - // console.log(`test return ${code} ${msg}`) + // console.log(`onConnect return ${code} ${msg}`) assert.strictEqual(code, DENY); - assert.strictEqual(msg, 'host [127.0.0.2] is listed on bl.spamcop.net'); + assert.ok(/is listed on/.test(msg)); done(); }, this.connection)