From 3e6b2b0de6b8775b62c4db9da78d81047705d35a Mon Sep 17 00:00:00 2001 From: NickOvt Date: Mon, 23 Mar 2026 11:20:10 +0200 Subject: [PATCH 1/2] ZMS-4: PAN-279746 fix --- lib/sender.js | 40 +++++++++++++++--------- lib/tls-retry.js | 60 +++++++++++++++++++++++++++++++++++ test/tls-retry-test.js | 71 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 14 deletions(-) create mode 100644 lib/tls-retry.js create mode 100644 test/tls-retry-test.js diff --git a/lib/sender.js b/lib/sender.js index 53f16eb..96705d6 100644 --- a/lib/sender.js +++ b/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, @@ -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); diff --git a/lib/tls-retry.js b/lib/tls-retry.js new file mode 100644 index 0000000..17e8729 --- /dev/null +++ b/lib/tls-retry.js @@ -0,0 +1,60 @@ +'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 isTlsRetryError(err) { + return !!(err && !err.cert && (err.code === 'ETLS' || err.code === 'ECONNRESET' || err.code === 'ETIMEDOUT' || 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.isTlsRetryError = isTlsRetryError; +module.exports.shouldRetryWithReducedClientHello = shouldRetryWithReducedClientHello; +module.exports.shouldRetryWithoutTls = shouldRetryWithoutTls; diff --git a/test/tls-retry-test.js b/test/tls-retry-test.js new file mode 100644 index 0000000..50c3ef9 --- /dev/null +++ b/test/tls-retry-test.js @@ -0,0 +1,71 @@ +'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.shouldRetryWithReducedClientHello(true, false, timeoutErr)); + test.ok(!tlsRetry.shouldRetryWithReducedClientHello(true, true, timeoutErr)); + test.ok(!tlsRetry.shouldRetryWithReducedClientHello(false, false, timeoutErr)); + + 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(); +}; From 18fb6303b33394c0c826f4f65893ed751fbd2317 Mon Sep 17 00:00:00 2001 From: NickOvt Date: Wed, 25 Mar 2026 12:14:55 +0200 Subject: [PATCH 2/2] correctly test mx-connect timeout --- lib/sender.js | 2 +- lib/tls-retry.js | 7 ++++++- test/tls-retry-test.js | 17 +++++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/lib/sender.js b/lib/sender.js index 96705d6..1b9d732 100644 --- a/lib/sender.js +++ b/lib/sender.js @@ -1408,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; diff --git a/lib/tls-retry.js b/lib/tls-retry.js index 17e8729..44548aa 100644 --- a/lib/tls-retry.js +++ b/lib/tls-retry.js @@ -27,8 +27,12 @@ function getTlsOptions(servername, enforceTLS, reducedClientHello) { 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' || err.code === 'ETIMEDOUT' || TLS_ERROR_RE.test(err.message || ''))); + return !!(err && !err.cert && (err.code === 'ETLS' || err.code === 'ECONNRESET' || isNetworkTimeout(err) || TLS_ERROR_RE.test(err.message || ''))); } function isTlsFallbackError(err) { @@ -55,6 +59,7 @@ module.exports.REDUCED_TLS_CLIENT_HELLO_OPTIONS = REDUCED_TLS_CLIENT_HELLO_OPTIO 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; diff --git a/test/tls-retry-test.js b/test/tls-retry-test.js index 50c3ef9..aaf97ff 100644 --- a/test/tls-retry-test.js +++ b/test/tls-retry-test.js @@ -31,6 +31,7 @@ module.exports['Retry reduced ClientHello only once for TLS handshake errors'] = 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)); @@ -38,6 +39,22 @@ module.exports['Retry reduced ClientHello only once for TLS handshake errors'] = 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));