Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tls: getTicketKeys()/setTicketKeys() #2227

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions doc/api/tls.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -630,6 +630,21 @@ Returns the bound address, the address family name and port of the
server as reported by the operating system. See [net.Server.address()][] for
more information.

### server.getTicketKeys()

Returns `Buffer` instance holding the keys currently used for
encryption/decryption of the [TLS Session Tickets][]

### server.setTicketKeys(keys)

Updates the keys for encryption/decryption of the [TLS Session Tickets][].

NOTE: the buffer should be 48 bytes long. It will be split into 3 parts inside
of the OpenSSL for HMAC key, AES key, and Ticket Key name.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the second sentence is an irrelevant implementation detail?


NOTE: the change is effective only for the future server connections. Existing
or currently pending server connections will use previous keys.

### server.addContext(hostname, context)

Add secure context that will be used if client request's SNI hostname is
Expand Down Expand Up @@ -835,3 +850,4 @@ The numeric representation of the local port.
[asn1.js]: http://npmjs.org/package/asn1.js
[OCSP request]: http://en.wikipedia.org/wiki/OCSP_stapling
[TLS recommendations]: https://wiki.mozilla.org/Security/Server_Side_TLS
[TLS Session Tickets]: https://www.ietf.org/rfc/rfc5077.txt
14 changes: 12 additions & 2 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -812,13 +812,23 @@ exports.createServer = function(options, listener) {

Server.prototype._getServerData = function() {
return {
ticketKeys: this._sharedCreds.context.getTicketKeys().toString('hex')
ticketKeys: this.getTicketKeys().toString('hex')
};
};


Server.prototype._setServerData = function(data) {
this._sharedCreds.context.setTicketKeys(new Buffer(data.ticketKeys, 'hex'));
this.setTicketKeys(new Buffer(data.ticketKeys, 'hex'));
};


Server.prototype.getTicketKeys = function getTicketKeys(keys) {
return this._sharedCreds.context.getTicketKeys(keys);
};


Server.prototype.setTicketKeys = function setTicketKeys(keys) {
this._sharedCreds.context.setTicketKeys(keys);
};


Expand Down
32 changes: 27 additions & 5 deletions test/parallel/test-tls-ticket.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,37 @@ var serverCount = 0;
function createServer() {
var id = serverCount++;

var counter = 0;
var previousKey = null;

var server = tls.createServer({
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'),
ticketKeys: keys
}, function(c) {
serverLog.push(id);
c.end();

counter++;

// Rotate ticket keys
if (counter === 1) {
previousKey = server.getTicketKeys();
server.setTicketKeys(crypto.randomBytes(48));
} else if (counter === 2) {
server.setTicketKeys(previousKey);
} else {
throw new Error('UNREACHABLE');
}
});

return server;
}

var servers = [ createServer(), createServer(),
createServer(), createServer(),
createServer(), createServer() ];
var naturalServers = [ createServer(), createServer(), createServer() ];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this imply there are unnatural servers as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they are just clones :(


// 3x servers
var servers = naturalServers.concat(naturalServers).concat(naturalServers);

// Create one TCP server and balance sockets to multiple TLS server instances
var shared = net.createServer(function(c) {
Expand All @@ -54,7 +70,7 @@ function start(callback) {
session: sess,
rejectUnauthorized: false
}, function() {
sess = s.getSession() || sess;
sess = sess || s.getSession();
ticketLog.push(s.getTLSTicket().toString('hex'));
});
s.on('close', function() {
Expand All @@ -70,8 +86,14 @@ function start(callback) {

process.on('exit', function() {
assert.equal(ticketLog.length, serverLog.length);
for (var i = 0; i < serverLog.length - 1; i++) {
for (var i = 0; i < naturalServers.length - 1; i++) {
assert.notEqual(serverLog[i], serverLog[i + 1]);
assert.equal(ticketLog[i], ticketLog[i + 1]);

// 2nd connection should have different ticket
assert.notEqual(ticketLog[i], ticketLog[i + naturalServers.length]);

// 3rd connection should have the same ticket
assert.equal(ticketLog[i], ticketLog[i + naturalServers.length * 2]);
}
});