Release v1.1.0 (#38)

* fix: timeout DNS 1 second before plugin * use shared haraka/.github workflows * add another test case
haraka · Jul 21, 2022 · 2fc9693 · 2fc9693
1 parent 44ca102
commit 2fc9693
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 100 deletions.
diff --git 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
diff --git 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
@@ -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,65 +54,43 @@ 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<ptr_names.length; i++) {
-        const ptr_domain = ptr_names[i].toLowerCase()
-        results[ptr_domain] = []
+    for (const ptr_domain of ptr_names) {
 
         // Make sure TLD is valid
-        if (!tlds.get_organizational_domain(ptr_domain)) {
+        if (!tlds.get_organizational_domain(ptr_domain.toLowerCase())) {
             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(this, `domain: ${ptr_domain}`)
-        pending_queries++
-
-        net_utils.get_ips_by_host(ptr_domain, (err, ips) => {
-            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
@@ -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
@@ -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()
-        })
-    })
-
-})