changed how we wrap TLS connections that we've peaked at

lib/modules/tls.js

@@ -40,18 +40,47 @@ module.exports.create = function (deps, config, netHandler) {
   , 'localAddress'
   , 'localPort'
   ];
-  function wrapSocket(socket, opts) {
+  function wrapSocket(socket, opts, cb) {
-    if (!opts.hyperPeek) {
+    var reader = require('socket-pair').create(function (err, writer) {
+      if (typeof cb === 'function') {
+        process.nextTick(cb);
+      }
+      if (err) {
+        reader.emit('error', err);
+        return;
+      }
+
       process.nextTick(function () {
         socket.unshift(opts.firstChunk);
       });
-    }
 
-    var wrapped = require('tunnel-packer').wrapSocket(socket);
+      socket.pipe(writer);
-    addressNames.forEach(function (name) {
+      writer.pipe(socket);
-      wrapped[name] = opts[name] || wrapped[name];
+
+      socket.on('error', function (err) {
+        console.log('wrapped TLS socket error', err);
+        reader.emit('error', err);
+      });
+      writer.on('error', function (err) {
+        console.error('socket-pair writer error', err);
+        // If the writer had an error the reader probably did too, and I don't think we'll
+        // get much out of emitting this on the original socket, so logging is enough.
+      });
+
+      socket.on('close', writer.destroy.bind(writer));
+      writer.on('close', socket.destroy.bind(socket));
     });
-    return wrapped;
+
+    // We can't set these properties the normal way because there is a getter without a setter,
+    // but we can use defineProperty. We reuse the descriptor even though we will be manipulating
+    // it because we will only ever set the value and we set it every time.
+    var descriptor = {enumerable: true, configurable: true, writable: true};
+    addressNames.forEach(function (name) {
+      descriptor.value = opts[name] || extractSocketProp(socket, name);
+      Object.defineProperty(reader, name, descriptor);
+    });
+
+    return reader;
   }
 
   var le = greenlock.create({
@@ -244,18 +273,26 @@ module.exports.create = function (deps, config, netHandler) {
     , opts.remoteAddress || socket.remoteAddress
     );
 
+    var wrapped;
+    // We can't emit the connection to the TLS server until we don't the connection is fully
+    // opened, otherwise it might hang open when the decrypted side is destroyed.
+    // https://github.com/nodejs/node/issues/14605
+    function emitSock() {
+      terminateServer.emit('connection', wrapped);
+    }
     if (opts.hyperPeek) {
       // This connection was peeked at using a method that doesn't interferre with the TLS
       // server's ability to handle it properly. Currently the only way this happens is
       // with tunnel connections where we have the first chunk of data before creating the
       // new connection (thus removing need to get data off the new connection).
-      terminateServer.emit('connection', socket);
+      wrapped = socket;
+      process.nextTick(emitSock);
     }
     else {
       // The hyperPeek flag wasn't set, so we had to read data off of this connection, which
       // means we can no longer use it directly in the TLS server.
       // See https://github.com/nodejs/node/issues/8752 (node's internal networking layer == 💩 sometimes)
-      terminateServer.emit('connection', wrapSocket(socket, opts));
+      wrapped = wrapSocket(socket, opts, emitSock);
     }
   }