feat(pg-connection-string): warn if non-standard ssl options are used

[hjr3]

In preparation for v3.0.0, we start warning users to be explicit about the sslmode they want.

[cloudflare-workers-and-pages]

Deploying node-postgres with    Cloudflare Pages

Latest commit:	6bdb0b7
Status:	✅  Deploy successful!
Preview URL:	https://f5d22a97.node-postgres.pages.dev
Branch Preview URL:	https://push-qkwyoukwlmvl.node-postgres.pages.dev

View logs

[charmander]

Could warn this way for repeat callers: https://nodejs.org/api/process.html#avoiding-duplicate-warnings

[hjr3]

Could warn this way for repeat callers: https://nodejs.org/api/process.html#avoiding-duplicate-warnings

TIL - implemented

[jawj]

Has the change that drives this warning been discussed elsewhere?

Usually I would be all for adopting the standard or upstream semantics — but I actually think this is an example where libpq gets it badly wrong and current node-postgres has historically done much better. libpq'ssslmode=require really sounds secure but it absolutely is not: this is a serious footgun that people are shooting themselves in the foot with all the time.

The change here means that, if and when developers upgrade node-postgres to 9.0, connections that used to be secure will suddenly become insecure. I mean, hopefully they'll see the warning and do something about it ... but surely many will not.

IMHO it would be much better to have users actively opt in to this downgrade in TLS protection. That would mean keeping the uselibpqcompat flag, and keeping it false by default.

As I understand it, node-postgres historically treats every sslmode from require upwards as not just verify-full, but as sslrootcert=system. IMHO this is also behaviour that libpq got wrong and node-postgres got right. If the developer asks for verify-full and doesn't provide a root CA cert, libpq throws. In the same situation, node-postgres validates against the system root CA certs. That matches the behaviour of https and is the logic I think most current developers would expect.

Is it proposed to match libpq here too, and throw an error if the user specifies sslmode=verify-full without also providing a certificate? I don't think it should be — but that's also where the logic of matching libpq takes you.

[hjr3]

@jawj there was a long discussion on #2709