fix: omit SNI for IP-based hostnames

src/message-io.ts

@@ -2,7 +2,7 @@ import DuplexPair from 'native-duplexpair';
 
 import { Duplex } from 'stream';
 import * as tls from 'tls';
-import { Socket } from 'net';
+import { isIP, Socket } from 'net';
 import { EventEmitter } from 'events';
 
 import Debug from './debug';
@@ -73,7 +73,13 @@ class MessageIO extends EventEmitter {
       const securePair = this.securePair = {
         cleartext: tls.connect({
           socket: duplexpair.socket1 as Socket,
-          servername: hostname,
+          // The `host` is used to verify the server's certificate identity.
+          // It is not used to establish a connection as a `socket` is
+          // specified.
+          host: hostname,
+          // RFC 6066 does not allow IP addresses to be used as the server
+          // name, so omit the SNI extension in that case.
+          servername: isIP(hostname) ? '' : hostname,
           secureContext: secureContext,
           rejectUnauthorized: !trustServerCertificate
         }),

test/fixtures/loopback-ip.crt

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDOzCCAiOgAwIBAgIUOagp5aI+GnvGz2kGV/dacO4z8+kwDQYJKoZIhvcNAQEL
+BQAwEzERMA8GA1UEAwwIbG9vcGJhY2swIBcNMjYwNzE3MTA1MTI5WhgPMjEyNjA2
+MjMxMDUxMjlaMBMxETAPBgNVBAMMCGxvb3BiYWNrMIIBIjANBgkqhkiG9w0BAQEF
+AAOCAQ8AMIIBCgKCAQEArSY7wJmq2PkFrf5/U9cAQmBhQiMhMKSav4may7JXEqyJ
+iS1esCDBk4kr0tdWAFX7od3l4Kmd21LfsqNCAdiJqsOpxc51CdyTp1gSAiesHOee
+b9t9fDvRu5vnRYWkAg3JHf4En09LGpnVMJ0fS9g9Jj5Oj+W+RTOnHZ5C5ZJgumhz
+TCv3UxfEoFA009hNHf0qYRGWJUG27Wcuo6AakPhJAaMhVQfi730AsIJkkHMSZWDa
++CjhaNALLtNsyL9uUROtITZX/oX95ke8RCFB1oZ2Tq45MdWcj8OHY1t7c7tpOl/f
+tmbc3casio21iDiYe6FR29XE3+qNg6D04YdRE7VM/wIDAQABo4GEMIGBMB0GA1Ud
+DgQWBBThheYYWTAI/HjE2Oq6YLP/YdF55DAfBgNVHSMEGDAWgBThheYYWTAI/HjE
+2Oq6YLP/YdF55DAPBgNVHRMBAf8EBTADAQH/MCEGA1UdEQQaMBiHBH8AAAGHEAAA
+AAAAAAAAAAAAAAAAAAEwCwYDVR0PBAQDAgWgMA0GCSqGSIb3DQEBCwUAA4IBAQBu
+vUt7/fKxpuvhzScGfYcrDXHCVPBYG31PiqB3E95AWxSxE9pwAczVWiG8hm8ObhHy
+V2UD0D0oZhrrqfGfiIpdggvO7GZot6oZbBj9G9OrxDvjniKJ1ZFoDD+aMruA+Ulg
+6bm2fg1GRyYtON9zc+uTgDeCQxe3vuW59+z0r4+M4jdbA9DDjU1PGWkjbTspKUuA
+e1+Ceb5RWseBNCkS39oq5IX6av3CwboxEn35xAZGO4HZD9WIiMUbm3Zh7RU3ylrQ
+A9heYMjqxteiYQBXHK0A08CFLoXJAkPYtNRePVS99Hwn0tL4NCQCEDvDyDDkmFEr
+Il8meQR4AMX5EoAjl5D3
+-----END CERTIFICATE-----

test/fixtures/loopback-ip.key

@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCtJjvAmarY+QWt
+/n9T1wBCYGFCIyEwpJq/iZrLslcSrImJLV6wIMGTiSvS11YAVfuh3eXgqZ3bUt+y
+o0IB2Imqw6nFznUJ3JOnWBICJ6wc555v2318O9G7m+dFhaQCDckd/gSfT0samdUw
+nR9L2D0mPk6P5b5FM6cdnkLlkmC6aHNMK/dTF8SgUDTT2E0d/SphEZYlQbbtZy6j
+oBqQ+EkBoyFVB+LvfQCwgmSQcxJlYNr4KOFo0Asu02zIv25RE60hNlf+hf3mR7xE
+IUHWhnZOrjkx1ZyPw4djW3tzu2k6X9+2ZtzdxqyKjbWIOJh7oVHb1cTf6o2DoPTh
+h1ETtUz/AgMBAAECggEAU2MYX7chyoCTNapdE7l1jIHyFPHqKAz94cSZYgQvXvTs
+/71z3orH7w+epVnDN0Ixf1DoRnwpY9jTlqp9PXpVQoIcnwDA6NDr4ynM8nC8uObr
+LJ+eHViJtzpr9NVZmQueke0luLttRhBT1ae1zMcFHAfhufdA3P71OpRfT0RhhSPA
+LLj8q26e21RVViCS6rlTyPqZwTAM8+numV455t5N651MGchbUmnFfTCmgL5g9DlV
+dDSMSC3izH3XvrcN0LKp9ILopJNi2nQikA75KYDeiz2J36WIUJFuddUpmw+CywAg
+tZtx0A0UpXPCojFzoYUB0boBfUbAtP+LCf4CYeeZYQKBgQDtnU6JJVM2a/1ntTIu
+GIKjIgXb0jJYA0j9EFdJegQmsIcn9CFdr4na9Bl0QeFtPoQ9LaQRry92pFzXL07U
+X6ASF7cv5FTjU3cDD+GgOdPDIwjZ1c4jVInNqt1JDodAreSxRAxK9f/kMRHBEdco
+va6oB9XJJDoKyylTEbYT8CapZwKBgQC6i/6Pi0sp40CUGHPr9co5GiuEXyWkeZQX
+Xb65diSVN9ufpUnz4LvY5o27lxcr0oE9gDSjXj9Fw3p98XAcQb8txapUkjPE0zmb
+XsUxlC5kfiJnJxXUk6uME+xl0T/rqc2bo68K9VS0Hcim6+RydZvQ4oUj43bGyk89
+KGW7ehHIqQKBgFAXN8OOay/q0m1Ea/eRl2b5LtRbU1DLOqzh9ynzELchxUy4Qw7a
+/jJpGLNRCXTTH6unWKgFTpBE8m4tqfp3iPLQP5TK97nwBitoDPr+wtwBURhrsqHB
++vjx4MihNwGcG3uMnkAeDvSRfZSAOO+oj9hfW7YCmfXNaM5xJ9gsmDt9AoGANdcf
+4ogeanmHcvbXxjuLYQ23j53yfFUGs/O/j0GU72hOv/XxsSykqsZoE8NY0bIQ0RJG
+nvwdoOH5YY4kgFNNfUV2krAbtuwLzVb97QBKn4B0J3d1aoQwV96MNRq+qyK4D/qf
+E7eY+d86wUqGBa8CCLjIGoKVDmznj8wwCHsD1PkCgYEAvO+PBa7poGWmGfw04cJ5
+zt7vdjag/YNob95hbHAUaJ8V+5qi8hrF6EiztTSfu5pSUFcJn8Ir1PTL9lGQP48N
+Mtrc4cZ+rK8jTJq6zjAqzAZR8s1+9/cH8XQuZ9mqw7fc5HS3D+b6Pon3c5zYAwAV
+4lmBEggRtnMfLaIewQABj4s=
+-----END PRIVATE KEY-----

test/unit/message-io-test.ts

@@ -3,7 +3,7 @@ import { once } from 'events';
 import { assert } from 'chai';
 import { promisify } from 'util';
 import DuplexPair from 'native-duplexpair';
-import { TLSSocket } from 'tls';
+import { checkServerIdentity, type PeerCertificate, TLSSocket } from 'tls';
 import { readFileSync } from 'fs';
 import { Duplex } from 'stream';
 
@@ -363,6 +363,30 @@ describe('MessageIO', function() {
       securePair.encrypted.destroy();
     });
 
+    /**
+     * Forwards TLS handshake data between the given `MessageIO` and the
+     * server side of the secure pair, unwrapping it from / wrapping it into
+     * `PRELOGIN` messages.
+     */
+    async function forwardTlsHandshake(io: MessageIO, rounds: number) {
+      for (let i = 0; i < rounds; i++) {
+        const message = await io.readMessage();
+        for await (const chunk of message) {
+          securePair.encrypted.write(chunk);
+        }
+
+        await once(securePair.encrypted, 'readable');
+
+        const chunks = [];
+        let chunk;
+        while (chunk = securePair.encrypted.read()) {
+          chunks.push(chunk);
+        }
+
+        io.sendMessage(TYPE.PRELOGIN, Buffer.concat(chunks));
+      }
+    }
+
     it('performs TLS negotiation', async function() {
       await Promise.all([
         // Client side
@@ -518,6 +542,119 @@ describe('MessageIO', function() {
       ]);
     });
 
+    it('sends the hostname via the SNI extension', async function() {
+      await Promise.all([
+        // Client side
+        (async () => {
+          const io = new MessageIO(clientConnection, packetSize, debug);
+
+          await io.startTls({}, 'localhost', true);
+
+          assert(io.tlsNegotiationComplete);
+        })(),
+
+        // Server side
+        (async () => {
+          const io = new MessageIO(serverConnection, packetSize, debug);
+
+          const onSecure = once(securePair.cleartext, 'secure');
+
+          await forwardTlsHandshake(io, 2);
+
+          await onSecure;
+
+          assert.strictEqual(securePair.cleartext.servername, 'localhost');
+        })()
+      ]);
+    });
+
+    describe('when connecting to an IP address', function() {
+      beforeEach(function() {
+        // Replace the server side of the secure pair with one that uses a
+        // certificate that carries IP address SANs (and no DNS SANs).
+        securePair.cleartext.destroy();
+        securePair.encrypted.destroy();
+
+        const duplexpair = new DuplexPair();
+
+        securePair = {
+          cleartext: new TLSSocket(duplexpair.socket1 as Socket, {
+            key: readFileSync('./test/fixtures/loopback-ip.key'),
+            cert: readFileSync('./test/fixtures/loopback-ip.crt'),
+            isServer: true,
+            ciphers: 'ECDHE-RSA-AES128-GCM-SHA256',
+            // TDS 7.x only supports TLS versions up to TLS v1.2
+            maxVersion: 'TLSv1.2'
+          }),
+          encrypted: duplexpair.socket2
+        };
+      });
+
+      it('omits the SNI extension and validates the certificate against the IPv4 address', async function() {
+        await Promise.all([
+          // Client side
+          (async () => {
+            const io = new MessageIO(clientConnection, packetSize, debug);
+
+            await io.startTls({
+              ca: [readFileSync('./test/fixtures/loopback-ip.crt')]
+            }, '127.0.0.1', false);
+
+            assert(io.tlsNegotiationComplete);
+          })(),
+
+          // Server side
+          (async () => {
+            const io = new MessageIO(serverConnection, packetSize, debug);
+
+            const onSecure = once(securePair.cleartext, 'secure');
+
+            await forwardTlsHandshake(io, 2);
+
+            await onSecure;
+
+            assert.notOk(securePair.cleartext.servername);
+          })()
+        ]);
+      });
+
+      it('omits the SNI extension and validates the certificate against the IPv6 address', async function() {
+        // Some Node.js versions are affected by an upstream regression that
+        // prevents IPv6 addresses from being matched against `IP Address`
+        // SANs. See https://github.com/nodejs/node/issues/64144
+        const cert = { subject: { CN: 'dummy' }, subjectaltname: 'IP Address:0:0:0:0:0:0:0:1' };
+        if (checkServerIdentity('::1', cert as PeerCertificate) !== undefined) {
+          this.skip();
+        }
+
+        await Promise.all([
+          // Client side
+          (async () => {
+            const io = new MessageIO(clientConnection, packetSize, debug);
+
+            await io.startTls({
+              ca: [readFileSync('./test/fixtures/loopback-ip.crt')]
+            }, '::1', false);
+
+            assert(io.tlsNegotiationComplete);
+          })(),
+
+          // Server side
+          (async () => {
+            const io = new MessageIO(serverConnection, packetSize, debug);
+
+            const onSecure = once(securePair.cleartext, 'secure');
+
+            await forwardTlsHandshake(io, 2);
+
+            await onSecure;
+
+            assert.notOk(securePair.cleartext.servername);
+          })()
+        ]);
+      });
+    });
+
     it('handles errors happening before TLS negotiation has sent any data', async function() {
       await Promise.all([
         // Client side