Skip to content

cannot use tls.TLSSocket created from net.Socket after first 'data' event #8752

Description

@coolaj86

tried on node v6.3 and v6.6

My goal is to mux a socket to use either tcp or tls after inspecting the first packet to see if it's a hello.

The problem I have is that I could not pass the socket to tls.TLSSocket (or tls.Server, or https.Server) after the 'data' event (even though I replay it) and have it emit data.

issue-8752.js:

//
//
// SETUP
//
//
'use strict';

var net = require('net');
var tls = require('tls');
var http = require('http');
var tlsOpts = {
  key: require('fs').readFileSync('./privkey.pem')
, cert: require('fs').readFileSync('./fullchain.pem')
};
var httpServer = http.createServer(function (req, res) {
  res.end('Hello, World!');
});
var tcp3000 = net.createServer();

tcp3000.listen(3000, function () {
  console.log('listening on 3000');
});



tcp3000.on('connection', function (socket) {
  console.log('connection incoming');
  // this works when I put it here, but I don't know if it's tls yet here
  // httpsServer.emit('connection', socket);

  socket.once('data', function (chunk) {
    console.log('data incoming');

    if (/http\/1/i.test(chunk.toString())) {

      console.log("looks like http, continue");

      // this works as expected
      httpServer.emit('connection', socket);

    } else {

      //**********************************************************
      //
      // PROBLEM
      //
      //**********************************************************
      console.log("doesn't look like http, try tls");

      // none of these methods work:
      // httpsServer.emit('connection', socket);  // this didn't work
      // tlsServer.emit('connection', socket);    // this didn't work either
      var tlsSocket = new tls.TLSSocket(socket, { secureContext: tls.createSecureContext(tlsOpts) });
      tlsSocket.on('data', function (chunk) {
        // never called
        console.log('chunk', chunk);
      });

    }

    // replay first packet
    socket.emit('data', chunk);
    // I've also tried this, which didn't work at all:
    // socket.unshift(chunk);
    // socket.emit('readable');
  });

});
curl https://local.test.ppl.family
connection incoming
data incoming
looks like http, continue
curl: (35) gnutls_handshake() failed: An unexpected TLS packet was received.

From the documentation I've found I don't think I'm doing anything wrong and the same process works with a plain tcp socket, so it makes me wonder if there isn't something amiss in the internals.

Also, the following does work (even though it's useless):

'use strict';

var net = require('net');
var http = require('http');
var https = require('https');
var tlsOpts = {
  key: require('fs').readFileSync('./privkey.pem')
, cert: require('fs').readFileSync('./fullchain.pem')
};
var httpServer = http.createServer(function (req, res) {
  res.end('Hello, World!');
});
var httpsServer = https.createServer(tlsOpts, function (req, res) {
  res.end('Hello, Encrypted World!');
});
var tcp3000 = net.createServer();

tcp3000.listen(3000, function () {
  console.log('listening on 3000');
});



tcp3000.on('connection', function (socket) {
  // if I never let the 'data' event fire, it works as expected.
  httpsServer.emit('connection', socket);
});

Activity

  1. coolaj86 commented on Sep 23, 2016

    @coolaj86
    Author

    I found a workaround which suggests this must be a bug and not intentional behavior.

    Even with the workaround, if I try to use tls.TLSSocket directly I get back an error:

    curl: (35) Unknown SSL protocol error in connection to example.com:-9800
    

    However, this allows me to at least use tls.Server.emit('connection'):

          console.log("doesn't look like http, try tls");
    
          var myDuplex = new Duplex();
          myDuplex._write = function (chunk, encoding, cb) {
            socket.write(chunk, encoding);
            cb();
          };
          myDuplex._read = function (size) {
            var chunk = socket.read(size);
            if (chunk) { this.push(chunk); }
          };
          socket.on('data', function (chunk) {
            myDuplex.push(chunk);
          });
    
          tlsServer.emit('connection', myDuplex);

    And tlsServer looks like this:

    var tlsServer = tls.createServer(tlsOpts, function (tlsSocket) {
      httpServer.emit('connection', tlsSocket);
    });
  2. added
    tlsIssues and PRs related to the tls subsystem.
    on Sep 23, 2016
  3. mscdex commented on Sep 23, 2016

    @mscdex
    Contributor

    Did you try using the recommended way of upgrading a socket? For example:

    var tlssocket = tls.connect({ socket: socket }, function() {
      console.log('Socket upgraded!');
      // use `tlssocket`
    });

    Also on a semi-related note, you shouldn't make any assumptions about chunks of data from a stream. The substring you're checking for could be cut across boundaries.

  4. mscdex commented on Sep 23, 2016

    @mscdex
    Contributor

    If you're wanting to serve http and https over the same port, you might try httpolyglot.

  5. coolaj86 commented on Sep 27, 2016

    @coolaj86
    Author

    I'm not trying to perform a socket upgrade. The socket is already tls. Nevertheless, when I replace my hacky code with your code I got this result:

    events.js:160
          throw er; // Unhandled 'error' event
          ^
    
    Error: socket hang up
        at TLSSocket.onHangUp (_tls_wrap.js:1092:19)
        at TLSSocket.g (events.js:291:16)
        at emitNone (events.js:91:20)
        at TLSSocket.emit (events.js:185:7)
        at endReadableNT (_stream_readable.js:974:12)
        at _combinedTickCallback (internal/process/next_tick.js:74:11)
        at process._tickCallback (internal/process/next_tick.js:98:9)
    

    I'm working on a tunneling service, which happens to include serving both http and https on the same port, but is a bit more complicated than just that.

    The end goal is to allow many authenticated clients to tunnel anything - http, https, and even ssh or openvpn (wrapped in openssl as to appear to be https and to get the sni header) - and that will work in school dorms, HOAs, public libraries, corporate networks, airplanes, and other harsh network environments (and on all OSes without requiring admin privileges, iptables, etc).

    Similar concept to localtunnel.me but with arbitrary encrypted traffic and eventually that could even work on a chrome book in the browser.

  6. mscdex commented on Sep 27, 2016

    @mscdex
    Contributor

    FWIW as far as SSH goes, you may run into issues with buggy SSH clients since I've seen some that actually wait for the server to respond with their ident string first before sending theirs (even though this kind of goes against the RFC, depending on your interpretation).

    Also, tunneling OpenVPN over TCP? I think most clients would be using UDP for performance? Either way, wrapping OpenVPN traffic with encrypted OpenSSL traffic seems like a lot of overhead.

  7. coolaj86 commented on Sep 28, 2016

    @coolaj86
    Author

    I don't want to pull this thread away from the core issue of figuring out how to wrap and unwrap net and tls sockets, so I continued the tangent here: https://gist.github.com/coolaj86/f7c39baf83c32525f7c88ed13643e11d

  8. added
    streamIssues and PRs related to Node.js streams.
    on Dec 13, 2016
  9. Trott commented on Jul 15, 2017

    @Trott
    Member

    This is still an issue in Node.js 8.x?

  10. coolaj86 commented on May 4, 2018

    @coolaj86
    Author

    This is still an issues in node v9.11.1

  11. Trott commented on May 5, 2018

    @Trott
    Member

    @nodejs/crypto @nodejs/streams

  12. bnoordhuis commented on May 7, 2018

    @bnoordhuis
    Member

    The reason the example doesn't work is that tls.TLSSocket short-circuits the net.Socket machinery for performance reasons. That's its sole raison d'être really, and not something we'll change.

    You can use tls.createSecurePair(). It's deprecated but if it supports a use case that tls.TLSSocket does not, we can look into undeprecating it.

  13. coolaj86 commented on May 12, 2018

    @coolaj86
    Author

    @bnoordhuis The use case is building a net proxy service that can inspect incoming traffic and route it accordingly.

    We don't know ahead of time if the incoming message is tls or not (or if it's http or https), so we need to inspect the first packet and then pump the stream through the rest of the stack.

    Could you add a peek() function into the tcp API so that we can inspect and determine what it is?

    We'd also like to be able to track the bytes going over the wire per-session. For some traffic we authenticate with https or wss much later, but we need access to the byte count from the tcp object (to know total packet size, including the increase caused by encryption).

  14. indutny commented on May 12, 2018

    @indutny
    Member

    I haven't been following this conversation closely, but it should be possible with JSStreamWrap: https://github.com/nodejs/node/blob/master/lib/internal/wrap_js_stream.js

  15. coolaj86 commented on May 12, 2018

    @coolaj86
    Author

    @indutny by "it" do you mean add a peek() function to a net.Socket that would allow it to be passed to create a tls.TLSSocket without crashing?

  16. indutny commented on May 12, 2018

    @indutny
    Member

    The original question is about prepending the data to the socket, and wrapping it with tls.Socket. This is possible through that API, you'll have to wrap the socket into a stream.Duplex, though.

  17. monster860 commented on May 24, 2018

    @monster860

    I found a workaround that does not involve weird wrapping stuff.

    So what I did is I replaced the socket.emit('data', chunk) with socket._handle.onread(chunk.length, chunk)

  18. coolaj86 commented on May 24, 2018

    @coolaj86
    Author

    @monster860 I'll have to try that out. Back when I created this issue that also failed in my use case, but I suspect the API has changed.

    I also found that if you need an async event between the first packet and being ready to receive the next packet, you need to use socket.once('data', whatever) followed by socket.pause(), which will be resumed by the socket.on('data', whateverelse) after the callback.

    // peek
    socket.once('data', function (firstChunk) {
      // stop flow
      socket.pause();
    
      // supposing this works, as you suggest
      socket._handle.onread(firstChunk.length, firstChunk)
      // what I do presently
      // socket.unshift(firstChunk);
    
      // determine if this is tls or plain, and what to do about it (maybe it looks like http or tunneled ssh)
      checkItOut(firstChunk, function (err, result) {
    
        // do what needs to be done
        if ('doit' === result) {
          socket.on('data', doIt);
        } else if ('doother' === result) {
          socket.on('data', doother);
        } else {
          socket.on('data', dolast);
        }
        socket.resume();
    
      });
    });
  19. monster860 commented on May 24, 2018

    @monster860

    Hm... Here's what I'm doing:

    let proxies = {
    	http: http.createServer((req, res) => {
    		res.writeHead(301, {"Location": "https://" + req.headers['host'] + req.url});
    		res.end();
    	}),
    	https: https.createServer(server_config.http_opts, http_handler)
    };
    net.createServer((socket) => {
    	socket.once('data', (buffer) => {
    		socket.pause();
    		let byte = buffer[0];
    		let protocol = byte == 22 ? 'https' : 'http';
    		let proxy = proxies[protocol];
    		proxy.emit('connection', socket);
    		socket._handle.onread(firstChunk.length, firstChunk);
    		socket.resume();
    	});
    }).listen(server_config.port);
  20. bnoordhuis commented on May 25, 2018

    @bnoordhuis
    Member

    Since we won't be making changes and workarounds exist, I'll go close this out.

    Could you add a peek() function into the tcp API so that we can inspect and determine what it is?

    No new API should be needed, you can peek by inserting e.g. a stream.Duplex between the net.Socket and tls.TLSSocket.

  21. coolaj86 commented on May 25, 2018

    @coolaj86
    Author

    @monster860 It looks like you and I are working on similar things right now. You might find Greenlock for Express.js or Greenlock for node.js to be valuable.

  22. coolaj86 commented on Jun 9, 2018

    @coolaj86
    Author

    @monster860 Have you tested your solution with connections that persist and data that is larger than the size of a single TCP packet?

    When I remove my workaround I'm still seeing the same behavior where the second data packet never comes through.

  23. coolaj86 commented on Jun 9, 2018

    @coolaj86
    Author

    @bnoordhuis Does piping it through a duplex make it slower to decode the TLS than passing a net socket directly? It seems like there's a lot of latency.

  24. bnoordhuis commented on Jun 9, 2018

    @bnoordhuis
    Member

    I doubt it makes much of a difference, I would expect the cost of encryption/decryption to dominate. Have you profiled it?

  25. monster860 commented on Jun 9, 2018

    @monster860

    @coolaj86 My application involves websockets, so yes.

    I've also tried piping through a duplex and that introduces a noticable lag.

  26. coolaj86 commented on Aug 31, 2018

    @coolaj86
    Author

    @monster860 Thought you might like to know that there's an almost undocumented readable event that will save you all sorts of trouble - no need to pause() or resume() and you'll have far fewer issues with waterlines in buffers.

    https://nodejs.org/api/stream.html#stream_event_readable

    I wish I had known about this years ago. I don't know why the net and tls docs haven't been updated. I can't think of any reason why a data event would be more useful than readable.

  27. dkostenko-dev commented on Jan 6, 2020

    @dkostenko-dev

    How can i write own handshake protocol with the help of sockets (and may be 'net' module) like https with request/response encryption over node.js express server logic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    streamIssues and PRs related to Node.js streams.tlsIssues and PRs related to the tls subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions