Permalink
Browse files

tls: mitigate session renegotiation attacks

The TLS protocol allows (and sometimes requires) clients to renegotiate the
session. However, renegotiation requires a disproportional amount of server-side
resources, particularly CPU time, which makes it a potential vector for
denial-of-service attacks.

To mitigate this issue, we keep track of and limit the number of renegotiation
requests over time, emitting an error if the threshold is exceeded.
  • Loading branch information...
1 parent 756ab13 commit 1d8950dd84169fd3ebec382f3fe70e4d9fc6402d @bnoordhuis committed Feb 15, 2012
Showing with 165 additions and 0 deletions.
  1. +52 −0 lib/tls.js
  2. +16 −0 src/node_crypto.cc
  3. +2 −0 src/node_crypto.h
  4. +95 −0 test/pummel/test-tls-ci-reneg-attack.js
View
52 lib/tls.js
@@ -27,6 +27,14 @@ var stream = require('stream');
var END_OF_FILE = 42;
var assert = require('assert').ok;
+// Allow {TLS_CLIENT_RENEG_LIMIT} client-initiated session renegotiations
+// every {TLS_CLIENT_RENEG_WINDOW} seconds. An error event is emitted if more
+// renegotations are seen. The settings are applied to all remote client
+// connections.
+exports.TLS_CLIENT_RENEG_LIMIT = 3;
+exports.TLS_CLIENT_RENEG_WINDOW = 60;
+
+
var debug;
if (process.env.NODE_DEBUG && /tls/.test(process.env.NODE_DEBUG)) {
debug = function(a) { console.error('TLS:', a); };
@@ -539,6 +547,37 @@ EncryptedStream.prototype._pusher = function(pool, offset, length) {
};
+function onhandshakestart() {
+ debug('onhandshakestart');
+
+ var self = this, ssl = this.ssl;
+ ssl.handshakes++;
+
+ if (ssl.handshakes === 1) {
+ function timeout() {
+ ssl.handshakes = 0;
+ ssl.timer = null;
+ }
+ ssl.timer = setTimeout(timeout, exports.TLS_CLIENT_RENEG_WINDOW * 1000);
+ }
+ else if (ssl.handshakes >= exports.TLS_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.
+ process.nextTick(function() {
+ var err = new Error('TLS session renegotiation attack detected.');
+ if (self.cleartext) self.cleartext.emit('error', err);
+ });
+ }
+}
+
+
+function onhandshakedone() {
+ // for future use
+ debug('onhandshakedone');
+}
+
+
/**
* Provides a pair of streams to do encrypted communication.
*/
@@ -585,6 +624,13 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized,
this._isServer ? this._requestCert : options.servername,
this._rejectUnauthorized);
+ if (this._isServer) {
+ this.ssl.onhandshakestart = onhandshakestart.bind(this);
+ this.ssl.onhandshakedone = onhandshakedone.bind(this);
+ this.ssl.handshakes = 0;
+ this.ssl.timer = null;
+ }
+
if (process.features.tls_sni) {
if (this._isServer && options.SNICallback) {
this.ssl.setSNICallback(options.SNICallback);
@@ -727,6 +773,12 @@ SecurePair.prototype.destroy = function() {
var self = this;
this._doneFlag = true;
+
+ if (this.ssl.timer) {
+ clearTimeout(this.ssl.timer);
+ this.ssl.timer = null;
+ }
+
this.ssl.error = null;
this.ssl.close();
this.ssl = null;
View
16 src/node_crypto.cc
@@ -911,6 +911,8 @@ Handle<Value> Connection::New(const Arguments& args) {
SSL_set_app_data(p->ssl_, p);
+ if (is_server) SSL_set_info_callback(p->ssl_, SSLInfoCallback);
+
#ifdef OPENSSL_NPN_NEGOTIATED
if (is_server) {
// Server should advertise NPN protocols
@@ -973,6 +975,20 @@ Handle<Value> Connection::New(const Arguments& args) {
}
+void Connection::SSLInfoCallback(const SSL *ssl, int where, int ret) {
+ if (where & SSL_CB_HANDSHAKE_START) {
+ HandleScope scope;
+ Connection* c = static_cast<Connection*>(SSL_get_app_data(ssl));
+ MakeCallback(c->handle_, "onhandshakestart", 0, NULL);
+ }
+ if (where & SSL_CB_HANDSHAKE_DONE) {
+ HandleScope scope;
+ Connection* c = static_cast<Connection*>(SSL_get_app_data(ssl));
+ MakeCallback(c->handle_, "onhandshakedone", 0, NULL);
+ }
+}
+
+
Handle<Value> Connection::EncIn(const Arguments& args) {
HandleScope scope;
View
2 src/node_crypto.h
@@ -190,6 +190,8 @@ class Connection : ObjectWrap {
}
private:
+ static void SSLInfoCallback(const SSL *ssl, int where, int ret);
+
BIO *bio_read_;
BIO *bio_write_;
SSL *ssl_;
View
95 test/pummel/test-tls-ci-reneg-attack.js
@@ -0,0 +1,95 @@
+// Copyright Joyent, Inc. and other Node contributors.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a
+// copy of this software and associated documentation files (the
+// "Software"), to deal in the Software without restriction, including
+// without limitation the rights to use, copy, modify, merge, publish,
+// distribute, sublicense, and/or sell copies of the Software, and to permit
+// persons to whom the Software is furnished to do so, subject to the
+// following conditions:
+//
+// The above copyright notice and this permission notice shall be included
+// in all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
+// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
+// USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+var common = require('../common');
+var assert = require('assert');
+var spawn = require('child_process').spawn;
+var tls = require('tls');
+var fs = require('fs');
+
+// renegotiation limits to test
+var LIMITS = [0, 1, 2, 3, 5, 10, 16];
+
+if (process.platform === 'win32') {
+ console.log("Skipping test, you probably don't have openssl installed.");
+ process.exit();
+}
+
+(function() {
+ var n = 0;
+ function next() {
+ if (n >= LIMITS.length) return;
+ tls.TLS_CLIENT_RENEG_LIMIT = LIMITS[n++];
+ test(next);
+ }
+ next();
+})();
+
+function test(next) {
+ var options = {
+ cert: fs.readFileSync(common.fixturesDir + '/test_cert.pem'),
+ key: fs.readFileSync(common.fixturesDir + '/test_key.pem')
+ };
+
+ 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();
+ });
+ conn.pipe(conn);
+ });
+
+ server.listen(common.PORT, function() {
+ var args = ('s_client -connect 127.0.0.1:' + common.PORT).split(' ');
+ var child = spawn('openssl', args);
+
+ child.stdout.pipe(process.stdout);
+ child.stderr.pipe(process.stderr);
+
+ // count handshakes, start the attack after the initial handshake is done
+ var handshakes = 0;
+ child.stderr.on('data', function(data) {
+ handshakes += (('' + data).match(/verify return:1/g) || []).length;
+ if (handshakes === 2) spam();
+ });
+
+ var exited = false;
+ child.on('exit', function() {
+ // TLS_CLIENT_RENEG_LIMIT + 1 because the start of the last renegotiation
+ // is logged (and therefore counted), +2 for the initial handshake
+ if (tls.TLS_CLIENT_RENEG_LIMIT <= 1)
@isaacs
isaacs added a line comment Feb 16, 2012

Please wrap if/else blocks in braces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ assert.equal(handshakes, 4);
+ else
+ assert.equal(handshakes, 2 * tls.TLS_CLIENT_RENEG_LIMIT);
+ server.close();
+ exited = true;
+ process.nextTick(next);
+ });
+
+ // simulate renegotiation attack
+ function spam() {
+ if (exited) return;
+ child.stdin.write("R\n");
+ setTimeout(spam, 250);
+ }
+ });
+}

1 comment on commit 1d8950d

@isaacs

Barring minor style nit, looks good to me. Doesn't seem to break any new tests. Thanks for taking this on!

I thought you were going to try to get this onto 0.6. Are you still planning on doing that? The commit looks like it has a relatively minor collision on 0.6, but I didn't pursue it very far.

Please sign in to comment.