This repository has been archived by the owner. It is now read-only.
Permalink
Browse files

tls: don't use a timer to track renegotiations

It makes tls.createSecurePair(null, true) hang until the timer expires.

Using a timer here is silly. Use a timestamp instead.
  • Loading branch information...
bnoordhuis committed Oct 8, 2012
1 parent 16a9dac commit 76ddf06f1081e51c163f1c5bc3d37895ed2c62c6
Showing with 42 additions and 8 deletions.
  1. +14 −8 lib/tls.js
  2. +28 −0 test/simple/test-tls-handshake-nohang.js
View
@@ -700,15 +700,21 @@ EncryptedStream.prototype._pusher = function(pool, offset, length) {
function onhandshakestart() {
debug('onhandshakestart');
- var self = this, ssl = this.ssl;
+ var self = this;
+ var ssl = self.ssl;
+ var now = Date.now();
+
+ assert(now >= ssl.lastHandshakeTime);
- if (ssl.timer === null) {
- ssl.timer = setTimeout(function timeout() {
- ssl.handshakes = 0;
- ssl.timer = null;
- }, exports.CLIENT_RENEG_WINDOW * 1000);
+ if ((now - ssl.lastHandshakeTime) >= exports.CLIENT_RENEG_WINDOW * 1000) {
+ ssl.handshakes = 0;
}
- else if (++ssl.handshakes > exports.CLIENT_RENEG_LIMIT) {
+
+ var first = (ssl.lastHandshakeTime === 0);
+ ssl.lastHandshakeTime = now;
+ if (first) return;
+
+ 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.
@@ -810,8 +816,8 @@ function SecurePair(credentials, isServer, requestCert, rejectUnauthorized,
this.ssl.onhandshakedone = onhandshakedone.bind(this);
this.ssl.onclienthello = onclienthello.bind(this);
this.ssl.onnewsession = onnewsession.bind(this);
+ this.ssl.lastHandshakeTime = 0;
this.ssl.handshakes = 0;
- this.ssl.timer = null;
}
if (process.features.tls_sni) {
@@ -0,0 +1,28 @@
+// 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 tls = require('tls');
+
+// neither should hang
+tls.createSecurePair(null, false, false, false);
+tls.createSecurePair(null, true, false, false);

0 comments on commit 76ddf06

Please sign in to comment.