Skip to content

Commit e643f5b

Browse files
committed
Revert "set tls.sessionIdContext property (haraka#1740)"
This reverts commit 2f9dbb6.
1 parent 749efdb commit e643f5b

File tree

1 file changed

+57
-30
lines changed

1 file changed

+57
-30
lines changed

tls_socket.js

Lines changed: 57 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
/*----------------------------------------------------------------------------------------------*/
55

66
var tls = require('tls');
7+
var constants = require('constants');
8+
var crypto = require('crypto');
79
var util = require('util');
810
var net = require('net');
911
var stream = require('stream');
1012
var log = require('./logger');
1113
var EventEmitter = require('events');
1214

13-
var secureContext;
14-
1515
var ocsp;
1616
try {
1717
ocsp = require('ocsp');
@@ -208,28 +208,6 @@ if (ocsp) {
208208

209209
exports.ocsp = ocsp;
210210

211-
function _getSecureContext(options) {
212-
if (secureContext) return secureContext;
213-
214-
if (options === undefined) options = {};
215-
216-
if (options.requestCert === undefined) {
217-
options.requestCert = true;
218-
}
219-
if (options.rejectUnauthorized === undefined) {
220-
options.rejectUnauthorized = false;
221-
}
222-
if (!options.sessionIdContext) {
223-
options.sessionIdContext = 'haraka';
224-
};
225-
if (!options.sessionTimeout) {
226-
// options.sessionTimeout = 1;
227-
};
228-
229-
secureContext = tls.createSecureContext(options);
230-
return secureContext;
231-
}
232-
233211
function createServer(cb) {
234212
var serv = net.createServer(function (cryptoSocket) {
235213

@@ -241,16 +219,47 @@ function createServer(cb) {
241219
socket.clean();
242220
cryptoSocket.removeAllListeners('data');
243221

222+
// Set SSL_OP_ALL for maximum compatibility with broken clients
223+
// See http://www.openssl.org/docs/ssl/SSL_CTX_set_options.html
244224
if (!options) options = {};
245-
225+
// TODO: bug in Node means we can't do this until it's fixed
226+
// options.secureOptions = constants.SSL_OP_ALL;
227+
228+
// Setting secureProtocol to 'SSLv23_method' and secureOptions to
229+
// constants.SSL_OP_NO_SSLv2/3 are used to disable SSLv2 and SSLv3
230+
// protocol support, to prevent DROWN and POODLE attacks at least.
231+
// Node's docs here are super unhelpful, e.g.
232+
// <https://nodejs.org/api/tls.html#tls_tls_createserver_options_secureconnectionlistener>
233+
// doesn't even mention secureOptions. Some digging reveals the
234+
// relevant openssl docs: secureProtocol is documented in
235+
// <https://www.openssl.org/docs/manmaster/ssl/ssl.html#DEALING-WITH-PROTOCOL-METHODS>,
236+
// (note: you'll want to select the correct openssl version that
237+
// node was compiled against, instead of master), and secureOptions
238+
// are documented in
239+
// <https://www.openssl.org/docs/manmaster/ssl/SSL_CTX_set_options.html>
240+
// (again, select the appropriate openssl version, not master).
241+
// One caveat: it doesn't seem like all options are actually
242+
// compiled into node. To see which ones are, fire up node and
243+
// examine the object returned by require('constants').
244+
245+
// use cached secureContext as the relevant options are static
246246
if (!options.secureContext) {
247+
options.secureProtocol = options.secureProtocol || 'SSLv23_method';
248+
options.secureOptions = options.secureOptions |
249+
constants.SSL_OP_NO_SSLv2 | constants.SSL_OP_NO_SSLv3;
247250

248-
options.secureContext = _getSecureContext(options);
251+
if (options.requestCert === undefined) {
252+
options.requestCert = true;
253+
}
254+
if (options.rejectUnauthorized === undefined) {
255+
options.rejectUnauthorized = false;
256+
}
257+
options.secureContext = (tls.createSecureContext || crypto.createCredentials)(options);
249258
options.isServer = true;
250259
if (options.enableOCSPStapling) {
251260
if (ocsp) {
252261
options.server = pseudoServ;
253-
pseudoServ._sharedCreds = secureContext;
262+
pseudoServ._sharedCreds = options.secureContext;
254263
} else {
255264
log.logerr("OCSP Stapling cannot be enabled because the ocsp module is not loaded");
256265
}
@@ -314,9 +323,27 @@ function connect (port, host, cb) {
314323
socket.clean();
315324
cryptoSocket.removeAllListeners('data');
316325

326+
// Set SSL_OP_ALL for maximum compatibility with broken servers
327+
// See http://www.openssl.org/docs/ssl/SSL_CTX_set_options.html
317328
if (!options) options = {};
318-
319-
options.secureContext = _getSecureContext(options);
329+
// TODO: bug in Node means we can't do this until it's fixed
330+
// options.secureOptions = constants.SSL_OP_ALL;
331+
332+
// See comments around similar code in createServer above for what's
333+
// going on here.
334+
options.secureProtocol = options.secureProtocol || 'SSLv23_method';
335+
options.secureOptions = options.secureOptions |
336+
constants.SSL_OP_NO_SSLv2 | constants.SSL_OP_NO_SSLv3;
337+
338+
if (options) {
339+
if (options.requestCert === undefined) {
340+
options.requestCert = true;
341+
}
342+
if (options.rejectUnauthorized === undefined) {
343+
options.rejectUnauthorized = false;
344+
}
345+
}
346+
options.sslcontext = (tls.createSecureContext || crypto.createCredentials)(options);
320347
options.socket = cryptoSocket;
321348

322349
var cleartext = new tls.connect(options);
@@ -338,7 +365,7 @@ function connect (port, host, cb) {
338365
if (cb2) cb2(cleartext.authorized, cleartext.authorizationError, cert, cipher);
339366
});
340367

341-
// cleartext._controlReleased = true;
368+
// cleartext._controlReleased = true;
342369
socket.cleartext = cleartext;
343370

344371
if (socket._timeout) {

0 commit comments

Comments
 (0)