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

Consider making the TLS implementation used in shuttle-shared-db configurable #690

Closed
aumetra opened this issue Mar 7, 2023 · 5 comments
Assignees
Labels
T-Feature Request A request for a new feature

Comments

@aumetra
Copy link
Contributor

aumetra commented Mar 7, 2023

(Opening this issue based on a question asked on Discord)

Currently the SQLx connection pool provisioned by shuttle-shared-db uses native-tls to establish a secure connection to the database server.
Would it be thinkable to gate these behind features like rustls and native-tls, where one of them is set as the default? (this would be a breaking change)


Looking at the SQLx source code, they use the webpki-roots certificates for their trust anchors.
So if Shuttle installs their own CA certificates, using the rustls feature will break it.

@oddgrd oddgrd added T-Improvement Improvement or addition to existing features question labels Mar 8, 2023
@moritzheiber
Copy link

This is especially relevant if you're using sqlx in your application already (e.g. to run migrations or query the database) and don't wish to rely on native-tls as a feature of the crate (i.e. you cannot have the sqlx dependency of shuttle-shared-db built with native-tls and your own version of sqlx with e.g. rustls)

@oddgrd oddgrd added T-Feature Request A request for a new feature and removed T-Improvement Improvement or addition to existing features labels Mar 28, 2023
@oddgrd
Copy link
Contributor

oddgrd commented Mar 28, 2023

The thing I'm not sure about here is that since we use native-tls everywhere else (which we might also be able to change, but that needs investigation), will using rustls here then conflict? If someone wants to explore this in a PR and prove that it works, I think we can add this feature flag to shuttle-shared-db.

@utterstep
Copy link
Contributor

@oddgrd can you assign this to me & @vroussea (Team V²)? :)
We'd like to try this now, as #864 PR passed the review.

@oddgrd
Copy link
Contributor

oddgrd commented May 5, 2023

@utterstep awesome! I sent you an invite now to get the triager role on this repo, this should allow you to add labels and set assignments

@aumetra
Copy link
Contributor Author

aumetra commented May 11, 2023

Closed by #870

@aumetra aumetra closed this as completed May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Feature Request A request for a new feature
Projects
None yet
Development

No branches or pull requests

4 participants