Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bnoordhuis authored and BethGriggs committed Mar 20, 2019
1 parent 91620b8 commit 7573b55
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/_tls_legacy.js
Expand Up @@ -659,6 +659,10 @@ function onclienthello(hello) {
if (err) return self.socket.destroy(err);

setImmediate(function() {
// SecurePair might have been destroyed in the time window
// between callback() and this function.
if (!self.ssl) return;

self.ssl.loadSession(session);
self.ssl.endParser();

Expand Down
80 changes: 80 additions & 0 deletions test/parallel/test-tls-async-cb-after-socket-end-securepair.js
@@ -0,0 +1,80 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const fixtures = require('../common/fixtures');
const SSL_OP_NO_TICKET = require('crypto').constants.SSL_OP_NO_TICKET;
const assert = require('assert');
const net = require('net');
const tls = require('tls');

const options = {
secureOptions: SSL_OP_NO_TICKET,
key: fixtures.readSync('test_key.pem'),
cert: fixtures.readSync('test_cert.pem')
};

const server = net.createServer(function(socket) {
const sslcontext = tls.createSecureContext(options);
sslcontext.context.setCiphers('RC4-SHA:AES128-SHA:AES256-SHA');

const pair = tls.createSecurePair(sslcontext, true, false, false, { server });

assert.ok(pair.encrypted.writable);
assert.ok(pair.cleartext.writable);

pair.encrypted.pipe(socket);
socket.pipe(pair.encrypted);

pair.on('error', () => {}); // Expected, client s1 closes connection.
});

let sessionCb = null;
let client = null;

server.on('newSession', common.mustCall(function(key, session, done) {
done();
}));

server.on('resumeSession', common.mustCall(function(id, cb) {
sessionCb = cb;

next();
}));

server.listen(0, function() {
const clientOpts = {
port: this.address().port,
rejectUnauthorized: false,
session: false
};

const s1 = tls.connect(clientOpts, function() {
clientOpts.session = s1.getSession();
console.log('1st secure');

s1.destroy();
const 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);
}

0 comments on commit 7573b55

Please sign in to comment.