Skip to content

Commit

Permalink
tls: fix check for reused session
Browse files Browse the repository at this point in the history
When TLS Session Ticket is renewed by server - no Certificate record is
to the client. We are prepared for empty certificate in this case, but
this relies on the session reuse check, which was implemented
incorrectly and was returning false when the TLS Session Ticket was
renewed.

Use session reuse check provided by OpenSSL instead.

Fix: #2304
PR-URL: #2312
Reviewed-By: Shigeki Ohtsu <[email protected]>
  • Loading branch information
indutny authored and Fishrock123 committed Aug 11, 2015
1 parent 51b6bdc commit 581850b
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 12 deletions.
13 changes: 1 addition & 12 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -584,17 +584,6 @@ TLSSocket.prototype._start = function() {
this._handle.start();
};

TLSSocket.prototype._isSessionResumed = function _isSessionResumed(session) {
if (!session)
return false;

var next = this.getSession();
if (!next)
return false;

return next.equals(session);
};

TLSSocket.prototype.setServername = function(name) {
this._handle.setServername(name);
};
Expand Down Expand Up @@ -1011,7 +1000,7 @@ exports.connect = function(/* [port, host], options, cb */) {

// Verify that server's identity matches it's certificate's names
// Unless server has resumed our existing session
if (!verifyError && !socket._isSessionResumed(options.session)) {
if (!verifyError && !socket.isSessionReused()) {
var cert = socket.getPeerCertificate();
verifyError = options.checkServerIdentity(hostname, cert);
}
Expand Down
56 changes: 56 additions & 0 deletions test/parallel/test-https-resume-after-renew.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';
var common = require('../common');
var fs = require('fs');
var https = require('https');
var crypto = require('crypto');

var options = {
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'),
ca: fs.readFileSync(common.fixturesDir + '/keys/ca1-cert.pem')
};

var server = https.createServer(options, function(req, res) {
res.end('hello');
});

var aes = new Buffer(16);
aes.fill('S');
var hmac = new Buffer(16);
hmac.fill('H');

server._sharedCreds.context.enableTicketKeyCallback();
server._sharedCreds.context.onticketkeycallback = function(name, iv, enc) {
if (enc) {
var newName = new Buffer(16);
var newIV = crypto.randomBytes(16);
newName.fill('A');
} else {
// Renew
return [ 2, hmac, aes ];
}

return [ 1, hmac, aes, newName, newIV ];
};

server.listen(common.PORT, function() {
var addr = this.address();

function doReq(callback) {
https.request({
method: 'GET',
port: addr.port,
servername: 'agent1',
ca: options.ca
}, function(res) {
res.resume();
res.once('end', callback);
}).end();
}

doReq(function() {
doReq(function() {
server.close();
});
});
});

0 comments on commit 581850b

Please sign in to comment.