diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index 8054c7c8..6164e578 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -1,5 +1,6 @@ import thrift from 'thrift'; import os from 'os'; +import tls from 'tls'; import { EventEmitter } from 'events'; import TCLIService from '../thrift/TCLIService'; @@ -196,6 +197,15 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I https: true, socketTimeout: options.socketTimeout, proxy: options.proxy, + // `customCaCert` is ADDITIVE: Node's `ca` option replaces the system trust + // store, so we append the custom cert to the built-in roots to keep public + // Databricks warehouses trusted while also trusting the caller's CA. + ca: options.customCaCert === undefined ? undefined : [...tls.rootCertificates, options.customCaCert.toString()], + // Client certificate + key for mutual TLS (mTLS). Both must be supplied together. + cert: options.clientCert, + key: options.clientKey, + // Validate the server certificate unless the caller explicitly opts out. + rejectUnauthorized: options.checkServerCertificate ?? true, headers: { 'User-Agent': buildUserAgentString(options.userAgentEntry), }, diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index 8c56019c..e8a511d1 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -68,7 +68,9 @@ export default class HttpConnection implements IConnectionProvider { const httpsAgentOptions: https.AgentOptions = { ...this.getAgentDefaultOptions(), minVersion: 'TLSv1.2', - rejectUnauthorized: false, + // Validate the server certificate by default; only skip verification when + // the caller explicitly opts out via `rejectUnauthorized: false`. + rejectUnauthorized: this.options.rejectUnauthorized ?? true, ca: this.options.ca, cert: this.options.cert, key: this.options.key, diff --git a/lib/connection/contracts/IConnectionOptions.ts b/lib/connection/contracts/IConnectionOptions.ts index 340b6fae..cf4e9ba3 100644 --- a/lib/connection/contracts/IConnectionOptions.ts +++ b/lib/connection/contracts/IConnectionOptions.ts @@ -19,7 +19,11 @@ export default interface IConnectionOptions { socketTimeout?: number; proxy?: ProxyOptions; - ca?: Buffer | string; + ca?: Buffer | string | Array; cert?: Buffer | string; key?: Buffer | string; + + // Whether the TLS server certificate is validated against the trusted CA set. + // When omitted the connection provider treats it as `true` (validation enabled). + rejectUnauthorized?: boolean; } diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index b1b2bb7b..3668d8d8 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -59,6 +59,46 @@ export type ConnectionOptions = { proxy?: ProxyOptions; enableMetricViewMetadata?: boolean; + /** + * Verify the server's TLS certificate on the primary Thrift transport. + * Secure-by-default: omitting this leaves full chain + hostname verification + * enabled (`true`), matching Node's `https` default, the JDBC/ODBC drivers, + * and the SEA/kernel backend. + * + * Setting it to `false` disables server certificate verification entirely + * (any self-signed, expired, or wrong-hostname certificate is accepted), + * which exposes the connection — including bearer-token auth headers — to + * man-in-the-middle attacks. Only use `false` for local development against a + * trusted endpoint, and prefer supplying `customCaCert` instead. + * + * Mirrors the `checkServerCertificate` option on the SEA backend. + */ + checkServerCertificate?: boolean; + + /** + * PEM-encoded CA certificate (string or `Buffer`) added to the trust store + * **on top of** the system roots — for TLS-inspecting proxies or on-prem + * internal CAs. Because it is additive, connections to public Databricks + * warehouses (trusted via the system roots) keep working. + * + * Mirrors the `customCaCert` option on the SEA backend. + */ + customCaCert?: Buffer | string; + + /** + * PEM-encoded client certificate (string or `Buffer`) presented to the server + * for mutual TLS (mTLS). Must be supplied together with `clientKey`. Leave + * both unset for the usual token/OAuth flows, which do not require a client + * certificate. + */ + clientCert?: Buffer | string; + + /** + * PEM-encoded private key (string or `Buffer`) for `clientCert`, used for + * mutual TLS (mTLS). Must be supplied together with `clientCert`. + */ + clientKey?: Buffer | string; + /** * Retry-policy knobs governing how the driver retries retryable requests. * They apply to **both** backends: the Thrift `HttpRetryPolicy` reads them diff --git a/tests/unit/DBSQLClient.test.ts b/tests/unit/DBSQLClient.test.ts index 4cf16838..7675662c 100644 --- a/tests/unit/DBSQLClient.test.ts +++ b/tests/unit/DBSQLClient.test.ts @@ -62,6 +62,67 @@ describe('DBSQLClient.connect', () => { expect(connectionOptions.path).to.equal(path); }); + it('should validate the server certificate by default', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions'](connectOptions); + + expect(connectionOptions.rejectUnauthorized).to.be.true; + }); + + it('should map checkServerCertificate to rejectUnauthorized', async () => { + const client = new DBSQLClient(); + + expect(client['getConnectionOptions']({ ...connectOptions, checkServerCertificate: false }).rejectUnauthorized).to + .be.false; + expect(client['getConnectionOptions']({ ...connectOptions, checkServerCertificate: true }).rejectUnauthorized).to.be + .true; + }); + + it('should not set a custom CA when customCaCert is omitted', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions'](connectOptions); + + expect(connectionOptions.ca).to.be.undefined; + }); + + it('should add customCaCert on top of the system root certificates (additive)', async () => { + const client = new DBSQLClient(); + const customCaCert = '-----BEGIN CERTIFICATE-----\ncustom\n-----END CERTIFICATE-----\n'; + + const connectionOptions = client['getConnectionOptions']({ ...connectOptions, customCaCert }); + + expect(connectionOptions.ca).to.be.an('array'); + const ca = connectionOptions.ca as Array; + // The custom cert must be present... + expect(ca).to.include(customCaCert); + // ...alongside the built-in system roots (so public warehouses still validate). + expect(ca.length).to.be.greaterThan(1); + }); + + it('should not set client cert/key when mTLS options are omitted', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions'](connectOptions); + + expect(connectionOptions.cert).to.be.undefined; + expect(connectionOptions.key).to.be.undefined; + }); + + it('should map clientCert/clientKey to cert/key for mutual TLS', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions']({ + ...connectOptions, + clientCert: 'client-cert', + clientKey: 'client-key', + }); + + expect(connectionOptions.cert).to.equal('client-cert'); + expect(connectionOptions.key).to.equal('client-key'); + }); + it('should initialize connection state', async () => { const client = new DBSQLClient(); diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index 44d38a69..ec37e72f 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -26,7 +26,7 @@ describe('HttpConnection.connect', () => { expect(anotherConnection).to.eq(thriftConnection); }); - it('should set SSL certificates and disable rejectUnauthorized', async () => { + it('should set SSL certificates and validate the server certificate by default', async () => { const connection = new HttpConnection( { host: 'localhost', @@ -42,12 +42,46 @@ describe('HttpConnection.connect', () => { const thriftConnection = await connection.getThriftConnection(); - expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.false; + expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.true; expect(thriftConnection.config.agent.options.ca).to.be.eq('ca'); expect(thriftConnection.config.agent.options.cert).to.be.eq('cert'); expect(thriftConnection.config.agent.options.key).to.be.eq('key'); }); + it('should allow disabling server certificate validation via rejectUnauthorized', async () => { + const connection = new HttpConnection( + { + host: 'localhost', + port: 10001, + path: '/hive', + https: true, + rejectUnauthorized: false, + }, + new ClientContextStub(), + ); + + const thriftConnection = await connection.getThriftConnection(); + + expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.false; + }); + + it('should keep server certificate validation enabled when rejectUnauthorized is true', async () => { + const connection = new HttpConnection( + { + host: 'localhost', + port: 10001, + path: '/hive', + https: true, + rejectUnauthorized: true, + }, + new ClientContextStub(), + ); + + const thriftConnection = await connection.getThriftConnection(); + + expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.true; + }); + it('should initialize http agents', async () => { const connection = new HttpConnection( {