OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2026-01-15 12:15:26 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

tls: fix off-by-one error in renegotiation check

Browse Source 
Make CLIENT_RENEG_LIMIT inclusive instead of exclusive, i.e. a limit of 2
means the peer can renegotiate twice, not just once.

Update pummel/test-tls-ci-reneg-attack accordingly and make it less timing
sensitive (and run faster) while we're at it.
This commit is contained in:
Ben Noordhuis
2012-06-18 04:05:21 +02:00
parent ae5b0e1fc1
commit ff552ddbaa
2 changed files with 11 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
lib/tls.js
View File

@@ -572,16 +572,15 @@ function onhandshakestart() {
debug('onhandshakestart');
var self = this, ssl = this.ssl;
ssl.handshakes++;
if (ssl.handshakes === 1) {
if (ssl.timer === null) {
function timeout() {
ssl.handshakes = 0;
ssl.timer = null;
}
ssl.timer = setTimeout(timeout, exports.CLIENT_RENEG_WINDOW * 1000);
}
else if (ssl.handshakes >= exports.CLIENT_RENEG_LIMIT) {
else if (++ssl.handshakes > exports.CLIENT_RENEG_LIMIT) {
// Defer the error event to the next tick. We're being called from OpenSSL's
// state machine and OpenSSL is not re-entrant. We cannot allow the user's
// callback to destroy the connection right now, it would crash and burn.

14
test/pummel/test-tls-ci-reneg-attack.js
View File

@@ -49,11 +49,14 @@ function test(next) {
key: fs.readFileSync(common.fixturesDir + '/test_key.pem')
};
var seenError = false;
var server = tls.createServer(options, function(conn) {
conn.on('error', function(err) {
console.error('Caught exception: ' + err);
assert(/TLS session renegotiation attack/.test(err));
conn.destroy();
seenError = true;
});
conn.pipe(conn);
});
@@ -67,16 +70,17 @@ function test(next) {
// count handshakes, start the attack after the initial handshake is done
var handshakes = 0;
var renegs = 0;
child.stderr.on('data', function(data) {
if (seenError) return;
handshakes += (('' + data).match(/verify return:1/g) || []).length;
if (handshakes === 2) spam();
renegs += (('' + data).match(/RENEGOTIATING/g) || []).length;
});
child.on('exit', function() {
// with a renegotiation limit <= 1, we always see 4 handshake markers:
// two for the initial handshake and another two for the attempted
// renegotiation
assert.equal(handshakes, 2 * Math.max(2, tls.CLIENT_RENEG_LIMIT));
assert.equal(renegs, tls.CLIENT_RENEG_LIMIT + 1);
server.close();
process.nextTick(next);
});
@@ -94,7 +98,7 @@ function test(next) {
function spam() {
if (closed) return;
child.stdin.write('R\n');
setTimeout(spam, 250);
setTimeout(spam, 50);
}
});
}