Skip to content

Commit e329020

Browse files
committed
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: nodejs/node-v0.x-archive#8780 Fix: nodejs#1696
1 parent 6ccbe75 commit e329020

File tree

2 files changed

+86
-0
lines changed

2 files changed

+86
-0
lines changed

lib/_tls_wrap.js

+15
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ function loadSession(self, hello, cb) {
6161
if (err)
6262
return cb(err);
6363

64+
if (!self._handle)
65+
return cb(new Error('Socket is closed'));
66+
6467
// NOTE: That we have disabled OpenSSL's internal session storage in
6568
// `node_crypto.cc` and hence its safe to rely on getting servername only
6669
// from clienthello or this place.
@@ -91,6 +94,9 @@ function loadSNI(self, servername, cb) {
9194
if (err)
9295
return cb(err);
9396

97+
if (!self._handle)
98+
return cb(new Error('Socket is closed'));
99+
94100
// TODO(indutny): eventually disallow raw `SecureContext`
95101
if (context)
96102
self._handle.sni_context = context.context || context;
@@ -127,6 +133,9 @@ function requestOCSP(self, hello, ctx, cb) {
127133
if (err)
128134
return cb(err);
129135

136+
if (!self._handle)
137+
return cb(new Error('Socket is closed'));
138+
130139
if (response)
131140
self._handle.setOCSPResponse(response);
132141
cb(null);
@@ -157,6 +166,9 @@ function oncertcb(info) {
157166
if (err)
158167
return self.destroy(err);
159168

169+
if (!self._handle)
170+
return cb(new Error('Socket is closed'));
171+
160172
self._handle.certCbDone();
161173
});
162174
});
@@ -179,6 +191,9 @@ function onnewsession(key, session) {
179191
return;
180192
once = true;
181193

194+
if (!self._handle)
195+
return cb(new Error('Socket is closed'));
196+
182197
self._handle.newSessionDone();
183198

184199
self._newSessionPending = false;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
var common = require('../common');
2+
3+
var assert = require('assert');
4+
var path = require('path');
5+
var fs = require('fs');
6+
var constants = require('constants');
7+
8+
if (!common.hasCrypto) {
9+
console.log('1..0 # Skipped: missing crypto');
10+
process.exit();
11+
}
12+
13+
var tls = require('tls');
14+
15+
var options = {
16+
secureOptions: constants.SSL_OP_NO_TICKET,
17+
key: fs.readFileSync(path.join(common.fixturesDir, 'test_key.pem')),
18+
cert: fs.readFileSync(path.join(common.fixturesDir, 'test_cert.pem'))
19+
};
20+
21+
var server = tls.createServer(options, function(c) {
22+
});
23+
24+
var sessionCb = null;
25+
var client = null;
26+
27+
server.on('newSession', function(key, session, done) {
28+
done();
29+
});
30+
31+
server.on('resumeSession', function(id, cb) {
32+
sessionCb = cb;
33+
34+
next();
35+
});
36+
37+
server.listen(1443, function() {
38+
var clientOpts = {
39+
port: 1443,
40+
rejectUnauthorized: false,
41+
session: false
42+
};
43+
44+
var s1 = tls.connect(clientOpts, function() {
45+
clientOpts.session = s1.getSession();
46+
console.log('1st secure');
47+
48+
s1.destroy();
49+
var s2 = tls.connect(clientOpts, function(s) {
50+
console.log('2nd secure');
51+
52+
s2.destroy();
53+
}).on('connect', function() {
54+
console.log('2nd connected');
55+
client = s2;
56+
57+
next();
58+
});
59+
});
60+
})
61+
62+
function next() {
63+
if (!client || !sessionCb)
64+
return;
65+
66+
client.destroy();
67+
setTimeout(function() {
68+
sessionCb();
69+
server.close();
70+
}, 100);
71+
}

0 commit comments

Comments
 (0)