Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add TDS8.0 Support for tedious #1522

Merged
merged 42 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
4b27035
feat: add support for TDS8.0
MichaelSun90 Jul 19, 2022
96c88cb
Merge branch 'master' into NewTLSSupport
mShan0 Feb 2, 2023
60c1a38
fix: fix type check for `options.encrypt` (#1517)
mShan0 Feb 8, 2023
bf32f85
chore: Add logic for read secure context
MichaelSun90 Feb 8, 2023
e458d12
chore: update the PR based on the comments
MichaelSun90 Feb 15, 2023
2165a2a
chore: add logic for read secure context
MichaelSun90 Feb 8, 2023
3a9213a
chore: update the PR based on the comments
MichaelSun90 Feb 15, 2023
f654831
Merge branch 'NewTLSSupport' of https://github.com/tediousjs/tedious …
MichaelSun90 Feb 15, 2023
39b1379
Merge branch 'master' into NewTLSSupport
MichaelSun90 Feb 16, 2023
b31c287
chore: changes based on comments
MichaelSun90 Feb 17, 2023
eabc9d5
Merge branch 'NewTLSSupport' of https://github.com/tediousjs/tedious …
MichaelSun90 Feb 17, 2023
0fcbe8a
chore: add tests and fixed some minor issue
MichaelSun90 Feb 23, 2023
ef108d6
chore: remove it.only from test
MichaelSun90 Mar 2, 2023
a448262
test: add additional encrypt config unit tests (#1525)
mShan0 Mar 3, 2023
a6ffebf
Merge branch 'master' into NewTLSSupport
arthurschreiber Mar 22, 2023
fee6ee7
Add `extendedKeyUsage` option.
arthurschreiber Mar 23, 2023
30f0ccd
Try some other options.
arthurschreiber Mar 23, 2023
7891302
Merge branch 'master' of https://github.com/tediousjs/tedious into Ne…
arthurschreiber Mar 26, 2023
cbe4089
Add some config debug output.
arthurschreiber Mar 26, 2023
76a146a
Ensure proper line endings.
arthurschreiber Mar 26, 2023
2252a43
ci: add latest build of sql server 2022
mShan0 Mar 27, 2023
88ef29f
Merge pull request #1530 from tediousjs/mark-update-windows-ci-build
mShan0 Mar 27, 2023
7818752
wait for files to finish download
mShan0 Mar 27, 2023
53dd486
enable updates
mShan0 Mar 27, 2023
e5b4c6f
ci: revert to download box files directly, also download and install …
arthurschreiber Mar 28, 2023
d5f2bdb
ci: fix update download path
arthurschreiber Mar 28, 2023
81c52a2
ci: fix starting the sql server install
arthurschreiber Mar 28, 2023
f675c79
chore: fix test for other protocol versions
MichaelSun90 Mar 28, 2023
04c76a7
chore: code style change and error handling
MichaelSun90 Apr 6, 2023
c955670
Merge branch 'master' of https://github.com/tediousjs/tedious into Ne…
arthurschreiber Apr 8, 2023
3b8074d
Fix incorrect merge conflict resolution.
arthurschreiber Apr 8, 2023
f2e45ad
test: rework to skip if TDS 8.0 is not available
arthurschreiber May 31, 2023
a6d414a
Merge branch 'master' into NewTLSSupport
arthurschreiber Jun 7, 2023
c2aa47f
fix: remove the unecessary error output form connection-test
MichaelSun90 Jun 7, 2023
47bd587
Only check `sys.dm_os_host_info` on SQL Server versions where it is a…
arthurschreiber Jun 28, 2023
d579439
Merge branch 'master' into NewTLSSupport
MichaelSun90 Jun 28, 2023
20ef404
chore: lint issue
MichaelSun90 Jun 29, 2023
c20367b
chore: lint issue for socket
MichaelSun90 Jun 29, 2023
a9524dc
chore: connection to certain db
MichaelSun90 Jun 29, 2023
9d52330
Merge branch 'master' into NewTLSSupport
MichaelSun90 Jul 17, 2023
1859352
Merge branch 'master' into NewTLSSupport
MichaelSun90 Jul 19, 2023
c3e8b67
chore: remove the db constrain
MichaelSun90 Jul 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ jobs:

- name: Generate TLS Certificate
run: |
openssl req -x509 -newkey rsa:2048 -nodes -sha256 -subj '/CN=localhost' -keyout ./test/fixtures/mssql.key -out ./test/fixtures/mssql.crt
openssl req -x509 -newkey rsa:4096 -nodes -addext "extendedKeyUsage = serverAuth" -subj '/CN=localhost' -keyout ./test/fixtures/mssql.key -out ./test/fixtures/mssql.crt

- name: Start containers
run: |
Expand Down Expand Up @@ -192,10 +192,9 @@ jobs:
run: |
Push-Location C:\temp

Invoke-WebRequest -Uri https://download.microsoft.com/download/3/8/d/38de7036-2433-4207-8eae-06e247e17b25/SQLServer2022-DEV-x64-ENU.exe -OutFile sqlsetup.exe
Invoke-WebRequest -Uri https://download.microsoft.com/download/3/8/d/38de7036-2433-4207-8eae-06e247e17b25/SQLServer2022-DEV-x64-ENU.box -OutFile sqlsetup.box

Start-Process -Wait -FilePath ./sqlsetup.exe -ArgumentList /qs, /x:setup
Invoke-WebRequest -Uri https://download.microsoft.com/download/c/c/9/cc9c6797-383c-4b24-8920-dc057c1de9d3/SQL2022-SSEI-Dev.exe -OutFile SQL2022-SSEI-Dev.exe
Start-Process -Wait -FilePath ./SQL2022-SSEI-Dev.exe -ArgumentList /ACTION=Download, /MEDIAPATH=C:\temp, /MEDIATYPE=CAB, /QUIET
Start-Process -Wait -FilePath ./SQLServer2022-DEV-x64-ENU.exe -ArgumentList /qs, /x:setup

.\setup\setup.exe /q /ACTION=Install /INSTANCENAME=MSSQLSERVER /FEATURES=SQLEngine /UPDATEENABLED=0 /SQLSVCACCOUNT='NT SERVICE\MSSQLSERVER' /SQLSYSADMINACCOUNTS='BUILTIN\ADMINISTRATORS' /TCPENABLED=1 /NPENABLED=0 /IACCEPTSQLSERVERLICENSETERMS /SQLCOLLATION=SQL_Latin1_General_CP1_CI_AS /USESQLRECOMMENDEDMEMORYLIMITS
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: We can use /SECURITYMODE='SQL' to enable mixed authentication on installation, and use /SAPWD='...' to set the password. That would allow us to skip changing these settings (and the server restart).


Expand Down Expand Up @@ -246,9 +245,10 @@ jobs:
'-----END CERTIFICATE-----'
)
# Output PEM file to the path
$output | Out-File -FilePath test\fixtures\mssql.crt -Encoding ascii
$output -join "`n" | Out-File -FilePath test\fixtures\mssql.crt -Encoding ascii -NoNewLine

- name: Set up CI configuration
shell: bash
run: |
mkdir ~/.tedious

Expand All @@ -270,6 +270,8 @@ jobs:
}
}' | jq --arg certificate "$(cat ./test/fixtures/mssql.crt)" '.config.options.cryptoCredentialsDetails.ca |= $certificate' > ~/.tedious/test-connection.json

cat ~/.tedious/test-connection.json

- name: Upgrade npm
run: npm install -g npm
if: ${{ matrix.node-version == '6.x' }}
Expand Down
92 changes: 69 additions & 23 deletions src/connection.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import crypto from 'crypto';
import os from 'os';
import * as tls from 'tls';
import { Socket } from 'net';
import dns from 'dns';

Expand Down Expand Up @@ -365,7 +366,7 @@ export interface InternalConnectionOptions {
enableImplicitTransactions: null | boolean;
enableNumericRoundabort: null | boolean;
enableQuotedIdentifier: null | boolean;
encrypt: boolean;
encrypt: string | boolean;
encryptionKeyStoreProviders: KeyStoreProviderMap | undefined;
fallbackToDefaultDb: boolean;
instanceName: undefined | string;
Expand Down Expand Up @@ -660,11 +661,12 @@ export interface ConnectionOptions {
enableQuotedIdentifier?: boolean;

/**
* A boolean determining whether or not the connection will be encrypted. Set to `true` if you're on Windows Azure.
* A string value that can be only set to 'strict', which indicates the usage TDS 8.0 protocol. Otherwise,
* a boolean determining whether or not the connection will be encrypted.
*
* (default: `false`)
* (default: `true`)
*/
encrypt?: boolean;
encrypt?: string | boolean;

/**
* By default, if the database requested by [[database]] cannot be accessed,
Expand Down Expand Up @@ -818,6 +820,10 @@ export interface ConnectionOptions {
*/
trustServerCertificate?: boolean;

/**
*
*/
serverName?: string;
/**
* A boolean determining whether to return rows as arrays or key-value collections.
*
Expand Down Expand Up @@ -1479,10 +1485,11 @@ class Connection extends EventEmitter {

this.config.options.enableQuotedIdentifier = config.options.enableQuotedIdentifier;
}

if (config.options.encrypt !== undefined) {
if (typeof config.options.encrypt !== 'boolean') {
throw new TypeError('The "config.options.encrypt" property must be of type boolean.');
if (config.options.encrypt !== 'strict') {
throw new TypeError('The "encrypt" property must be set to "strict", or of type boolean.');
}
}

this.config.options.encrypt = config.options.encrypt;
Expand Down Expand Up @@ -1642,6 +1649,13 @@ class Connection extends EventEmitter {
this.config.options.trustServerCertificate = config.options.trustServerCertificate;
}

if (config.options.serverName !== undefined) {
if (typeof config.options.serverName !== 'string') {
throw new TypeError('The "config.options.serverName" property must be of type string.');
}
this.config.options.serverName = config.options.serverName;
}

if (config.options.useColumnNames !== undefined) {
if (typeof config.options.useColumnNames !== 'boolean') {
throw new TypeError('The "config.options.useColumnNames" property must be of type boolean.');
Expand Down Expand Up @@ -1967,21 +1981,52 @@ class Connection extends EventEmitter {

connect(connectOpts, dns.lookup, signal).then((socket) => {
process.nextTick(() => {
socket.on('error', (error) => { this.socketError(error); });
socket.on('close', () => { this.socketClose(); });
socket.on('end', () => { this.socketEnd(); });
socket.setKeepAlive(true, KEEP_ALIVE_INITIAL_DELAY);
const secureContext = tls.createSecureContext(this.secureContextOptions);
if (this.config.options.encrypt === 'strict') {
const encryptOptions = {
host: this.config.server,
socket: socket,
ALPNProtocols: ['tds/8.0'],
secureContext: secureContext,
servername: this.config.options.serverName ? this.config.options.serverName : this.config.server,
};
const encryptsocket = tls.connect(encryptOptions, () => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What will happen if someone connects using strict encryption to a SQL Server instance that does not support it?
How do we need to handle that, and how do we signal to the user that something is wrong? How do we help them understand what is wrong?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If user uses a strict to connect to a SQL server instance that does not support it, a socket error will be returned as : " Client network socket disconnected before secure TLS connection was established"
Server side message:
"The login packet used to open the connection is structurally invalid; the connection has been closed. Please contact the vendor of the client library. [CLIENT: 172.28.192.1]
"
what do you think that we consume this message internally, and throw a custom error with a bit more information, so we can guide user through this

socket = encryptsocket;

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

this.messageIo = new MessageIO(socket, this.config.options.packetSize, this.debug);

this.socket = socket;

this.closed = false;
this.debug.log('connected to ' + this.config.server + ':' + this.config.options.port);

this.sendPreLogin();
this.transitionTo(this.STATE.SENT_PRELOGIN);
});

console.log('using TDS 8.0 strict TLS encryption');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this might be some leftover log output.

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

this.messageIo = new MessageIO(socket, this.config.options.packetSize, this.debug);
this.messageIo.on('secure', (cleartext) => { this.emit('secure', cleartext); });
this.messageIo = new MessageIO(socket, this.config.options.packetSize, this.debug);
this.messageIo.on('secure', (cleartext) => { this.emit('secure', cleartext); });

this.socket = socket;
this.socket = socket;

this.closed = false;
this.debug.log('connected to ' + this.config.server + ':' + this.config.options.port);
this.closed = false;
this.debug.log('connected to ' + this.config.server + ':' + this.config.options.port);

this.sendPreLogin();
this.transitionTo(this.STATE.SENT_PRELOGIN);
this.sendPreLogin();
this.transitionTo(this.STATE.SENT_PRELOGIN);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this duplication?

}
});
}, (err) => {
this.clearConnectTimer();
Expand Down Expand Up @@ -2231,10 +2276,12 @@ class Connection extends EventEmitter {
* @private
*/
sendPreLogin() {
const [ , major, minor, build ] = /^(\d+)\.(\d+)\.(\d+)/.exec(version) ?? [ '0.0.0', '0', '0', '0' ];

const [, major, minor, build] = /^(\d+)\.(\d+)\.(\d+)/.exec(version) ?? ['0.0.0', '0', '0', '0'];
const payload = new PreloginPayload({
encrypt: this.config.options.encrypt,
// If encrypt setting is set to 'strict', then we should have already done the encryption before calling
// this function. Therefore, the encrypt will be set to false here.
// Otherwise, we will set encrypt here based on the encrypt Boolean value from the configuration.
encrypt: typeof this.config.options.encrypt === 'boolean' && this.config.options.encrypt,
version: { major: Number(major), minor: Number(minor), build: Number(build), subbuild: 0 }
});

Expand Down Expand Up @@ -3155,16 +3202,15 @@ Connection.prototype.STATE = {
if (preloginPayload.fedAuthRequired === 1) {
this.fedAuthRequired = true;
}

if (preloginPayload.encryptionString === 'ON' || preloginPayload.encryptionString === 'REQ') {
if ('strict' !== this.config.options.encrypt && (preloginPayload.encryptionString === 'ON' || preloginPayload.encryptionString === 'REQ')) {
if (!this.config.options.encrypt) {
this.emit('connect', new ConnectionError("Server requires encryption, set 'encrypt' config option to true.", 'EENCRYPT'));
return this.close();
}

try {
this.transitionTo(this.STATE.SENT_TLSSSLNEGOTIATION);
await this.messageIo.startTls(this.secureContextOptions, this.routingData?.server ?? this.config.server, this.config.options.trustServerCertificate);
await this.messageIo.startTls(this.secureContextOptions, this.config.options.serverName ? this.config.options.serverName : this.routingData?.server ?? this.config.server, this.config.options.trustServerCertificate);
} catch (err: any) {
return this.socketError(err);
}
Expand Down
3 changes: 2 additions & 1 deletion src/tds-versions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ export const versions: { [key: string]: number } = {
'7_2': 0x72090002,
'7_3_A': 0x730A0003,
'7_3_B': 0x730B0003,
'7_4': 0x74000004
'7_4': 0x74000004,
'8_0': 0x08000000
};

export const versionsByValue: { [key: number]: string } = {};
Expand Down
46 changes: 39 additions & 7 deletions test/integration/connection-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const os = require('os');
import Connection from '../../src/connection';
import { ConnectionError, RequestError } from '../../src/errors';
import Request from '../../src/request';
import { versions } from '../../src/tds-versions';

function getConfig() {
const config = JSON.parse(
Expand Down Expand Up @@ -530,24 +531,45 @@ describe('Ntlm Test', function() {
});

describe('Encrypt Test', function() {
it('should encrypt', function(done) {
const config = getConfig();
config.options.encrypt = true;

/**
* @this {Mocha.Context}
* @param {Mocha.Done} done
* @param {import("../../src/connection").ConnectionConfiguration} config
* @param {string} tdsKey
* @returns {void}
*/
function runEncryptTest(done, config, tdsKey) {
const connection = new Connection(config);

connection.connect(function(err) {
assert.ifError(err);
connection.execSql(request);
});

connection.close();
const request = new Request(
`SELECT c.protocol_version
FROM sys.dm_exec_connections AS c
JOIN sys.dm_exec_sessions AS s
ON c.session_id = s.session_id
WHERE c.session_id = @@SPID`, (err, rowCount) => {
assert.ifError(err);
assert.strictEqual(rowCount, 1);

connection.close();
});

request.on('row', function(columns) {
// console.log(versions[tdsKey],columns[0].value)
assert.strictEqual(columns.length, 1);
assert.strictEqual(versions[tdsKey], columns[0].value);
});

connection.on('end', function() {
done();
});

connection.on('databaseChange', function(database) {
assert.strictEqual(database, config.options.database);
assert.strictEqual(database, config.options?.database);
});

connection.on('secure', function(cleartext) {
Expand All @@ -560,9 +582,19 @@ describe('Encrypt Test', function() {
// console.log("#{info.number} : #{info.message}")
});

return connection.on('debug', function(text) {
connection.on('debug', function(text) {
// console.log(text)
});
}
it('should encrypt', function(done) {
const config = getConfig();
config.options.encrypt = true;
runEncryptTest.call(this, done, config, '7_4');
});
it('encrypt with TDS8.0', function(done) {
const config = getConfig();
config.options.encrypt = 'strict';
runEncryptTest.call(this, done, config, '8_0');
});
});

Expand Down
36 changes: 36 additions & 0 deletions test/unit/connection-config-validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,40 @@ describe('Connection configuration validation', function() {
new Connection(config);
});
});

it('bad encrypt value type', () => {
const numberEncrypt = 0;
config.options.encrypt = numberEncrypt;
assert.throws(() => {
new Connection(config);
});
});

it('bad encrypt string', () => {
config.options.encrypt = 'false';
assert.throws(() => {
new Connection(config);
});
});

it('good false encrypt value', () => {
config.options.encrypt = false;
const connection = new Connection(config);
assert.strictEqual(connection.config.options.encrypt, false);
ensureConnectionIsClosed(connection, () => {});
});

it('good true encrypt value', () => {
config.options.encrypt = true;
const connection = new Connection(config);
assert.strictEqual(connection.config.options.encrypt, true);
ensureConnectionIsClosed(connection, () => {});
});

it('good strict encrypt value', () => {
config.options.encrypt = 'strict';
const connection = new Connection(config);
assert.strictEqual(connection.config.options.encrypt, 'strict');
ensureConnectionIsClosed(connection, () => {});
});
});