Skip to content

Commit 3e6b2b0

Browse files
committed
ZMS-4: PAN-279746 fix
1 parent ee40254 commit 3e6b2b0

3 files changed

Lines changed: 157 additions & 14 deletions

File tree

lib/sender.js

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ const os = require('os');
99
const bounces = require('./bounces');
1010
const Headers = require('@zone-eu/mailsplit').Headers;
1111
const SMTPConnection = require('nodemailer/lib/smtp-connection');
12-
const tls = require('tls');
1312
const dkimSign = require('./dkim-sign');
1413
const StreamHash = require('./stream-hash');
1514
const EventEmitter = require('events');
@@ -20,6 +19,7 @@ const mxConnect = require('mx-connect');
2019
const addressTools = require('./address-tools');
2120
const libmime = require('libmime');
2221
const db = require('./db');
22+
const tlsRetry = require('./tls-retry');
2323
const base32 = require('base32.js');
2424
const { gelfCode, emitGelf } = require('./log-gelf');
2525

@@ -1097,6 +1097,7 @@ class Sender extends EventEmitter {
10971097
let mxLastError = false;
10981098
let ignoreMXHosts = [];
10991099
let ignoreTLS = !!this.zone.ignoreTLS || false;
1100+
let reducedClientHelloHosts = new Set();
11001101

11011102
let tryConnect = () => {
11021103
delivery.mxPort = delivery.mxPort || this.zone.port || 25;
@@ -1295,6 +1296,7 @@ class Sender extends EventEmitter {
12951296
);
12961297

12971298
delivery.mxHostname = mx.hostname || mx.host;
1299+
let reducedClientHello = reducedClientHelloHosts.has(delivery.mxHostname);
12981300

12991301
let options = {
13001302
servername: mx.hostname,
@@ -1319,11 +1321,7 @@ class Sender extends EventEmitter {
13191321
connectionTimeout: 5 * 60 * 1000,
13201322
greetingTimeout: 2 * 60 * 1000,
13211323

1322-
tls: {
1323-
servername: mx.hostname,
1324-
rejectUnauthorized: enforceTLS,
1325-
minVersion: enforceTLS ? 'TLSv1.2' : 'TLSv1'
1326-
},
1324+
tls: tlsRetry.getTlsOptions(mx.hostname, enforceTLS, reducedClientHello),
13271325

13281326
transactionLog: true,
13291327
logger: {
@@ -1370,13 +1368,27 @@ class Sender extends EventEmitter {
13701368
err.temporary = true;
13711369
}
13721370

1373-
if (
1374-
(err.code === 'ETLS' ||
1375-
/SSL23_GET_SERVER_HELLO|\/deps\/openssl|ssl3_check|ssl3_get_record|SSL routines/i.test(err.message) ||
1376-
err.code === 'ECONNRESET') &&
1377-
!ignoreTLS &&
1378-
!enforceTLS
1379-
) {
1371+
let tlsAttempted = tlsRetry.isTlsAttempted(connection, options.secure);
1372+
1373+
if (tlsRetry.shouldRetryWithReducedClientHello(tlsAttempted, reducedClientHello, err)) {
1374+
log.info(
1375+
this.logName,
1376+
'id=%s %s.%s ERRCONNECT [%s] Failed to connect to %s[%s] using %s, retrying once with a reduced TLS ClientHello. %s',
1377+
delivery.sessionId,
1378+
delivery.id,
1379+
delivery.seq,
1380+
connId,
1381+
mx.hostname,
1382+
mx.host,
1383+
options.secure ? 'SSL/TLS' : 'STARTTLS',
1384+
err.message
1385+
);
1386+
plugins.handler.runHooks('sender:tlserror', [delivery, options, err], () => false);
1387+
reducedClientHelloHosts.add(delivery.mxHostname);
1388+
return tryConnect();
1389+
}
1390+
1391+
if (tlsRetry.shouldRetryWithoutTls(ignoreTLS, enforceTLS, options.secure, err)) {
13801392
// STARTTLS failed, try again, this time without encryption
13811393
log.info(
13821394
this.logName,
@@ -1482,7 +1494,7 @@ class Sender extends EventEmitter {
14821494
}
14831495
// still have not returned, this means we have an unexpected connection close
14841496
let err = new Error('Unexpected socket close');
1485-
if (connection._socket instanceof tls.TLSSocket || (connection._socket && connection._socket.upgrading)) {
1497+
if (tlsRetry.isTlsAttempted(connection)) {
14861498
err.code = 'ETLS';
14871499
}
14881500
_onError(err);

lib/tls-retry.js

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
'use strict';
2+
3+
const tls = require('tls');
4+
5+
const REDUCED_TLS_CLIENT_HELLO_OPTIONS = Object.freeze({
6+
minVersion: 'TLSv1.3',
7+
maxVersion: 'TLSv1.3',
8+
ciphers: 'TLS_AES_128_GCM_SHA256:TLS_CHACHA20_POLY1305_SHA256:TLS_AES_256_GCM_SHA384',
9+
sigalgs: 'ecdsa_secp256r1_sha256:ecdsa_secp384r1_sha384:rsa_pss_rsae_sha256:rsa_pss_rsae_sha384:rsa_pkcs1_sha256:rsa_pkcs1_sha384',
10+
ecdhCurve: 'X25519:P-256:P-384'
11+
});
12+
13+
const TLS_ERROR_RE =
14+
/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;
15+
16+
function getTlsOptions(servername, enforceTLS, reducedClientHello) {
17+
let tlsOptions = {
18+
servername,
19+
rejectUnauthorized: enforceTLS,
20+
minVersion: enforceTLS ? 'TLSv1.2' : 'TLSv1'
21+
};
22+
23+
if (reducedClientHello) {
24+
tlsOptions = Object.assign(tlsOptions, REDUCED_TLS_CLIENT_HELLO_OPTIONS);
25+
}
26+
27+
return tlsOptions;
28+
}
29+
30+
function isTlsRetryError(err) {
31+
return !!(err && !err.cert && (err.code === 'ETLS' || err.code === 'ECONNRESET' || err.code === 'ETIMEDOUT' || TLS_ERROR_RE.test(err.message || '')));
32+
}
33+
34+
function isTlsFallbackError(err) {
35+
return !!(err && (err.code === 'ETLS' || err.code === 'ECONNRESET' || TLS_ERROR_RE.test(err.message || '')));
36+
}
37+
38+
function isTlsAttempted(connection, secure) {
39+
return !!(
40+
secure ||
41+
(connection &&
42+
(connection.secure || connection.upgrading || (connection._socket && connection._socket.upgrading) || connection._socket instanceof tls.TLSSocket))
43+
);
44+
}
45+
46+
function shouldRetryWithReducedClientHello(tlsAttempted, reducedClientHello, err) {
47+
return !!(tlsAttempted && !reducedClientHello && isTlsRetryError(err));
48+
}
49+
50+
function shouldRetryWithoutTls(ignoreTLS, enforceTLS, secure, err) {
51+
return !!(!ignoreTLS && !enforceTLS && !secure && isTlsFallbackError(err));
52+
}
53+
54+
module.exports.REDUCED_TLS_CLIENT_HELLO_OPTIONS = REDUCED_TLS_CLIENT_HELLO_OPTIONS;
55+
module.exports.getTlsOptions = getTlsOptions;
56+
module.exports.isTlsFallbackError = isTlsFallbackError;
57+
module.exports.isTlsAttempted = isTlsAttempted;
58+
module.exports.isTlsRetryError = isTlsRetryError;
59+
module.exports.shouldRetryWithReducedClientHello = shouldRetryWithReducedClientHello;
60+
module.exports.shouldRetryWithoutTls = shouldRetryWithoutTls;

test/tls-retry-test.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
'use strict';
2+
3+
const tlsRetry = require('../lib/tls-retry');
4+
5+
module.exports['Build default TLS options'] = test => {
6+
let tlsOptions = tlsRetry.getTlsOptions('mx.example', false, false);
7+
8+
test.equal(tlsOptions.servername, 'mx.example');
9+
test.equal(tlsOptions.rejectUnauthorized, false);
10+
test.equal(tlsOptions.minVersion, 'TLSv1');
11+
test.ok(!('maxVersion' in tlsOptions));
12+
13+
test.done();
14+
};
15+
16+
module.exports['Build reduced ClientHello TLS options'] = test => {
17+
let tlsOptions = tlsRetry.getTlsOptions('mx.example', true, true);
18+
19+
test.equal(tlsOptions.servername, 'mx.example');
20+
test.equal(tlsOptions.rejectUnauthorized, true);
21+
test.equal(tlsOptions.minVersion, 'TLSv1.3');
22+
test.equal(tlsOptions.maxVersion, 'TLSv1.3');
23+
test.equal(tlsOptions.ciphers, tlsRetry.REDUCED_TLS_CLIENT_HELLO_OPTIONS.ciphers);
24+
test.equal(tlsOptions.sigalgs, tlsRetry.REDUCED_TLS_CLIENT_HELLO_OPTIONS.sigalgs);
25+
test.equal(tlsOptions.ecdhCurve, tlsRetry.REDUCED_TLS_CLIENT_HELLO_OPTIONS.ecdhCurve);
26+
27+
test.done();
28+
};
29+
30+
module.exports['Retry reduced ClientHello only once for TLS handshake errors'] = test => {
31+
let timeoutErr = new Error('Timeout');
32+
timeoutErr.code = 'ETIMEDOUT';
33+
34+
test.ok(tlsRetry.shouldRetryWithReducedClientHello(true, false, timeoutErr));
35+
test.ok(!tlsRetry.shouldRetryWithReducedClientHello(true, true, timeoutErr));
36+
test.ok(!tlsRetry.shouldRetryWithReducedClientHello(false, false, timeoutErr));
37+
38+
test.done();
39+
};
40+
41+
module.exports['Treat STARTTLS upgrade state as a TLS attempt'] = test => {
42+
test.ok(tlsRetry.isTlsAttempted({ upgrading: true }, false));
43+
test.ok(tlsRetry.isTlsAttempted({ secure: true }, false));
44+
test.ok(tlsRetry.isTlsAttempted(false, true));
45+
test.ok(!tlsRetry.isTlsAttempted({}, false));
46+
47+
test.done();
48+
};
49+
50+
module.exports['Do not retry reduced ClientHello for certificate failures'] = test => {
51+
let certErr = new Error('certificate has expired');
52+
certErr.code = 'ETLS';
53+
certErr.cert = { subject: 'CN=mx.example' };
54+
55+
test.ok(!tlsRetry.shouldRetryWithReducedClientHello(true, false, certErr));
56+
test.ok(tlsRetry.shouldRetryWithoutTls(false, false, false, certErr));
57+
58+
test.done();
59+
};
60+
61+
module.exports['Only downgrade STARTTLS to plaintext when TLS is not policy-required'] = test => {
62+
let tlsErr = new Error('ssl3_get_record:wrong version number');
63+
tlsErr.code = 'ETLS';
64+
65+
test.ok(tlsRetry.shouldRetryWithoutTls(false, false, false, tlsErr));
66+
test.ok(!tlsRetry.shouldRetryWithoutTls(false, false, true, tlsErr));
67+
test.ok(!tlsRetry.shouldRetryWithoutTls(true, false, false, tlsErr));
68+
test.ok(!tlsRetry.shouldRetryWithoutTls(false, true, false, tlsErr));
69+
70+
test.done();
71+
};

0 commit comments

Comments
 (0)