Skip to content

Commit 7573b55

Browse files
bnoordhuisBethGriggs
authored andcommittedMar 20, 2019
tls: fix legacy SecurePair clienthello race window
There is a time window between the first and the last step of processing the clienthello event and the SecurePair may have been destroyed during that interval. Fixes: #26428 PR-URL: #26452 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
1 parent 91620b8 commit 7573b55

File tree

2 files changed

+84
-0
lines changed

2 files changed

+84
-0
lines changed
 

‎lib/_tls_legacy.js

+4
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,10 @@ function onclienthello(hello) {
659659
if (err) return self.socket.destroy(err);
660660

661661
setImmediate(function() {
662+
// SecurePair might have been destroyed in the time window
663+
// between callback() and this function.
664+
if (!self.ssl) return;
665+
662666
self.ssl.loadSession(session);
663667
self.ssl.endParser();
664668

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const fixtures = require('../common/fixtures');
7+
const SSL_OP_NO_TICKET = require('crypto').constants.SSL_OP_NO_TICKET;
8+
const assert = require('assert');
9+
const net = require('net');
10+
const tls = require('tls');
11+
12+
const options = {
13+
secureOptions: SSL_OP_NO_TICKET,
14+
key: fixtures.readSync('test_key.pem'),
15+
cert: fixtures.readSync('test_cert.pem')
16+
};
17+
18+
const server = net.createServer(function(socket) {
19+
const sslcontext = tls.createSecureContext(options);
20+
sslcontext.context.setCiphers('RC4-SHA:AES128-SHA:AES256-SHA');
21+
22+
const pair = tls.createSecurePair(sslcontext, true, false, false, { server });
23+
24+
assert.ok(pair.encrypted.writable);
25+
assert.ok(pair.cleartext.writable);
26+
27+
pair.encrypted.pipe(socket);
28+
socket.pipe(pair.encrypted);
29+
30+
pair.on('error', () => {}); // Expected, client s1 closes connection.
31+
});
32+
33+
let sessionCb = null;
34+
let client = null;
35+
36+
server.on('newSession', common.mustCall(function(key, session, done) {
37+
done();
38+
}));
39+
40+
server.on('resumeSession', common.mustCall(function(id, cb) {
41+
sessionCb = cb;
42+
43+
next();
44+
}));
45+
46+
server.listen(0, function() {
47+
const clientOpts = {
48+
port: this.address().port,
49+
rejectUnauthorized: false,
50+
session: false
51+
};
52+
53+
const s1 = tls.connect(clientOpts, function() {
54+
clientOpts.session = s1.getSession();
55+
console.log('1st secure');
56+
57+
s1.destroy();
58+
const s2 = tls.connect(clientOpts, function(s) {
59+
console.log('2nd secure');
60+
61+
s2.destroy();
62+
}).on('connect', function() {
63+
console.log('2nd connected');
64+
client = s2;
65+
66+
next();
67+
});
68+
});
69+
});
70+
71+
function next() {
72+
if (!client || !sessionCb)
73+
return;
74+
75+
client.destroy();
76+
setTimeout(function() {
77+
sessionCb();
78+
server.close();
79+
}, 100);
80+
}

0 commit comments

Comments
 (0)
Please sign in to comment.