Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop validating server certificates in SslMode.Prefer & SslMode.Require #3988

Closed
Tracked by #4006
cfbao opened this issue Sep 23, 2021 · 3 comments · Fixed by #4005
Closed
Tracked by #4006

Stop validating server certificates in SslMode.Prefer & SslMode.Require #3988

cfbao opened this issue Sep 23, 2021 · 3 comments · Fixed by #4005

Comments

@cfbao
Copy link

cfbao commented Sep 23, 2021

Steps to reproduce

Open a Npsql connection with SslMode = SslMode.Prefer or SslMode = SslMode.Require.
SSL connection will fail unless server cert is added in local cert store.

The issue

This isn't an "issue" in and of itself, and it can even be argued that this should be the desired behaviour.
However, libpq has SSL modes of the same names (and a few more), and they behave differently from Npgsql's. Given that libpq underlies some widely used PostgreSQL clients (e.g. psql & pgAdmin) and the Java JDBC driver also has the same behaviour, Npgsql's different behaviour can catch people off guard, and give people the inaccurate impression that it's broken.

libpq's SSL modes
JDBC's SSL modes

Notably, libpq & JDBC don't validate server certificates by default ("prefer"), or even in the "require" mode.

For example, we recently tried AWS Aurora PostgreSQL's IAM authentication. It requires SSL, but Aurora's server cert isn't signed by a public CA.
Everything worked immediately when we tried with pgAdmin, psql, and Flyway, but failed unexpectedly with Npgsql and FluentMigrator (which uses Npgsql).
It took us quite a while to figure out that the different defaults for SSL modes (disable vs prefer) and their different behaviours (validate certs or not) are the root cause.

I noticed a few open issues about moving Npgsql's SSL modes to be more aligned with libpq (#3652, #3684). It'd be nice if their cert validation behaviour can be aligned too.
Even if Npgsql won't make this change, it's still worth noting the differences with libpq in documentation.

Further technical details

Npgsql version: Npgsql 5.0.10
PostgreSQL version: 13.3 (AWS Aurora PostgreSQL)
Operating system: Windows & Debian

@vonzshik
Copy link
Contributor

Yeah, that's definitely one of the things on my list to bring npgsql's ssl implementation closer to how libpq operates.

I'm going to assign this one to myself and add it to 6.0 milestone. There is a high chance we will have to push ssl rework to 7.0 but I'll do my best to push a pr in the next few weeks.

@vonzshik vonzshik self-assigned this Sep 23, 2021
@vonzshik vonzshik added this to the 6.0.0 milestone Sep 23, 2021
@roji roji changed the title SslMode.Prefer & SslMode.Require validate server certificates Stop validating server certificates in SslMode.Prefer & SslMode.Require Oct 3, 2021
@roji
Copy link
Member

roji commented Oct 3, 2021

After offline discussion, we don't think we can just change the meaning of SslMode.Require to stop validating certificates; this would silently cause users to become vulnerable to potential security issues.

The solution adopted is the following (thanks @NinoFloris):

  • For 6.0, if you specify SslMode=Require, you must also specify Trust Server Certificate=true; not specifying TSC or specifying true will cause an exception to be thrown, which directs users to the new VerifyCA/VerifyFull for validation, or to keep (or add) Trust Server Certificate=true if they really want SSL without validation.
  • These will steer validating users away from Require to Verify*, which is what we want. Users who do want SSL without validation will end up with TSC=true.
  • At some later point (Npgsql 7?), we can relax the requirement to specify TSC with SslMode (making it not validate like libpq), and possibly remove TSC altogether (start throwing if it's specified).

We do think it's fine to stop validation for SslMode.Prefer, since it already allows non-SSL connections.

@cfbao
Copy link
Author

cfbao commented Oct 5, 2021

this sounds like a brilliant transition plan! thanks everyone

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

Successfully merging a pull request may close this issue.

3 participants