fix: omit SNI for IP-based hostnames

[lpinca]

Refs: #1237

[arthurschreiber]

Thanks for the fix! While testing this I found one issue with it, though.

servername does double duty in Node's TLS client: it is the SNI value, but it is also the first candidate for the hostname that the certificate identity check runs against. When a socket is passed to tls.connect() without a host option, that check falls back through options.servername || options.host || options.socket._host || 'localhost'. With servername: '' and no host, the server's certificate ends up being validated against 'localhost'.

Reproduced with a server certificate carrying IP Address:127.0.0.1 in its SANs and trustServerCertificate: false:

• before this PR (servername: '127.0.0.1'): handshake succeeds (with the DEP0123 warning),
• with this PR: ERR_TLS_CERT_ALTNAME_INVALID: Hostname/IP does not match certificate's altnames: Host: localhost. is not cert's CN: 127.0.0.1.

So connecting to an IP address with encrypt: true, trustServerCertificate: false and a certificate with matching IP Address SANs would regress from working to failing.

The fix is to also pass host: hostname — since a socket is provided it is never used to establish a connection, it only feeds the identity check. This mirrors what Connection#wrapWithTls (the TDS 8.0 strict encryption path) already does. I've left it as a suggestion on the diff, and the patch below additionally adds unit tests (IPv4/IPv6/hostname SNI behavior, validated against a new long-lived fixture certificate that carries only IP SANs, so the localhost fallback cannot slip through) — the IPv4 test fails with exactly the error above if the host option is removed again.

Two notes from writing the tests:

• IPv6 identity validation against IP Address SANs is currently broken upstream in Node 22.23.1+ / 24.17+ / 26.4+ by nodejs/node#64144 (domainToASCII('::1') returns '', so tls.checkServerIdentity() never takes the IP branch). That's independent of this PR and fail-closed; the IPv6 test feature-detects it and skips on affected versions.
• Bracketed IPv6 literals ([::1]) would not be caught by isIP(), but they can't reach this code path anyway — lookupAllAddresses only accepts unbracketed IPv6 for direct connections — so no extra handling is needed.

If you enable "Allow edits from maintainers" on this PR I'm happy to push the commit directly; otherwise you can apply it with git am from the patch below.

Full patch (fix + tests + fixtures)

From 2e5685d3e522b7f8e8711027e4898c1ea79c52bd Mon Sep 17 00:00:00 2001
From: Arthur Schreiber <schreiber.arthur@googlemail.com>
Date: Fri, 17 Jul 2026 10:57:46 +0000
Subject: [PATCH] fix: preserve server identity check when omitting SNI

When a socket is passed to `tls.connect()` without a `host` option,
the certificate identity check falls back to `options.servername ||
options.host || options.socket._host || 'localhost'`. Setting
`servername` to an empty string for IP addresses therefore caused the
server's certificate to be validated against `'localhost'` instead of
the IP address, breaking connections to IP addresses that use
`trustServerCertificate: false` with a certificate carrying matching
`IP Address` SANs.

Pass the target host explicitly so that certificate validation keeps
using it while the SNI extension remains omitted for IP addresses.

The IPv6 test is skipped on Node.js versions affected by
https://github.com/nodejs/node/issues/64144, where
`tls.checkServerIdentity()` cannot match IPv6 addresses against
`IP Address` SANs at all.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 src/message-io.ts             |   6 ++
 test/fixtures/loopback-ip.crt |  20 +++++
 test/fixtures/loopback-ip.key |  28 +++++++
 test/unit/message-io-test.ts  | 143 +++++++++++++++++++++++++++++++++-
 4 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 test/fixtures/loopback-ip.crt
 create mode 100644 test/fixtures/loopback-ip.key

diff --git a/src/message-io.ts b/src/message-io.ts
index fac8d75..0346eb8 100644
--- a/src/message-io.ts
+++ b/src/message-io.ts
@@ -73,6 +73,12 @@ class MessageIO extends EventEmitter {
       const securePair = this.securePair = {
         cleartext: tls.connect({
           socket: duplexpair.socket1 as Socket,
+          // 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
diff --git a/test/fixtures/loopback-ip.crt b/test/fixtures/loopback-ip.crt
new file mode 100644
index 0000000..8fb9c8c
--- /dev/null
+++ b/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-----
diff --git a/test/fixtures/loopback-ip.key b/test/fixtures/loopback-ip.key
new file mode 100644
index 0000000..59c6bfc
--- /dev/null
+++ b/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-----
diff --git a/test/unit/message-io-test.ts b/test/unit/message-io-test.ts
index 3acfa9a..1976565 100644
--- a/test/unit/message-io-test.ts
+++ b/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';
 
@@ -339,7 +339,9 @@ describe('MessageIO', function() {
   });
 
   describe('#startTls', function() {
-    let securePair: { encrypted: Duplex, cleartext: TLSSocket };
+    // `@types/node` does not declare the server-side `servername` getter
+    // on `TLSSocket`, so widen the type here.
+    let securePair: { encrypted: Duplex, cleartext: TLSSocket & { servername?: string } };
 
     beforeEach(function() {
       const duplexpair = new DuplexPair();
@@ -362,6 +364,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
@@ -517,6 +543,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
-- 
2.42.0

[lpinca]

If you enable "Allow edits from maintainers"

Done.

[arthurschreiber]

Now that maintainer edits are enabled, I've pushed the commit from the review directly (2e5685d) — the inline suggestion and patch above are superseded and can be resolved.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.29%. Comparing base (2862a72) to head (0b5bf30).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1728      +/-   ##
==========================================
- Coverage   80.24%   79.29%   -0.96%     
==========================================
  Files          90       90              
  Lines        4910     4911       +1     
  Branches      923      924       +1     
==========================================
- Hits         3940     3894      -46     
- Misses        665      720      +55     
+ Partials      305      297       -8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

🎉 This PR is included in version 20.0.5 🎉

The release is available on:

• npm package (@next dist-tag)
• GitHub release

Your semantic-release bot 📦🚀