ZMS-4: PAN-279746 fix

lib/sender.js

@@ -9,7 +9,6 @@ const os = require('os');
 const bounces = require('./bounces');
 const Headers = require('@zone-eu/mailsplit').Headers;
 const SMTPConnection = require('nodemailer/lib/smtp-connection');
-const tls = require('tls');
 const dkimSign = require('./dkim-sign');
 const StreamHash = require('./stream-hash');
 const EventEmitter = require('events');
@@ -20,6 +19,7 @@ const mxConnect = require('mx-connect');
 const addressTools = require('./address-tools');
 const libmime = require('libmime');
 const db = require('./db');
+const tlsRetry = require('./tls-retry');
 const base32 = require('base32.js');
 const { gelfCode, emitGelf } = require('./log-gelf');
 
@@ -1097,6 +1097,7 @@ class Sender extends EventEmitter {
         let mxLastError = false;
         let ignoreMXHosts = [];
         let ignoreTLS = !!this.zone.ignoreTLS || false;
+        let reducedClientHelloHosts = new Set();
 
         let tryConnect = () => {
             delivery.mxPort = delivery.mxPort || this.zone.port || 25;
@@ -1295,6 +1296,7 @@ class Sender extends EventEmitter {
                 );
 
                 delivery.mxHostname = mx.hostname || mx.host;
+                let reducedClientHello = reducedClientHelloHosts.has(delivery.mxHostname);
 
                 let options = {
                     servername: mx.hostname,
@@ -1319,11 +1321,7 @@ class Sender extends EventEmitter {
                     connectionTimeout: 5 * 60 * 1000,
                     greetingTimeout: 2 * 60 * 1000,
 
-                    tls: {
-                        servername: mx.hostname,
-                        rejectUnauthorized: enforceTLS,
-                        minVersion: enforceTLS ? 'TLSv1.2' : 'TLSv1'
-                    },
+                    tls: tlsRetry.getTlsOptions(mx.hostname, enforceTLS, reducedClientHello),
 
                     transactionLog: true,
                     logger: {
@@ -1370,13 +1368,27 @@ class Sender extends EventEmitter {
                         err.temporary = true;
                     }
 
-                    if (
-                        (err.code === 'ETLS' ||
-                            /SSL23_GET_SERVER_HELLO|\/deps\/openssl|ssl3_check|ssl3_get_record|SSL routines/i.test(err.message) ||
-                            err.code === 'ECONNRESET') &&
-                        !ignoreTLS &&
-                        !enforceTLS
-                    ) {
+                    let tlsAttempted = tlsRetry.isTlsAttempted(connection, options.secure);
+
+                    if (tlsRetry.shouldRetryWithReducedClientHello(tlsAttempted, reducedClientHello, err)) {
+                        log.info(
+                            this.logName,
+                            'id=%s %s.%s ERRCONNECT [%s] Failed to connect to %s[%s] using %s, retrying once with a reduced TLS ClientHello. %s',
+                            delivery.sessionId,
+                            delivery.id,
+                            delivery.seq,
+                            connId,
+                            mx.hostname,
+                            mx.host,
+                            options.secure ? 'SSL/TLS' : 'STARTTLS',
+                            err.message
+                        );
+                        plugins.handler.runHooks('sender:tlserror', [delivery, options, err], () => false);
+                        reducedClientHelloHosts.add(delivery.mxHostname);
+                        return tryConnect();
+                    }
+
+                    if (tlsRetry.shouldRetryWithoutTls(ignoreTLS, enforceTLS, options.secure, err)) {
                         // STARTTLS failed, try again, this time without encryption
                         log.info(
                             this.logName,
@@ -1396,7 +1408,7 @@ class Sender extends EventEmitter {
                         return tryConnect();
                     }
 
-                    if (err.code === 'ETIMEDOUT') {
+                    if (tlsRetry.isNetworkTimeout(err)) {
                         // retry to some other MX
                         ignoreMXHosts.push(mx.host);
                         mxLastError = err;
@@ -1482,7 +1494,7 @@ class Sender extends EventEmitter {
                         }
                         // still have not returned, this means we have an unexpected connection close
                         let err = new Error('Unexpected socket close');
-                        if (connection._socket instanceof tls.TLSSocket || (connection._socket && connection._socket.upgrading)) {
+                        if (tlsRetry.isTlsAttempted(connection)) {
                             err.code = 'ETLS';
                         }
                         _onError(err);

lib/tls-retry.js

@@ -0,0 +1,65 @@
+'use strict';
+
+const tls = require('tls');
+
+const REDUCED_TLS_CLIENT_HELLO_OPTIONS = Object.freeze({
+    minVersion: 'TLSv1.3',
+    maxVersion: 'TLSv1.3',
+    ciphers: 'TLS_AES_128_GCM_SHA256:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_256_GCM_SHA384',
+    sigalgs: 'ecdsa_secp256r1_sha256:ecdsa_secp384r1_sha384:rsa_pss_rsae_sha256:rsa_pss_rsae_sha384:rsa_pkcs1_sha256:rsa_pkcs1_sha384',
+    ecdhCurve: 'X25519:P-256:P-384'
+});
+
+const TLS_ERROR_RE =
+    /SSL23_GET_SERVER_HELLO|\/deps\/openssl|ssl3_check|ssl3_get_record|SSL routines|disconnected\s+before\s+secure\s+TLS\s+connection\s+was\s+established|alert handshake failure|wrong version number/i;
+
+function getTlsOptions(servername, enforceTLS, reducedClientHello) {
+    let tlsOptions = {
+        servername,
+        rejectUnauthorized: enforceTLS,
+        minVersion: enforceTLS ? 'TLSv1.2' : 'TLSv1'
+    };
+
+    if (reducedClientHello) {
+        tlsOptions = Object.assign(tlsOptions, REDUCED_TLS_CLIENT_HELLO_OPTIONS);
+    }
+
+    return tlsOptions;
+}
+
+function isNetworkTimeout(err) {
+    return !!(err && (err.code === 'ETIMEDOUT' || (err.category === 'network' && /timed out/i.test(err.message || err.response || ''))));
+}
+
+function isTlsRetryError(err) {
+    return !!(err && !err.cert && (err.code === 'ETLS' || err.code === 'ECONNRESET' || isNetworkTimeout(err) || TLS_ERROR_RE.test(err.message || '')));
+}
+
+function isTlsFallbackError(err) {
+    return !!(err && (err.code === 'ETLS' || err.code === 'ECONNRESET' || TLS_ERROR_RE.test(err.message || '')));
+}
+
+function isTlsAttempted(connection, secure) {
+    return !!(
+        secure ||
+        (connection &&
+            (connection.secure || connection.upgrading || (connection._socket && connection._socket.upgrading) || connection._socket instanceof tls.TLSSocket))
+    );
+}
+
+function shouldRetryWithReducedClientHello(tlsAttempted, reducedClientHello, err) {
+    return !!(tlsAttempted && !reducedClientHello && isTlsRetryError(err));
+}
+
+function shouldRetryWithoutTls(ignoreTLS, enforceTLS, secure, err) {
+    return !!(!ignoreTLS && !enforceTLS && !secure && isTlsFallbackError(err));
+}
+
+module.exports.REDUCED_TLS_CLIENT_HELLO_OPTIONS = REDUCED_TLS_CLIENT_HELLO_OPTIONS;
+module.exports.getTlsOptions = getTlsOptions;
+module.exports.isTlsFallbackError = isTlsFallbackError;
+module.exports.isTlsAttempted = isTlsAttempted;
+module.exports.isNetworkTimeout = isNetworkTimeout;
+module.exports.isTlsRetryError = isTlsRetryError;
+module.exports.shouldRetryWithReducedClientHello = shouldRetryWithReducedClientHello;
+module.exports.shouldRetryWithoutTls = shouldRetryWithoutTls;

test/tls-retry-test.js

@@ -0,0 +1,88 @@
+'use strict';
+
+const tlsRetry = require('../lib/tls-retry');
+
+module.exports['Build default TLS options'] = test => {
+    let tlsOptions = tlsRetry.getTlsOptions('mx.example', false, false);
+
+    test.equal(tlsOptions.servername, 'mx.example');
+    test.equal(tlsOptions.rejectUnauthorized, false);
+    test.equal(tlsOptions.minVersion, 'TLSv1');
+    test.ok(!('maxVersion' in tlsOptions));
+
+    test.done();
+};
+
+module.exports['Build reduced ClientHello TLS options'] = test => {
+    let tlsOptions = tlsRetry.getTlsOptions('mx.example', true, true);
+
+    test.equal(tlsOptions.servername, 'mx.example');
+    test.equal(tlsOptions.rejectUnauthorized, true);
+    test.equal(tlsOptions.minVersion, 'TLSv1.3');
+    test.equal(tlsOptions.maxVersion, 'TLSv1.3');
+    test.equal(tlsOptions.ciphers, tlsRetry.REDUCED_TLS_CLIENT_HELLO_OPTIONS.ciphers);
+    test.equal(tlsOptions.sigalgs, tlsRetry.REDUCED_TLS_CLIENT_HELLO_OPTIONS.sigalgs);
+    test.equal(tlsOptions.ecdhCurve, tlsRetry.REDUCED_TLS_CLIENT_HELLO_OPTIONS.ecdhCurve);
+
+    test.done();
+};
+
+module.exports['Retry reduced ClientHello only once for TLS handshake errors'] = test => {
+    let timeoutErr = new Error('Timeout');
+    timeoutErr.code = 'ETIMEDOUT';
+
+    test.ok(tlsRetry.isNetworkTimeout(timeoutErr));
+    test.ok(tlsRetry.shouldRetryWithReducedClientHello(true, false, timeoutErr));
+    test.ok(!tlsRetry.shouldRetryWithReducedClientHello(true, true, timeoutErr));
+    test.ok(!tlsRetry.shouldRetryWithReducedClientHello(false, false, timeoutErr));
+
+    test.done();
+};
+
+module.exports['Recognize mxConnect-generated network timeouts'] = test => {
+    let timeoutErr = new Error('Network error when connecting to MX server mx.example[192.0.2.1] for example.com: Connection timed out when connecting to MX server');
+    timeoutErr.category = 'network';
+    timeoutErr.temporary = true;
+    timeoutErr.response = 'Network error: Connection timed out when connecting to MX server';
+
+    let connectionErr = new Error('Network error when connecting to MX server mx.example[192.0.2.1] for example.com: Connection refused');
+    connectionErr.category = 'network';
+    connectionErr.temporary = true;
+
+    test.ok(tlsRetry.isNetworkTimeout(timeoutErr));
+    test.ok(!tlsRetry.isNetworkTimeout(connectionErr));
+
+    test.done();
+};
+
+module.exports['Treat STARTTLS upgrade state as a TLS attempt'] = test => {
+    test.ok(tlsRetry.isTlsAttempted({ upgrading: true }, false));
+    test.ok(tlsRetry.isTlsAttempted({ secure: true }, false));
+    test.ok(tlsRetry.isTlsAttempted(false, true));
+    test.ok(!tlsRetry.isTlsAttempted({}, false));
+
+    test.done();
+};
+
+module.exports['Do not retry reduced ClientHello for certificate failures'] = test => {
+    let certErr = new Error('certificate has expired');
+    certErr.code = 'ETLS';
+    certErr.cert = { subject: 'CN=mx.example' };
+
+    test.ok(!tlsRetry.shouldRetryWithReducedClientHello(true, false, certErr));
+    test.ok(tlsRetry.shouldRetryWithoutTls(false, false, false, certErr));
+
+    test.done();
+};
+
+module.exports['Only downgrade STARTTLS to plaintext when TLS is not policy-required'] = test => {
+    let tlsErr = new Error('ssl3_get_record:wrong version number');
+    tlsErr.code = 'ETLS';
+
+    test.ok(tlsRetry.shouldRetryWithoutTls(false, false, false, tlsErr));
+    test.ok(!tlsRetry.shouldRetryWithoutTls(false, false, true, tlsErr));
+    test.ok(!tlsRetry.shouldRetryWithoutTls(true, false, false, tlsErr));
+    test.ok(!tlsRetry.shouldRetryWithoutTls(false, true, false, tlsErr));
+
+    test.done();
+};