fix(pg-connection-string): get closer to libpq semantics for sslmode

[pmalouin]

Summary

We have found that the handling of the sslmode connection string parameter is inconsistent with other PG libraries and with the reference libpq documentation. This PR proposes some changes to sslmode behavior that are more aligned with libpq.

Detailed sslmode behavior

Here is the list of all sslmode values and their expected behavior with this PR:

sslmode=verify-full

Require an SSL connection and verify the CA and server identity.
No changes in this PR.

sslmode=verify-ca (changed)

Require an SSL connection and verify the CA, but not the server identity. This is achieved by setting ssl.checkServerIdentity to a no-op function (see docs). Previously, this mode behaved like verify-full but that was not consistent with the libpq implementation. SECURITY WARNING: Using sslmode=verify-ca requires specifying a CA with sslrootcert. If a public CA is used, verify-ca allows connections to a server that somebody else may have registered with the CA, making you vulnerable to Man-in-the-Middle attacks.

sslmode=require (changed)

If a root CA is provided via the sslrootcert parameter of the connection string, it behaves like verify-ca. Otherwise, require an SSL connection but do not verify CA and server identity (ssl.rejectUnauthorized is set to false).
Previously, this mode behaved like verify-full but that was not consistent with the libpq implementation.

sslmode=no-verify

Require an SSL connection but do not verify CA and server identity (ssl.rejectUnauthorized is set to false).
No changes in this PR. Note: this mode is not documented in libpq and does not appear to be broadly supported in other libraries, but has been kept as-is for the sake of backwards-compatibility. One option could be to mark it as deprecated since sslmode=require could be an alternative, but doing so might have little value for this project.

sslmode=prefer (changed)

Require an SSL connection but do not verify CA and server identity (ssl.rejectUnauthorized is set to false). Previously, this mode behaved like verify-full but that was not consistent with the libpq implementation.
In reality, this mode should be even less strict and support a fallback logic from SSL to non-SSL connection if SSL is not accepted by the server. Implementing a fallback logic seems to be more complex to solve and I did not dare touch this, but this could eventually be addressed if users of this library deem this mode valuable.

sslmode=allow

Not supported by this library.
No changes in this PR. For this mode also, there could be an opportunity to implement a fallback logic from non-SSL to SSL, but I did not dare touch this and I don't have data that suggests that this might be valuable for this project.

sslmode=disable

Only try a non-SSL connection.
No changes in this PR

An important note is that this PR potentially introduces semver breaking changes, in particular because it relaxes the security constraints of some sslmode values:

• sslmode=prefer is less strict, users should switch to sslmode=verify-full to keep parity.
• sslmode=require is less strict, users should switch to sslmode=verify-full to keep parity.
• sslmode=verify-ca is less strict, users should switch to sslmode=verify-full to keep parity.

Prior discussions about sslmode

I believe this PR addresses concerns raised in these two GH issues in the past:
#2281
#2009

In particular, there has been one case where the sslmode=verify-ca is currently too strict when connecting through AWS RDS Proxy, but the work-around of using sslmode=no-verify would disable CA verification completely.

Other languages/libraries and their support for sslmode

Just as a reference, these two libraries are also trying to be more or less consistent with libpq:

• Golang - pq
  • Supports disable, require, verify-ca andverify-full only.
• PostgreSQL JDBC Driver
  • Supports all 6 modes.

---

Thanks for considering this change and please let me know how I can polish this further for acceptance 🙏

[cyx]

@charmander do we have an idea when this will land / get released as part of pg9?

[ceefour]

Please merge this as it fixes #2281 and #2375

[ceefour]

Help @brianc to merge this

[ThisIsMissEm]

@brianc / @hjr3 what's the status on this? We've a PR on Mastodon blocked by this change: mastodon/mastodon#25483

[charmander]

@ThisIsMissEm Is that PR blocked on this? It looks more like it would be made unnecessary by this.

[ThisIsMissEm]

@ThisIsMissEm Is that PR blocked on this? It looks more like it would be made unnecessary by this.

I'm saying "blocked" in the sense of if upstream is going to fix, that'd be preferable to a fix in our code for it

[charmander]

@ThisIsMissEm On second read, it looks like the Mastodon PR is necessary either way. Unless you were planning on deleting all of that code if this were merged and released soon enough?

[hjr3]

This PR introduces breaking changes. We should probably:

• merge feat(pg-connection-string): ClientConfig helper functions #3128
• release version 2.8.0
• merge this PR
• see if there are any other breaking changes that need to be made
• release version 3.0

[ThisIsMissEm]

This PR introduces breaking changes. We should probably:

• merge feat(pg-connection-string): ClientConfig helper functions #3128
• release version 2.7.0
• merge this PR
• see if there are any other breaking changes that need to be made
• release version 3.0

@hjr3 that sounds good to me!

[ThisIsMissEm]

Just wanted to follow up and see if there's any plans for forwards progress on this?

[Trott]

Over on Mastodon, @ThisIsMissEm asked if I might know some Node.js TLS/SSL people who might be able to review this. I hope I'm not pinging too widely, but I'm certainly interested in trying to unblock things for the Mastodon project so....hey, can anyone review? @indutny @jasnell @joyeecheung @tniessen @pimterry @panva @mertcanaltin

[charmander]

I’m not really up to speed on the Mastodon context here, but some information, if it helps:

• This implementation is (AFAIK) as correct as it can be in terms of translating libpq sslmodes to pg configuration. But although it’s documented, there is one sslmode hazard in libpq we could probably improve on.

  The difference between verify-ca and verify-full depends on the policy of the root CA. If a public CA is used, verify-ca allows connections to a server that somebody else may have registered with the CA. In this case, verify-full should always be used. If a local CA is used, or even a self-signed certificate, using verify-ca often provides enough protection.

• I’ve already approved this PR, but since it’s a breaking change it isn’t going to be a default until pg 9. There are a lot of changes that would ideally go in pg 9, and I don’t expect it to come out soon. Maybe a new major version of pg-connection-string could be released sooner, if you’d be interested in adding a dependency on a newer version of pg-connection-string and using it manually to pass pg a configuration object instead of a connection string.

• As @ThisIsMissEm mentioned in the linked pull request, pg doesn’t support sslmode=prefer (and it doesn’t support allow either). I think prefer is a bad idea (because it adds complexity and provides a false sense of security), assume/hope it’s only the libpq default for historical reasons, and don’t expect pg to support it in the future.

[ThisIsMissEm]

@charmander sslmode=prefer is explicitly what our users have asked for because we support it on the ruby side of the project which uses the native pg implementation

[charmander]

@charmander sslmode=prefer is explicitly what our users have asked for because we support it on the ruby side of the project which uses the native pg implementation

@ThisIsMissEm In the end it’s up to @brianc, but I’m against supporting sslmode=prefer in pg – it’s convenient and unsafe, which isn’t a good combo.

Looking back at more of the context and questions, e.g.

I'm wanting an expert opinion on the changes in the upstream PR before I make any further changes here, since I'm by no means an expert on TLS/SSL, and I'm cautious about introducing accidental security vulnerabilities here.

sslmode=require is implemented in this PR without adding any vulnerabilities relative to libpq, but libpq’s sslmode=require is inherently vulnerable to MITM even though it might sound like “require normal SSL” by the name – it doesn’t check certificates at all, so it only provides opportunistic encryption. Most people should be using either sslmode=verify-ca (with sslrootcert) or sslmode=verify-full. verify-full is the one that gives the basic guarantees of TLS that are defaults in places like Node’s node:tls module (+ most other platforms) and web browsers.

(Re: managed database providers: It looks like DigitalOcean, the managed database provider in mastodon/mastodon#11445, is giving bad advice by encouraging people to connect with sslmode=require over the internet, and only mentioning specifying sslrootcert as an afterthought, plus a quick link to libpq documentation. Heroku is even worse: require over the internet, require from generic code, no mention of the lack of guarantees anywhere. Google Cloud SQL is fully safe, but it does its own thing and passes pre-connected streams to pg. AWS RDS documents a secure way to do it, as well as what look like some less secure ways.)

[hjr3]

@charmander

I’ve already approved this PR, but since it’s a breaking change it isn’t going to be a default until pg 9.

I suggested a path forward here: #2709 (comment)

It is not clear to me why we would need to wait until pg 9 to release pg-connection-string v3.0

[charmander]

@hjr3 As I mentioned in the same comment:

Maybe a new major version of pg-connection-string could be released sooner, if you’d be interested in adding a dependency on a newer version of pg-connection-string and using it manually to pass pg a configuration object instead of a connection string.

i.e. we don’t need to wait, but the new major version of pg-connection-string won’t be the default dependency in pg (that it uses to parse connectionString) until pg 9, so people will still need to make application-side changes.

[hjr3]

@hjr3 As I mentioned in the same comment:

Maybe a new major version of pg-connection-string could be released sooner, if you’d be interested in adding a dependency on a newer version of pg-connection-string and using it manually to pass pg a configuration object instead of a connection string.

i.e. we don’t need to wait, but the new major version of pg-connection-string won’t be the default dependency in pg (that it uses to parse connectionString) until pg 9, so people will still need to make application-side changes.

I somehow missed that. My apologies. I understand and agree with what you are saying.

I do think it is fairly common for teams to use newer versions of pg-connection-string with pg.

[ThisIsMissEm]

In mastodon, we already directly pull in pg-connection-string, specifically to try to better handle connection options from server operators. Would be fully in favor of a release that includes this and #3128. Would sponsorship or something help move this along? (I might be able to get some support for that)

[hjr3]

I want to continue working on pg-connection-string and I do not need sponsorship. The goal of iceddev/pg-connection-string#37 was to make it easier to keep packages in sync with each other. Unfortunately, I lost my merge permissions when the transition was made.

If @brianc can give me merge permissions, I can move the package forward. If there are concerns with my ability to merge other packages, I started #3129 which should allow us to enforce the proper permissions.

[ThisIsMissEm]

@hjr3 that was more directed at @brianc

[brianc]

hey sorry I had the flu last week...literally never been that sick before in my life. still only about 80% back to health. Yeah @hjr3 I've "known" you for a long time on github, and trust you. happy to give you merge permissions! You might need to ping me if you need a critical release done to npm at any time, but even though sometimes I go awol for a week or two I am always watching the repo...just hard to do computer stuff when you think you might actually die from the flu! Instead of merging this myself I'm going to give you permissions @hjr3 so we can make sure they work. :)

[charmander]

same as my previous review

[brianc]

I'd probably say just release 3.0.0 with this as default and thoroughly document the breaking changes

since this is part of the monorepo it isn't independently versioned exactly. I need to maintain backwards compatibility with node-postgres as it stands now unless we want to do a major version bump of PG if this is a breaking change....which I'd rather avoid doing. An opt-in flag or some way to turn this on via the connection string would be a nice default behavior for now, and then when I flip to [email protected] and have the proper documentation & upgrade guide and stuff we can switch the defaults. Does that help / make sense @hjr3 ? Thanks for your help on this btw!

I originally wanted to merge this in as-is and publish a 3.0.0. I now wonder if we should add a flag (.e.g. sslcompat=libpq) to enable this behavior in 2.x.x so users have an upgrade path. We can then make this the default behavior in 3.0.0.

Yeah any kinda flag you can add to make this an opt-in thing for now would be perfect! 👍

[brianc]

note: the tests are failing due to some istambul test coverage metric I never set up myself...feel free to lower the threshold slightly if that's the nudge it needs to get over the finish line. :)

[hjr3]

The more I think about this, the more I am convinced that this should be behind a flag and then we can figure out in the far-term future if we ever want to make this a default. I am very concerned about changing something like sslmode=prefer from being an alias to sslmode=verify-full to a much weaker sslmode.

All that is to say: no major version upgrade is planned.

[brianc]

All that is to say: no major version upgrade is planned.

awesome - lmk if you need help gettin' the PR over the finish line! I'm all in favor of not breaking changes! =D

behind a flag

that would be like another, optional, non-"standard" query param in the connection string, yea?

[brianc]

radical!

…

On Thu, Apr 10, 2025 at 12:42 PM Herman J. Radtke III < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/pg-connection-string/index.js <#2709 (comment)> : > case 'no-verify': { config.ssl.rejectUnauthorized = false break } + case 'require': { + if (config.sslrootcert) { + // If a root CA is specified, behavior of `sslmode=require` will be the same as that of `verify-ca` + config.ssl.checkServerIdentity = function () {} + } else { + config.ssl.rejectUnauthorized = false + } + break + } + case 'verify-ca': { + config.ssl.checkServerIdentity = function () {} I think where we are at is: 1. put the check back in place to match libpq semantics 2. improve the error message to warn folks about mitm attacks 3. make the sslmode documentation tradeoffs more clear, especially in libpq mode I will send an updated PR soon ™️ — Reply to this email directly, view it on GitHub <#2709 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAMHIP5FDGZTA6EQXWKE6L2Y2USHAVCNFSM5O7W67N2U5DIOJSWCZC7NNSXTPCQOVWGYUTFOF2WK43UKJSXM2LFO45TENZVG43TEOJZGM4A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ThisIsMissEm]

rather than using a value from the connection string, I think it'd be better to have parse take an option: parse(str: string, useLibpqCompat: boolean) — that way connection URLs don't have non-standard parameters?

[Xiretza]

Then every application using this library has to implement this out-of-band, right? That's simply not going to happen, so from a user perspective this effectively wouldn't fix the issue of not supporting standard libpq connection strings.

[ThisIsMissEm]

I think this would be an application concern, not a user choice?

[hjr3]

Many libraries that wrap pg accept a DSN. Those libraries would have to be updated to have whatever function that accepts the DSN also accept this extra parameter.

Change request addressed

[charmander]

I don’t know what the best way forward is. It seems like there should be at least a code-side way to set the compatibility flag, as @ThisIsMissEm brought up, if we’re going the route of a compatibility flag? This doesn’t preclude supporting sslcompat=libpq in the query string, if people think that’s also important enough. Or the equivalent of a code-side compatibility flag could be a separate major version of pg-connection-string that implements this as a breaking change…?

Anyway, my suggestion would be:

• new major version of pg-connection-string using the more libpq-compatible interpretation of sslmode
• pg continues to depend on the same older version range of pg-connection-string, with any future changes to pg-connection-string informed by that (i.e. maybe more backporting than usual)
• introduce a deprecation warning on the old version of pg-connection-string when sslmodes of require or verify-ca are used, recommending better replacements:
  • verify-full if the connection string is already being used successfully with no further changes, or
  • installing the new version of pg-connection-string and calling into it directly instead of going through pg’s connectionString option, if the library user is currently patching the ssl property after parsing to achieve the effect of one of the modes
• another deprecation warning with just the latter recommendation (plus mention of the standard form sslmode=require) when sslmode=no-verify is used

Side note: It’s important to fix the pg surprise where the entire ssl configuration is replaced if SSL-related options exist in connectionString before sslmode=require becomes weaker in the interpretation of connectionString – i.e. in pg@9, under this plan – but it’s less of an issue when mixing pg@8 with the newer pg-connection-string, since the option merging is then exposed as top-level JavaScript operations with clear precedence.

[hjr3]

I have added a the useLibpqCompat option to parse to set libpq compatibility. I also renamed the sslcompat flag to uselibpqcompat to match the parse option.

I have created #3421 for the major version upgrade roadmap. I think there are a number of things we want to fix if we do a major version. The desired changes, how we plan and communicate the changes and tracking of those changes will happen in that issue.

[hjr3]

I plan on merging this in a few days unless someone objects.