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.

diff --git a/lib/tls.js b/lib/tls.js
index dc32787..6390d7e 100644 (file)
--- a/lib/tls.js
+++ b/lib/tls.js
@@ -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) {

diff --git a/test/simple/test-tls-handshake-nohang.js b/test/simple/test-tls-handshake-nohang.js
new file mode 100644 (file)
index 0000000..de36ebb
--- /dev/null
+++ b/test/simple/test-tls-handshake-nohang.js
@@ -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);