Fix #462: verify Thrift TLS server certificate by default [PECOBLR-3837][SEC-20280]

lib/DBSQLClient.ts

@@ -1,5 +1,7 @@
 import thrift from 'thrift';
 import os from 'os';
+import fs from 'fs';
+import tls from 'tls';
 
 import { EventEmitter } from 'events';
 import TCLIService from '../thrift/TCLIService';
@@ -14,7 +16,7 @@ import IAuthentication from './connection/contracts/IAuthentication';
 import HttpConnection from './connection/connections/HttpConnection';
 import IConnectionOptions from './connection/contracts/IConnectionOptions';
 import HiveDriverError from './errors/HiveDriverError';
-import { buildUserAgentString } from './utils';
+import { buildUserAgentString, normalizePemBytes } from './utils';
 import IBackend from './contracts/IBackend';
 import { InternalConnectionOptions } from './contracts/InternalConnectionOptions';
 import ThriftBackend from './thrift-backend/ThriftBackend';
@@ -190,14 +192,78 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I
  this.logger.log(LogLevel.info, 'Created DBSQLClient');
  }
 
+ // Node folds `NODE_EXTRA_CA_CERTS` into the default trust store ONLY when the
+ // `ca` option is left unset — and `tls.rootCertificates` does not include those
+ // extra roots. Since we set `ca` explicitly to append `customCaCert`, we must
+ // re-read `NODE_EXTRA_CA_CERTS` ourselves so callers relying on it (e.g. a
+ // corporate proxy) do not silently lose those roots.
+ private static getExtraCaCerts(): Array<string> {
+ const extraCertsPath = process.env.NODE_EXTRA_CA_CERTS;
+ if (!extraCertsPath) {
+ return [];
+ }
+ try {
+ return [fs.readFileSync(extraCertsPath, 'utf8')];
+ } catch {
+ // Node itself silently ignores an unreadable NODE_EXTRA_CA_CERTS; mirror that.
+ return [];
+ }
+ }
+
  private getConnectionOptions(options: ConnectionOptions): IConnectionOptions {
+ // mTLS requires both a client certificate and its private key. If exactly one is
+ // supplied, Node fails deep in the TLS handshake with an opaque error, so surface
+ // a clear client-side message instead.
+ const hasClientCert = options.clientCert !== undefined;
+ const hasClientKey = options.clientKey !== undefined;
+ if (hasClientCert !== hasClientKey) {
+ throw new HiveDriverError(
+ `DBSQLClient: mutual TLS requires both clientCert and clientKey; only \`${
+ hasClientCert ? 'clientCert' : 'clientKey'
+ }\` was supplied. Provide the matching ${hasClientCert ? '`clientKey` (private key)' : '`clientCert`'}.`,
+ );
+ }
+
+ // Validate the PEM inputs up front with the same ordered BEGIN…END check the
+ // kernel path uses (normalizePemBytes), so a truncated/headerless/DER blob is
+ // rejected here with a named, actionable error instead of surfacing as an
+ // opaque failure deep in Node's TLS handshake.
+ const clientCert =
+ options.clientCert === undefined
+ ? undefined
+ : normalizePemBytes(options.clientCert, 'clientCert', 'certificate', 'DBSQLClient');
+ const clientKey =
+ options.clientKey === undefined
+ ? undefined
+ : normalizePemBytes(options.clientKey, 'clientKey', 'private key', 'DBSQLClient');
+
  return {
  host: options.host,
  port: options.port || 443,
  path: prependSlash(options.path),
  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 AND any roots
+ // supplied via NODE_EXTRA_CA_CERTS to keep public Databricks warehouses
+ // trusted while also trusting the caller's CA.
+ ca:
+ options.customCaCert === undefined
+ ? undefined
+ : [
+ ...tls.rootCertificates,
+ ...DBSQLClient.getExtraCaCerts(),
+ // Push the normalized Buffer as-is (Node's `ca` accepts a mixed
+ // Array<string | Buffer>) to match the cert/key treatment and keep
+ // byte-fidelity for Buffer inputs instead of round-tripping through utf-8.
+ normalizePemBytes(options.customCaCert, 'customCaCert', 'certificate', 'DBSQLClient'),
+ ],
+ // Client certificate + key for mutual TLS (mTLS). Both must be supplied together.
+ cert: clientCert,
+ key: clientKey,
+ // Validate the server certificate unless the caller explicitly opts out.
+ rejectUnauthorized: options.checkServerCertificate ?? true,
  headers: {
  'User-Agent': buildUserAgentString(options.userAgentEntry),
  },

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,

lib/connection/contracts/IConnectionOptions.ts

@@ -19,7 +19,11 @@ export default interface IConnectionOptions {
  socketTimeout?: number;
  proxy?: ProxyOptions;
 
- ca?: Buffer | string;
+ ca?: Buffer | string | Array<Buffer | string>;
  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;
 }

lib/contracts/IDBSQLClient.ts

@@ -59,6 +59,54 @@ 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 built-in roots — for TLS-inspecting proxies or on-prem
+ * internal CAs. Because it is additive, connections to public Databricks
+ * warehouses keep working.
+ *
+ * Note: supplying this rebuilds the trust store from Node's **bundled Mozilla
+ * roots** (`tls.rootCertificates`) plus any roots from the `NODE_EXTRA_CA_CERTS`
+ * environment variable, then appends this certificate. It does **not** include
+ * OS-installed roots that Node would otherwise consult (e.g. on Node >= 22 run
+ * with `--use-system-ca`). If you rely on an enterprise root installed in the
+ * OS trust store, add it explicitly via `NODE_EXTRA_CA_CERTS` or `customCaCert`
+ * when using this option.
+ *
+ * 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

lib/kernel/KernelAuth.ts

@@ -16,7 +16,7 @@ import { ConnectionOptions } from '../contracts/IDBSQLClient';
 import { InternalConnectionOptions } from '../contracts/InternalConnectionOptions';
 import AuthenticationError from '../errors/AuthenticationError';
 import HiveDriverError from '../errors/HiveDriverError';
-import { buildUserAgentString } from '../utils';
+import { buildUserAgentString, normalizePemBytes } from '../utils';
 
 /**
  * Default local listener port for the U2M authorization-code callback.
@@ -267,50 +267,6 @@ export function isBlankOrReserved(s: string): boolean {
 /** napi-rs marshals `maxConnections` as a `u32`; reject values it can't hold. */
 const MAX_U32 = 0xffffffff;
 
-/**
- * Normalise a PEM input (`string` or `Buffer`) accepted on the public
- * surface into the `Buffer` the napi shape requires. Does a light,
- * ordered BEGIN…END sanity check so a truncated/headerless blob (or a
- * stray page that merely contains the literals out of order, e.g. a
- * proxy-intercept page) is rejected here rather than surfacing as an
- * opaque kernel TLS error. The bytes are NOT fully parsed in JS — that
- * is deferred to the kernel, which returns a meaningful error on a
- * malformed PEM/key.
- *
- * `kind` selects the expected block: `'certificate'` matches a
- * `CERTIFICATE` block; `'private key'` matches any `… PRIVATE KEY` block
- * (PKCS#8 `PRIVATE KEY`, PKCS#1 `RSA PRIVATE KEY`, SEC1 `EC PRIVATE KEY`).
- *
- * Throws `HiveDriverError` when the value is empty or (for strings)
- * lacks the expected PEM header.
- */
-function normalizePemBytes(value: Buffer | string, optionName: string, kind: 'certificate' | 'private key'): Buffer {
- if (typeof value === 'string') {
- const re =
- kind === 'certificate'
- ? /-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/
- : /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]+?-----END [A-Z0-9 ]*PRIVATE KEY-----/;
- if (!re.test(value)) {
- const expected =
- kind === 'certificate'
- ? "a '-----BEGIN CERTIFICATE-----' … '-----END CERTIFICATE-----' block"
- : "a 'BEGIN … PRIVATE KEY' / 'END … PRIVATE KEY' PEM block (PKCS#8, PKCS#1, or SEC1)";
- throw new HiveDriverError(
- `kernel backend: \`${optionName}\` string does not look like a PEM ${kind} (expected ${expected}). ` +
- 'Pass PEM text or a Buffer of PEM bytes.',
- );
- }
- return Buffer.from(value, 'utf8');
- }
- if (Buffer.isBuffer(value)) {
- if (value.length === 0) {
- throw new HiveDriverError(`kernel backend: \`${optionName}\` Buffer is empty.`);
- }
- return value;
- }
- throw new HiveDriverError(`kernel backend: \`${optionName}\` must be a PEM string or a Buffer.`);
-}
-
 /**
  * Normalise the public TLS options into the napi shape.
  *
@@ -321,11 +277,13 @@ function normalizePemBytes(value: Buffer | string, optionName: string, kind: 'ce
  * master verify toggle is on). Mirrors Python's `tls_verify_hostname`.
  * - `customCaCert` accepts a PEM string or `Buffer`; normalised to a
  * `Buffer` via {@link normalizePemBytes}.
- * - `clientCertPem` / `clientKeyPem` carry the mutual-TLS client identity.
- * They must be supplied **together** — supplying only one is rejected
- * here with an actionable error (rather than waiting for the kernel's
- * `InvalidArgument` at `openSession`). Each accepts a PEM string or
- * `Buffer`, normalised the same way.
+ * - `clientCertPem` / `clientKeyPem` (or their public aliases
+ * `clientCert` / `clientKey`) carry the mutual-TLS client identity. The
+ * internal `*Pem` names win when both are present. They must be supplied
+ * **together** — supplying only one is rejected here with an actionable
+ * error (rather than waiting for the kernel's `InvalidArgument` at
+ * `openSession`). Each accepts a PEM string or `Buffer`, normalised the
+ * same way.
  *
  * Throws `HiveDriverError` when a cert/key is empty, mis-typed, lacks the
  * expected PEM header, or when only one half of the mTLS pair is set.
@@ -334,8 +292,17 @@ export function buildKernelTlsOptions(options: ConnectionOptions): KernelTlsOpti
  // Read the kernel-only fields through the purpose-built internal options type
  // rather than an ad-hoc inline cast, so the shape can't silently drift from
  // its declaration and a typo'd key fails to compile.
- const { checkServerCertificate, checkServerCertificateHostname, customCaCert, clientCertPem, clientKeyPem } =
- options as ConnectionOptions & InternalConnectionOptions;
+ const merged = options as ConnectionOptions & InternalConnectionOptions;
+ const { checkServerCertificate, checkServerCertificateHostname, customCaCert } = merged;
+
+ // The public mTLS options are `clientCert`/`clientKey` (see `ConnectionOptions`);
+ // the internal kernel-only aliases are `clientCertPem`/`clientKeyPem`. Accept
+ // both here — preferring the explicit internal alias when present — so a caller
+ // who sets the public `clientCert`/`clientKey` and runs on the kernel backend
+ // still gets mTLS configured instead of having their client identity silently
+ // dropped.
+ const clientCertPem = merged.clientCertPem ?? merged.clientCert;
+ const clientKeyPem = merged.clientKeyPem ?? merged.clientKey;
 
  const tls: KernelTlsOptions = {};
 
@@ -348,7 +315,7 @@ export function buildKernelTlsOptions(options: ConnectionOptions): KernelTlsOpti
  }
 
  if (customCaCert !== undefined) {
- tls.customCaCert = normalizePemBytes(customCaCert, 'customCaCert', 'certificate');
+ tls.customCaCert = normalizePemBytes(customCaCert, 'customCaCert', 'certificate', 'kernel backend');
  }
 
  // mTLS client identity. Enforce both-or-neither up front so a caller who
@@ -365,8 +332,18 @@ export function buildKernelTlsOptions(options: ConnectionOptions): KernelTlsOpti
  );
  }
  if (hasCert && hasKey) {
- tls.clientCertPem = normalizePemBytes(clientCertPem as Buffer | string, 'clientCertPem', 'certificate');
- tls.clientKeyPem = normalizePemBytes(clientKeyPem as Buffer | string, 'clientKeyPem', 'private key');
+ tls.clientCertPem = normalizePemBytes(
+ clientCertPem as Buffer | string,
+ 'clientCertPem',
+ 'certificate',
+ 'kernel backend',
+ );
+ tls.clientKeyPem = normalizePemBytes(
+ clientKeyPem as Buffer | string,
+ 'clientKeyPem',
+ 'private key',
+ 'kernel backend',
+ );
  }
 
  return tls;

lib/utils/index.ts

@@ -4,6 +4,7 @@ import formatProgress, { ProgressUpdateTransformer } from './formatProgress';
 import LZ4 from './lz4';
 import * as ProtocolVersion from './protocolVersion';
 import serializeQueryTags from './queryTags';
+import normalizePemBytes from './normalizePemBytes';
 
 export {
  definedOrError,
@@ -13,4 +14,5 @@ export {
  LZ4,
  ProtocolVersion,
  serializeQueryTags,
+ normalizePemBytes,
 };

lib/utils/normalizePemBytes.ts

@@ -0,0 +1,52 @@
+import HiveDriverError from '../errors/HiveDriverError';
+
+/**
+ * Normalise a PEM input (`string` or `Buffer`) accepted on the public surface
+ * into a `Buffer`. Does a light, ordered BEGIN…END sanity check so a
+ * truncated/headerless/DER blob (or a stray page that merely contains the
+ * literals out of order, e.g. a proxy-intercept page) is rejected here rather
+ * than surfacing as an opaque TLS handshake error further down. The bytes are
+ * NOT fully parsed in JS — that is deferred to the TLS stack, which returns a
+ * meaningful error on a malformed PEM/key.
+ *
+ * `kind` selects the expected block: `'certificate'` matches a `CERTIFICATE`
+ * block; `'private key'` matches any `… PRIVATE KEY` block (PKCS#8 `PRIVATE
+ * KEY`, PKCS#1 `RSA PRIVATE KEY`, SEC1 `EC PRIVATE KEY`).
+ *
+ * `backendLabel` prefixes the error message so the caller (e.g. `DBSQLClient`
+ * on the Thrift path, `kernel backend` on the kernel path) is named accurately.
+ *
+ * Throws `HiveDriverError` when the value is empty or (for strings) lacks the
+ * expected PEM header.
+ */
+export default function normalizePemBytes(
+ value: Buffer | string,
+ optionName: string,
+ kind: 'certificate' | 'private key',
+ backendLabel: string,
+): Buffer {
+ if (typeof value === 'string') {
+ const re =
+ kind === 'certificate'
+ ? /-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/
+ : /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]+?-----END [A-Z0-9 ]*PRIVATE KEY-----/;
+ if (!re.test(value)) {
+ const expected =
+ kind === 'certificate'
+ ? "a '-----BEGIN CERTIFICATE-----' … '-----END CERTIFICATE-----' block"
+ : "a 'BEGIN … PRIVATE KEY' / 'END … PRIVATE KEY' PEM block (PKCS#8, PKCS#1, or SEC1)";
+ throw new HiveDriverError(
+ `${backendLabel}: \`${optionName}\` string does not look like a PEM ${kind} (expected ${expected}). ` +
+ 'Pass PEM text or a Buffer of PEM bytes.',
+ );
+ }
+ return Buffer.from(value, 'utf8');
+ }
+ if (Buffer.isBuffer(value)) {
+ if (value.length === 0) {
+ throw new HiveDriverError(`${backendLabel}: \`${optionName}\` Buffer is empty.`);
+ }
+ return value;
+ }
+ throw new HiveDriverError(`${backendLabel}: \`${optionName}\` must be a PEM string or a Buffer.`);
+}