Skip to content

Support libpq's channel_binding and require_auth (and enforce them against every authentication request) - #3746

Open
jawj wants to merge 4 commits into
brianc:masterfrom
jawj:master
Open

Support libpq's channel_binding and require_auth (and enforce them against every authentication request)#3746
jawj wants to merge 4 commits into
brianc:masterfrom
jawj:master

Conversation

@jawj

@jawj jawj commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

This builds on my earlier PR, #3356, which added SCRAM-SHA-256-PLUS behind the enableChannelBinding option. As I noted then, that option let us enable channel binding but gave no way to require it. Being opt-in and non-standard, I suspect rather few users yet get the benefit of it.

This PR moves to using a libpq-style channel_binding parameter. It also introduces a libpq-style require_auth parameter, and makes sure both are enforced everywhere they need to be.

These changes make some connections more secure by default. They also provide users with the option to make connections even more secure, by specifying channel_binding=require or specific authentication modes, such as require_auth=scram-sha-256, that limit the damage that can be done by an MITM attack. And they bring node-postgres's connection options into closer alignment with libpq.

Because channel_binding=require authenticates the server, it also silences the deprecation warning that sslmode=prefer, sslmode=require and sslmode=verify-ca currently emit (this matters to Neon users, for example, who see those warnings even though the weaker guarantees they warn about cannot be exploited when the channel is bound).

channel_binding

  • Takes libpq's three values, disable, prefer and require, from the client config, a connection string, or PGCHANNELBINDING.
  • The default is now prefer: channel binding is used whenever the connection is over SSL and the server offers SCRAM-SHA-256-PLUS, and an unbound exchange is used otherwise. This matches libpq's default.
  • enableChannelBinding is still honored, with true meaning "prefer" and false meaning "disable" (it also accepts the three levels), but is now documented as deprecated.
  • channel_binding: 'require' needs SSL, so it is an error to combine it with SSL disabled — unless you are using the native (libpq) client, which negotiates SSL of its own accord and reports for itself if it ends up without.

require_auth

Names the authentication method(s) the server is allowed to ask for, with libpq's semantics: a comma-separated allow-list, or a block-list in which every entry is negated with !, plus none for a connection where the server asks for nothing. It can be set in the config, a connection string, or PGREQUIREAUTH — which an explicit require_auth: '' overrides, as in libpq.

  • The methods libpq recognizes are password, md5, scram-sha-256, gss, sspi and oauth, of which this client implements the first three. A setting that includes only methods we cannot satisfy (e.g. require_auth: 'gss') is an error at construction-time (but e.g. require_auth: 'gss,md5' is fine). The native client passes the setting to libpq and defers to it, so the full set is accepted there.
  • channel_binding: 'require' implies require_auth: 'scram-sha-256', since no other method supports channel binding. A require_auth value that rules out SCRAM contradicts that, so is also an error.

Enforcing them

The kind of bug I filed CVE-2025-49146 for against pgjdbc is a risk here — where channel_binding=require was honored inside the SCRAM exchange but a server could simply ask for a plain password instead!

So the check lives in one place that every authentication request passes through, modelled on libpq's check_expected_areq:

  • Requirements are checked before the client answers, so a refused request neither sends a password nor invokes a password callback.
  • The same check runs on AuthenticationOk, and again where the connection becomes usable, so a server cannot evade a requirement by declaring the client authenticated without asking for anything — or by sending no authentication message at all. libpq accepts nothing but an authentication request at that point in its handshake, whereas this client listens for every message from the start, so it needs the second check.
  • Authentication is answered once, whatever the two new parameters are set to. A server that asks again, having already said what it wanted and been given it, is refused.
  • A refused authentication request now stays refused. A server can pipeline an entire successful login into one packet, and the messages following the refused request were still acted on: the connection callback was invoked a second time, this time reporting success, a connect event was emitted, and the client was left looking usable. Such a client now reports the failure once and refuses to answer anything further.
  • Hashing an MD5 password and computing a SCRAM proof each take a turn of the event loop, and Connection#end() writes its Terminate before ending the stream, so both re-check that the connection has not been given up on before writing their answer.

Two related tightenings in the SCRAM code: the client counts itself channel-bound only once the server's signature has been verified, and only for SCRAM-SHA-256-PLUS, so an unbound exchange cannot satisfy require; and a server offering SCRAM-SHA-256-PLUS on a connection that is not encrypted is refused.

Naming

Both parameters keep libpq's spelling (the same as client_encoding and application_name do, for example). Since both exist to refuse weak authentication, a camelCased channelBinding or requireAuth throws rather than being quietly ignored, as does a channel binding level that is not one of the supported three. Both options can also be set on a Pool, which passes its options to the clients it creates.

Changing the default

Changing default behaviour was a concern last time round. prefer only ever upgrades a connection that is already over SSL, to a server that offers channel binding, and falls back to an unbound exchange otherwise, so there is nothing for a server to be surprised by. The one case where an outcome changes is a server whose certificate uses a signature algorithm the parser doesn't know (Ed448, for instance): that connection now fails rather than silently authenticating unbound. The same thing can happen in libpq. In either case, channel_binding: 'disable' restores the old outcome.

Tests

  • packages/pg/test/unit/client/auth-flow-tests.js drives a client through complete exchanges against a scripted SCRAM server using a real certificate, computing the server's replies from what the client actually sent, so it checks the gs2 header and the binding data rather than taking the client's word for either. This helps with @charmander's point on Add support for SCRAM-SHA-256-PLUS i.e. channel binding #3356 that the tests there would have passed with no implementation of channel binding at all. It covers each of the refusals above, including the pipelined login and the post-refusal races.
  • require-auth-tests.js covers the parsing and the requirement check. The ConnectionParameters and pg-connection-string tests cover resolution from config, connection strings and the environment, precedence between them, and each error.
  • The SCRAM integration tests now run over real SSL and assert whether the exchange was bound, and PGTESTNOSSL is no longer set in CI, so the SSL and channel binding tests actually run there (perhaps there is a good reason for setting PGTESTNOSSL in CI, though?). packages/pg/script/test-server.sh starts the same image CI uses under either podman or docker, for anyone who wants to run them locally.

Docs are in docs/pages/features/ssl.mdx and the pg-connection-string README, with a suggested CHANGELOG entry too.

DefinitelyTyped's @types/pg will need channel_binding and require_auth adding. I'll send a PR for that if and when these changes land.

@jawj
jawj requested a review from hjr3 as a code owner August 12, 2026 15:28
@jawj

jawj commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

@hjr3 or @brianc: any thoughts on this?

@charmander

Copy link
Copy Markdown
Collaborator

(For what it’s worth: I haven’t reviewed the diff yet, but the PR description sounds perfect.)

@charmander charmander left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(Partial review that doesn’t cover the core logic. Test refactoring suggestions needn’t block merge.)

Thanks for implementing all this!

Comment on lines +30 to +32
const value = [channelBinding, enableChannelBinding, process.env.PGCHANNELBINDING, defaults.channel_binding].find(
(candidate) => candidate !== undefined && candidate !== null
)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const value = [channelBinding, enableChannelBinding, process.env.PGCHANNELBINDING, defaults.channel_binding].find(
(candidate) => candidate !== undefined && candidate !== null
)
const value = channelBinding ?? enableChannelBinding ?? process.env.PGCHANNELBINDING ?? defaults.channel_binding

but shouldn’t enableChannelBinding be the only one with support for boolean values (and emit a deprecation warning when used)?

const value = channelBinding ?? convertBoolean(enableChannelBinding) ?? process.env.PGCHANNELBINDING ?? defaults.channel_binding
if (!channelBindingLevels.includes(value)) {
  throw 
}
return value

(where convertBoolean returns null, 'disable' + warning, or 'prefer' + warning)

Comment thread packages/pg/lib/client.js
return this._channelBinding
}

// Changing the level after construction re-derives what the server has to do, so that

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does changing the level after construction need to be supported at all? If it’s just for strict backwards compatibility with enableChannelBinding (whose being settable was incidental, yeah?), I think it should only be possible via set enableChannelBinding and emit a deprecation warning.

client.connection.emit(name, msg)
}

const until = async function (predicate, description) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I know it’s a test, but uses of this busy wait look like they wouldn’t be too hard to replace with a promise resolved in startClient and a method provided by MemoryStream?

Comment on lines +160 to +162
await awaitQuiet()

assert.deepStrictEqual(connecting.errors, [])

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

And it looks like this kind of thing should be a flag to startClient that no errors are expected.

Comment on lines +483 to +484
await until(() => release !== undefined, 'the password to be asked for')
release()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

and ;(await release)(), with release being made into a promise.

Comment on lines +506 to +509
assert(!err)
assert.equal(
pgCString.indexOf("channel_binding='require'") !== -1,
true,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert(!err)
assert.equal(
pgCString.indexOf("channel_binding='require'") !== -1,
true,
assert.ifError(err)
assert(
pgCString.includes("channel_binding='require'"),

Comment thread CHANGELOG.md

We do not include break-fix version release in this file.

## pg@8.24.0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
## pg@8.24.0
## Unreleased

@hjr3 hjr3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall, this looks great. One minor piece of feedback on Options.channelBinding but this does not have to be blocking a merge.

// 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We introduced Options.useLibpqCompat because we had no other way of signaling a change in the parsing behavior. My hope was to remove Options once we made the major version bump.

Setting Options.channelBinding is not changing parsing behavior but instead adding another way to modify the resulting config object.

If someone wants to add channel binding, they could parse and then add it to the resulting config object:

const config = parse('...');
config.channel_binding = 'require';

If it is to prevent the warning, then:

const url = new URL('pg:///?sslmode=require')
if (url.searchParams.has('channel_binding') === false) {
  url.searchParams.append('channel_binding', 'require')
}
const config = parse(url.toString());

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants