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

Upgrade to rustls 0.23 #3399

Merged
merged 1 commit into from
Aug 4, 2024
Merged

Upgrade to rustls 0.23 #3399

merged 1 commit into from
Aug 4, 2024

Conversation

djc
Copy link
Contributor

@djc djc commented Aug 2, 2024

Upgrade rustls to 0.23.x. The tricky part here is that rustls since 0.22 exposes two CryptoProvider implementations here. So far I've exposed separate features for each, but this still runs the risk that a project that ends up with crates selecting rustls with different crypto providers picks an unexpected one. I guess there's not much of a solution for that, but open to feedback about what you think the right direction is for sqlx.

@djc djc force-pushed the rustls-0.23 branch 2 times, most recently from 22a380a to 8351d62 Compare August 2, 2024 15:29
sqlx-core/Cargo.toml Outdated Show resolved Hide resolved
@djc djc force-pushed the rustls-0.23 branch 3 times, most recently from 8de5060 to 048e04c Compare August 2, 2024 22:03
@djc
Copy link
Contributor Author

djc commented Aug 2, 2024

Feels like the test matrix gets fairly extreme. IMO it doesn't really make sense to test the matrix of TLS implementations (x crypto providers) against all database versions, but that's sort of orthogonal.

@abonander
Copy link
Collaborator

It is rather important, because it's caught incompatibilities that have crept in upstream: #3091

@djc
Copy link
Contributor Author

djc commented Aug 2, 2024

Oh, I could absolutely see testing recent versions of the databases against all TLS stacks (and potentially crypto providers), just not sure it's all that valuable to do this against the entire back catalog of database versions.

@abonander
Copy link
Collaborator

It's not the entire back catalog, the policy is the oldest and newest actively supported versions.

MariaDB is a bit of a special case because we added the verylatest tag at the request of one of the maintainers.

If there are any versions that don't fit with this policy, it's just because the file hasn't been updated.

As for testing Ring vs aws-lc-rs, it depends on whether or not RusTLS guarantees they support the same cipher suites, etc. If they do, then just testing one is fine. Otherwise, it seems prudent to cover them both.

I don't care as much anymore about the number of CI passes as I used to. I'm perfectly happy to let Micro$oft foot the bill.

@djc
Copy link
Contributor Author

djc commented Aug 2, 2024

Crypto providers might definitely have different cipher suites.

Copy link
Collaborator

@abonander abonander left a comment

Choose a reason for hiding this comment

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

@djc
Copy link
Contributor Author

djc commented Aug 3, 2024

Can you add a note about crypto providers to https://github.com/launchbadge/sqlx/blob/main/src/lib.md#tls-support?

Done.

@abonander abonander merged commit a892ebc into launchbadge:main Aug 4, 2024
86 checks passed
jrasanen pushed a commit to jrasanen/sqlx that referenced this pull request Oct 14, 2024
jrasanen pushed a commit to jrasanen/sqlx that referenced this pull request Oct 14, 2024
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.

3 participants