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

Respect rustls default crypto provider after upgrading rustls to 0.23 #2201

Closed
wants to merge 4 commits into from

Conversation

thynson
Copy link

@thynson thynson commented May 7, 2024

Description of changes:

This is a follow up of #2200, making s2n-quic-rustls respect the CryptoProvider::get_default().

Call-outs:

Code that relying on aws_lc_rs directly was replaced with CryptoProvider::get_default().unwrap().

Testing:

Existing unit tests and examples pass.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@thynson thynson changed the title feat: respect rustls default crypto provider Respect rustls default crypto provider after upgrading rustls to 0.23 May 7, 2024
@toidiu
Copy link
Contributor

toidiu commented May 9, 2024

@thynson can you please share more about why you don't/aren't able to use the aws-lc-rs provider?

We would like to avoid supporting multiple providers since it makes integration less clean (as can be seen from the default_crypto_provider() code you submitted).

@thynson
Copy link
Author

thynson commented May 9, 2024

One of my teammate is trying to port and deploy our app to an embedded system, but failed to cross compile aws_lc_rs for various reason. For this reason we are staying on an old version internal fork of s2n-quic to avoid diverging the code base for now.
While my goal is to deploy our app on the cloud, actually I would like to switch to aws_lc_rs if it's possible for maximum throughput.

@thynson
Copy link
Author

thynson commented May 9, 2024

Actually we're blocked on #2069 too.

@toidiu
Copy link
Contributor

toidiu commented May 9, 2024

@thynson Thanks for opening this PR and raising the issue. We considered what it would take to support both CryptoProviders and decided that it is better to only support a single CryptoProvider in the variant of aws-lc-rs.

Why choose aws-lc-rs:

In order to provider good performance/sercurity/interop we extensively test our library in our CI. Since each new feature requires more testing, we must carefully choose which features to support. In this instance we decided that it is better to not expose rustls's underlying CryptoProvider feature and instead set one by default. We have decided to only support aws-lc-rs because it is the default provider that rustls ships with and also because it offers FIPs compliance.

Still a build issue?

aws/aws-lc-rs#297
Since aws-lc-rs has removed its CMake dependency, I wonder if building aws-lc-rs is still an issue for you. We also encourage you to open an issue with aws-lc-rs to help resolve build issues.

@toidiu toidiu deleted the branch aws:ak-updateRustls May 9, 2024 23:11
@toidiu toidiu closed this May 9, 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.

2 participants