diff --git a/CHANGELOG.md b/CHANGELOG.md index 77d44b38..75e3c168 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Add the `https` connection option: set `https: false` to connect to `host` over plain HTTP (port 80 by default), for example through a trusted local reverse proxy. HTTPS remains the default, and plain HTTP should only be used on a trusted local or private link. (#503) - CloudFetch retry-exhaustion errors now identify the download and the last failure: the `RetryError` message reads `CloudFetch result download failed: (last error: )`. `` is `HTTP : ` (or `HTTP ` when the status text is empty) for HTTP failures, and `network error `, `network error ()` (e.g. `aborted`, when there is no `code`) or a bare `network error` for network failures (the raw network-error message, which can contain the presigned URL, is not copied). If no cause can be determined, the ` (last error: …)` suffix is omitted. The `RetryError` instance, `errorCode` and `payload` (including the node-fetch `Request` / `Response`) are unchanged. (#471) - Report untyped NULL columns as STRING in kernel result metadata, matching Thrift. (#470) - Reject non-scalar query parameter values at bind time with a `ParameterError` on both the Thrift and kernel backends, instead of stringifying them (which could leave a Thrift operation stuck before FINISHED). Newly rejected on the Thrift backend: arrays, plain objects, boxed primitives (e.g. `new String('x')`, `new Number(1)`), functions, symbols, and a `DBSQLParameter` that wraps another `DBSQLParameter`. Primitives, `Date`, `Int64`, and `DBSQLParameter` are still accepted. diff --git a/CONNECTION_PARAMETERS.md b/CONNECTION_PARAMETERS.md index af46726c..7e95eb03 100644 --- a/CONNECTION_PARAMETERS.md +++ b/CONNECTION_PARAMETERS.md @@ -42,7 +42,8 @@ backend (the default), the **SEA / Kernel** backend, or both. | ----------------------- | --------- | :----: | :----: | ------------- | -------------------------------------------------------------------------------------- | | `host` | `string` | ✅ | ✅ | — (required) | Required on both. | | `path` | `string` | ✅ | ✅ | — (required) | Kernel also derives `x-databricks-org-id` from a `?o=` query parameter. | -| `port` | `number` | ✅ | ❌ | `443` / — | Kernel ignores this field. Include a non-default port in `host` for Kernel. | +| `port` | `number` | ✅ | ⚠️ | `443` / — | Kernel uses it only with `https: false`; otherwise put a non-default port in `host`. | +| `https` | `boolean` | ✅ | ✅ | `true` | `false` selects plain HTTP; see [Plain HTTP transport](#plain-http-transport). | | `userAgentEntry` | `string` | ✅ | ✅ | — | Folded into the connector `User-Agent` on both. | | `useKernel` (internal) | `boolean` | — | — | `false` | Selects the backend: `false`/omitted → Thrift, `true` → Kernel. | | `clientId` (deprecated) | `string` | ✅ | ✅ | — | Runtime-only alias for `userAgentEntry`. `userAgentEntry` wins when both are supplied. | @@ -96,6 +97,34 @@ disabling verification. | `clientKey` | `Buffer \| string` | ✅ | ✅ | — | Public mTLS private key. Must be paired with `clientCert`; PKCS#8 is recommended for Kernel portability. | | `clientCertPem` / `clientKeyPem` (internal) | `Buffer \| string` | ❌ | ✅ | — | Runtime-only Kernel aliases for the public mTLS pair. They take precedence over `clientCert`/`clientKey` when both pairs are supplied. | +## Plain HTTP transport + +Set `https: false` to connect to `host` over plain HTTP, for example through a +trusted local reverse proxy that forwards to Databricks over HTTPS (and may +inject credentials, with `authType: 'custom'` returning no headers): + +```ts +await client.connect({ + host: 'localhost', + port: 3015, + path: '/sql/1.0/warehouses/', + https: false, + authType: 'custom', + provider: { authenticate: async () => ({}) }, +}); +``` + +Plain HTTP is unencrypted: SQL text, results, and any credentials the driver +sends can be read or altered on the network. Use it only on a trusted local or +private link. With `https: false`, `port` defaults to `80`, the TLS options +above have no effect, and `clientCert`/`clientKey` are rejected. + +On Thrift, `https: false` applies to the SQL connection only. OAuth +(`databricks-oauth`), token federation, telemetry, and feature-flag requests +still use HTTPS to `host`. Kernel uses `http://host[:port]` for all of its +requests, including its own OAuth and telemetry calls. CloudFetch downloads use +the presigned URLs returned by the server on both backends. + ## Results and type rendering | Option | Type | Thrift | Kernel | Default Value | Note | diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index ea381bf3..0eb50c70 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -229,6 +229,15 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I ); } + // Only an explicit `false` opts out of TLS. + const https = options.https !== false; + // http.Agent would silently drop the client identity; the kernel rejects it too. + if (!https && hasClientCert) { + throw new HiveDriverError( + 'DBSQLClient: mutual TLS (`clientCert`/`clientKey`) requires HTTPS; remove them or do not set `https: false`.', + ); + } + // 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 @@ -244,9 +253,9 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I return { host: options.host, - port: options.port || 443, + port: options.port || (https ? 443 : 80), path: prependSlash(options.path), - https: true, + https, socketTimeout: options.socketTimeout, proxy: options.proxy, // `customCaCert` is ADDITIVE: Node's `ca` option replaces the system trust diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index 0a6f416b..cd42b151 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -75,6 +75,20 @@ export type ConnectionOptions = { */ escapeMetadataCatalogName?: boolean; + /** + * Connect to `host` over HTTPS (`true`, the default) or plain HTTP (`false`). + * + * Plain HTTP is unencrypted: SQL, results, and any credentials the driver + * sends can be read and modified on the network. Use `false` only for a + * trusted local or private link, such as a reverse proxy that forwards to + * Databricks over HTTPS. With `false`, `port` defaults to 80, the TLS options + * have no effect, and `clientCert`/`clientKey` are rejected. + * + * On the Thrift backend this applies to the SQL connection only; OAuth, token + * federation, telemetry, and feature-flag requests still use HTTPS. + */ + https?: boolean; + /** * Verify the server's TLS certificate on the primary Thrift transport. * Secure-by-default: omitting this leaves full chain + hostname verification diff --git a/lib/kernel/KernelAuth.ts b/lib/kernel/KernelAuth.ts index 53484d8e..6df0f22a 100644 --- a/lib/kernel/KernelAuth.ts +++ b/lib/kernel/KernelAuth.ts @@ -818,6 +818,20 @@ export function buildKernelProxyOptions(options: ConnectionOptions): KernelProxy return { proxy: out }; } +// The kernel prepends `https://` to a bare host but keeps an explicit `http://`. +// It otherwise ignores `port`, so carry it into the URL unless `host` already has one. +function buildKernelHostName(options: ConnectionOptions): string { + if (options.https !== false) { + return options.host; + } + // Strip a trailing `/` like the Thrift URL builder does. + const host = options.host.replace(/\/$/, ''); + // `name:port` or `[ipv6]:port`; a bare IPv6 literal like `::1` has no port. + const hasPort = /^(\[[^\]]+\]|[^:]+):\d+$/.test(host); + const port = options.port && !hasPort ? `:${options.port}` : ''; + return `http://${host}${port}`; +} + export function buildKernelConnectionOptions(options: ConnectionOptions): KernelNativeConnectionOptions { const { authType } = options as { authType?: string }; @@ -831,7 +845,7 @@ export function buildKernelConnectionOptions(options: ConnectionOptions): Kernel KernelHttpOptions & KernelProxyOptions & KernelFederationOptions = { - hostName: options.host, + hostName: buildKernelHostName(options), httpPath: prependSlash(options.path), // Match the NodeJS Thrift driver, which surfaces INTERVAL columns as // strings. The kernel defaults to native Arrow interval/duration types; diff --git a/tests/e2e/http_transport.test.ts b/tests/e2e/http_transport.test.ts new file mode 100644 index 00000000..d3a3dfb0 --- /dev/null +++ b/tests/e2e/http_transport.test.ts @@ -0,0 +1,81 @@ +import { expect } from 'chai'; +import http from 'http'; +import { AddressInfo, Socket } from 'net'; +import httpProxy from 'http-proxy'; +import { DBSQLClient } from '../../lib'; +import { ConnectionOptions } from '../../lib/contracts/IDBSQLClient'; +import { InternalConnectionOptions } from '../../lib/contracts/InternalConnectionOptions'; + +import config from './utils/config'; + +// Application --HTTP--> local reverse proxy --HTTPS--> Databricks +describe('Plain HTTP transport through a local reverse proxy', () => { + const requests: Array<{ method?: string; url?: string }> = []; + const sockets = new Set(); + let server: http.Server; + let port: number; + + before(async () => { + const proxy = httpProxy.createProxyServer({ target: `https://${config.host}`, changeOrigin: true, secure: true }); + server = http.createServer((req, res) => { + requests.push({ method: req.method, url: req.url }); + proxy.web(req, res, {}, (error) => { + res.writeHead(502); + res.end(String(error)); + }); + }); + server.on('connection', (socket: Socket) => { + sockets.add(socket); + socket.on('close', () => sockets.delete(socket)); + }); + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', resolve); + }); + port = (server.address() as AddressInfo).port; + }); + + after(async () => { + const closed = new Promise((resolve) => { + server.close(() => resolve()); + }); + sockets.forEach((socket) => socket.destroy()); + await closed; + }); + + beforeEach(() => { + requests.length = 0; + }); + + [false, true].forEach((useKernel) => { + it(`runs a query with https: false (${useKernel ? 'kernel' : 'thrift'} backend)`, async () => { + const client = new DBSQLClient(); + await client.connect({ + host: '127.0.0.1', + port, + path: config.path, + token: config.token, + https: false, + useKernel, + } as ConnectionOptions & InternalConnectionOptions); + + const session = await client.openSession({ initialCatalog: config.catalog, initialSchema: config.schema }); + const operation = await session.executeStatement('SELECT 1 AS one'); + const rows = (await operation.fetchAll()) as Array>; + await operation.close(); + await session.close(); + await client.close(); + + expect(rows).to.have.length(1); + expect(Number(rows[0].one)).to.equal(1); + // The SQL traffic went through the plain-HTTP proxy: Thrift to the + // warehouse path, the kernel to the SQL Statement Execution API. + if (useKernel) { + expect(requests.some((request) => request.url?.startsWith('/api/2.0/sql/'))).to.be.true; + } else { + expect(requests.length).to.be.gte(1); + const path = config.path.startsWith('/') ? config.path : `/${config.path}`; + expect(requests.every((request) => request.url === path)).to.be.true; + } + }); + }); +}); diff --git a/tests/e2e/proxy.test.ts b/tests/e2e/proxy.test.ts index cd6e8ae4..de807d5a 100644 --- a/tests/e2e/proxy.test.ts +++ b/tests/e2e/proxy.test.ts @@ -65,26 +65,16 @@ describe('Proxy', () => { try { const client = new DBSQLClient(); - // Our proxy mock is HTTP -> HTTPS, but DBSQLClient is hard-coded to use HTTPS. - // Here we override default behavior to make DBSQLClient work with HTTP proxy - // @ts-expect-error TS2341: Property getConnectionOptions is private - const originalGetConnectionOptions = client.getConnectionOptions; - // @ts-expect-error TS2341: Property getConnectionOptions is private - client.getConnectionOptions = (...args) => { - const result = originalGetConnectionOptions.apply(client, args); - result.https = false; - result.port = 80; - return result; - }; - const clientConfig = client.getConfig(); sinon.stub(client, 'getConfig').returns(clientConfig); + // Our proxy mock is HTTP -> HTTPS, so the client speaks plain HTTP to it. const connection = await client.connect({ host: config.host, path: config.path, token: config.token, proxy: proxy.config, + https: false, }); await connection.openSession({ diff --git a/tests/unit/DBSQLClient.test.ts b/tests/unit/DBSQLClient.test.ts index 090d7ede..a313198c 100644 --- a/tests/unit/DBSQLClient.test.ts +++ b/tests/unit/DBSQLClient.test.ts @@ -21,6 +21,7 @@ import IThriftClient from '../../lib/contracts/IThriftClient'; import IAuthentication from '../../lib/connection/contracts/IAuthentication'; import AuthProviderStub from './.stubs/AuthProviderStub'; import ConnectionProviderStub from './.stubs/ConnectionProviderStub'; +import HiveDriverError from '../../lib/errors/HiveDriverError'; import { TProtocolVersion } from '../../thrift/TCLIService_types'; import TelemetryClientProvider from '../../lib/telemetry/TelemetryClientProvider'; import FeatureFlagCache from '../../lib/FeatureFlagCache'; @@ -199,6 +200,56 @@ describe('DBSQLClient.connect', () => { ); }); + it('should use HTTPS on port 443 by default', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions']({ host: 'h.databricks.com', path: '', token: 'x' }); + + expect(connectionOptions.https).to.be.true; + expect(connectionOptions.port).to.equal(443); + }); + + it('should use plain HTTP on port 80 when https is false', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions']({ + host: 'localhost', + path: '', + token: 'x', + https: false, + }); + + expect(connectionOptions.https).to.be.false; + expect(connectionOptions.port).to.equal(80); + }); + + it('should keep an explicit port when https is false', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions']({ ...connectOptions, port: 3015, https: false }); + + expect(connectionOptions.https).to.be.false; + expect(connectionOptions.port).to.equal(3015); + }); + + it('should only switch to plain HTTP for an explicit https: false', async () => { + const client = new DBSQLClient(); + + expect(client['getConnectionOptions']({ ...connectOptions, https: true }).https).to.be.true; + expect(client['getConnectionOptions']({ ...connectOptions, https: undefined }).https).to.be.true; + // Untyped JS callers: anything but the boolean `false` keeps HTTPS. + expect(client['getConnectionOptions']({ ...connectOptions, https: 'false' } as any).https).to.be.true; + }); + + it('should reject a mutual TLS client identity when https is false', async () => { + const client = new DBSQLClient(); + + // Rejected before PEM parsing, so placeholder values are enough. + expect(() => + client['getConnectionOptions']({ ...connectOptions, https: false, clientCert: 'cert', clientKey: 'key' }), + ).to.throw(HiveDriverError, /requires HTTPS/); + }); + it('should initialize connection state', async () => { const client = new DBSQLClient(); diff --git a/tests/unit/connection/connections/PlainHttpTransport.test.ts b/tests/unit/connection/connections/PlainHttpTransport.test.ts new file mode 100644 index 00000000..07d150bf --- /dev/null +++ b/tests/unit/connection/connections/PlainHttpTransport.test.ts @@ -0,0 +1,128 @@ +import { expect, AssertionError } from 'chai'; +import http, { IncomingHttpHeaders } from 'http'; +import { AddressInfo, Socket } from 'net'; +import thrift from 'thrift'; +import DBSQLClient from '../../../../lib/DBSQLClient'; +import TCLIService from '../../../../thrift/TCLIService'; +import { TStatusCode, TProtocolVersion } from '../../../../thrift/TCLIService_types'; + +const warehousePath = '/sql/1.0/warehouses/abc'; + +// Minimal plain-HTTP Thrift endpoint standing in for a trusted local reverse proxy. +class LocalThriftServer { + public readonly requests: Array<{ method?: string; url?: string; encrypted: boolean; headers: IncomingHttpHeaders }> = + []; + + private readonly server: http.Server; + + // The driver's keep-alive agent holds sockets open; `server.close()` waits for them. + private readonly sockets = new Set(); + + constructor() { + const handle = { guid: Buffer.from('6ba7b8109dad11d180b400c04fd430c8', 'hex'), secret: Buffer.alloc(16, 2) }; + this.server = thrift.createWebServer({ + services: { + [warehousePath]: { + processor: TCLIService, + // Single-argument handlers select the generated processor's promise style. + handler: { + OpenSession: async (_req: unknown) => ({ + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + serverProtocolVersion: TProtocolVersion.SPARK_CLI_SERVICE_PROTOCOL_V8, + sessionHandle: { sessionId: handle }, + }), + CloseSession: async (_req: unknown) => ({ status: { statusCode: TStatusCode.SUCCESS_STATUS } }), + }, + }, + }, + } as any) as unknown as http.Server; + this.server.prependListener('request', (req: http.IncomingMessage) => { + this.requests.push({ + method: req.method, + url: req.url, + encrypted: Boolean((req.socket as any).encrypted), + headers: req.headers, + }); + }); + this.server.on('connection', (socket: Socket) => { + this.sockets.add(socket); + socket.on('close', () => this.sockets.delete(socket)); + }); + } + + public async start(): Promise { + await new Promise((resolve) => { + this.server.listen(0, '127.0.0.1', resolve); + }); + return (this.server.address() as AddressInfo).port; + } + + public async stop(): Promise { + const closed = new Promise((resolve) => { + this.server.close(() => resolve()); + }); + this.sockets.forEach((socket) => socket.destroy()); + await closed; + } +} + +describe('DBSQLClient plain HTTP transport', () => { + let server: LocalThriftServer; + let port: number; + + beforeEach(async () => { + server = new LocalThriftServer(); + port = await server.start(); + }); + + afterEach(async () => { + await server.stop(); + }); + + const baseOptions = () => ({ + host: '127.0.0.1', + port, + path: warehousePath, + telemetryEnabled: false, + retryMaxAttempts: 1, + authType: 'custom' as const, + // The reverse proxy injects credentials, so the driver sends none. + provider: { authenticate: async () => ({}) }, + }); + + it('opens and closes a session over plain HTTP with https: false', async () => { + const client = new DBSQLClient(); + await client.connect({ ...baseOptions(), https: false }); + + const session = await client.openSession(); + await session.close(); + await client.close(); + + expect(server.requests).to.have.length(2); + for (const request of server.requests) { + expect(request.method).to.equal('POST'); + expect(request.url).to.equal(warehousePath); + expect(request.encrypted).to.be.false; + expect(request.headers.authorization).to.be.undefined; + } + }); + + it('keeps using HTTPS when https is omitted', async () => { + const client = new DBSQLClient(); + await client.connect(baseOptions()); + + try { + await client.openSession(); + expect.fail('a TLS handshake against a plain-HTTP server should fail'); + } catch (error) { + if (error instanceof AssertionError) { + throw error; + } + } finally { + await client.close(); + } + + // The TLS ClientHello never parses as an HTTP request. + expect(server.requests).to.have.length(0); + }); +}); diff --git a/tests/unit/kernel/connectionOptions.test.ts b/tests/unit/kernel/connectionOptions.test.ts index 954e0296..1aa032d6 100644 --- a/tests/unit/kernel/connectionOptions.test.ts +++ b/tests/unit/kernel/connectionOptions.test.ts @@ -63,6 +63,55 @@ describe('KernelAuth connection options — geospatial result representation', ( }); }); +describe('KernelAuth connection options — https transport', () => { + it('forwards host unchanged by default (kernel prepends https://)', () => { + expect(buildKernelConnectionOptions(opts({})).hostName).to.equal('h.databricks.com'); + expect(buildKernelConnectionOptions(opts({ https: true })).hostName).to.equal('h.databricks.com'); + }); + + it('selects plain HTTP via an explicit http:// host when https is false', () => { + expect(buildKernelConnectionOptions(opts({ https: false })).hostName).to.equal('http://h.databricks.com'); + }); + + it('carries the port into the http:// host (matching the Thrift URL)', () => { + expect(buildKernelConnectionOptions(opts({ host: 'localhost', port: 3015, https: false })).hostName).to.equal( + 'http://localhost:3015', + ); + // A port already embedded in `host` (the kernel convention) is preserved. + expect(buildKernelConnectionOptions(opts({ host: 'localhost:3015', https: false })).hostName).to.equal( + 'http://localhost:3015', + ); + }); + + it('does not double the port when it is set in both host and port', () => { + expect(buildKernelConnectionOptions(opts({ host: 'localhost:3015', port: 3015, https: false })).hostName).to.equal( + 'http://localhost:3015', + ); + expect(buildKernelConnectionOptions(opts({ host: '[::1]:3015', port: 3015, https: false })).hostName).to.equal( + 'http://[::1]:3015', + ); + }); + + it('strips a trailing slash from host before adding the port (matching Thrift)', () => { + expect(buildKernelConnectionOptions(opts({ host: 'localhost/', port: 3015, https: false })).hostName).to.equal( + 'http://localhost:3015', + ); + expect(buildKernelConnectionOptions(opts({ host: 'localhost:3015/', port: 3015, https: false })).hostName).to.equal( + 'http://localhost:3015', + ); + }); + + it('does not mistake an IPv6 literal for an embedded port', () => { + expect(buildKernelConnectionOptions(opts({ host: '[::1]', port: 3015, https: false })).hostName).to.equal( + 'http://[::1]:3015', + ); + // Same URL shape as the Thrift transport, which also requires brackets. + expect(buildKernelConnectionOptions(opts({ host: '::1', port: 3015, https: false })).hostName).to.equal( + 'http://::1:3015', + ); + }); +}); + describe('KernelAuth connection options — maxConnections', () => { it('forwards a valid positive integer', () => { const native = buildKernelConnectionOptions(opts({ maxConnections: 10 })) as { maxConnections?: number };