Different behaviour between new tls.TLSSocket() followed by connect(..) versus tls.connect(..)

[sregger]

• Version: v10.16.3
• Platform: Windows 10
• Subsystem: tls

I'm not sure whether this is an incorrect usage issue or a follow up of #3963. Alternatively it may be covered by documentation I missed or may need to be added.

I'm adding enterprise proxy (NTLM, Kerberos) support to the tls module by overriding the createConnection method and using the SSPI native library on Windows.

To do this I'm attempting to use the following code to create a TLS Socket in nodejs

const socket = new net.Socket();
socket.setEncoding('utf8');
const tlsSocket = new tls.TLSSocket()

socket.on('connect', (connectionData) => {
  // Do extra work with socket to authenticate with the proxy using SSPI
  // then switch to a tls socket
  tlsSocket.connect({
    host: targetHost,
    port: targetPort,
    socket: socket,
  })
}

But this is causing an ECONNREFUSED error indicating that the socket setup is not taking affect with tlsSocket. The events I see in this flow on tlsSocket are

• lookup
• error (for ECONNREFUSED)
• _tlsError (for ECONNREFUSED)
• close

But if I change the code to

const socket = new net.Socket();
socket.setEncoding('utf8');

socket.on('connect', (connectionData) => {
  // Do extra work with socket to authenticate with the proxy using SSPI
  // then switch to a tls socket
  let tlsSocket = tls.connect({
    host: targetHost,
    port: targetPort,
    socket: socket,
  })
}

It works and the tlsSocket events are

• resume
• data
• readable
• end
• prefinish
• finish
• close

Why does this fail if I create the constructor version of the TLSSocket and connect it later? i.e. Rather than when I use tls.connect(..) which passes.

I need the reference to tlsSocket so the createConnection function can return it immediately. I have been able to use the createConnection callback instead with the second approach. However this brakes an OCSP agent I'm using. i.e. As it incorrectly (I think) assumes createConnection always returns the socket object rather than undefined followed by a callback.

[tniessen]

Can you elaborate why you need the TLSSocket before it is connected?

I solved a similar problem by creating a Duplex stream that buffers data until the TLSSocket is connected, but I don't fully understand your use case, so it might not be applicable.

[sregger]

Can you elaborate why you need the TLSSocket before it is connected?

My understanding of the createConnection(...) method is that it is synchronous and needs to immediately return a socket object *. The socket object should already have had the connect(..) method called according to the documentation. But end users should expect for it to not yet be connected/writable and listen for the connect event.

* - nodejs seem to support returning undefined followed by a callback once ready. But I'm not sure this is intentional or if it accidentally works for me.

See https://nodejs.org/api/net.html#net_net_createconnection

In my case I need to initially connect a net socket. Using the HTTP CONNECT method with this socket I have to authenticate with the proxy (which can involve up to 2 network calls and responses). Then, once I receive a 200 OK from the proxy to indicate the authentication succeeded, I have to upgrade the socket to a secure tlsSocket.

That is not possible to achieve synchronously. So I'm hoping to achieve it by creating the tlsSocket object initially and having it connect(..) later.

I solved a similar problem by creating a Duplex stream that buffers data until the TLSSocket is connected, but I don't fully understand your use case, so it might not be applicable.

I don't understand that solution. If someone calls createConnection(..) and gets a duplex stream back instead of a socket I'd assume it'd break them?

Currently I'm returning undefined and later calling a callback passed to createConnection(..). But initially I tried the following

let writable = false
const ons = []
const onces = []

// Create a 'proxy' (not network proxy) object to allow us control the value of the
// 'writable' property and when the 'connect' event is fired
const socketProxy = new Proxy(tlsSocket, {
  get(target, property) {
    if (property === 'writable') return writable;
    if (['on', 'once'].includes(property)) return (event, eventCallback) => {
      if (event === 'connect' && !writable) {
        if ('on' === property)
          ons.push(eventCallback)
        else
          onces.push(eventCallback)
      }
      else {
        // Listen for events other that 'connect' goes directly to the target
        // Similarly if the socket is writable go directly to the target
        return target[property];
      }
    }

    // Any non-overriden functions are passed on
    return target[property];
  }

i.e. I return a Proxy of the socket which I mark as not connected (writable) until the tlsSocket is connected. But this does not work due to the issue above.

[tniessen]

I don't understand that solution. If someone calls createConnection(..) and gets a duplex stream back instead of a socket I'd assume it'd break them?

TCP sockets and TLS sockets are Duplex streams in Node.js, and TLS can work on top of any other Duplex stream as well, it does not have to be a socket.

[sregger]

Excuse my ignorance but can you provide an example or hints on how to set that up? I've experience with streams but I'm not groking this.

After looking through the _tls_wrap.js code in node.js, and further experiments, I'm coming to the conclusion that it's not possible to create a new TLSSocket(..) object without having it start a connection. I had assumed it should be possible by not passing in any arguments to the constructor and manually calling connect to start it. But that only seems to lead to a ECONNREFUSED.

[tniessen]

If you need to perform custom steps between connecting the TCP socket and performing the TLS handshake, think of your code as an additional network layer that is between TCP and TLS. I did something similar for a network protocol prototype recently. The code isn't great, but I hope it will demonstrate what I mean :)

1. The "layer" implementation as a custom Duplex stream.
2. Put your "layer" on top of TCP.
3. Do whatever you want on top of your own layer. I mostly tested with HTTPS and HTTP, but TLS should also "just work", even though the API is slightly different.

[bnoordhuis]

This issue seems to have run its course. I'll go ahead and close it out.

[sregger]

Agree with the closure.

In case it helps in the end I had to remove the OCSP module and rely upon the callback approach (which accidentally works for TLS).

While it "feels" reasonable for the above approach to work in reality this section of the core code is (IMO) not well written and a little inconsistent with itself. i.e. As the above solution would work for a TCP, but not for a TLS, connection.

[tniessen]

this section of the core code is (IMO) not well written and a little inconsistent with itself. i.e. As the above solution would work for a TCP, but not for a TLS, connection.

Feel free to propose / implement a fix :)

[sregger]

Feel free to propose / implement a fix :)

A more than fair response. Internally I cringe at the idea of how many libraries, projects, etc I'd break.

[Antonius-S]

I'm implementing proxy support as well and got overwhelmed about how difficult is to inject some custom code between plain socket and tls. TLS supports starting and closing secure channel at any time over an existing connection, I suppose this would be a nightmare to implement in Node whereas it should be as simple as

sock.connect();
tlsSock = new TLSSocket(sock);
tlsSock.startTLS();
...do something inside secure channel
tlsSock.shutdownTLS();

or even

tlsSock = new TLSSocket();
tlsSock.connect(port, host);
tlsSock.write('plain text');
tlsSock.startTLS();
tlsSock.write('encrypted text');
tlsSock.shutdownTLS();
tlsSock.write('plain text');