-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(postgres): Support direct SSL connections to Postgres 17+ servers #3879
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
base: main
Are you sure you want to change the base?
Conversation
Pushed an update which adds the new parameter to the |
Hi @abonander. Is there anything we could do to help prioritizing/reviewing/merging this PR? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Request changes
is just to incorporate the recent changes to the existing ci job into this pr's new job step.
Push is a rebase onto the latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Job step revision lgtm, as does the rest of the change.
I'm not a maintainer so @abonander will have to review it also.
Remove "Ignored for Unix domain socket" notes that no longer apply.
This PR adds support for the
sslnegotiation
connection parameter which has been available since Postgres 17.0. See thelibpq
reference here and the 17.0 release notes.The motivation for this setting is to allow clients to skip the Postgres-specific preample while establishing a secure connection to the server. The benefits of this are:
mitmproxy
for example doesn't understand the Postgres protocol, but using a direct connection allows it to process Postgres traffic. I have seen corporate proxies that have incompatibilities with the Postgres protocol, and cause issues for clients.Postgres 17+ servers support this option, and several other drivers support it client side. Users of
sqlx
can now configure the client to connect directly instead of doing the preamble. For older server versions, there is asslnegotiation=postgres
fallback which uses the traditional preamble.As part of this change I have started including the
postgresql
ALPN in the client SSL handshake, since Postgres servers require it when establishing a direct connection. From my testing (and by its design) it should be ignored by servers that do not pay attention to it.Does your PR solve an issue?
There doesn't seem to be an issue for this feature at the moment. I can create one if required. The closest I could find was #3264, but the Postgres 17 feature was just mentioned there, and the actual issue seemed to be unrelated.I created a companion issue, fixes #3880.
Is this a breaking change?
Not as far as I can tell.
Though this feature is only supported in newer Postgres versions (17+), the default of
PgSslNegotiation::Postgres
maintains existing behaviour used by older library versions. This is shown by the fact that all Postgres 13 Docker integration tests pass in the GitHub workflows.There are a few changes which could impact this, but I don't have enough context to determine if they constitute breaking changes. Please consider the following:
postgresql
. Servers typically ignore this (the Postgres 13 Docker integration tests show this) if they don't care about the ALPN value. It's possible in some use case the ALPN causes a server to reject the connection, but I haven't seen this in practice.build_url
method now includes thesslnegotiation
setting. I'm not sure how this method is used in practice outside of the test suite, but the additional parameter may impact users relying on the structure of that return value.