From b93c6eae7a962818c869f8c8e2de53a96f080866 Mon Sep 17 00:00:00 2001 From: George MacKerron Date: Wed, 12 Aug 2026 15:23:39 +0100 Subject: [PATCH 1/3] Add support for channel_binding and require_auth connection parameters, following libpq --- .github/workflows/ci.yml | 3 +- CHANGELOG.md | 8 + LOCAL_DEV.md | 17 + README.md | 4 +- docs/pages/features/ssl.mdx | 34 +- packages/pg-connection-string/README.md | 2 + packages/pg-connection-string/index.d.ts | 11 + packages/pg-connection-string/index.js | 10 +- packages/pg-connection-string/test/parse.ts | 67 ++ packages/pg/lib/channel-binding.js | 36 + packages/pg/lib/client.js | 180 ++++- packages/pg/lib/connection-parameters.js | 70 +- packages/pg/lib/crypto/sasl.js | 57 +- packages/pg/lib/defaults.js | 8 + packages/pg/lib/native/client.js | 2 +- packages/pg/lib/require-auth.js | 146 ++++ packages/pg/script/test-server.sh | 77 +++ .../integration/client/sasl-scram-tests.js | 132 +++- .../pg/test/unit/client/auth-flow-tests.js | 625 ++++++++++++++++++ .../pg/test/unit/client/require-auth-tests.js | 167 +++++ .../pg/test/unit/client/sasl-scram-tests.js | 103 ++- .../connection-parameters/creation-tests.js | 296 +++++++++ 22 files changed, 1996 insertions(+), 59 deletions(-) create mode 100644 packages/pg/lib/channel-binding.js create mode 100644 packages/pg/lib/require-auth.js create mode 100755 packages/pg/script/test-server.sh create mode 100644 packages/pg/test/unit/client/auth-flow-tests.js create mode 100644 packages/pg/test/unit/client/require-auth-tests.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f0751c56..731f38166 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,7 +64,8 @@ jobs: PGPASSWORD: postgres PGHOST: localhost PGDATABASE: ci_db_test - PGTESTNOSSL: 'true' + # PGTESTNOSSL is deliberately unset: the postgres-ssl service image above has SSL + # configured, so the SSL and SCRAM channel binding tests can run for real here. SCRAM_TEST_PGUSER: scram_test SCRAM_TEST_PGPASSWORD: test4scram SCRAM_TEST_PGUSER_UNICODE: scram_unicode_test diff --git a/CHANGELOG.md b/CHANGELOG.md index f8158747e..cf36532f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,14 @@ For richer information consult the commit log on github with referenced pull req We do not include break-fix version release in this file. +## pg@8.24.0 + +- Add support for the `channel_binding` connection parameter (in a connection string, the client config or `PGCHANNELBINDING`): `"disable"`, `"prefer"` or `"require"`, following libpq. Also change default from `disable` to `prefer`, so channel binding is used whenever the server offers it. The previous `enableChannelBinding` boolean option is retained but deprecated: `true` maps to `"prefer"`. +- Also add support for the `require_auth`/`PGREQUIREAUTH` connection parameter, which specifies which authentication method(s) the client will accept from the server. +- Both parameters retain libpq's spelling. A camelCased `channelBinding` or `requireAuth` throw an error rather than leaving a security setting silently ignored, and an unrecognized channel binding value is refused the same way. +- These requirements are enforced against every authentication request, not only SCRAM exchanges, so a server cannot evade `channel_binding=require` by requesting some other kind of authentication (a downgrade recorded against another driver as [CVE-2025-49146](https://www.cve.org/CVERecord?id=CVE-2025-49146)). +- The native (libpq) client validates `channel_binding` and `require_auth` natively, against libpq's wider range of supported auth types. + ## pg@8.23.0 - Add support for query [`pipelineing`](https://github.com/brianc/node-postgres/pull/3652). diff --git a/LOCAL_DEV.md b/LOCAL_DEV.md index 3bbd9b456..879adaa9a 100644 --- a/LOCAL_DEV.md +++ b/LOCAL_DEV.md @@ -1,5 +1,22 @@ # Local development +## In a container + +The quickest way to get a server the whole suite can run against, SSL included, is the +script that starts the same image CI uses. It works with either podman or docker, and +prints the environment variables to export: + +```sh +packages/pg/script/test-server.sh # start it +packages/pg/script/test-server.sh stop # remove it again +``` + +SSL is worth having even if you are not working on it, since the SCRAM channel binding +tests are skipped without it. Pass `POSTGRES_VERSION` to test against another release, +e.g. `POSTGRES_VERSION=13 packages/pg/script/test-server.sh`. + +## On the host + Steps to install and configure Postgres on Mac for developing against locally 1. Install homebrew diff --git a/README.md b/README.md index ecd94e792..5c02d19ad 100644 --- a/README.md +++ b/README.md @@ -90,8 +90,8 @@ If your change involves breaking backwards compatibility please please point tha 1. Clone the repo 2. Ensure you have installed libpq-dev in your system (the native bindings are built in the test process) 3. From your workspace root run `yarn` and then `yarn lerna bootstrap` -4. Ensure you have a PostgreSQL instance running with SSL enabled and an empty database for tests. _note: you can skip the tests requring SSL by setting the environment variable `PGTESTNOSSL=1` if you're not changing any SSL related code_. -5. Ensure you have the proper environment variables configured for connecting to your postgres instance. Using the standard `PG*` environment variables like `PGUSER` and `PGPASSWORD` etc... +4. Ensure you have a PostgreSQL instance running with SSL enabled and an empty database for tests. Running `packages/pg/script/test-server.sh` starts one in a container, or see [LOCAL_DEV.md](./LOCAL_DEV.md) to configure your own. _note: you can skip the tests requring SSL by setting the environment variable `PGTESTNOSSL=1` if you're not changing any SSL related code_. +5. Ensure you have the proper environment variables configured for connecting to your postgres instance. Using the standard `PG*` environment variables like `PGUSER` and `PGPASSWORD` etc... The script in step 4 prints the ones the SCRAM tests need. 6. Run `yarn test` to run all the tests. ## Troubleshooting and FAQ diff --git a/docs/pages/features/ssl.mdx b/docs/pages/features/ssl.mdx index 6a29ed739..9709bae6b 100644 --- a/docs/pages/features/ssl.mdx +++ b/docs/pages/features/ssl.mdx @@ -76,14 +76,40 @@ Direct negotiation requests the `postgresql` ALPN protocol during the TLS handsh ## Channel binding -If the PostgreSQL server offers SCRAM-SHA-256-PLUS (i.e. channel binding) for TLS/SSL connections, you can enable this as follows: +Channel binding ties SCRAM authentication to the TLS connection it takes place on. A server that does not hold both the private key for the certificate it presented _and_ the user's password hash cannot successfully authenticate with channel binding. Channel binding thus authenticates the server to the client even when the certificate itself is not verified. It requires a TLS connection and the SCRAM-SHA-256-PLUS authentication mechanism, which PostgreSQL 11 and newer offer over SSL for roles whose password is stored using `scram-sha-256`. + +The `channel_binding` option takes the same three values as [the corresponding libpq parameter](https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-CHANNEL-BINDING): + +- `'require'` refuses to authenticate at all unless the exchange is bound to the server's certificate +- `'prefer'` (now the default) uses channel binding whenever the server offers it, and authenticates without it otherwise +- `'disable'` never uses it ```js -const client = new Client({ ...config, enableChannelBinding: true}) +const client = new Client({ ...config, channel_binding: 'require' }) +``` + +It can also be supplied via a connection string, or the `PGCHANNELBINDING` environment variable. For example: + +```js +const config = { + connectionString: 'postgres://user-and-password@host:port/db?sslmode=require&channel_binding=require', +} ``` -or +The earlier, boolean `enableChannelBinding` option remains available (`true` maps to `'prefer'` and `false` to `'disable'`), but should be considered deprecated. + +## Requiring an authentication method + +A server chooses which authentication method to ask the client for, so a server subject to an MITM attack can ask for a weaker method than expected. For example, it might request the user's plaintext password instead of SCRAM, or treat the client as authenticated without asking for anything at all. The `require_auth` option pins down what authentication methods we will accept from the server, exactly like the [libpq parameter of the same name](https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-REQUIRE-AUTH): ```js -const pool = new Pool({ ...config, enableChannelBinding: true}) +const client = new Client({ ...config, require_auth: 'scram-sha-256' }) ``` + +The value is a comma-separated list of methods, any of which the server may use, or a list in which every entry is negated with `!`, naming methods it may not use. `none` stands for a connection where the server asks for nothing (so `require_auth: 'none'` only accepts access without authentication, and `require_auth: '!none'` insists simply that the server authenticates us somehow). + +The methods libpq recognizes are `password`, `md5`, `scram-sha-256`, `gss`, `sspi` and `oauth`. Of these, node-postgres implements the first three. Naming only methods it cannot perform is an error rather than a connection that could never succeed. The native client passes the setting to libpq, which performs the full set (according to its build settings). It can equally be set in a connection string or with the `PGREQUIREAUTH` environment variable. + +Setting `channel_binding: 'require'` implies `require_auth: 'scram-sha-256'`, since no other method can be bound. Combining it with a `require_auth` that rules SCRAM out is an error. + +Note that both `channel_binding` and `require_auth` keep libpq's snake_cased spelling. A camelCased `channelBinding` or `requireAuth` is not recognized, and throws an error rather than connecting without the protection requested. diff --git a/packages/pg-connection-string/README.md b/packages/pg-connection-string/README.md index 5475f63bf..5cb56b56b 100644 --- a/packages/pg-connection-string/README.md +++ b/packages/pg-connection-string/README.md @@ -98,6 +98,8 @@ Query parameters follow a `?` character, including the following special query p * `sslcert=` - reads data from the given file and includes the result as `ssl.cert` * `sslkey=` - reads data from the given file and includes the result as `ssl.key` * `sslrootcert=` - reads data from the given file and includes the result as `ssl.ca` + * `channel_binding=` - sets the `channel_binding` property, which the client acts on. As in libpq, these three values are the only ones accepted. + * `require_auth=` - sets the `require_auth` property, which the client acts on, naming the authentication method(s) the server is allowed to ask for. The value is passed through unchanged but validated by the client, which accepts libpq's methods (`password`, `md5`, `gss`, `sspi`, `scram-sha-256`, `oauth` and `none`). Optionally, all methods may be negated with `!`, which makes this a block-list instead of an allow-list. A bare relative URL, such as `salesdata`, will indicate a database name while leaving other properties empty. diff --git a/packages/pg-connection-string/index.d.ts b/packages/pg-connection-string/index.d.ts index 4b305299e..2bdda2030 100644 --- a/packages/pg-connection-string/index.d.ts +++ b/packages/pg-connection-string/index.d.ts @@ -2,9 +2,16 @@ import { ClientConfig } from 'pg' export function parse(connectionString: string, options?: Options): ConnectionOptions +// Use of SCRAM channel binding, as libpq's channel_binding parameter defines it +export type ChannelBinding = 'disable' | 'prefer' | 'require' + export interface Options { // Use libpq semantics when interpreting the connection string useLibpqCompat?: boolean + // The channel binding setting held by the caller, for cases where it was not + // given in the connection string. A value of 'require' suppresses the sslmode + // deprecation warning, since the server is then authenticated by the binding. + channelBinding?: ChannelBinding } interface SSLConfig { @@ -23,6 +30,10 @@ export interface ConnectionOptions { client_encoding?: string ssl?: boolean | string | SSLConfig sslnegotiation?: 'postgres' | 'direct' + channel_binding?: ChannelBinding + // The authentication method(s) the server may ask for, as libpq's require_auth + // parameter defines them: a comma-separated list, optionally negated with '!' + require_auth?: string application_name?: string fallback_application_name?: string diff --git a/packages/pg-connection-string/index.js b/packages/pg-connection-string/index.js index 7ee302976..c09f1acf1 100644 --- a/packages/pg-connection-string/index.js +++ b/packages/pg-connection-string/index.js @@ -136,6 +136,14 @@ function parse(str, options = {}) { } } } else { + // A required channel binding authenticates the server to the client, so none + // of the weaker libpq sslmode guarantees warned about below can be exploited. + // Only the connection string is visible here, so the caller passes on any + // setting it holds itself, and the connection string takes precedence. + // Note: options.channelBinding may be boolean rather than string, but testing + // against `"require"` remains correct (neither boolean value means the same). + const channelBinding = config.channel_binding || options.channelBinding + switch (config.sslmode) { case 'disable': { config.ssl = false @@ -145,7 +153,7 @@ function parse(str, options = {}) { case 'require': case 'verify-ca': case 'verify-full': { - if (config.sslmode !== 'verify-full') { + if (config.sslmode !== 'verify-full' && channelBinding !== 'require') { deprecatedSslModeWarning(config.sslmode) } break diff --git a/packages/pg-connection-string/test/parse.ts b/packages/pg-connection-string/test/parse.ts index c2a537581..96da9023e 100644 --- a/packages/pg-connection-string/test/parse.ts +++ b/packages/pg-connection-string/test/parse.ts @@ -3,6 +3,7 @@ const expect = chai.expect chai.should() import { parse } from '../' +import type { Options } from '../' describe('parse', function () { it('using connection string in client constructor', function () { @@ -454,6 +455,72 @@ describe('parse', function () { }).to.throw() }) + describe('channel binding', function () { + // The sslmode deprecation warning is emitted at most once per process, so + // each case asserts against a freshly loaded copy of the module. + function warningsFrom(connectionString: string, options?: Options): string[] { + const modulePath = require.resolve('../index.js') + delete require.cache[modulePath] + const freshParse = require(modulePath).parse as typeof parse + const warnings: string[] = [] + const emitWarning = process.emitWarning + process.emitWarning = ((warning: string | Error) => { + warnings.push(String(warning)) + }) as typeof process.emitWarning + try { + freshParse(connectionString, options) + } finally { + process.emitWarning = emitWarning + delete require.cache[modulePath] + } + return warnings + } + + it('configuration parameter channel_binding=require', function () { + const subject = parse('pg:///?channel_binding=require') + subject.channel_binding?.should.equal('require') + }) + + it('configuration parameter channel_binding=prefer', function () { + const subject = parse('pg:///?channel_binding=prefer') + subject.channel_binding?.should.equal('prefer') + }) + + it('configuration parameter channel_binding=disable', function () { + const subject = parse('pg:///?channel_binding=disable') + subject.channel_binding?.should.equal('disable') + }) + + it('channel_binding does not change the ssl configuration', function () { + const subject = parse('pg:///?sslmode=require&channel_binding=require') + subject.ssl?.should.eql({}) + }) + + it('channel_binding=require suppresses the sslmode deprecation warning', function () { + for (const sslmode of ['prefer', 'require', 'verify-ca']) { + warningsFrom(`pg:///?sslmode=${sslmode}&channel_binding=require`).should.eql([]) + } + }) + + it('other channel_binding values leave the sslmode deprecation warning in place', function () { + for (const channelBinding of ['', 'prefer', 'disable']) { + const warnings = warningsFrom(`pg:///?sslmode=require&channel_binding=${channelBinding}`) + warnings.should.have.length(1) + warnings[0].should.match(/SECURITY WARNING/) + } + }) + + it('channelBinding option suppresses the warning when the connection string omits it', function () { + warningsFrom('pg:///?sslmode=require', { channelBinding: 'require' }).should.eql([]) + }) + + it('a channel_binding connection string parameter takes precedence over the option', function () { + const warnings = warningsFrom('pg:///?sslmode=require&channel_binding=disable', { channelBinding: 'require' }) + warnings.should.have.length(1) + warnings[0].should.match(/SECURITY WARNING/) + }) + }) + it('allow other params like max, ...', function () { const subject = parse('pg://myhost/db?max=18&min=4') subject.max?.should.equal('18') diff --git a/packages/pg/lib/channel-binding.js b/packages/pg/lib/channel-binding.js new file mode 100644 index 000000000..77130a5e0 --- /dev/null +++ b/packages/pg/lib/channel-binding.js @@ -0,0 +1,36 @@ +'use strict' + +// Support for libpq's channel_binding parameter, which says whether SCRAM authentication +// has to be bound to the server's certificate: +// https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-CHANNEL-BINDING + +const defaults = require('./defaults') + +const channelBindingLevels = ['disable', 'prefer', 'require'] + +// Accepts the levels libpq's channel_binding parameter defines, plus the booleans that +// pg's original enableChannelBinding option took. Any other non-string keeps its +// historical truthiness, so previously working configs keep working. A string that is not +// a level is refused rather than read as the weakest one that resembles it. +const normalizeChannelBinding = function (value) { + if (typeof value !== 'string') { + return value ? 'prefer' : 'disable' + } + if (!channelBindingLevels.includes(value)) { + throw new Error( + `Invalid channel_binding value: "${value}". Valid values are "disable", "prefer" and "require" (or a boolean).` + ) + } + return value +} + +// channel_binding, being libpq's own spelling, wins over the older +// enableChannelBinding option, then the environment, then the default. +const resolveChannelBinding = function (channelBinding, enableChannelBinding) { + const value = [channelBinding, enableChannelBinding, process.env.PGCHANNELBINDING, defaults.channel_binding].find( + (candidate) => candidate !== undefined && candidate !== null + ) + return normalizeChannelBinding(value) +} + +module.exports = { channelBindingLevels, normalizeChannelBinding, resolveChannelBinding } diff --git a/packages/pg/lib/client.js b/packages/pg/lib/client.js index 7a2fc9a64..8e6415198 100644 --- a/packages/pg/lib/client.js +++ b/packages/pg/lib/client.js @@ -2,6 +2,8 @@ const EventEmitter = require('events').EventEmitter const utils = require('./utils') const nodeUtils = require('util') const sasl = require('./crypto/sasl') +const { checkAuthRequest, resolveAuthRequirement } = require('./require-auth') +const { normalizeChannelBinding } = require('./channel-binding') const TypeOverrides = require('./type-overrides') const ConnectionParameters = require('./connection-parameters') @@ -84,7 +86,23 @@ class Client extends EventEmitter { this._activeQuery = null this._txStatus = null - this.enableChannelBinding = Boolean(c.enableChannelBinding) // set true to use SCRAM-SHA-256-PLUS when offered + // Use of SCRAM-SHA-256-PLUS: 'require', 'prefer' (when the server offers it) or + // 'disable'. ConnectionParameters resolves this from the channel_binding and + // enableChannelBinding options and the PGCHANNELBINDING environment variable. + this._channelBinding = this.connectionParameters.channel_binding + // What the server has to do to authenticate itself, from require_auth and + // channel_binding, or null if any supported method will do + this._authRequirement = this.connectionParameters.authRequirement + // Whether the client has done all the authenticating it is going to do, and whether + // that included binding the exchange to the server's certificate + this._authFinished = false + this._channelBound = false + // Whether a requirement has been broken, which nothing later can put right. Anything + // computed for an authentication request has to consult this again before writing its + // answer: hashing a password and computing a SCRAM proof each take a turn of the event + // loop, and Connection#end() sends its Terminate before ending the stream, so a write + // that arrived in the meantime would still reach the server. + this._authAborted = false this.scramMaxIterations = coerceNumberOrDefault(c.scramMaxIterations, sasl.DEFAULT_MAX_SCRAM_ITERATIONS) this.connection = c.connection || @@ -116,6 +134,30 @@ class Client extends EventEmitter { this._connectionTimeoutMillis = c.connectionTimeoutMillis || 0 } + get channelBinding() { + return this._channelBinding + } + + // Changing the level after construction re-derives what the server has to do, so that + // the two cannot come to disagree over whether channel binding is mandatory. The value + // is checked as it would have been in the constructor, so that a level this client does + // not know cannot pass for the weakest one. + set channelBinding(value) { + this._channelBinding = normalizeChannelBinding(value) + this._authRequirement = resolveAuthRequirement(this.connectionParameters.require_auth, this._channelBinding) + } + + // Kept in step with channelBinding, since this was the option's original name and + // shape. Levels pass through, so assigning 'require' does not weaken to 'prefer', and + // booleans mean what they always did. + get enableChannelBinding() { + return this.channelBinding !== 'disable' + } + + set enableChannelBinding(value) { + this.channelBinding = value + } + get activeQuery() { activeQueryDeprecationNotice() return this._activeQuery @@ -255,6 +297,7 @@ class Client extends EventEmitter { con.on('authenticationSASL', this._handleAuthSASL.bind(this)) con.on('authenticationSASLContinue', this._handleAuthSASLContinue.bind(this)) con.on('authenticationSASLFinal', this._handleAuthSASLFinal.bind(this)) + con.on('authenticationOk', this._handleAuthenticationOk.bind(this)) con.on('backendKeyData', this._handleBackendKeyData.bind(this)) con.on('error', this._handleErrorEvent.bind(this)) con.on('errorMessage', this._handleErrorMessage.bind(this)) @@ -273,6 +316,16 @@ class Client extends EventEmitter { _getPassword(cb) { const con = this.connection + // Looking a password up can be asynchronous, and a requirement can be broken while it + // is in flight, so what was permitted when the lookup began is checked again before + // anything is answered with. + const answer = () => { + if (this._authAborted) { + return + } + cb() + } + if (typeof this.password === 'function') { this._Promise .resolve() @@ -287,13 +340,13 @@ class Client extends EventEmitter { } else { this.connectionParameters.password = this.password = null } - cb() + answer() }) .catch((err) => { con.emit('error', err) }) } else if (this.password !== null) { - cb() + answer() } else { try { const pgPass = require('pgpass') @@ -302,7 +355,7 @@ class Client extends EventEmitter { pgPassDeprecationNotice() this.connectionParameters.password = this.password = pass } - cb() + answer() }) } catch (e) { this.emit('error', e) @@ -310,17 +363,77 @@ class Client extends EventEmitter { } } + // Fails the connection: the caller hears of the error through connect(), and the + // connection is closed so that a server which carries on regardless cannot reach a + // usable session. The messages that carry on regardless may already be here, since a + // server can pipeline the whole of a successful login into one packet, so the failure + // is recorded rather than left to be inferred from the connection being closed. + _abortAuthentication(err) { + this._authAborted = true + this._queryable = false + this.connection.emit('error', err) + this.connection.end() + } + + // Mirrors libpq's check_expected_areq. Every authentication request is judged here + // before the client answers it, or even looks up a password, so that a server cannot + // escape a requirement by asking for a method whose handler forgot to check. That was + // CVE-2025-49146: pgjdbc honored channel_binding=require within a SCRAM exchange, but + // a server could ask for a plain password instead and face no such requirement. + _authRequestAllowed(method) { + // Nothing is answered once a requirement has been broken, not even a request that + // would have been permitted on its own: the connection is already on its way out. + if (this._authAborted) { + return false + } + + // Authentication happens once. The server asks for one method and then says whether + // it was enough, so a further request means a server after something it has not been + // given: the password itself, say, from a client that had proved knowing it through + // SCRAM. Postgres stores only a SCRAM verifier, and someone in the middle holding a + // stolen one can complete that exchange, so what is asked for here is worth refusing + // whatever require_auth says. + if (this._authFinished && method !== 'none') { + this._abortAuthentication( + new Error(`The server requested ${method} authentication after the client had already authenticated`) + ) + return false + } + + const reason = checkAuthRequest({ + requirement: this._authRequirement, + method, + authFinished: this._authFinished, + channelBound: this._channelBound, + }) + + if (reason === null) { + return true + } + + this._abortAuthentication(new Error(reason)) + return false + } + _handleAuthCleartextPassword(msg) { + if (!this._authRequestAllowed('password')) return + this._getPassword(() => { this.connection.password(this.password) + // as in libpq: having sent a password, we expect no further authentication request + this._authFinished = true }) } _handleAuthMD5Password(msg) { + if (!this._authRequestAllowed('md5')) return + this._getPassword(async () => { try { const hashedPassword = await crypto.postgresMd5PasswordHash(this.user, this.password, msg.salt) + if (this._authAborted) return this.connection.password(hashedPassword) + this._authFinished = true } catch (e) { this.emit('error', e) } @@ -328,43 +441,57 @@ class Client extends EventEmitter { } _handleAuthSASL(msg) { + if (!this._authRequestAllowed('scram-sha-256')) return + this._getPassword(() => { try { - this.saslSession = sasl.startSession( - msg.mechanisms, - this.enableChannelBinding && this.connection.stream, - this.scramMaxIterations - ) + this.saslSession = sasl.startSession(msg.mechanisms, { + channelBinding: this.channelBinding, + sslInUse: Boolean(this.ssl), + stream: this.connection.stream, + scramMaxIterations: this.scramMaxIterations, + }) this.connection.sendSASLInitialResponseMessage(this.saslSession.mechanism, this.saslSession.response) } catch (err) { - this.connection.emit('error', err) + this._abortAuthentication(err) } }) } async _handleAuthSASLContinue(msg) { + if (!this._authRequestAllowed('scram-sha-256')) return + try { - await sasl.continueSession( - this.saslSession, - this.password, - msg.data, - this.enableChannelBinding && this.connection.stream - ) + await sasl.continueSession(this.saslSession, this.password, msg.data, this.connection.stream) + if (this._authAborted) return this.connection.sendSCRAMClientFinalMessage(this.saslSession.response) } catch (err) { - this.connection.emit('error', err) + this._abortAuthentication(err) } } _handleAuthSASLFinal(msg) { + if (!this._authRequestAllowed('scram-sha-256')) return + try { + const { mechanism } = this.saslSession sasl.finalizeSession(this.saslSession, msg.data) this.saslSession = null + // The server has proved that it holds the verifier for our password, and with + // SCRAM-SHA-256-PLUS that proof is tied to the certificate of this TLS session. + this._authFinished = true + this._channelBound = mechanism === 'SCRAM-SHA-256-PLUS' } catch (err) { - this.connection.emit('error', err) + this._abortAuthentication(err) } } + _handleAuthenticationOk() { + // 'none' is require_auth's name for a connection involving no authentication + // request, which is what this message amounts to if nothing preceded it + this._authRequestAllowed('none') + } + _handleBackendKeyData(msg) { this.processID = msg.processID this.secretKey = msg.secretKey @@ -372,6 +499,23 @@ class Client extends EventEmitter { _handleReadyForQuery(msg) { if (this._connecting) { + // A server that declares the client logged in anyway does not get to make it so: + // this message may have arrived in the same packet as the request that was refused, + // and reporting a successful connection now would undo the refusal. + if (this._authAborted) { + return + } + + // The last word on any requirement, since this is where the connection becomes + // usable. An AuthenticationOk is judged as it arrives, but a server can reach this + // point without having sent one: unlike libpq, which accepts nothing but an + // authentication request at this stage of its handshake, this client is listening + // for every message from the start. 'none' is require_auth's name for a connection + // that involved no authentication request at all. + if (!this._authRequestAllowed('none')) { + return + } + this._connecting = false this._connected = true clearTimeout(this.connectionTimeoutHandle) diff --git a/packages/pg/lib/connection-parameters.js b/packages/pg/lib/connection-parameters.js index 37987fd68..7936fa378 100644 --- a/packages/pg/lib/connection-parameters.js +++ b/packages/pg/lib/connection-parameters.js @@ -6,6 +6,10 @@ const defaults = require('./defaults') const parse = require('pg-connection-string').parse // parses a connection string +const { resolveAuthRequirement } = require('./require-auth') + +const { resolveChannelBinding } = require('./channel-binding') + const val = function (key, config, envVar) { if (config[key]) { return config[key] @@ -22,6 +26,20 @@ const val = function (key, config, envVar) { return envVar || defaults[key] } +// These two are spelled as libpq spells them, as client_encoding and application_name +// are, so a camelCased attempt at one is not recognized. Both exist to refuse weak +// authentication, so proceeding without them would make exactly the connection they were +// set to prevent: an error, and not a warning, which the connection would outlive. +const libpqSpellings = { channelBinding: 'channel_binding', requireAuth: 'require_auth' } + +const rejectCamelCasedOptions = function (config) { + for (const [camelCased, libpqSpelling] of Object.entries(libpqSpellings)) { + if (config[camelCased] !== undefined) { + throw new Error(`The ${camelCased} option is not recognized: spell it ${libpqSpelling}, as libpq does.`) + } + } +} + const readSSLConfigFromEnvironment = function () { switch (process.env.PGSSLMODE) { case 'disable': @@ -50,16 +68,31 @@ const add = function (params, config, paramName) { } class ConnectionParameters { - constructor(config) { + // `native` says these parameters are for libpq rather than this library's own protocol + // implementation, which decides who checks what: libpq negotiates SSL of its own accord + // and performs authentication methods this library does not, so a configuration it can + // honor must not be rejected here. + constructor(config, { native = false } = {}) { + // The boolean enableChannelBinding option is only honored in the client config: + // a connection string carries libpq's channel_binding parameter instead. + const enableChannelBinding = typeof config === 'string' ? undefined : config && config.enableChannelBinding + // if a string is passed, it is a raw connection string so we parse it into a config config = typeof config === 'string' ? parse(config) : config || {} // if the config has a connectionString defined, parse IT into the config we use // this will override other default values with what is stored in connectionString if (config.connectionString) { - config = Object.assign({}, config, parse(config.connectionString)) + // The parser suppresses its sslmode deprecation warning when channel binding + // is required, so it needs any setting that is not in the connection string. + const channelBinding = resolveChannelBinding(config.channel_binding, enableChannelBinding) + config = Object.assign({}, config, parse(config.connectionString, { channelBinding })) } + // After the merge, so that a camelCased query parameter is caught as well: the parser + // passes through anything it does not recognize. + rejectCamelCasedOptions(config) + this.user = val('user', config) this.database = val('database', config) @@ -111,6 +144,30 @@ class ConnectionParameters { throw new Error('sslnegotiation=direct requires SSL to be enabled') } + // Use of SCRAM channel binding: 'require', 'prefer' (the default, using it when + // the server offers it) or 'disable'. + this.channel_binding = resolveChannelBinding(config.channel_binding, enableChannelBinding) + // This client only encrypts a connection when asked to, so requiring a binding to the + // server's certificate without SSL could never be satisfied. libpq, on the other hand, + // negotiates SSL by default, and reports the shortfall itself if it ends up without. + if (!native && this.channel_binding === 'require' && !this.ssl) { + throw new Error('channel_binding=require requires SSL to be enabled') + } + + // The authentication method(s) the server may ask for. The requirement derived from + // it, which the client enforces, also carries any channel binding requirement. + // An explicitly empty value requires nothing and is honored over the environment, as + // it is in libpq's own conninfo; val() would fall through to the environment instead, + // since it tests truthiness. It is kept as an empty string rather than dropped, so + // that it reaches libpq: absence there is not the same thing, because libpq reads + // PGREQUIREAUTH itself for a parameter the conninfo does not mention, and an empty one + // it takes to require nothing. The cost is that a libpq older than PostgreSQL 16 will + // reject the parameter, but only for someone who asked for it by name. + const requireAuth = + config.require_auth !== undefined ? config.require_auth : val('require_auth', config, 'PGREQUIREAUTH') + this.require_auth = requireAuth ?? undefined + this.authRequirement = resolveAuthRequirement(this.require_auth, this.channel_binding, { native }) + this.client_encoding = val('client_encoding', config) this.replication = val('replication', config) // a domain socket begins with '/' @@ -157,6 +214,15 @@ class ConnectionParameters { add(params, ssl, 'sslcert') add(params, ssl, 'sslrootcert') add(params, this, 'sslnegotiation') + // Only when it differs from libpq's own default, so that we neither say anything + // redundant nor pass an unknown parameter to a libpq older than PostgreSQL 11. + if (this.channel_binding !== 'prefer') { + add(params, this, 'channel_binding') + } + // Needs no such guard: this is undefined unless it was asked for, and add() skips + // undefined values, so a libpq older than PostgreSQL 16 is never passed a parameter + // it would reject. + add(params, this, 'require_auth') if (this.database) { params.push('dbname=' + quoteParamValue(this.database)) diff --git a/packages/pg/lib/crypto/sasl.js b/packages/pg/lib/crypto/sasl.js index ea63b2413..280bce6d4 100644 --- a/packages/pg/lib/crypto/sasl.js +++ b/packages/pg/lib/crypto/sasl.js @@ -32,9 +32,38 @@ function saslprep(password) { const DEFAULT_MAX_SCRAM_ITERATIONS = 100000 -function startSession(mechanisms, stream, scramMaxIterations = DEFAULT_MAX_SCRAM_ITERATIONS) { +function startSession(mechanisms, options = {}) { + const { + channelBinding = 'prefer', + sslInUse = false, + stream, + scramMaxIterations = DEFAULT_MAX_SCRAM_ITERATIONS, + } = options + + // Binding to 'tls-server-end-point' means hashing the server's certificate, so it + // takes a TLS connection that can produce one. Some streams cannot — a socket in a + // Cloudflare Worker, for instance. + const canBindChannel = sslInUse && Boolean(stream) && typeof stream.getPeerCertificate === 'function' + const useChannelBinding = canBindChannel && channelBinding !== 'disable' + + if (channelBinding === 'require') { + if (!sslInUse) { + throw new Error('SASL: Channel binding is required, but SSL is not in use') + } + if (!canBindChannel) { + throw new Error('SASL: Channel binding is required, but this connection cannot provide the server certificate') + } + } + + // A server offering channel binding over an unencrypted connection suggests SSL was + // stripped in transit. A real server should abort such an exchange itself to defeat + // downgrade attacks; to defend against MITM attacks, we do so too. + if (!sslInUse && mechanisms.includes('SCRAM-SHA-256-PLUS')) { + throw new Error('SASL: Server offered SCRAM-SHA-256-PLUS authentication over a non-SSL connection') + } + const candidates = ['SCRAM-SHA-256'] - if (stream) candidates.unshift('SCRAM-SHA-256-PLUS') // higher-priority, so placed first + if (useChannelBinding) candidates.unshift('SCRAM-SHA-256-PLUS') // higher-priority, so placed first const mechanism = candidates.find((candidate) => mechanisms.includes(candidate)) @@ -42,17 +71,24 @@ function startSession(mechanisms, stream, scramMaxIterations = DEFAULT_MAX_SCRAM throw new Error('SASL: Only mechanism(s) ' + candidates.join(' and ') + ' are supported') } - if (mechanism === 'SCRAM-SHA-256-PLUS' && typeof stream.getPeerCertificate !== 'function') { - // this should never happen if we are really talking to a Postgres server - throw new Error('SASL: Mechanism SCRAM-SHA-256-PLUS requires a certificate') + if (channelBinding === 'require' && mechanism !== 'SCRAM-SHA-256-PLUS') { + throw new Error( + 'SASL: Channel binding is required, but the server did not offer an authentication method that supports it' + ) } const clientNonce = crypto.randomBytes(18).toString('base64') - const gs2Header = mechanism === 'SCRAM-SHA-256-PLUS' ? 'p=tls-server-end-point' : stream ? 'y' : 'n' + + // 'y' tells the server we could have bound the channel but were not offered the + // chance, which is how it detects a stripped SCRAM-SHA-256-PLUS mechanism; 'n' says + // we cannot bind at all. The server checks that the client-final message repeats + // this flag, so the session carries it rather than deriving it a second time. + const gs2Header = mechanism === 'SCRAM-SHA-256-PLUS' ? 'p=tls-server-end-point' : useChannelBinding ? 'y' : 'n' return { mechanism, clientNonce, + gs2Header, response: gs2Header + ',,n=*,r=' + clientNonce, message: 'SASLInitialResponse', scramMaxIterations, @@ -96,19 +132,16 @@ async function continueSession(session, password, serverData, stream) { const clientFirstMessageBare = 'n=*,r=' + session.clientNonce const serverFirstMessage = 'r=' + sv.nonce + ',s=' + sv.salt + ',i=' + sv.iteration - // without channel binding: - let channelBinding = stream ? 'eSws' : 'biws' // 'y,,' or 'n,,', base64-encoded - - // override if channel binding is in use: + let bindingData = Buffer.from(session.gs2Header + ',,') if (session.mechanism === 'SCRAM-SHA-256-PLUS') { const peerCert = stream.getPeerCertificate().raw let hashName = signatureAlgorithmHashFromCertificate(peerCert) if (hashName === 'MD5' || hashName === 'SHA-1') hashName = 'SHA-256' const certHash = await crypto.hashByName(hashName, peerCert) - const bindingData = Buffer.concat([Buffer.from('p=tls-server-end-point,,'), Buffer.from(certHash)]) - channelBinding = bindingData.toString('base64') + bindingData = Buffer.concat([bindingData, Buffer.from(certHash)]) } + const channelBinding = bindingData.toString('base64') const clientFinalMessageWithoutProof = 'c=' + channelBinding + ',r=' + sv.nonce const authMessage = clientFirstMessageBare + ',' + serverFirstMessage + ',' + clientFinalMessageWithoutProof diff --git a/packages/pg/lib/defaults.js b/packages/pg/lib/defaults.js index 427243f50..351ae9f6c 100644 --- a/packages/pg/lib/defaults.js +++ b/packages/pg/lib/defaults.js @@ -52,6 +52,14 @@ module.exports = { // SSL negotiation style: 'postgres' (traditional SSLRequest) or 'direct' sslnegotiation: undefined, + // use of SCRAM channel binding: 'require', 'prefer' or 'disable' + channel_binding: 'prefer', + + // authentication method(s) the server may ask for, as a comma-separated list of + // 'password', 'md5', 'scram-sha-256' and 'none', each optionally negated with '!'. + // Undefined accepts any method this client supports. + require_auth: undefined, + application_name: undefined, fallback_application_name: undefined, diff --git a/packages/pg/lib/native/client.js b/packages/pg/lib/native/client.js index d305713d6..ad2c5caca 100644 --- a/packages/pg/lib/native/client.js +++ b/packages/pg/lib/native/client.js @@ -41,7 +41,7 @@ const Client = (module.exports = function (config) { // keep these on the object for legacy reasons // for the time being. TODO: deprecate all this jazz - const cp = (this.connectionParameters = new ConnectionParameters(config)) + const cp = (this.connectionParameters = new ConnectionParameters(config, { native: true })) if (config.nativeConnectionString) cp.nativeConnectionString = config.nativeConnectionString this.user = cp.user diff --git a/packages/pg/lib/require-auth.js b/packages/pg/lib/require-auth.js new file mode 100644 index 000000000..4c0f6e88c --- /dev/null +++ b/packages/pg/lib/require-auth.js @@ -0,0 +1,146 @@ +'use strict' + +// Support for libpq's require_auth parameter, which pins down the authentication +// method(s) the server is allowed to ask for: +// https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNECT-REQUIRE-AUTH + +// Every method libpq recognizes. A connection string written for libpq should not be +// rejected here merely for naming a method this client does not implement, so gss, sspi +// and oauth are accepted: they simply never match a request. +const authMethods = ['password', 'md5', 'gss', 'sspi', 'scram-sha-256', 'oauth'] + +// What this client can actually perform. GSS, SSPI and OAuth are not implemented. +const supportedAuthMethods = ['password', 'md5', 'scram-sha-256'] + +const quotedList = function (values) { + return values.map((value) => `"${value}"`).join(', ') +} + +// Follows libpq: elements are comma-separated, and either all of them are negated with +// a leading '!', in which case the list starts from everything permitted and subtracts, +// or none are, in which case it starts from nothing permitted and adds. Whitespace is +// not trimmed and repetition is rejected. +const parseRequireAuth = function (requireAuth) { + const elements = requireAuth.split(',') + const negated = elements[0].startsWith('!') + + // 'none' is not a method but a statement about whether the server must ask at all, + // so it is tracked separately from the set of methods. + const allowedMethods = new Set(negated ? authMethods : []) + let authRequired = !negated + const seen = new Set() + + for (const element of elements) { + if (element.startsWith('!') !== negated) { + throw new Error(`Invalid require_auth value: "${requireAuth}". Negated methods cannot be mixed with plain ones.`) + } + + const method = negated ? element.slice(1) : element + + if (method !== 'none' && !authMethods.includes(method)) { + throw new Error( + `Invalid require_auth value: "${requireAuth}". Valid methods are ${quotedList(authMethods)} and "none", ` + + `each optionally negated with "!".` + ) + } + + if (seen.has(method)) { + throw new Error(`Invalid require_auth value: "${requireAuth}". Method "${method}" is specified more than once.`) + } + seen.add(method) + + if (method === 'none') { + // 'none' permits a connection the server never challenges, such as trust or + // certificate authentication; '!none' insists that it challenges. + authRequired = negated + } else if (negated) { + allowedMethods.delete(method) + } else { + allowedMethods.add(method) + } + } + + return { authRequired, allowedMethods } +} + +// Resolves what the server must do to authenticate itself to us, from the require_auth +// and channel_binding settings, or null if any method this client supports will do. +// Throws when no method this client can perform would satisfy the settings, rather than +// leaving a connection that could only ever fail. `native` says the connection is libpq's +// to make, which widens the methods that count as performable. +const resolveAuthRequirement = function (requireAuth, channelBinding, { native = false } = {}) { + let requirement = null + + if (requireAuth) { + const { authRequired, allowedMethods } = parseRequireAuth(requireAuth) + requirement = { + authRequired, + allowedMethods, + channelBindingRequired: false, + description: `require_auth="${requireAuth}"`, + } + } + + if (channelBinding === 'require') { + // Only a channel-bound SCRAM exchange will do. libpq keeps this check separate from + // require_auth; treating it as a requirement in its own right means every + // authentication request is judged in one place. + if (requirement && !requirement.allowedMethods.has('scram-sha-256')) { + throw new Error( + `channel_binding=require cannot be satisfied by ${requirement.description}, ` + + `which does not permit scram-sha-256 authentication` + ) + } + requirement = { + authRequired: true, + allowedMethods: new Set(['scram-sha-256']), + channelBindingRequired: true, + description: 'channel_binding=require', + } + } + + // libpq performs GSS, SSPI and OAuth authentication, which this library does not, and + // reports for itself when the build in use lacks support for one of them. + const performableMethods = native ? authMethods : supportedAuthMethods + + if (requirement && requirement.authRequired && !performableMethods.some((m) => requirement.allowedMethods.has(m))) { + throw new Error( + `${requirement.description} cannot be satisfied: this client can only perform ` + + `${quotedList(performableMethods)} authentication` + ) + } + + return requirement +} + +// Mirrors libpq's check_expected_areq: a single check every authentication request +// passes through before the client answers it. `method` is the method the server asked +// for, or 'none' for an AuthenticationOk message, which is require_auth's own name for +// a connection involving no authentication request. Returns null if the request is +// permitted, or the reason it is not. +const checkAuthRequest = function ({ requirement, method, authFinished, channelBound }) { + if (!requirement) { + return null + } + + if (method === 'none') { + // The server says the client is in, so any requirement has to be satisfied by now. + // Otherwise a server evades it by never asking at all, as trust authentication + // does, or by abandoning a SCRAM exchange before it proves who it is. + if (requirement.channelBindingRequired && !channelBound) { + return `The server authenticated the client without channel binding, but ${requirement.description} was set` + } + if (requirement.authRequired && !authFinished) { + return `The server did not complete authentication, but ${requirement.description} was set` + } + return null + } + + if (!requirement.allowedMethods.has(method)) { + return `The server requested ${method} authentication, but ${requirement.description} was set` + } + + return null +} + +module.exports = { resolveAuthRequirement, checkAuthRequest, authMethods, supportedAuthMethods } diff --git a/packages/pg/script/test-server.sh b/packages/pg/script/test-server.sh new file mode 100755 index 000000000..62f0984c6 --- /dev/null +++ b/packages/pg/script/test-server.sh @@ -0,0 +1,77 @@ +#!/bin/bash +# Runs a PostgreSQL server for the integration tests in a container, using the same image +# as .github/workflows/ci.yml. That image has SSL configured, which the official +# PostgreSQL images do not, and without it the SCRAM channel binding tests cannot run. +# Works with either podman or docker. +# +# packages/pg/script/test-server.sh # start it, and print the environment to export +# packages/pg/script/test-server.sh stop # remove it again +# +# POSTGRES_VERSION, PORT, NAME, IMAGE and CONTAINER_ENGINE can each be overridden. +set -euo pipefail + +ENGINE=${CONTAINER_ENGINE:-} +if [ -z "$ENGINE" ]; then + for candidate in podman docker; do + if command -v "$candidate" >/dev/null; then + ENGINE=$candidate + break + fi + done +fi +if [ -z "$ENGINE" ]; then + echo "Neither podman nor docker was found. Set CONTAINER_ENGINE to the one to use." >&2 + exit 1 +fi + +NAME=${NAME:-node-postgres-test} +PORT=${PORT:-5432} +POSTGRES_VERSION=${POSTGRES_VERSION:-18} +IMAGE=${IMAGE:-ghcr.io/railwayapp-templates/postgres-ssl:$POSTGRES_VERSION} + +if [ "${1:-start}" = stop ]; then + exec "$ENGINE" rm -f "$NAME" +fi + +"$ENGINE" rm -f "$NAME" >/dev/null 2>&1 || true + +# PGDATA is pinned because the PostgreSQL 18 image defaults it to a versioned +# subdirectory, which this image's entrypoint rejects. See the same note in ci.yml. +"$ENGINE" run -d --name "$NAME" -p "$PORT:5432" \ + -e POSTGRES_USER=postgres \ + -e POSTGRES_PASSWORD=postgres \ + -e POSTGRES_HOST_AUTH_METHOD=md5 \ + -e POSTGRES_DB=ci_db_test \ + -e PGDATA=/var/lib/postgresql/data \ + "$IMAGE" >/dev/null + +printf 'waiting for %s' "$NAME" +for _ in $(seq 1 60); do + if "$ENGINE" exec "$NAME" pg_isready -q 2>/dev/null; then break; fi + printf . + sleep 1 +done +echo + +# The roles the SCRAM tests look for, as ci.yml creates them. Their passwords are stored +# using scram-sha-256 whatever the server's own default is, so that the server offers +# SCRAM-SHA-256-PLUS once SSL is in use. +"$ENGINE" exec -i "$NAME" psql -U postgres -d ci_db_test -v ON_ERROR_STOP=1 -q <<'SQL' +SET password_encryption = 'scram-sha-256'; +CREATE ROLE scram_test LOGIN PASSWORD 'test4scram'; +CREATE ROLE scram_unicode_test LOGIN PASSWORD U&'IX-\2168'; +SQL + +"$ENGINE" exec "$NAME" psql -U postgres -d ci_db_test -tAc \ + "select version() || ', ssl=' || current_setting('ssl')" + +cat < { - const client = new pg.Client({ ...config, enableChannelBinding: true }) - let usingChannelBinding = false - let hasPeerCert = false +// Whether SSL is in use decides which mechanisms the server offers, so these tests say +// so themselves rather than inheriting whatever PGSSLMODE happens to hold. +const sslConfig = { ...config, ssl: { rejectUnauthorized: false } } +const noSslConfig = { ...config, ssl: false } + +// The SASL session is discarded as soon as authentication finishes, so the mechanism has +// to be noted while the exchange is still in flight. Returns it alongside whether the +// connection was encrypted, since a test asserting on one wants to be sure of the other. +async function authenticate(clientConfig) { + const client = new pg.Client(clientConfig) + let mechanism = null client.connection.once('authenticationSASLContinue', () => { - hasPeerCert = client.connection.stream.getPeerCertificate === 'function' - usingChannelBinding = client.saslSession.mechanism === 'SCRAM-SHA-256-PLUS' + mechanism = client.saslSession.mechanism }) await client.connect() - assert.ok(usingChannelBinding || !hasPeerCert, 'Should be using SCRAM-SHA-256-PLUS for authentication if using SSL') + const encrypted = Boolean(client.connection.stream.encrypted) + const { rows } = await client.query('SELECT 1 AS one') + assert.strictEqual(rows[0].one, 1, 'the connection should be usable once authenticated') await client.end() + return { mechanism, encrypted } +} + +suite.test('sasl/scram authenticates without channel binding when SSL is not in use', async () => { + // channel_binding defaults to 'prefer', and a server only offers SCRAM-SHA-256-PLUS + // over SSL, so an unbound exchange is the negotiated outcome here. + const { mechanism, encrypted } = await authenticate(noSslConfig) + assert.strictEqual(encrypted, false, 'this test is meant to run over an unencrypted connection') + assert.strictEqual(mechanism, 'SCRAM-SHA-256') }) suite.test('can connect using sasl/scram with channel binding disabled', async () => { @@ -69,6 +91,61 @@ suite.test('can connect using sasl/scram with channel binding disabled', async ( await client.end() }) +if (process.env.PGTESTNOSSL) { + suite.test('skipping SCRAM channel binding tests (PGTESTNOSSL)', () => {}) +} else { + // A bound exchange only completes if the server agrees with the certificate hash the + // client computed, so these tests check the binding itself and not merely which + // mechanism was chosen. + suite.test('sasl/scram binds the channel when channel_binding=require', async () => { + const { mechanism, encrypted } = await authenticate({ ...sslConfig, channel_binding: 'require' }) + assert.ok(encrypted, 'expected the connection to be upgraded to a TLS socket') + assert.strictEqual(mechanism, 'SCRAM-SHA-256-PLUS') + }) + + suite.test('sasl/scram binds the channel by default when SSL is in use', async () => { + // channel_binding defaults to 'prefer', which takes the server up on its offer + const { mechanism } = await authenticate(sslConfig) + assert.strictEqual(mechanism, 'SCRAM-SHA-256-PLUS') + }) + + suite.test('sasl/scram leaves the channel unbound when channel_binding=disable', async () => { + const { mechanism } = await authenticate({ ...sslConfig, channel_binding: 'disable' }) + assert.strictEqual(mechanism, 'SCRAM-SHA-256') + }) + + suite.test('channel_binding in a connection string binds the channel', async () => { + const user = encodeURIComponent(config.user) + const password = encodeURIComponent(config.password) + const host = config.host || helper.config.host + const port = config.port || helper.config.port + const database = config.database || helper.config.database + const params = 'sslmode=no-verify&channel_binding=require' + const connectionString = `postgres://${user}:${password}@${host}:${port}/${database}?${params}` + + const { mechanism, encrypted } = await authenticate({ connectionString }) + assert.ok(encrypted, 'expected the connection to be upgraded to a TLS socket') + assert.strictEqual(mechanism, 'SCRAM-SHA-256-PLUS') + }) + + suite.test('the deprecated enableChannelBinding option still governs channel binding', async () => { + const enabled = await authenticate({ ...sslConfig, enableChannelBinding: true }) + assert.strictEqual(enabled.mechanism, 'SCRAM-SHA-256-PLUS') + + const disabled = await authenticate({ ...sslConfig, enableChannelBinding: false }) + assert.strictEqual(disabled.mechanism, 'SCRAM-SHA-256') + }) + + suite.test('channel_binding=require is satisfied alongside require_auth=scram-sha-256', async () => { + const { mechanism } = await authenticate({ + ...sslConfig, + channel_binding: 'require', + require_auth: 'scram-sha-256', + }) + assert.strictEqual(mechanism, 'SCRAM-SHA-256-PLUS') + }) +} + suite.test('sasl/scram fails when password is wrong', async () => { const client = new pg.Client({ ...config, @@ -109,6 +186,47 @@ suite.test('sasl/scram fails when password is empty', async () => { assert.ok(usingSasl, 'Should be using SASL for authentication') }) +suite.test('require_auth permits the method the server asks for', async () => { + const { mechanism } = await authenticate({ ...config, require_auth: 'scram-sha-256' }) + assert.ok(mechanism, 'expected a SCRAM exchange to have taken place') +}) + +suite.test('require_auth permits the method the server asks for when named by exclusion', async () => { + const { mechanism } = await authenticate({ ...config, require_auth: '!password,!md5' }) + assert.ok(mechanism, 'expected a SCRAM exchange to have taken place') +}) + +suite.test('require_auth refuses a server asking for a method it does not name', async () => { + const client = new pg.Client({ ...config, require_auth: 'md5' }) + await assert.rejects(() => client.connect(), { + message: 'The server requested scram-sha-256 authentication, but require_auth="md5" was set', + }) +}) + +suite.test('require_auth=none refuses a server that demands a password', async () => { + const client = new pg.Client({ ...config, require_auth: 'none' }) + await assert.rejects(() => client.connect(), { + message: 'The server requested scram-sha-256 authentication, but require_auth="none" was set', + }) +}) + +suite.test('PGREQUIREAUTH is honored', async () => { + const pgRequireAuth = process.env.PGREQUIREAUTH + process.env.PGREQUIREAUTH = 'md5' + try { + const client = new pg.Client(config) + await assert.rejects(() => client.connect(), { + message: 'The server requested scram-sha-256 authentication, but require_auth="md5" was set', + }) + } finally { + if (pgRequireAuth === undefined) { + delete process.env.PGREQUIREAUTH + } else { + process.env.PGREQUIREAUTH = pgRequireAuth + } + } +}) + /** * SASLprep regression coverage. RFC 5802 / RFC 4013 require the SCRAM client * to normalize the password (B.1 mapping → NFKC → prohibition + bidi check) diff --git a/packages/pg/test/unit/client/auth-flow-tests.js b/packages/pg/test/unit/client/auth-flow-tests.js new file mode 100644 index 000000000..d2008a4bb --- /dev/null +++ b/packages/pg/test/unit/client/auth-flow-tests.js @@ -0,0 +1,625 @@ +'use strict' + +// Drives a Client through complete authentication exchanges against a scripted server, +// which is where requirements like channel_binding=require and require_auth have to +// hold: CVE-2025-49146 was a client that enforced channel binding within its SCRAM +// exchange, but answered a plain password request without a murmur. + +const assert = require('assert') +const fs = require('fs') +const path = require('path') +const helper = require('./test-helper') +const Connection = require('../../../lib/connection') +const crypto = require('../../../lib/crypto/utils') +const { Client, MemoryStream } = helper + +const suite = new helper.Suite() + +const password = 'sekret' + +// A real certificate, so the channel binding path hashes something a server could +// actually have presented +const serverCertificate = Buffer.from( + fs + .readFileSync(path.join(__dirname, '..', '..', 'tls', 'test-server.crt'), 'utf8') + .replace(/-----[^-]+-----|\s/g, ''), + 'base64' +) + +// Starts a client on a stream that records what it writes, so that authentication +// messages can be fed to it and its answers inspected. With tls, the stream can produce +// a peer certificate, as a TLS socket would. +const startClient = function (config = {}, { tls = false } = {}) { + const stream = new MemoryStream() + stream.end = function () { + this.ended = true + } + if (tls) { + stream.getPeerCertificate = () => ({ raw: serverCertificate }) + } + + const client = new Client({ connection: new Connection({ stream }), password, ...config }) + const errors = [] + // Successful outcomes are recorded as well as failures, so that a test can tell a + // refused connection from one that was refused and then reported as connected anyway + const callbacks = [] + const connects = [] + client.on('connect', () => connects.push(true)) + client.connect((err) => { + callbacks.push(err) + if (err) errors.push(err) + }) + stream.packets.length = 0 + + return { client, stream, errors, callbacks, connects } +} + +// Sends a message from the server to the client +const send = function ({ client }, name, msg = {}) { + client.connection.emit(name, msg) +} + +const until = async function (predicate, description) { + const deadline = Date.now() + 2000 + while (!predicate()) { + if (Date.now() > deadline) throw new Error(`timed out waiting for ${description}`) + await new Promise((resolve) => setImmediate(resolve)) + } +} + +// Authentication handlers do real crypto, so their answers have to be waited for rather +// than assumed to have arrived by the next tick +const awaitPackets = function ({ stream }, count) { + return until(() => stream.packets.length >= count, `the client to send ${count} packet(s)`) +} + +const awaitError = async function ({ errors }) { + await until(() => errors.length > 0, 'the client to report an error') + return errors[0] +} + +// Long enough that a handler which was going to answer would have done so, so that +// 'nothing was sent' means it +const awaitQuiet = function () { + return new Promise((resolve) => setTimeout(resolve, 25)) +} + +// Message types the client has written, e.g. ['p'] for a password message or ['X'] for a +// Terminate: enough to tell whether it answered a request or hung up on it +const sentTypes = function ({ stream }) { + return stream.packets.map((packet) => String.fromCharCode(packet[0])) +} + +// A scripted SCRAM-SHA-256 server. Its replies are computed from the messages the client +// actually sent, so the client's own view of the exchange, and of the channel binding in +// particular, is never taken on trust. +const scramServer = { + salt: Buffer.from('0123456789abcdef'), + iterations: 4096, + + firstMessage(clientNonce) { + return `r=${clientNonce}serverpart,s=${this.salt.toString('base64')},i=${this.iterations}` + }, + + async signature(clientNonce, clientFinalMessageWithoutProof) { + const saltedPassword = await crypto.deriveKey(password, this.salt, this.iterations) + const serverKey = await crypto.hmacSha256(saltedPassword, 'Server Key') + const authMessage = [`n=*,r=${clientNonce}`, this.firstMessage(clientNonce), clientFinalMessageWithoutProof].join( + ',' + ) + + return Buffer.from(await crypto.hmacSha256(serverKey, authMessage)).toString('base64') + }, +} + +const parseSASLInitialResponse = function (packet) { + const body = packet.subarray(5) // past the type byte and the length + const terminator = body.indexOf(0) + + return { + mechanism: body.subarray(0, terminator).toString(), + // the response follows its own four-byte length + response: body.subarray(terminator + 5).toString(), + } +} + +// Runs a whole exchange from the server's side, stopping short of the AuthenticationOk, +// and reports what the client chose to do +const runSASLExchange = async function (connecting, mechanisms) { + send(connecting, 'authenticationSASL', { mechanisms }) + await awaitPackets(connecting, 1) + + const { mechanism, response } = parseSASLInitialResponse(connecting.stream.packets[0]) + const clientNonce = response + .split(',') + .find((part) => part.startsWith('r=')) + .slice(2) + + send(connecting, 'authenticationSASLContinue', { data: scramServer.firstMessage(clientNonce) }) + await awaitPackets(connecting, 2) + + const clientFinalMessage = connecting.stream.packets[1].subarray(5).toString() + const withoutProof = clientFinalMessage.slice(0, clientFinalMessage.indexOf(',p=')) + + send(connecting, 'authenticationSASLFinal', { data: `v=${await scramServer.signature(clientNonce, withoutProof)}` }) + await awaitQuiet() + + return { + mechanism, + gs2Header: response.split(',,')[0], + channelBinding: withoutProof.split(',')[0].slice(2), + } +} + +suite.test('a cleartext password request is answered by default', async function () { + const connecting = startClient() + + send(connecting, 'authenticationCleartextPassword') + await awaitPackets(connecting, 1) + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.deepStrictEqual(sentTypes(connecting), ['p']) +}) + +suite.test('an md5 password request is answered by default', async function () { + const connecting = startClient() + + send(connecting, 'authenticationMD5Password', { salt: Buffer.from([1, 2, 3, 4]) }) + await awaitPackets(connecting, 1) + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.deepStrictEqual(sentTypes(connecting), ['p']) +}) + +suite.test('an immediate AuthenticationOk is accepted by default', async function () { + // trust authentication: nothing is asked of the client, and nothing is required of it + const connecting = startClient() + + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.deepStrictEqual(sentTypes(connecting), []) +}) + +suite.test('a SCRAM exchange is completed by default', async function () { + const connecting = startClient() + + const exchange = await runSASLExchange(connecting, ['SCRAM-SHA-256']) + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.strictEqual(exchange.mechanism, 'SCRAM-SHA-256') + assert.strictEqual(connecting.client._authFinished, true) +}) + +suite.test('channel_binding=require refuses a cleartext password request', async function () { + const connecting = startClient({ ssl: true, channel_binding: 'require' }, { tls: true }) + + send(connecting, 'authenticationCleartextPassword') + const error = await awaitError(connecting) + await awaitQuiet() + + assert.strictEqual(error.message, 'The server requested password authentication, but channel_binding=require was set') + // the password must not have been sent: only a Terminate, closing the connection + assert.deepStrictEqual(sentTypes(connecting), ['X']) + assert.strictEqual(connecting.stream.ended, true) +}) + +suite.test('channel_binding=require refuses an md5 password request', async function () { + const connecting = startClient({ ssl: true, channel_binding: 'require' }, { tls: true }) + + send(connecting, 'authenticationMD5Password', { salt: Buffer.from([1, 2, 3, 4]) }) + const error = await awaitError(connecting) + await awaitQuiet() + + assert.strictEqual(error.message, 'The server requested md5 authentication, but channel_binding=require was set') + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('channel_binding=require refuses an immediate AuthenticationOk', async function () { + const connecting = startClient({ ssl: true, channel_binding: 'require' }, { tls: true }) + + send(connecting, 'authenticationOk') + const error = await awaitError(connecting) + + assert.strictEqual( + error.message, + 'The server authenticated the client without channel binding, but channel_binding=require was set' + ) + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('channel_binding=require refuses an exchange that cannot be bound', async function () { + const connecting = startClient({ ssl: true, channel_binding: 'require' }, { tls: true }) + + // a server that offers only the mechanism without channel binding + send(connecting, 'authenticationSASL', { mechanisms: ['SCRAM-SHA-256'] }) + const error = await awaitError(connecting) + await awaitQuiet() + + assert.match(error.message, /Channel binding is required, but the server did not offer/) + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('channel_binding=require completes a bound SCRAM-SHA-256-PLUS exchange', async function () { + const connecting = startClient({ ssl: true, channel_binding: 'require' }, { tls: true }) + + const exchange = await runSASLExchange(connecting, ['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS']) + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.strictEqual(exchange.mechanism, 'SCRAM-SHA-256-PLUS') + assert.strictEqual(exchange.gs2Header, 'p=tls-server-end-point') + // the binding data carries the certificate hash, not just the gs2 header + assert.ok(exchange.channelBinding.length > 'p=tls-server-end-point,,'.length) + assert.strictEqual(connecting.client._channelBound, true) +}) + +suite.test('channel binding is used when preferred and offered, and skipped when disabled', async function () { + const preferring = startClient({ ssl: true }, { tls: true }) + const preferred = await runSASLExchange(preferring, ['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS']) + + assert.deepStrictEqual(preferring.errors, []) + assert.strictEqual(preferred.mechanism, 'SCRAM-SHA-256-PLUS') + + const disabling = startClient({ ssl: true, channel_binding: 'disable' }, { tls: true }) + const disabled = await runSASLExchange(disabling, ['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS']) + + assert.deepStrictEqual(disabling.errors, []) + assert.strictEqual(disabled.mechanism, 'SCRAM-SHA-256') + assert.strictEqual(disabled.gs2Header, 'n') + assert.strictEqual(disabling.client._channelBound, false) +}) + +suite.test('the enableChannelBinding option still turns channel binding on', async function () { + const connecting = startClient({ ssl: true, enableChannelBinding: true }, { tls: true }) + + const exchange = await runSASLExchange(connecting, ['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS']) + + assert.deepStrictEqual(connecting.errors, []) + assert.strictEqual(exchange.mechanism, 'SCRAM-SHA-256-PLUS') +}) + +suite.test('requiring channel binding after construction is enforced too', async function () { + const connecting = startClient({ ssl: true }, { tls: true }) + connecting.client.enableChannelBinding = 'require' + + send(connecting, 'authenticationCleartextPassword') + const error = await awaitError(connecting) + await awaitQuiet() + + assert.strictEqual(error.message, 'The server requested password authentication, but channel_binding=require was set') + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('a channel binding level set after construction is checked as one given to it', function () { + const { client } = startClient({ ssl: true }, { tls: true }) + + for (const value of ['Require', 'required', 'prefer ', '']) { + assert.throws(() => { + client.channelBinding = value + }, /Invalid channel_binding value/) + assert.strictEqual(client.channelBinding, 'prefer', 'a refused value should not take effect') + } + + // The option's original shape, which was a boolean, still means what it did. + client.enableChannelBinding = false + assert.strictEqual(client.channelBinding, 'disable') + client.enableChannelBinding = true + assert.strictEqual(client.channelBinding, 'prefer') +}) + +suite.test('require_auth=scram-sha-256 refuses a cleartext password request', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + send(connecting, 'authenticationCleartextPassword') + const error = await awaitError(connecting) + await awaitQuiet() + + assert.strictEqual( + error.message, + 'The server requested password authentication, but require_auth="scram-sha-256" was set' + ) + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('require_auth=scram-sha-256 refuses an AuthenticationOk before any exchange', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + send(connecting, 'authenticationOk') + const error = await awaitError(connecting) + + assert.strictEqual( + error.message, + 'The server did not complete authentication, but require_auth="scram-sha-256" was set' + ) +}) + +suite.test('require_auth=scram-sha-256 completes an unbound exchange without SSL', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + const exchange = await runSASLExchange(connecting, ['SCRAM-SHA-256']) + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.strictEqual(exchange.mechanism, 'SCRAM-SHA-256') + assert.strictEqual(exchange.gs2Header, 'n') + assert.strictEqual(connecting.client._authFinished, true) +}) + +suite.test('require_auth=password answers a cleartext request but refuses md5', async function () { + const answering = startClient({ require_auth: 'password' }) + send(answering, 'authenticationCleartextPassword') + await awaitPackets(answering, 1) + + assert.deepStrictEqual(answering.errors, []) + assert.deepStrictEqual(sentTypes(answering), ['p']) + + const refusing = startClient({ require_auth: 'password' }) + send(refusing, 'authenticationMD5Password', { salt: Buffer.from([1, 2, 3, 4]) }) + await awaitError(refusing) + await awaitQuiet() + + assert.deepStrictEqual(sentTypes(refusing), ['X']) +}) + +suite.test('require_auth=!password refuses a cleartext request but answers md5', async function () { + const refusing = startClient({ require_auth: '!password' }) + send(refusing, 'authenticationCleartextPassword') + const error = await awaitError(refusing) + await awaitQuiet() + + assert.strictEqual( + error.message, + 'The server requested password authentication, but require_auth="!password" was set' + ) + assert.deepStrictEqual(sentTypes(refusing), ['X']) + + const answering = startClient({ require_auth: '!password' }) + send(answering, 'authenticationMD5Password', { salt: Buffer.from([1, 2, 3, 4]) }) + await awaitPackets(answering, 1) + + assert.deepStrictEqual(answering.errors, []) + assert.deepStrictEqual(sentTypes(answering), ['p']) +}) + +suite.test('require_auth=none accepts an immediate AuthenticationOk', async function () { + const connecting = startClient({ require_auth: 'none' }) + + send(connecting, 'authenticationOk') + await awaitQuiet() + + assert.deepStrictEqual(connecting.errors, []) + assert.deepStrictEqual(sentTypes(connecting), []) +}) + +suite.test('a password provider is not consulted for a refused request', async function () { + // Looking a password up can reach out to a credential service, so the requirement has + // to be settled before anything else happens. + let consulted = false + const connecting = startClient({ + require_auth: 'scram-sha-256', + password: () => { + consulted = true + return password + }, + }) + + send(connecting, 'authenticationCleartextPassword') + await awaitError(connecting) + await awaitQuiet() + + assert.strictEqual(consulted, false) + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +// A server can pipeline an entire successful login into a single packet, in which case the +// messages that follow a refused request have already arrived and are dispatched after it. +// Everything below is that situation: the refusal has to be the last word. +const sendSuccessfulLogin = function (connecting) { + send(connecting, 'authenticationOk') + send(connecting, 'backendKeyData', { processID: 1, secretKey: 2 }) + send(connecting, 'readyForQuery', { status: 'I' }) +} + +suite.test('a refusal is not undone by messages that were already in flight', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + send(connecting, 'authenticationCleartextPassword') + sendSuccessfulLogin(connecting) + await awaitQuiet() + + assert.strictEqual(connecting.errors.length, 1, 'the refusal should be reported once') + assert.strictEqual( + connecting.errors[0].message, + 'The server requested password authentication, but require_auth="scram-sha-256" was set' + ) + assert.strictEqual(connecting.callbacks.length, 1, 'connect() should be answered exactly once') + assert.deepStrictEqual(connecting.connects, [], 'no connect event should be emitted') + assert.strictEqual(connecting.client._connected, false) + assert.deepStrictEqual(sentTypes(connecting), ['X']) + + await assert.rejects(() => connecting.client.query('SELECT 1'), /not queryable/) +}) + +suite.test('a refused SCRAM exchange is not rescued by a pipelined login', async function () { + const connecting = startClient({ ssl: true, channel_binding: 'require' }, { tls: true }) + + // The server offers only the mechanism that cannot be bound, so the client refuses to + // begin, and then it declares the client logged in as though nothing had happened. + send(connecting, 'authenticationSASL', { mechanisms: ['SCRAM-SHA-256'] }) + await awaitError(connecting) + sendSuccessfulLogin(connecting) + await awaitQuiet() + + assert.match(connecting.errors[0].message, /Channel binding is required, but the server did not offer/) + assert.strictEqual(connecting.callbacks.length, 1, 'connect() should be answered exactly once') + assert.deepStrictEqual(connecting.connects, [], 'no connect event should be emitted') + assert.strictEqual(connecting.client._connected, false) + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('an asynchronous password lookup is not answered once a refusal has happened', async function () { + let release + const connecting = startClient({ + require_auth: 'password', + password: () => new Promise((resolve) => (release = () => resolve(password))), + }) + + // The request is permitted, so the lookup begins... + send(connecting, 'authenticationCleartextPassword') + // ...but the same packet held a request that is not, and by the time the credential + // service answers there is nothing left to answer with. + send(connecting, 'authenticationMD5Password', { salt: Buffer.from([1, 2, 3, 4]) }) + const error = await awaitError(connecting) + await until(() => release !== undefined, 'the password to be asked for') + release() + await awaitQuiet() + + assert.strictEqual(error.message, 'The server requested md5 authentication, but require_auth="password" was set') + assert.deepStrictEqual(sentTypes(connecting), ['X'], 'the password must not be sent') +}) + +suite.test('a second authentication request is refused after the first was', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + send(connecting, 'authenticationCleartextPassword') + await awaitError(connecting) + + // Asking again with a method the setting does permit gets the server no further: the + // connection has been given up on, so there is nothing left to answer with. + send(connecting, 'authenticationSASL', { mechanisms: ['SCRAM-SHA-256'] }) + await awaitQuiet() + + assert.strictEqual(connecting.errors.length, 1, 'the refusal should be reported once') + assert.deepStrictEqual(sentTypes(connecting), ['X']) +}) + +suite.test('a request that follows finished authentication is refused', async function () { + const connecting = startClient() + await runSASLExchange(connecting, ['SCRAM-SHA-256']) + assert.deepStrictEqual(connecting.errors, [], 'the exchange itself should succeed') + + // Having satisfied itself that the client knows the password, the server asks to be told + // it. Nothing this client is configured with, up to and including require_auth, permits + // a second request, so this holds for a default configuration as much as a strict one. + send(connecting, 'authenticationCleartextPassword') + const error = await awaitError(connecting) + await awaitQuiet() + + assert.match(error.message, /already authenticated/) + assert.deepStrictEqual(sentTypes(connecting), ['p', 'p', 'X'], 'the two SCRAM messages and no more') + assert.deepStrictEqual(connecting.connects, []) +}) + +suite.test('a query queued before a refusal is not sent to the server', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + // What a caller queues while the handshake is still going on may be exactly what it did + // not want an unauthenticated server to see. It is rejected when the stream ends, which + // a real socket does once the Terminate below has flushed. + connecting.client.query('SELECT $1::text', ['a secret']).catch(() => {}) + + send(connecting, 'authenticationCleartextPassword') + await awaitError(connecting) + // The rest of a login the server had already pipelined behind its refused request. + send(connecting, 'backendKeyData', { processID: 1, secretKey: 2 }) + send(connecting, 'readyForQuery', { status: 'I' }) + await awaitQuiet() + + assert.deepStrictEqual(sentTypes(connecting), ['X'], 'nothing but Terminate should be written') + assert.strictEqual(connecting.client._queryable, false) +}) + +// libpq refuses anything but an authentication request at this point in its handshake, so a +// server cannot simply leave authentication out. This client listens for every message from +// the start, so a server that skips straight to declaring the client logged in has to be +// caught where the connection is completed. +suite.test('a server that skips authentication altogether satisfies nothing', async function () { + const cases = [ + { config: { require_auth: 'scram-sha-256' }, message: /did not complete authentication/ }, + { config: { ssl: true, channel_binding: 'require' }, message: /without channel binding/ }, + ] + + for (const { config, message } of cases) { + const connecting = startClient(config, { tls: true }) + + // Not one authentication message: the server, or someone in the middle holding a + // certificate this client was willing to accept, just says the client is in. + send(connecting, 'backendKeyData', { processID: 1, secretKey: 2 }) + send(connecting, 'readyForQuery', { status: 'I' }) + const error = await awaitError(connecting) + + assert.match(error.message, message) + assert.deepStrictEqual(connecting.connects, [], 'no connect event should be emitted') + assert.strictEqual(connecting.client._connected, false) + await assert.rejects(() => connecting.client.query('SELECT 1'), /not queryable/) + } +}) + +// Connection#end() writes its Terminate and only ends the stream once that has flushed, so +// a write that lands in between still reaches the server. Anything the client computes +// before answering — an md5 hash, a SCRAM proof — takes a turn of the event loop, which is +// long enough for a refusal to have happened. +suite.test('an md5 hash computed before a refusal is not sent after it', async function () { + const connecting = startClient({ require_auth: 'md5' }) + + // Permitted, so hashing begins, and suspends + send(connecting, 'authenticationMD5Password', { salt: Buffer.from([1, 2, 3, 4]) }) + // Refused, from the same packet, while the hash is still being computed + send(connecting, 'authenticationCleartextPassword') + await awaitError(connecting) + await awaitQuiet() + + assert.deepStrictEqual(sentTypes(connecting), ['X'], 'the hash must not follow the Terminate') +}) + +suite.test('a SCRAM proof computed before a refusal is not sent after it', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + send(connecting, 'authenticationSASL', { mechanisms: ['SCRAM-SHA-256'] }) + await awaitPackets(connecting, 1) + const { response } = parseSASLInitialResponse(connecting.stream.packets[0]) + const clientNonce = response + .split(',') + .find((part) => part.startsWith('r=')) + .slice(2) + + // The proof is derived from the password, which takes thousands of PBKDF2 iterations... + send(connecting, 'authenticationSASLContinue', { data: scramServer.firstMessage(clientNonce) }) + // ...and the server breaks the requirement while that is still going on + send(connecting, 'authenticationCleartextPassword') + await awaitError(connecting) + await awaitQuiet() + + assert.deepStrictEqual(sentTypes(connecting), ['p', 'X'], 'the proof must not follow the Terminate') +}) + +suite.test('an exchange in progress is not continued after a refusal', async function () { + const connecting = startClient({ require_auth: 'scram-sha-256' }) + + // The exchange begins, as the setting permits it to + send(connecting, 'authenticationSASL', { mechanisms: ['SCRAM-SHA-256'] }) + await awaitPackets(connecting, 1) + const { response } = parseSASLInitialResponse(connecting.stream.packets[0]) + const clientNonce = response + .split(',') + .find((part) => part.startsWith('r=')) + .slice(2) + + // Then the server asks for something the setting refuses, and carries on with the + // exchange as though it had not: continuing would put the client's proof on the wire. + send(connecting, 'authenticationCleartextPassword') + send(connecting, 'authenticationSASLContinue', { data: scramServer.firstMessage(clientNonce) }) + await awaitQuiet() + + assert.strictEqual(connecting.errors.length, 1, 'the refusal should be reported once') + assert.deepStrictEqual(sentTypes(connecting), ['p', 'X'], 'no client final message should follow the Terminate') +}) diff --git a/packages/pg/test/unit/client/require-auth-tests.js b/packages/pg/test/unit/client/require-auth-tests.js new file mode 100644 index 000000000..8f72f64f6 --- /dev/null +++ b/packages/pg/test/unit/client/require-auth-tests.js @@ -0,0 +1,167 @@ +'use strict' +const assert = require('assert') +const helper = require('./test-helper') +const { resolveAuthRequirement, checkAuthRequest } = require('../../../lib/require-auth') + +const suite = new helper.Suite() + +const allowedMethods = function (requireAuth, channelBinding) { + return [...resolveAuthRequirement(requireAuth, channelBinding).allowedMethods].sort() +} + +suite.test('nothing is required by default', function () { + assert.strictEqual(resolveAuthRequirement(undefined, 'prefer'), null) + assert.strictEqual(resolveAuthRequirement(undefined, 'disable'), null) + // libpq treats an empty setting as no setting at all + assert.strictEqual(resolveAuthRequirement('', 'prefer'), null) +}) + +suite.test('a plain list permits only the methods it names', function () { + const requirement = resolveAuthRequirement('md5,scram-sha-256', 'prefer') + + assert.deepStrictEqual([...requirement.allowedMethods].sort(), ['md5', 'scram-sha-256']) + assert.strictEqual(requirement.authRequired, true) + assert.strictEqual(requirement.channelBindingRequired, false) + assert.strictEqual(requirement.description, 'require_auth="md5,scram-sha-256"') +}) + +suite.test('a negated list permits everything it does not name', function () { + const requirement = resolveAuthRequirement('!password,!md5', 'prefer') + + assert.deepStrictEqual([...requirement.allowedMethods].sort(), ['gss', 'oauth', 'scram-sha-256', 'sspi']) + // as in libpq, a negated list starts from a connection that need not authenticate + assert.strictEqual(requirement.authRequired, false) +}) + +suite.test('none permits a server that never asks for authentication', function () { + const requirement = resolveAuthRequirement('none', 'prefer') + + assert.strictEqual(requirement.authRequired, false) + assert.deepStrictEqual([...requirement.allowedMethods], []) +}) + +suite.test('!none insists that the server asks for authentication', function () { + const requirement = resolveAuthRequirement('!none', 'prefer') + + assert.strictEqual(requirement.authRequired, true) + assert.deepStrictEqual(allowedMethods('!none', 'prefer'), [ + 'gss', + 'md5', + 'oauth', + 'password', + 'scram-sha-256', + 'sspi', + ]) +}) + +suite.test('methods this client cannot perform are accepted alongside ones it can', function () { + // A connection string shared with libpq should not be rejected out of hand: gss + // simply never matches a request pg is able to receive. + assert.deepStrictEqual(allowedMethods('gss,md5', 'prefer'), ['gss', 'md5']) + assert.deepStrictEqual(allowedMethods('scram-sha-256,sspi', 'prefer'), ['scram-sha-256', 'sspi']) +}) + +suite.test('a requirement this client could never satisfy is rejected', function () { + for (const requireAuth of ['gss', 'oauth', 'gss,sspi']) { + assert.throws(() => resolveAuthRequirement(requireAuth, 'prefer'), { + message: `require_auth="${requireAuth}" cannot be satisfied: this client can only perform "password", "md5", "scram-sha-256" authentication`, + }) + } + + // Negating every method it supports leaves the same dead end, unless an + // unauthenticated connection is still permitted + assert.throws(() => resolveAuthRequirement('!password,!md5,!scram-sha-256,!none', 'prefer'), /cannot be satisfied/) + assert.strictEqual(resolveAuthRequirement('!password,!md5,!scram-sha-256', 'prefer').authRequired, false) +}) + +suite.test('libpq performs methods this client cannot, so a native connection permits them', function () { + const requirement = resolveAuthRequirement('gss,sspi', 'prefer', { native: true }) + assert.deepStrictEqual([...requirement.allowedMethods].sort(), ['gss', 'sspi']) + + assert.throws(() => resolveAuthRequirement('gss,sspi', 'prefer'), { + message: + 'require_auth="gss,sspi" cannot be satisfied: this client can only perform "password", "md5", "scram-sha-256" authentication', + }) +}) + +suite.test('malformed values are rejected', function () { + assert.throws(() => resolveAuthRequirement('password,!md5', 'prefer'), { + message: 'Invalid require_auth value: "password,!md5". Negated methods cannot be mixed with plain ones.', + }) + assert.throws(() => resolveAuthRequirement('!md5,password', 'prefer'), /cannot be mixed/) + assert.throws(() => resolveAuthRequirement('md5,md5', 'prefer'), { + message: 'Invalid require_auth value: "md5,md5". Method "md5" is specified more than once.', + }) + assert.throws(() => resolveAuthRequirement('none,none', 'prefer'), /more than once/) + assert.throws(() => resolveAuthRequirement('scram-sha-1', 'prefer'), /Valid methods are/) + // As in libpq, whitespace is not trimmed: a typo here is a security problem + assert.throws(() => resolveAuthRequirement('md5, password', 'prefer'), /Valid methods are/) + assert.throws(() => resolveAuthRequirement('md5,', 'prefer'), /Valid methods are/) +}) + +suite.test('channel_binding=require narrows the requirement to bound SCRAM', function () { + for (const requireAuth of [undefined, 'scram-sha-256', 'md5,scram-sha-256', '!password']) { + const requirement = resolveAuthRequirement(requireAuth, 'require') + + assert.deepStrictEqual([...requirement.allowedMethods], ['scram-sha-256']) + assert.strictEqual(requirement.authRequired, true) + assert.strictEqual(requirement.channelBindingRequired, true) + assert.strictEqual(requirement.description, 'channel_binding=require') + } +}) + +suite.test('channel_binding=require conflicts with a require_auth that rules out SCRAM', function () { + for (const requireAuth of ['md5', 'password,md5', '!scram-sha-256', 'none']) { + assert.throws(() => resolveAuthRequirement(requireAuth, 'require'), { + message: `channel_binding=require cannot be satisfied by require_auth="${requireAuth}", which does not permit scram-sha-256 authentication`, + }) + } +}) + +suite.test('every request is permitted when nothing is required', function () { + for (const method of ['password', 'md5', 'scram-sha-256', 'none']) { + assert.strictEqual(checkAuthRequest({ requirement: null, method, authFinished: false, channelBound: false }), null) + } +}) + +suite.test('a request for a method that is not permitted is refused', function () { + const requirement = resolveAuthRequirement('scram-sha-256', 'prefer') + + assert.strictEqual(checkAuthRequest({ requirement, method: 'scram-sha-256' }), null) + assert.strictEqual( + checkAuthRequest({ requirement, method: 'password' }), + 'The server requested password authentication, but require_auth="scram-sha-256" was set' + ) + assert.strictEqual( + checkAuthRequest({ requirement, method: 'md5' }), + 'The server requested md5 authentication, but require_auth="scram-sha-256" was set' + ) +}) + +suite.test('an AuthenticationOk that completes no exchange is refused', function () { + const requirement = resolveAuthRequirement('scram-sha-256', 'prefer') + + assert.strictEqual( + checkAuthRequest({ requirement, method: 'none', authFinished: false }), + 'The server did not complete authentication, but require_auth="scram-sha-256" was set' + ) + assert.strictEqual(checkAuthRequest({ requirement, method: 'none', authFinished: true }), null) +}) + +suite.test('an AuthenticationOk needs no exchange when none is permitted', function () { + const requirement = resolveAuthRequirement('none', 'prefer') + + assert.strictEqual(checkAuthRequest({ requirement, method: 'none', authFinished: false }), null) +}) + +suite.test('an unbound exchange cannot satisfy channel_binding=require', function () { + // startSession refuses to pick a mechanism that cannot be bound, so this is the + // second line of defence: even a completed exchange has to have been bound. + const requirement = resolveAuthRequirement(undefined, 'require') + + assert.strictEqual( + checkAuthRequest({ requirement, method: 'none', authFinished: true, channelBound: false }), + 'The server authenticated the client without channel binding, but channel_binding=require was set' + ) + assert.strictEqual(checkAuthRequest({ requirement, method: 'none', authFinished: true, channelBound: true }), null) +}) diff --git a/packages/pg/test/unit/client/sasl-scram-tests.js b/packages/pg/test/unit/client/sasl-scram-tests.js index 02b0d4e6d..64c967757 100644 --- a/packages/pg/test/unit/client/sasl-scram-tests.js +++ b/packages/pg/test/unit/client/sasl-scram-tests.js @@ -19,8 +19,15 @@ suite.test('sasl/scram', function () { ) }) + // A TLS stream whose peer certificate we can hash, so channel binding is possible + const bindableStream = { getPeerCertificate() {} } + suite.test('returns expected session data for SCRAM-SHA-256 (channel binding disabled, offered)', function () { - const session = sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS']) + const session = sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS'], { + channelBinding: 'disable', + sslInUse: true, + stream: bindableStream, + }) assert.equal(session.mechanism, 'SCRAM-SHA-256') assert.equal(String(session.clientNonce).length, 24) @@ -30,7 +37,7 @@ suite.test('sasl/scram', function () { }) suite.test('returns expected session data for SCRAM-SHA-256 (channel binding enabled, not offered)', function () { - const session = sasl.startSession(['SCRAM-SHA-256'], { getPeerCertificate() {} }) + const session = sasl.startSession(['SCRAM-SHA-256'], { sslInUse: true, stream: bindableStream }) assert.equal(session.mechanism, 'SCRAM-SHA-256') assert.equal(String(session.clientNonce).length, 24) @@ -40,7 +47,10 @@ suite.test('sasl/scram', function () { }) suite.test('returns expected session data for SCRAM-SHA-256 (channel binding enabled, offered)', function () { - const session = sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS'], { getPeerCertificate() {} }) + const session = sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS'], { + sslInUse: true, + stream: bindableStream, + }) assert.equal(session.mechanism, 'SCRAM-SHA-256-PLUS') assert.equal(String(session.clientNonce).length, 24) @@ -49,6 +59,70 @@ suite.test('sasl/scram', function () { assert(session.response.match(/^p=tls-server-end-point,,n=\*,r=.{24}$/)) }) + suite.test('uses channel binding when it is required and offered', function () { + const session = sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS'], { + channelBinding: 'require', + sslInUse: true, + stream: bindableStream, + }) + + assert.equal(session.mechanism, 'SCRAM-SHA-256-PLUS') + assert.equal(session.gs2Header, 'p=tls-server-end-point') + }) + + suite.test('falls back to SCRAM-SHA-256 when the stream cannot provide a certificate', function () { + // A stream without getPeerCertificate (as in a Cloudflare Worker) cannot bind the + // channel, so it must claim no support with 'n': claiming 'y' while the server + // offered SCRAM-SHA-256-PLUS is a downgrade the server rejects. + const session = sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS'], { + sslInUse: true, + stream: {}, + }) + + assert.equal(session.mechanism, 'SCRAM-SHA-256') + assert.equal(session.gs2Header, 'n') + assert(session.response.match(/^n,,n=\*,r=.{24}$/)) + }) + + suite.test('fails when channel binding is required but SSL is not in use', function () { + assert.throws(() => sasl.startSession(['SCRAM-SHA-256'], { channelBinding: 'require' }), { + message: 'SASL: Channel binding is required, but SSL is not in use', + }) + }) + + suite.test('fails when channel binding is required but the stream cannot provide a certificate', function () { + assert.throws( + () => sasl.startSession(['SCRAM-SHA-256'], { channelBinding: 'require', sslInUse: true, stream: {} }), + { + message: 'SASL: Channel binding is required, but this connection cannot provide the server certificate', + } + ) + }) + + suite.test('fails when channel binding is required but not offered by the server', function () { + assert.throws( + () => + sasl.startSession(['SCRAM-SHA-256'], { + channelBinding: 'require', + sslInUse: true, + stream: bindableStream, + }), + { + message: + 'SASL: Channel binding is required, but the server did not offer an authentication method that supports it', + } + ) + }) + + suite.test('fails when SCRAM-SHA-256-PLUS is offered over a non-SSL connection', function () { + // Suggests SSL was stripped in transit, whatever the channel binding setting + for (const channelBinding of ['disable', 'prefer']) { + assert.throws(() => sasl.startSession(['SCRAM-SHA-256', 'SCRAM-SHA-256-PLUS'], { channelBinding }), { + message: 'SASL: Server offered SCRAM-SHA-256-PLUS authentication over a non-SSL connection', + }) + } + }) + suite.test('creates random nonces', function () { const session1 = sasl.startSession(['SCRAM-SHA-256']) const session2 = sasl.startSession(['SCRAM-SHA-256']) @@ -63,7 +137,7 @@ suite.test('sasl/scram', function () { }) suite.test('honors a custom scramMaxIterations', function () { - const session = sasl.startSession(['SCRAM-SHA-256'], null, 50) + const session = sasl.startSession(['SCRAM-SHA-256'], { scramMaxIterations: 50 }) assert.equal(session.scramMaxIterations, 50) }) @@ -213,6 +287,7 @@ suite.test('sasl/scram', function () { const session = { message: 'SASLInitialResponse', clientNonce: 'a', + gs2Header: 'n', scramMaxIterations: 5, } @@ -225,6 +300,7 @@ suite.test('sasl/scram', function () { const session = { message: 'SASLInitialResponse', clientNonce: 'a', + gs2Header: 'n', scramMaxIterations: 0, } @@ -237,6 +313,7 @@ suite.test('sasl/scram', function () { const session = { message: 'SASLInitialResponse', clientNonce: 'a', + gs2Header: 'n', } await sasl.continueSession(session, 'password', 'r=ab,s=abcd,i=1') @@ -248,9 +325,12 @@ suite.test('sasl/scram', function () { }) suite.test('sets expected session data (SCRAM-SHA-256, channel binding enabled)', async function () { + // 'y' is echoed from the session rather than inferred from the stream, so a + // connection that could have bound the channel still reports it consistently. const session = { message: 'SASLInitialResponse', clientNonce: 'a', + gs2Header: 'y', } await sasl.continueSession(session, 'password', 'r=ab,s=abcd,i=1', { getPeerCertificate() {} }) @@ -263,8 +343,8 @@ suite.test('sasl/scram', function () { suite.test('SASLprep maps non-ASCII space characters (RFC 3454 C.1.2) to U+0020 SPACE', async function () { // SASLprep probably misuses the C.1.2 table; U+200B, in particular, is listed in both the C.1.2 and B.1 tables. We treat it as a space for compatibility with PostgreSQL. - const sessionPrepped = { message: 'SASLInitialResponse', clientNonce: 'a' } - const sessionRef = { message: 'SASLInitialResponse', clientNonce: 'a' } + const sessionPrepped = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } + const sessionRef = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } await sasl.continueSession(sessionPrepped, '\u200bfoo\xa0bar', 'r=ab,s=abcd,i=1') await sasl.continueSession(sessionRef, ' foo bar', 'r=ab,s=abcd,i=1') @@ -278,8 +358,8 @@ suite.test('sasl/scram', function () { // must produce identical SCRAM output to 'IX'. This proves the prep // step is engaged on the SCRAM derivation path. Without the fix the // two would diverge and this assertion would fail. - const sessionPrepped = { message: 'SASLInitialResponse', clientNonce: 'a' } - const sessionRef = { message: 'SASLInitialResponse', clientNonce: 'a' } + const sessionPrepped = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } + const sessionRef = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } await sasl.continueSession(sessionPrepped, 'I\u00ADX', 'r=ab,s=abcd,i=1') await sasl.continueSession(sessionRef, 'IX', 'r=ab,s=abcd,i=1') @@ -293,8 +373,8 @@ suite.test('sasl/scram', function () { // PostgreSQL's server applies SASLprep when computing the verifier, so // a role created with U+2168 is stored as if it were 'IX'. The client // must do the same. - const sessionPrepped = { message: 'SASLInitialResponse', clientNonce: 'a' } - const sessionRef = { message: 'SASLInitialResponse', clientNonce: 'a' } + const sessionPrepped = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } + const sessionRef = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } await sasl.continueSession(sessionPrepped, '\u2168', 'r=ab,s=abcd,i=1') await sasl.continueSession(sessionRef, 'IX', 'r=ab,s=abcd,i=1') @@ -310,7 +390,7 @@ suite.test('sasl/scram', function () { // raw password. We snapshot the resulting SCRAM output as a regression // guard: if anyone ever swaps the order of operations, removes the // NFKC step, or accidentally strips ASCII bytes, this assertion trips. - const session = { message: 'SASLInitialResponse', clientNonce: 'a' } + const session = { message: 'SASLInitialResponse', clientNonce: 'a', gs2Header: 'n' } await sasl.continueSession(session, '\u0007abc', 'r=ab,s=abcd,i=1') @@ -324,6 +404,7 @@ suite.test('sasl/scram', function () { message: 'SASLInitialResponse', mechanism: 'SCRAM-SHA-256-PLUS', clientNonce: 'a', + gs2Header: 'p=tls-server-end-point', } await sasl.continueSession(session, 'password', 'r=ab,s=abcd,i=1', { diff --git a/packages/pg/test/unit/connection-parameters/creation-tests.js b/packages/pg/test/unit/connection-parameters/creation-tests.js index e326e2630..8d715a150 100644 --- a/packages/pg/test/unit/connection-parameters/creation-tests.js +++ b/packages/pg/test/unit/connection-parameters/creation-tests.js @@ -397,6 +397,302 @@ suite.test('sslnegotiation is read from PGSSLNEGOTIATION env var', function () { } }) +suite.test('channel_binding defaults to prefer', function () { + const subject = new ConnectionParameters({}) + assert.strictEqual(subject.channel_binding, 'prefer') +}) + +suite.test('channel_binding is read from config', function () { + for (const channel_binding of ['disable', 'prefer', 'require']) { + const subject = new ConnectionParameters({ ssl: true, channel_binding }) + assert.strictEqual(subject.channel_binding, channel_binding) + } +}) + +suite.test('channel_binding is read from a connection string', function () { + const subject = new ConnectionParameters({ connectionString: 'postgres://host/db?channel_binding=disable' }) + assert.strictEqual(subject.channel_binding, 'disable') +}) + +suite.test('channel_binding rejects invalid values', function () { + assert.throws(() => new ConnectionParameters({ channel_binding: 'bogus' }), /Invalid channel_binding value/) + assert.throws( + () => new ConnectionParameters({ connectionString: 'postgres://host/db?channel_binding=bogus' }), + /Invalid channel_binding value/ + ) +}) + +suite.test('channel_binding=require requires ssl', function () { + assert.throws( + () => new ConnectionParameters({ ssl: false, channel_binding: 'require' }), + /channel_binding=require requires SSL to be enabled/ + ) +}) + +suite.test('the boolean enableChannelBinding option maps onto the channel binding levels', function () { + assert.strictEqual(new ConnectionParameters({ enableChannelBinding: true }).channel_binding, 'prefer') + assert.strictEqual(new ConnectionParameters({ enableChannelBinding: false }).channel_binding, 'disable') +}) + +suite.test('enableChannelBinding also accepts the channel binding levels', function () { + const subject = new ConnectionParameters({ ssl: true, enableChannelBinding: 'require' }) + assert.strictEqual(subject.channel_binding, 'require') +}) + +suite.test('channel_binding takes precedence over enableChannelBinding', function () { + const subject = new ConnectionParameters({ channel_binding: 'disable', enableChannelBinding: true }) + assert.strictEqual(subject.channel_binding, 'disable') +}) + +suite.test('enableChannelBinding is ignored in a connection string', function () { + // Only libpq's channel_binding parameter is recognized there, so the default stands. + const subject = new ConnectionParameters({ connectionString: 'postgres://host/db?enableChannelBinding=disable' }) + assert.strictEqual(subject.channel_binding, 'prefer') +}) + +suite.test('a camelCased channelBinding or requireAuth is rejected rather than ignored', function () { + assert.throws( + () => new ConnectionParameters({ channelBinding: 'require' }), + /The channelBinding option is not recognized: spell it channel_binding/ + ) + assert.throws( + () => new ConnectionParameters({ requireAuth: 'scram-sha-256' }), + /The requireAuth option is not recognized: spell it require_auth/ + ) + + // the parser passes query parameters it does not recognize through as they are written, + // so a connection string is held to the same spelling + assert.throws( + () => new ConnectionParameters({ connectionString: 'postgres://host/db?channelBinding=require' }), + /The channelBinding option is not recognized/ + ) + + // the libpq spellings, and the legacy enableChannelBinding option, are what work + const subject = new ConnectionParameters({ + channel_binding: 'disable', + require_auth: 'md5', + enableChannelBinding: true, + }) + assert.strictEqual(subject.channel_binding, 'disable') + assert.strictEqual(subject.require_auth, 'md5') +}) + +suite.test('channel_binding is read from PGCHANNELBINDING env var', function () { + const original = process.env.PGCHANNELBINDING + process.env.PGCHANNELBINDING = 'disable' + try { + const subject = new ConnectionParameters({}) + assert.strictEqual(subject.channel_binding, 'disable') + } finally { + if (original === undefined) { + delete process.env.PGCHANNELBINDING + } else { + process.env.PGCHANNELBINDING = original + } + } +}) + +suite.test('channel_binding is included in libpq connection string when it is not the libpq default', function () { + const subject = new ConnectionParameters({ + user: 'brian', + host: 'localhost', + port: 5432, + database: 'postgres', + ssl: true, + channel_binding: 'require', + }) + subject.getLibpqConnectionString( + assert.calls(function (err, pgCString) { + assert(!err) + assert.equal( + pgCString.indexOf("channel_binding='require'") !== -1, + true, + 'libpqConnectionString should contain channel_binding' + ) + }) + ) +}) + +suite.test('channel_binding is omitted from libpq connection string when it is the libpq default', function () { + const subject = new ConnectionParameters({ + user: 'brian', + host: 'localhost', + port: 5432, + database: 'postgres', + }) + subject.getLibpqConnectionString( + assert.calls(function (err, pgCString) { + assert(!err) + assert.equal(pgCString.indexOf('channel_binding'), -1, 'libpqConnectionString should not contain channel_binding') + }) + ) +}) + +suite.test('require_auth is unset by default, requiring nothing of the server', function () { + const subject = new ConnectionParameters({}) + assert.strictEqual(subject.require_auth, undefined) + assert.strictEqual(subject.authRequirement, null) +}) + +suite.test('require_auth is read from config', function () { + const subject = new ConnectionParameters({ require_auth: 'md5' }) + assert.strictEqual(subject.require_auth, 'md5') + assert.deepStrictEqual([...subject.authRequirement.allowedMethods], ['md5']) +}) + +suite.test('require_auth is read from a connection string', function () { + const subject = new ConnectionParameters({ connectionString: 'postgres://host/db?require_auth=scram-sha-256' }) + assert.strictEqual(subject.require_auth, 'scram-sha-256') + assert.deepStrictEqual([...subject.authRequirement.allowedMethods], ['scram-sha-256']) +}) + +suite.test('require_auth is read from PGREQUIREAUTH env var', function () { + const original = process.env.PGREQUIREAUTH + process.env.PGREQUIREAUTH = 'password' + try { + assert.strictEqual(new ConnectionParameters({}).require_auth, 'password') + // config takes precedence over the environment + assert.strictEqual(new ConnectionParameters({ require_auth: 'md5' }).require_auth, 'md5') + + // an explicitly empty config value requires nothing, and says so in preference to the + // environment, rather than being treated as absent and falling through to it. It is + // kept verbatim so that libpq hears it as well, which the conninfo test below covers. + const explicitlyEmpty = new ConnectionParameters({ require_auth: '' }) + assert.strictEqual(explicitlyEmpty.require_auth, '') + assert.strictEqual(explicitlyEmpty.authRequirement, null) + + // as in libpq, an empty setting requires nothing rather than permitting nothing + process.env.PGREQUIREAUTH = '' + const subject = new ConnectionParameters({}) + assert.strictEqual(subject.require_auth, undefined) + assert.strictEqual(subject.authRequirement, null) + } finally { + if (original === undefined) { + delete process.env.PGREQUIREAUTH + } else { + process.env.PGREQUIREAUTH = original + } + } +}) + +suite.test('require_auth rejects values it could never satisfy', function () { + assert.throws(() => new ConnectionParameters({ require_auth: 'bogus' }), /Invalid require_auth value/) + assert.throws(() => new ConnectionParameters({ require_auth: 'gss' }), /cannot be satisfied/) + assert.throws( + () => new ConnectionParameters({ connectionString: 'postgres://host/db?require_auth=md5,!password' }), + /cannot be mixed/ + ) +}) + +suite.test('channel_binding=require narrows the requirement to bound SCRAM', function () { + const subject = new ConnectionParameters({ ssl: true, channel_binding: 'require', require_auth: 'md5,scram-sha-256' }) + assert.deepStrictEqual([...subject.authRequirement.allowedMethods], ['scram-sha-256']) + assert.strictEqual(subject.authRequirement.channelBindingRequired, true) + // the setting itself is passed through unchanged, for libpq to enforce in its own way + assert.strictEqual(subject.require_auth, 'md5,scram-sha-256') +}) + +suite.test('channel_binding=require conflicts with a require_auth that rules out SCRAM', function () { + assert.throws( + () => new ConnectionParameters({ ssl: true, channel_binding: 'require', require_auth: 'md5' }), + /channel_binding=require cannot be satisfied by require_auth="md5"/ + ) +}) + +// Parameters bound for libpq are libpq's to judge: it authenticates by methods this +// library does not implement, and it negotiates SSL whether or not one was configured +// here, so a configuration it can honor must not be refused on this side. +suite.test('parameters for libpq permit the methods libpq performs', function () { + const subject = new ConnectionParameters({ require_auth: 'gss' }, { native: true }) + assert.deepStrictEqual([...subject.authRequirement.allowedMethods], ['gss']) + assert.strictEqual(subject.require_auth, 'gss') + + // the same configuration cannot work with this library's own protocol implementation + assert.throws(() => new ConnectionParameters({ require_auth: 'gss' }), /cannot be satisfied/) +}) + +suite.test('parameters for libpq leave SSL negotiation to libpq', function () { + // ssl is stated either way, since earlier tests in this file leave defaults.ssl set. + // A falsy one is not passed on to libpq as an sslmode at all, so libpq goes on to + // negotiate SSL by its own default and can bind the channel after all. + const subject = new ConnectionParameters({ ssl: false, channel_binding: 'require' }, { native: true }) + assert.strictEqual(subject.channel_binding, 'require') + + assert.throws( + () => new ConnectionParameters({ ssl: false, channel_binding: 'require' }), + /requires SSL to be enabled/ + ) +}) + +suite.test('parameters for libpq are still checked for what libpq would reject', function () { + assert.throws(() => new ConnectionParameters({ require_auth: 'bogus' }, { native: true }), /Invalid require_auth/) + assert.throws(() => new ConnectionParameters({ require_auth: 'md5,!gss' }, { native: true }), /cannot be mixed/) + assert.throws( + () => new ConnectionParameters({ channel_binding: 'require', require_auth: 'gss' }, { native: true }), + /channel_binding=require cannot be satisfied by require_auth="gss"/ + ) + assert.throws( + () => new ConnectionParameters({ channel_binding: 'bogus' }, { native: true }), + /Invalid channel_binding/ + ) +}) + +suite.test('an explicitly empty require_auth reaches libpq, overriding the environment there too', function () { + const original = process.env.PGREQUIREAUTH + process.env.PGREQUIREAUTH = 'scram-sha-256' + + try { + const subject = new ConnectionParameters( + { user: 'brian', host: 'localhost', port: 5432, database: 'postgres', require_auth: '' }, + { native: true } + ) + assert.strictEqual(subject.require_auth, '') + assert.strictEqual(subject.authRequirement, null) + + subject.getLibpqConnectionString( + assert.calls(function (err, pgCString) { + assert(!err) + // Saying nothing would leave libpq to read PGREQUIREAUTH for itself, undoing the + // override; an empty value is how conninfo says that nothing is required. + assert.notStrictEqual(pgCString.indexOf("require_auth=''"), -1, pgCString) + }) + ) + } finally { + if (original === undefined) { + delete process.env.PGREQUIREAUTH + } else { + process.env.PGREQUIREAUTH = original + } + } +}) + +suite.test('require_auth is included in libpq connection string only when set', function () { + new ConnectionParameters({ + user: 'brian', + host: 'localhost', + port: 5432, + database: 'postgres', + }).getLibpqConnectionString( + assert.calls(function (err, pgCString) { + assert(!err) + assert.equal(pgCString.indexOf('require_auth'), -1, 'libpqConnectionString should not contain require_auth') + }) + ) + + new ConnectionParameters({ + user: 'brian', + host: 'localhost', + port: 5432, + database: 'postgres', + require_auth: 'scram-sha-256', + }).getLibpqConnectionString( + assert.calls(function (err, pgCString) { + assert(!err) + assert.notStrictEqual(pgCString.indexOf("require_auth='scram-sha-256'"), -1) + }) + ) +}) + suite.test('sslnegotiation is included in libpq connection string', function () { const subject = new ConnectionParameters({ user: 'brian', From aea2cb9c1a2cc34589e59fb8517001d81feff7a4 Mon Sep 17 00:00:00 2001 From: George MacKerron Date: Wed, 12 Aug 2026 16:46:17 +0100 Subject: [PATCH 2/3] Exclude Node 16 from SSL tests (libpq fails to get server cert in native driver) --- .github/workflows/ci.yml | 5 ++++- packages/pg/test/integration/gh-issues/2085-tests.js | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 90c336620..d348caa1a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,8 +64,11 @@ jobs: PGPASSWORD: postgres PGHOST: localhost PGDATABASE: ci_db_test - # PGTESTNOSSL is deliberately unset: the postgres-ssl service image above has SSL + # PGTESTNOSSL is generally unset: the postgres-ssl service image above has SSL # configured, so the SSL and SCRAM channel binding tests can run for real here. + # But Node 16 native build cannot retrieve the server certificate through libpq, + # so the SSL tests stay off there. + PGTESTNOSSL: ${{ matrix.node == 16 && 'true' || '' }} SCRAM_TEST_PGUSER: scram_test SCRAM_TEST_PGPASSWORD: test4scram SCRAM_TEST_PGUSER_UNICODE: scram_unicode_test diff --git a/packages/pg/test/integration/gh-issues/2085-tests.js b/packages/pg/test/integration/gh-issues/2085-tests.js index 80bc7f33a..9d84500c9 100644 --- a/packages/pg/test/integration/gh-issues/2085-tests.js +++ b/packages/pg/test/integration/gh-issues/2085-tests.js @@ -10,6 +10,16 @@ if (process.env.PGTESTNOSSL) { return } +// The native client leaves SSL to libpq, which uses the system OpenSSL. Node 16 and +// earlier statically link OpenSSL 1.1.1 and export its symbols into the process, where +// they collide with the OpenSSL 3 the system libpq is built against: the handshake +// completes, but no peer certificate can be retrieved, so libpq reports "certificate +// could not be obtained: no SSL error reported". Node 18 is the first release to bundle +// OpenSSL 3, and nothing on this side of the boundary can make an earlier one work. +if (helper.args.native && parseInt(process.versions.openssl, 10) < 3) { + return +} + suite.test('it should connect over ssl', async () => { const ssl = helper.args.native ? 'require' From 31242581ff8dfc7f694116429df5ef238249117d Mon Sep 17 00:00:00 2001 From: George MacKerron Date: Wed, 12 Aug 2026 17:39:19 +0100 Subject: [PATCH 3/3] Remove PGTESTNOSSL again, and guard the specific unavoidably-failing Node 16 SSL test --- .github/workflows/ci.yml | 5 +---- packages/pg/test/integration/gh-issues/2085-tests.js | 12 +++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d348caa1a..d256557cf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,11 +64,8 @@ jobs: PGPASSWORD: postgres PGHOST: localhost PGDATABASE: ci_db_test - # PGTESTNOSSL is generally unset: the postgres-ssl service image above has SSL + # PGTESTNOSSL is no longer set: the postgres-ssl service image above has SSL # configured, so the SSL and SCRAM channel binding tests can run for real here. - # But Node 16 native build cannot retrieve the server certificate through libpq, - # so the SSL tests stay off there. - PGTESTNOSSL: ${{ matrix.node == 16 && 'true' || '' }} SCRAM_TEST_PGUSER: scram_test SCRAM_TEST_PGPASSWORD: test4scram SCRAM_TEST_PGUSER_UNICODE: scram_unicode_test diff --git a/packages/pg/test/integration/gh-issues/2085-tests.js b/packages/pg/test/integration/gh-issues/2085-tests.js index 9d84500c9..b12708a08 100644 --- a/packages/pg/test/integration/gh-issues/2085-tests.js +++ b/packages/pg/test/integration/gh-issues/2085-tests.js @@ -11,11 +11,13 @@ if (process.env.PGTESTNOSSL) { } // The native client leaves SSL to libpq, which uses the system OpenSSL. Node 16 and -// earlier statically link OpenSSL 1.1.1 and export its symbols into the process, where -// they collide with the OpenSSL 3 the system libpq is built against: the handshake -// completes, but no peer certificate can be retrieved, so libpq reports "certificate -// could not be obtained: no SSL error reported". Node 18 is the first release to bundle -// OpenSSL 3, and nothing on this side of the boundary can make an earlier one work. +// earlier statically link OpenSSL 1.1.1 and export its symbols, which take precedence +// over the OpenSSL 3 that a current libpq is built against: libpq's calls to SSL_new and +// SSL_connect land in 1.1.1, while its call to SSL_get1_peer_certificate, a name 1.1.1 +// does not define, lands in OpenSSL 3 and reads a structure it does not recognize. So the +// handshake completes but no peer certificate can be retrieved, and libpq reports +// "certificate could not be obtained: no SSL error reported". Node 18 is the first release +// to bundle OpenSSL 3, and nothing on this side of the boundary can make an earlier one work. if (helper.args.native && parseInt(process.versions.openssl, 10) < 3) { return }