fix: set maximum supported TLS version to 1.2 (#1471)

TDS 7.x does not support any TLS version newer than 1.2, so when opening the TLS socket we should specify TLS 1.2 as the maximum version supported.
tediousjs · Jul 26, 2022 · 5015634 · 5015634
1 parent 7ed1661
commit 5015634
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 23 deletions.
diff --git a/src/connection.ts b/src/connection.ts
@@ -4,7 +4,7 @@ import { Socket } from 'net';
 import dns from 'dns';
 
 import constants from 'constants';
-import { createSecureContext, SecureContext, SecureContextOptions } from 'tls';
+import { SecureContextOptions } from 'tls';
 
 import { Readable } from 'stream';
 
@@ -888,7 +888,7 @@ class Connection extends EventEmitter {
   /**
    * @private
    */
-  secureContext: SecureContext;
+  secureContextOptions: SecureContextOptions;
   /**
    * @private
    */
@@ -1683,22 +1683,20 @@ class Connection extends EventEmitter {
       }
     }
 
-    let credentialsDetails = this.config.options.cryptoCredentialsDetails;
-    if (credentialsDetails.secureOptions === undefined) {
+    this.secureContextOptions = this.config.options.cryptoCredentialsDetails;
+    if (this.secureContextOptions.secureOptions === undefined) {
       // If the caller has not specified their own `secureOptions`,
       // we set `SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS` here.
       // Older SQL Server instances running on older Windows versions have
       // trouble with the BEAST workaround in OpenSSL.
       // As BEAST is a browser specific exploit, we can just disable this option here.
-      credentialsDetails = Object.create(credentialsDetails, {
+      this.secureContextOptions = Object.create(this.secureContextOptions, {
         secureOptions: {
           value: constants.SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS
         }
       });
     }
 
-    this.secureContext = createSecureContext(credentialsDetails);
-
     this.debug = this.createDebug();
     this.inTransaction = false;
     this.transactionDescriptors = [Buffer.from([0, 0, 0, 0, 0, 0, 0, 0])];
@@ -3172,7 +3170,7 @@ Connection.prototype.STATE = {
 
           try {
             this.transitionTo(this.STATE.SENT_TLSSSLNEGOTIATION);
-            await this.messageIo.startTls(this.secureContext, this.routingData?.server ?? this.config.server, this.config.options.trustServerCertificate);
+            await this.messageIo.startTls(this.secureContextOptions, this.routingData?.server ?? this.config.server, this.config.options.trustServerCertificate);
           } catch (err: any) {
             return this.socketError(err);
           }

diff --git a/src/message-io.ts b/src/message-io.ts
@@ -61,7 +61,13 @@ class MessageIO extends EventEmitter {
   }
 
   // Negotiate TLS encryption.
-  startTls(secureContext: tls.SecureContext, hostname: string, trustServerCertificate: boolean) {
+  startTls(credentialsDetails: tls.SecureContextOptions, hostname: string, trustServerCertificate: boolean) {
+    if (!credentialsDetails.maxVersion || !['TLSv1.2', 'TLSv1.1', 'TLSv1'].includes(credentialsDetails.maxVersion)) {
+      credentialsDetails.maxVersion = 'TLSv1.2';
+    }
+
+    const secureContext = tls.createSecureContext(credentialsDetails);
+
     return new Promise<void>((resolve, reject) => {
       const duplexpair = new DuplexPair();
       const securePair = this.securePair = {

diff --git a/test/integration/connection-test.js b/test/integration/connection-test.js
@@ -221,15 +221,10 @@ describe('Initiate Connect Test', function() {
     config.options.cryptoCredentialsDetails = {
       ciphers: '!ALL'
     };
-
-    try {
+    assert.throws(() => {
       const { createSecureContext } = require('tls');
       createSecureContext(config.options.cryptoCredentialsDetails);
-    } catch {
-      assert.throws(() => {
-        new Connection(config);
-      });
-    }
+    }, Error, new RegExp(/.*SSL routines.*no cipher match/));
 
     done();
   });

diff --git 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 { createSecureContext, TLSSocket } from 'tls';
+import { TLSSocket } from 'tls';
 import { readFileSync } from 'fs';
 import { Duplex } from 'stream';
 
@@ -368,7 +368,7 @@ describe('MessageIO', function() {
         (async () => {
           const io = new MessageIO(clientConnection, packetSize, debug);
 
-          await io.startTls(createSecureContext({}), 'localhost', true);
+          await io.startTls({}, 'localhost', true);
 
           assert(io.tlsNegotiationComplete);
         })(),
@@ -429,7 +429,7 @@ describe('MessageIO', function() {
         (async () => {
           const io = new MessageIO(clientConnection, packetSize, debug);
 
-          await io.startTls(createSecureContext({}), 'localhost', true);
+          await io.startTls({}, 'localhost', true);
 
           // Send a request (via TLS)
           io.sendMessage(TYPE.LOGIN7, payload);
@@ -525,10 +525,10 @@ describe('MessageIO', function() {
 
           let hadError = false;
           try {
-            await io.startTls(createSecureContext({
+            await io.startTls({
               // Use a cipher that causes an error immediately
               ciphers: 'NULL'
-            }), 'localhost', true);
+            }, 'localhost', true);
           } catch (err: any) {
             hadError = true;
 
@@ -555,10 +555,10 @@ describe('MessageIO', function() {
 
           let hadError = false;
           try {
-            await io.startTls(createSecureContext({
+            await io.startTls({
               // Use some cipher that's not supported on the server side
               ciphers: 'ECDHE-ECDSA-AES128-GCM-SHA256'
-            }), 'localhost', true);
+            }, 'localhost', true);
           } catch (err: any) {
             hadError = true;