smtp_forward: tighten up queue.wants routing (#3199)

* doc(smtp_forward): improve markdown formatting
haraka · Jun 13, 2023 · f3685a7 · f3685a7
1 parent c2ff876
commit f3685a7
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 66 deletions.
diff --git a/Changes.md b/Changes.md
@@ -3,8 +3,11 @@
 
 #### Fixed
 
+- feat(q_forward): add LMTP routing handling #3190
+- chore(q_forward): tighten up queue.wants handling #3190
+- fix(q_forward): correct path to next_hop
+- doc(q_forward): improve markdown formatting #3190
 - helo.checks: serveral fixes, #3191
-- q(forward): correct path to next_hop
 - parsing error leaking to SMTP communication #3176
 - rename redis command setex to setEx #3174
 

diff --git a/docs/plugins/queue/smtp_forward.md b/docs/plugins/queue/smtp_forward.md
@@ -1,27 +1,18 @@
-queue/smtp\_forward
+# queue/smtp\_forward
 ==================
 
-This plugin delivers to another mail server. This is a common setup when you
-want to have a mail server with a solid pedigree of outbound delivery to
-other hosts, and inbound delivery to users.
+This plugin delivers to another mail server. This is a common setup when you want to have a mail server with a solid pedigree of outbound delivery to other hosts, and inbound delivery to users.
 
-In comparison to `queue/smtp_proxy`, this plugin waits until queue time to
-attempt the ongoing connection. This can be a benefit in reducing connections
-to your inbound mail server when you have content filtering (such as
-spamassassin) enabled. A possible downside is that it also delays recipient
-validation that the ongoing mail server may provide until queue time.
+In comparison to `queue/smtp_proxy`, this plugin waits until queue time to attempt the ongoing connection. This can be a benefit in reducing connections to your inbound mail server when you have content filtering (such as spamassassin) enabled. A possible downside is that it also delays recipient validation that the ongoing mail server may provide until queue time.
 
-Configuration
+## Configuration
 -------------
 
-* smtp\_forward.ini
-
-  Configuration is stored in this file in the following keys:
+Configuration is stored in smtp\_forward.ini in the following keys:
 
   * enable\_outbound=[true]
 
-    SMTP forward outbound messages (set to false to enable Haraka's separate
-    Outbound mail routing (MX based delivery)).
+    SMTP forward outbound messages (set to false to enable Haraka's separate Outbound mail routing (MX based delivery)).
 
   * host=HOST
 
@@ -33,27 +24,21 @@ Configuration
 
   * connect\_timeout=SECONDS
 
-    The maximum amount of time to wait when creating a new connection
-    to the host.  Default: 30 seconds.
+    The maximum amount of time to wait when creating a new connection to the host.  Default: 30 seconds.
 
   * timeout=SECONDS
 
-    The amount of seconds to let a backend connection live idle in the
-    connection pool.  This should always be less than the global plugin
-    timeout, which should in turn be less than the connection timeout.
+    The amount of seconds to let a backend connection live idle in the connection pool.  This should always be less than the global plugin timeout, which should in turn be less than the connection timeout.
 
   * max\_connections=NUMBER
 
     Maximum number of connections at any given time. Default: 1000
 
   * enable\_tls=[true]
 
-    Enable TLS with the forward host (if supported). TLS uses options
-    from the tls plugin. If key and cert are provided in the the outbound section of the tls plugin,
-    that certificate will be used as a TLS Client Certificate.
+    Enable TLS with the forward host (if supported). TLS uses options from the tls plugin. If key and cert are provided in the the outbound section of the tls plugin, that certificate will be used as a TLS Client Certificate.
 
-    This option controls the use of TLS via `STARTTLS`. This plugin does not work with
-    SMTP over TLS.
+    This option controls the use of TLS via `STARTTLS`. This plugin does not work with SMTP over TLS.
 
   * auth\_type=[plain\|login]
 
@@ -69,9 +54,7 @@ Configuration
 
   * queue
 
-    Which queue plugin to use. Default: undefined. The default bahavior is to
-    use smtp_forward for inbound connections and outbound for relaying
-    connections. This option is used for complex mail routes.
+    Which queue plugin to use. Default: undefined. The default bahavior is to use smtp_forward for inbound connections and outbound for relaying connections. This option is used for complex mail routes.
 
   * check_sender=false
 
@@ -86,18 +69,13 @@ Configuration
 
 # Per-Domain Configuration
 
-More specific forward routes for domains can be defined. The domain is
-chosen based on the value of the `domain_selector` config variable.
+More specific forward routes for domains can be defined. The domain is chosen based on the value of the `domain_selector` config variable.
 
-When `domain_selector` is set to `rcpt_to` (the default), more specific
-routes are only honored for SMTP connections with a single recipient or SMTP
-connections where every recipient host is identical.
+When `domain_selector` is set to `rcpt_to` (the default), more specific routes are only honored for SMTP connections with a single recipient or SMTP connections where every recipient host is identical.
 
-When `domain_selector` is set to `mail_from`, the domain of the MAIL FROM
-address is used.
+When `domain_selector` is set to `mail_from`, the domain of the MAIL FROM address is used.
 
-enable\_outbound can be set or unset on a per-domain level to enable or disable
-forwarding for specific domains.
+enable\_outbound can be set or unset on a per-domain level to enable or disable forwarding for specific domains.
 
     # default SMTP host
     host=1.2.3.4

diff --git a/docs/plugins/queue/smtp_proxy.md b/docs/plugins/queue/smtp_proxy.md
@@ -1,4 +1,4 @@
-queue/smtp\_proxy
+# queue/smtp\_proxy
 ================
 
 This plugin delivers to another mail server. This is a common setup when you
@@ -12,24 +12,22 @@ particular one important facility to some setups is recipient filtering.
 However be aware that other than connect and HELO-time filtering, you will
 have as many connections to your ongoing SMTP server as you have to Haraka.
 
-Configuration
+## Configuration
 -------------
 
-* smtp\_proxy.ini
-
-  Configuration is stored in this file in the following keys:
+Configuration is stored in smtp\_proxy.ini in the following keys:
 
-    * enable\_outbound=[true]
+  * enable\_outbound=[true]
 
     SMTP proxy outbound messages (set to false to enable Haraka's
     separate Outbound mail routing (MX based delivery)).
 
   * host=HOST
 
     The host to connect to.
-    
+
   * port=PORT
-    
+
     The port to connect to.
 
   * connect\_timeout=SECONDS
@@ -38,17 +36,17 @@ Configuration
     to the host.  Default if unspecified is 30 seconds.
 
   * timeout=SECONDS
-    
+
     The amount of seconds to let a backend connection live idle in the
     proxy pool.  This should always be less than the global plugin timeout,
     which should in turn be less than the connection timeout.
 
   * max\_connections=NUMBER
-    
+
     Maximum number of connections to create at any given time.
 
   * enable\_tls=[true|yes|1]
- 
+
     Enable TLS with the forward host (if supported). TLS uses options from
     the tls plugin.
 

diff --git a/plugins/queue/smtp_forward.js b/plugins/queue/smtp_forward.js
@@ -26,10 +26,12 @@ exports.register = function () {
     this.register_hook('queue', 'queue_forward');
 
     if (this.cfg.main.enable_outbound) {
+        // deliver local message via smtp forward when relaying=true
         this.register_hook('queue_outbound', 'queue_forward');
-    }
 
-    this.register_hook('get_mx', 'get_mx'); // for relaying outbound messages
+        // may specify more specific routes for outbound
+        this.register_hook('get_mx', 'get_mx');
+    }
 }
 
 exports.load_smtp_forward_ini = function () {
@@ -75,7 +77,7 @@ exports.is_outbound_enabled = function (dom_cfg) {
     if ('enable_outbound' in dom_cfg) return dom_cfg.enable_outbound; // per-domain flag
 
     return this.cfg.main.enable_outbound; // follow the global configuration
-};
+}
 
 exports.check_sender = function (next, connection, params) {
     const txn = connection?.transaction;
@@ -99,7 +101,7 @@ exports.check_sender = function (next, connection, params) {
     }
 
     txn.results.add(this, {pass: 'mail_from'});
-    return next();
+    next();
 }
 
 exports.set_queue = function (connection, queue_wanted, domain) {
@@ -110,7 +112,6 @@ exports.set_queue = function (connection, queue_wanted, domain) {
     if (!queue_wanted) queue_wanted = dom_cfg.queue || this.cfg.main.queue;
     if (!queue_wanted) return true;
 
-
     let dst_host = dom_cfg.host || this.cfg.main.host;
     if (dst_host) dst_host = `smtp://${dst_host}`;
 
@@ -164,7 +165,7 @@ exports.check_recipient = function (next, connection, params) {
     // the MAIL FROM domain is not local and neither is the RCPT TO
     // Another RCPT plugin may vouch for this recipient.
     txn.results.add(this, {msg: 'rcpt!local'});
-    return next();
+    next();
 }
 
 exports.auth = function (cfg, connection, smtp_client) {
@@ -303,20 +304,24 @@ exports.queue_forward = function (next, connection) {
 
         smtp_client.on('bad_code', (code, msg) => {
             if (dead_sender() || !txn) return;
-            smtp_client.call_next(((code && code[0] === '5') ? DENY : DENYSOFT),
-                msg);
+            smtp_client.call_next(((code && code[0] === '5') ? DENY : DENYSOFT), msg);
             smtp_client.release();
         });
     });
 }
 
 exports.get_mx_next_hop = next_hop => {
-    const dest = url.parse(next_hop);
+    // queue.wants && queue.next_hop are mechanisms for fine-grained MX routing.
+    // Plugins can specify a queue to perform the delivery as well as a route. A
+    // plugin that uses this is qmail-deliverable, which can direct email delivery
+    // via smtp_forward, outbound (SMTP), and outbound (LMTP).
+    const dest = new url.URL(next_hop);
     const mx = {
         priority: 0,
-        port: dest.port || 25,
+        port: dest.port || (dest.protocol === 'lmtp:' ? 24 : 25),
         exchange: dest.hostname,
     }
+    if (dest.protocol === 'lmtp:') mx.using_lmtp = true;
     if (dest.auth) {
         mx.auth_type = 'plain';
         mx.auth_user = dest.auth.split(':')[0];
@@ -327,9 +332,11 @@ exports.get_mx_next_hop = next_hop => {
 
 exports.get_mx = function (next, hmail, domain) {
 
-    // hmail.todo not defined in tests.
-    if (hmail.todo.notes?.queue?.next_hop) {
-        return next(OK, this.get_mx_next_hop(hmail.todo.notes?.queue?.next_hop));
+    const qw = hmail.todo.notes.get('queue.wants')
+    if (qw && qw !== 'smtp_forward') return next()
+
+    if (qw === 'smtp_forward' && hmail.todo.notes.get('queue.next_hop')) {
+        return next(OK, this.get_mx_next_hop(hmail.todo.notes.get('queue.next_hop')));
     }
 
     const dom = this.cfg.main.domain_selector === 'mail_from' ? hmail.todo.mail_from.host.toLowerCase() : domain.toLowerCase();
@@ -341,8 +348,7 @@ exports.get_mx = function (next, hmail, domain) {
     }
 
     const mx_opts = [
-        'auth_type', 'auth_user', 'auth_pass', 'bind', 'bind_helo',
-        'using_lmtp'
+        'auth_type', 'auth_user', 'auth_pass', 'bind', 'bind_helo', 'using_lmtp'
     ]
 
     const mx = {
@@ -357,5 +363,5 @@ exports.get_mx = function (next, hmail, domain) {
         mx[o] = this.cfg[dom][o];
     })
 
-    return next(OK, mx);
+    next(OK, mx);
 }
diff --git a/tests/plugins/queue/smtp_forward.js b/tests/plugins/queue/smtp_forward.js
@@ -143,6 +143,16 @@ exports.get_mx = {
             test.done();
         }, this.hmail, 'undefined.com');
     },
+    'returns no outbound route when queue.wants !== smtp_forward' (test) {
+        test.expect(2);
+        this.hmail.todo.notes.set('queue.wants', 'outbound')
+        this.hmail.todo.notes.set('queue.next_hop', 'smtp://5.4.3.2:26')
+        this.plugin.get_mx((code, mx) => {
+            test.equal(code, undefined);
+            test.deepEqual(mx, undefined);
+            test.done();
+        }, this.hmail, 'undefined.com');
+    },
     'returns an outbound route for defined domains' (test) {
         test.expect(2);
         this.plugin.get_mx((code, mx) => {
@@ -165,7 +175,16 @@ exports.get_mx = {
             test.done();
         }, this.hmail, 'undefined.com');
     },
-
+    'sets using_lmtp when next_hop URL is lmtp' (test) {
+        test.expect(2);
+        this.hmail.todo.notes.set('queue.wants', 'smtp_forward')
+        this.hmail.todo.notes.set('queue.next_hop', 'lmtp://4.3.2.1')
+        this.plugin.get_mx((code, mx) => {
+            test.equal(code, OK);
+            test.deepEqual(mx, { priority: 0, port: 24, using_lmtp: true, exchange: '4.3.2.1' });
+            test.done();
+        }, this.hmail, 'undefined.com');
+    },
 }
 
 exports.is_outbound_enabled = {