MessageIO.prototype.startTls should follow tls.connect semantics

[arthurschreiber]

Because the TDS 7.x protocol performs TLS negotiation wrapped into TDS messages exchanged between client and server, we can't use tls.connect directly to wrap our tcp sockets into tls sockets. (We can and do this with TDS 8.x).

For TDS 7.x the TLS negotiation is implemented in MessageIO.prototype.startTls. We use a library called native-duplexpair to give us access to both the cleartext as well the encrypted sides of the tls connection, so we can take the handshake data, wrap it into TDS messages, and send it to the remote server.

We pass the cleartext side of the of the secure pair to tls.connect. tls.connect wraps this "stream" and sets up events to e.g. get notified if the stream is closed to close the tls socket accordingly.

This can be seen e.g. here: https://github.com/nodejs/node/blob/v16.x/lib/_tls_wrap.js#L632

Unfortunately, this logic works correctly if tls.connect is used to wrap a net.Socket instance, as we for example do when using in the TDS 8.x case.

But in the TDS 7.x case, this doesn't work right. If the underlying net.Socket is closed, this information is not passed through to the the duplexpair stream.

So we have cases where the net.Socket is closed, but the tls.Socket does not get notified. I think we also have cases the other way round - the tls.Socket could be or run into an error, but the net.Socket we have does not get closed correctly.

We need to look at how tls.connect passes this information around, and replicate this correctly in MessageIO.prototype.startTls.

[arthurschreiber]

I think MessageIO.prototype.startTls also does not handle correctly cases where the network socket is closed by the remote side during the TLS negotiation. I think this eventually gets handled correctly by the close and error events we define here on the socket:

tedious/src/connection.ts

Lines 1995 to 1997 in aaef4bf

	socket.on('error', (error) => { this.socketError(error); });
	socket.on('close', () => { this.socketClose(); });
	socket.on('end', () => { this.socketEnd(); });

But I don't think this is the correct place to handle this. Essentially, it would be best if startTls would allow simply "forgetting" about the original network socket, same as with tls.connect.