From 19afe9a16ba722e42f6b0602bd526a72925b2c35 Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Thu, 26 Mar 2026 15:29:48 +0100 Subject: [PATCH 1/4] fix: omit SNI for IP-based hostnames Refs: https://github.com/tediousjs/tedious/issues/1237 --- src/message-io.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/message-io.ts b/src/message-io.ts index 30a733ab6..fac8d75b1 100644 --- a/src/message-io.ts +++ b/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,7 @@ class MessageIO extends EventEmitter { const securePair = this.securePair = { cleartext: tls.connect({ socket: duplexpair.socket1 as Socket, - servername: hostname, + servername: isIP(hostname) ? '' : hostname, secureContext: secureContext, rejectUnauthorized: !trustServerCertificate }), From 2e5685d3e522b7f8e8711027e4898c1ea79c52bd Mon Sep 17 00:00:00 2001 From: Arthur Schreiber Date: Fri, 17 Jul 2026 10:57:46 +0000 Subject: [PATCH 2/4] 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 --- 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 fac8d75b1..0346eb861 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 000000000..8fb9c8c36 --- /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 000000000..59c6bfc67 --- /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 3acfa9a6b..19765659b 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 From ec53cf231a9009703c8dea7c737b9f60f408d59d Mon Sep 17 00:00:00 2001 From: Arthur Schreiber Date: Fri, 17 Jul 2026 11:30:40 +0000 Subject: [PATCH 3/4] test: align `servername` type widening with newer `@types/node` Recent `@types/node` versions declare the server-side `servername` getter on `TLSSocket` as `string | false | null`, which is incompatible with the `servername?: string` widening under `exactOptionalPropertyTypes`. Use the same union so the test compiles against both older and newer type definitions. Co-Authored-By: Claude Fable 5 --- test/unit/message-io-test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/message-io-test.ts b/test/unit/message-io-test.ts index 19765659b..9d814d4b3 100644 --- a/test/unit/message-io-test.ts +++ b/test/unit/message-io-test.ts @@ -339,9 +339,9 @@ describe('MessageIO', function() { }); describe('#startTls', function() { - // `@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 } }; + // Older versions of `@types/node` do not declare the server-side + // `servername` getter on `TLSSocket`, so widen the type here. + let securePair: { encrypted: Duplex, cleartext: TLSSocket & { servername?: string | false | null } }; beforeEach(function() { const duplexpair = new DuplexPair(); From 0b5bf30e669caa7a766fadcd17ad8431025f3d30 Mon Sep 17 00:00:00 2001 From: Arthur Schreiber Date: Fri, 17 Jul 2026 11:39:41 +0000 Subject: [PATCH 4/4] test: drop `servername` type widening Now that the branch is on current `master`, whose lockfile pins `@types/node` 26, the server-side `servername` getter is declared on `TLSSocket` directly and the widening is no longer needed. Co-Authored-By: Claude Fable 5 --- test/unit/message-io-test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/unit/message-io-test.ts b/test/unit/message-io-test.ts index f32f38fee..ab1376dfe 100644 --- a/test/unit/message-io-test.ts +++ b/test/unit/message-io-test.ts @@ -340,9 +340,7 @@ describe('MessageIO', function() { }); describe('#startTls', function() { - // Older versions of `@types/node` do not declare the server-side - // `servername` getter on `TLSSocket`, so widen the type here. - let securePair: { encrypted: Duplex, cleartext: TLSSocket & { servername?: string | false | null } }; + let securePair: { encrypted: Duplex, cleartext: TLSSocket }; beforeEach(function() { const duplexpair = new DuplexPair();