feat: Allow users to specify a prebuilt 'rustls' configuration for TLS#4051
feat: Allow users to specify a prebuilt 'rustls' configuration for TLS#4051LukeMathWalker wants to merge 1 commit intolaunchbadge:mainfrom
Conversation
1be3a99 to
803ecc2
Compare
31dc8df to
5c41b3b
Compare
5c41b3b to
720c92b
Compare
There was a problem hiding this comment.
Sorry, I've only just realized that you were waiting for feedback this whole time.
Yeah, I'm fine documenting that setting your own ClientConfig makes sslmode (edit: and pretty much all the SSL settings, right?) a no-op and that it can't be losslessly converted back to a URL.
For outputting to a URL, we could add a new parameter which would make it really obvious that there's been some lossy conversion, e.g. danger-custom-ssl-config=true. Then, if the user tries to connect with such a URL, we error if they also haven't set a custom ClientConfig, as a guard against accidentally connecting with the wrong SSL settings.
(Edit: I was thinking we could just add a hidden variant to PgSslMode to signal this but we neglected to mark it #[non_exhaustive] so technically that would be a breaking change.)
I think it's also worth looking at #4242 and trying to reconcile with those changes, as it touches a lot of the same things.
Does your PR solve an issue?
Closes #4049.
Is this a breaking change?
No.
There is a breaking change for
sqlx-core, but that's considered semver-exempt.Open Design questions
What should the interaction be between
sslmodeand a prebuiltrustlsconfiguration?In the current PR, if TLS is available, we behave as if the user specified
PgSslMode::VerifyFull.rustls::client::ClientConfigdoesn't expose the underlying server cert verifier, therefore it's not possible to wrap around it to disable hostname verification or cert verification.At the same time, I think it'd be surprising for a user that specified its own
rustlsconfiguration to get the kind of permissive behaviour thatPgSslMode::Preferimplies.Conversion into a URL is inevitably lossy
We can't convert a prebuilt
rustlsclient configuration into the three URL parameters thatlibpqsupports.At the moment, the PR ends up building a URL that doesn't have
ssl*parameters. Would it be preferable to fail the conversion entirely?Follow-up work
If we agree on the approach, I can add prebuilt
rustlssupport to MySQL; either in this PR or in a separate one.