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

feat: sslnegotiation and direct ssl for postgres 17 #1151

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sunng87
Copy link

@sunng87 sunng87 commented Jul 6, 2024

This patch adds Direct SSL support which is a new feature coming in PostgreSQL 17. With this option, we issue TLS handshake directly without SslRequest. This will save a roundtrip on connection opening.

I just follow how libpq is designed, to offer an option sslnegotiation to control this behaviour (because it won't work on Postgres 16 or earlier).

postgres/src/config.rs Outdated Show resolved Hide resolved
@@ -25,18 +26,20 @@ where
SslMode::Prefer | SslMode::Require => {}
Copy link
Owner

Choose a reason for hiding this comment

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

Should there be any special behavior for sslmode=prefer sslnegotiation=direct?

Copy link
Author

Choose a reason for hiding this comment

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

No. When sslmode is prefer or required, we go directly to ssl handshake just like other ssl based protocol, for example, https. And those TLS libraries will initialize the handshake process, we don't need to do anything here.

@sfackler
Copy link
Owner

sfackler commented Jul 6, 2024

Seems reasonable to me, but I think it might make sense to wait on merging until 17 releases.

@sunng87
Copy link
Author

sunng87 commented Jul 7, 2024

No problem. I'm using this to test my own postgresql wire protocol implementation

And the clippy issue seems not related to this patch, I can create another one to fix that. Edit: Just saw it's been fixed on default branch

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.

None yet

2 participants