From 5795e835a1021abdf803e1460501487adbac8d7c Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Thu, 14 May 2015 10:38:18 +0200 Subject: [PATCH] tls: emit errors on close whilst async action When loading session, OCSP response, SNI, always check that the `self._handle` is present. If it is not - the socket was closed - and we should emit the error instead of throwing an uncaught exception. Fix: https://github.com/joyent/node/issues/8780 Fix: https://github.com/iojs/io.js/issues/1696 PR-URL: https://github.com/nodejs/io.js/pull/1702 Reviewed-By: Trevor Norris --- lib/_tls_wrap.js | 15 ++++ .../test-tls-async-cb-after-socket-end.js | 73 +++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 test/parallel/test-tls-async-cb-after-socket-end.js diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 9c934b9ffbbe3f..63f1f6ccbf17ef 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -61,6 +61,9 @@ function loadSession(self, hello, cb) { if (err) return cb(err); + if (!self._handle) + return cb(new Error('Socket is closed')); + // NOTE: That we have disabled OpenSSL's internal session storage in // `node_crypto.cc` and hence its safe to rely on getting servername only // from clienthello or this place. @@ -91,6 +94,9 @@ function loadSNI(self, servername, cb) { if (err) return cb(err); + if (!self._handle) + return cb(new Error('Socket is closed')); + // TODO(indutny): eventually disallow raw `SecureContext` if (context) self._handle.sni_context = context.context || context; @@ -127,6 +133,9 @@ function requestOCSP(self, hello, ctx, cb) { if (err) return cb(err); + if (!self._handle) + return cb(new Error('Socket is closed')); + if (response) self._handle.setOCSPResponse(response); cb(null); @@ -157,6 +166,9 @@ function oncertcb(info) { if (err) return self.destroy(err); + if (!self._handle) + return cb(new Error('Socket is closed')); + self._handle.certCbDone(); }); }); @@ -179,6 +191,9 @@ function onnewsession(key, session) { return; once = true; + if (!self._handle) + return cb(new Error('Socket is closed')); + self._handle.newSessionDone(); self._newSessionPending = false; diff --git a/test/parallel/test-tls-async-cb-after-socket-end.js b/test/parallel/test-tls-async-cb-after-socket-end.js new file mode 100644 index 00000000000000..db9db87f59087c --- /dev/null +++ b/test/parallel/test-tls-async-cb-after-socket-end.js @@ -0,0 +1,73 @@ +'use strict'; + +var common = require('../common'); + +var assert = require('assert'); +var path = require('path'); +var fs = require('fs'); +var constants = require('constants'); + +if (!common.hasCrypto) { + console.log('1..0 # Skipped: missing crypto'); + process.exit(); +} + +var tls = require('tls'); + +var options = { + secureOptions: constants.SSL_OP_NO_TICKET, + key: fs.readFileSync(path.join(common.fixturesDir, 'test_key.pem')), + cert: fs.readFileSync(path.join(common.fixturesDir, 'test_cert.pem')) +}; + +var server = tls.createServer(options, function(c) { +}); + +var sessionCb = null; +var client = null; + +server.on('newSession', function(key, session, done) { + done(); +}); + +server.on('resumeSession', function(id, cb) { + sessionCb = cb; + + next(); +}); + +server.listen(1443, function() { + var clientOpts = { + port: 1443, + rejectUnauthorized: false, + session: false + }; + + var s1 = tls.connect(clientOpts, function() { + clientOpts.session = s1.getSession(); + console.log('1st secure'); + + s1.destroy(); + var s2 = tls.connect(clientOpts, function(s) { + console.log('2nd secure'); + + s2.destroy(); + }).on('connect', function() { + console.log('2nd connected'); + client = s2; + + next(); + }); + }); +}); + +function next() { + if (!client || !sessionCb) + return; + + client.destroy(); + setTimeout(function() { + sessionCb(); + server.close(); + }, 100); +}