add clamd check options for skipping (#2533)

* add check options for skipping
* more consistency with rspamd package
* removed dubious skip result

Changes.md

@@ -9,6 +9,8 @@
 
 ### Changes
 
+* clamd: add check.authenticated, check.private_ip, check.local_ip option 
+
 * dkim_sign: improved log messages #2499
 
 * ehlo_hello_message: config/ehlo_hello_message can be used to overwrite the EHLO/HELO msg replacing `, Haraka is at your service` #2498

docs/plugins/clamd.md

@@ -89,6 +89,29 @@ The following options are enabled by default in clamd but ClamAV suggests
 using them only for scoring.
 
     * Phishing=false
+
+## [check]
+
+The optional check section can allow skipping ClamAV check for remote connection
+meeting following criteria.
+
+- authenticated
+
+    Default: true
+
+    If true, messages from authenticated users will be scanned.
+
+- private\_ip
+
+    Default: true
+
+    If true, messages from private IPs will be scanned.
+
+- local\_ip
+
+    Default: true
+
+    If true, messages from localhost will be scanned.
 
 ## clamd.excludes
 
@@ -115,5 +138,4 @@ using them only for scoring.
 *.UNOFFICIAL
 # Phishing
 Heuristics.Phishing.*
-`````
-
+`````

plugins/clamd.js

@@ -90,6 +90,10 @@ exports.load_clamd_ini = function () {
             // clamd.conf options enabled by default, but prone to false
             // positives.
             '-reject.Phishing',
+
+            '+check.authenticated',
+            '+check.private_ip',
+            '+check.local_ip'
         ],
     }, function () {
         plugin.load_clamd_ini();
@@ -147,8 +151,11 @@ exports.register = function () {
 
 exports.hook_data = function (next, connection) {
     const plugin = this;
+
     if (!plugin.cfg.main.only_with_attachments) return next();
 
+    if (!plugin.should_check(connection)) return next();
+
     const txn = connection.transaction;
     txn.parse_body = true;
     txn.attachment_hooks(function (ctype, filename, body) {
@@ -165,6 +172,8 @@ exports.hook_data_post = function (next, connection) {
     const txn = connection.transaction;
     const cfg = plugin.cfg;
 
+    if (!plugin.should_check(connection)) return next();
+
     // Do we need to run?
     if (cfg.main.only_with_attachments && !txn.notes.clamd_found_attachment) {
         connection.logdebug(plugin, 'skipping: no attachments found');
@@ -315,6 +324,30 @@ exports.hook_data_post = function (next, connection) {
     try_next_host();
 }
 
+exports.should_check = function (connection) {
+    const plugin = this;
+
+    if (plugin.cfg.check.authenticated == false && connection.notes.auth_user) {
+        connection.transaction.results.add(plugin, { skip: 'authed'});
+        return false;
+    }
+
+    // necessary because local IPs are included in private IPs
+    if (plugin.cfg.check.local_ip == true && connection.remote.is_local) return true;
+
+    if (plugin.cfg.check.local_ip == false && connection.remote.is_local) {
+        connection.transaction.results.add(plugin, { skip: 'local_ip'});
+        return false;
+    }
+
+    if (plugin.cfg.check.private_ip == false && connection.remote.is_private) {
+        connection.transaction.results.add(plugin, { skip: 'private_ip'});
+        return false;
+    }
+
+    return true;
+}
+
 exports.send_clamd_predata = function (socket, cb) {
     socket.write("zINSTREAM\0", () => {
         const received = 'Received: from Haraka clamd plugin\r\n';

tests/plugins/clamd.js

@@ -91,7 +91,97 @@ exports.hook_data_post = {
         this.plugin.cfg.main.only_with_attachments=true;
         test.expect(1);
         const next = function () {
-            test.ok(this.connection.transaction.results.get('clamd').skip);
+            test.ok(this.connection.transaction.results.get('clamd').skip.length > 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'skip authenticated': function (test) {
+        this.connection.notes.auth_user = 'user';
+        this.plugin.cfg.check.authenticated = false;
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length > 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'checks local IP': function (test) {
+        this.connection.remote.is_local = true;
+        this.plugin.cfg.check.local_ip = true;
+
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length === 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'skips local IP': function (test) {
+        this.connection.remote.is_local = true;
+        this.plugin.cfg.check.local_ip = false;
+
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length > 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'checks private IP': function (test) {
+        this.connection.remote.is_private = true;
+        this.plugin.cfg.check.private_ip = true;
+
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length === 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'skips private IP': function (test) {
+        this.connection.remote.is_private = true;
+        this.plugin.cfg.check.private_ip = false;
+
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length > 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'checks public ip': function (test) {
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length === 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'skip localhost if check.local_ip = false and check.private_ip = true': function (test) {
+        this.connection.remote.is_local = true;
+        this.connection.remote.is_private = true;
+
+        this.plugin.cfg.check.local_ip = false;
+        this.plugin.cfg.check.private_ip = true;
+
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length > 0);
+            test.done();
+        }.bind(this);
+        this.plugin.hook_data_post(next, this.connection);
+    },
+    'checks localhost if check.local_ip = true and check.private_ip = false': function (test) {
+        this.connection.remote.is_local = true;
+        this.connection.remote.is_private = true;
+
+        this.plugin.cfg.check.local_ip = true;
+        this.plugin.cfg.check.private_ip = false;
+
+        test.expect(1);
+        const next = function () {
+            test.ok(this.connection.transaction.results.get('clamd').skip.length === 0);
             test.done();
         }.bind(this);
         this.plugin.hook_data_post(next, this.connection);
@@ -101,7 +191,7 @@ exports.hook_data_post = {
         this.plugin.cfg.main.max_size=512;
         test.expect(1);
         const next = function () {
-            test.ok(this.connection.transaction.results.get('clamd').skip);
+            test.ok(this.connection.transaction.results.get('clamd').skip.length > 0);
             test.done();
         }.bind(this);
         this.plugin.hook_data_post(next, this.connection);