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

Please support using aws-lc-rs in place of ring #1050

Open
1 of 2 tasks
joshtriplett opened this issue Jan 19, 2024 · 9 comments
Open
1 of 2 tasks

Please support using aws-lc-rs in place of ring #1050

joshtriplett opened this issue Jan 19, 2024 · 9 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@joshtriplett
Copy link
Contributor

joshtriplett commented Jan 19, 2024

Describe the feature

I'd like to be able to use aws-lc-rs in place of ring, throughout my dependency tree. I'd love to be able to do so with aws-sdk-rust as well.

Use Case

aws-lc-rs is faster. (EDIT: turns out it doesn't have a more compatible license, and still has code under the OpenSSL license.)

Proposed Solution

I'd propose either:

  1. Switching to aws-lc-rs as the default, or
  2. Introducing feature flags to control which backend to use.

Switching to aws-lc-rs would be the simplest, and would be in line with the general aws-sdk-rust policy of not controlling functionality via feature flags.

Using a feature flag would allow people who prefer ring to continue using it.

I'd be happy to implement either solution.

Other Information

#966 requested support for the FIPS mode specifically, but that issue wasn't accepted because the FIPS mode requires Go to build.

However, the non-FIPS mode does not require Go. It does require cmake, but that seems somewhat less onerous. Nonetheless, if depending on cmake is a showstopper for this, I'd be happy to add a feature-flag-based solution, so that people who prefer to keep using ring can do so.

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

A note for the community

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue, please leave a comment
@joshtriplett joshtriplett added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jan 19, 2024
@jdisanti jdisanti removed the needs-triage This issue or PR still needs to be triaged. label Jan 19, 2024
@jmklix jmklix added the p2 This is a standard priority issue label Jan 19, 2024
@jmklix
Copy link
Member

jmklix commented Jan 19, 2024

We plan to add this at some point in the future, but I don't have a timeline for when this will be supported with this sdk

@joshtriplett
Copy link
Contributor Author

@jmklix Would that timeline be affected if I wrote and submitted a patch implementing this?

(If so, which approach would you prefer? Switch over completely, or use a feature flag?)

@jdisanti
Copy link
Contributor

jdisanti commented Jan 20, 2024

Hey Josh,

We've talked about switching over to it as the default, but there has been some disagreement over that due to the requirement that cmake be installed for it to compile (and potentially even Go when FIPS is needed). The ideal path forward would be to remove these requirements and make it default, but that's probably a significant amount of work on the aws-lc-rs side of things.

Our general stance is to not use cargo features for switching TLS implementations, so doing it that way is a no-go.

You should be able to work around this for now by swapping out the HttpClient implementation in config. There are examples in aws-config. Then you can turn off the tls-rustls/rustls features to remove the dependency on crate you have licensing issues with. If you want to make your solution available for others, you could also host a separate crate that provides a HttpClient for aws-lc-rs until we get the default changed.

@joshtriplett
Copy link
Contributor Author

@jdisanti

We've talked about switching over to it as the default, but there has been some disagreement over that due to the requirement that cmake be installed for it to compile (and potentially even Go when FIPS is needed). The ideal path forward would be to remove these requirements and make it default, but that's probably a significant amount of work on the aws-lc-rs side of things.

I get that. It looks like aws/aws-lc-rs#297 tracks that for aws-lc-rs, though I don't see a corresponding issue tracking that for aws-lc itself. That does seem like the ideal solution, if possible.

You should be able to work around this for now by swapping out the HttpClient implementation in config. There are examples in aws-config. Then you can turn off the tls-rustls/rustls features to remove the dependency on crate you have licensing issues with.

I've found switching out HttpClient to be remarkably painful and error-prone, and I've had a number of issues attempting to do so in the past due to traveling the less well-trodden paths. I'm currently trying to reduce the degree of customization in my setup of the AWS SDK. That's why I'm trying to find a simpler solution than that.

Is that path (disabling those features) tested in CI these days?

@justsmth
Copy link

We've talked about switching over to it as the default, but there has been some disagreement over that due to the requirement that cmake be installed for it to compile (and potentially even Go when FIPS is needed).

This limitation is being worked on: aws/aws-lc-rs#317

@rcoh
Copy link
Contributor

rcoh commented Jan 25, 2024

I've found switching out HttpClient to be remarkably painful and error-prone, and I've had a number of issues attempting to do so in the past due to traveling the less well-trodden paths. I'm currently trying to reduce the degree of customization in my setup of the AWS SDK. That's why I'm trying to find a simpler solution than that.

Is that path (disabling those features) tested in CI these days?

Yeah, there is a CI test for it. We also overhauled the way that HttpClients are configured, so I think that it is (mostly) a much improved experience. The one remaining issue is configuring a different HttpClient for credential providers.

@rcoh
Copy link
Contributor

rcoh commented Jan 25, 2024

Another detail we'd probably want to iron out—I think we'd probably need to create some sort of DoCrypto trait of some sort to enable runtime switching without having to rewire everything

@yerke
Copy link

yerke commented May 18, 2024

Now that aws/aws-lc-rs#317 is merged and released, is it possible to reevaluate this issue please? Thanks.

@Velfi
Copy link
Contributor

Velfi commented May 24, 2024

I'm working on a guide for this. Here's an early version:

Developer Guide: Hyper 1.0

The AWS SDKs for Rust were developed at a time before hyper when 1.0, so they currently default to the v0.14 version of hyper. The team is in the process of replacing that default with a client based on the latest version of hyper, but we’re first looking for people to test this new client “in the wild” before we make it the default client.

Our new client is a drop-in replacement for the old one and it comes in three variants based on the crypto implementation underlying TLS:

  • Ring - “Safe, fast, small crypto using Rust.” This is the crypto implementation behind our current hyper v0.14 client.
  • AWS Libcrypto - A general-purpose cryptographic library maintained by the AWS Cryptography team for AWS and their customers, based on code from the Google BoringSSL project and the OpenSSL project.
  • AWS Libcrypto FIPS - A FIPS-compliant version of the above crate.

NOTE: Both versions of AWS Libcrypto have build dependencies that must be installed. For up-to-date info on these requirements, see that project’s README. If you experience issues building these crates, please file an issue here.

Once one of these implementations is chosen, you’re ready to go.

Creating a Hyper 1.0 client and using it with the SDK

Clients with hyper 1.0 support are currently located in the aws_smithy_experimental crate. Add that to your project’s Cargo.toml along with the service you want to use. A client can be set on shared config provided by the aws_config crate or on a service’s config struct.

use aws_smithy_experimental::hyper_1_0::{ CryptoMode, HyperClientBuilder };
use aws_smithy_runtime_api::client::behavior_version::BehaviorVersion;

#[tokio::main]
async fn main() {
    let http_client = HyperClientBuilder::default()
        // Here is were we choose a crypto mode.
        //
        // Our choices are:
        //
        // - `CryptoMode::Ring`
        // - `CryptoMode::AwsLc`
        // - `CryptoMode::AwsLcFips`
        //
        // In this example, we've chosen `ring` as the crypto provider.   
        .crypto_mode(CryptoMode::Ring)
        .build_https();
    let conf = aws_config::defaults(BehaviorVersion::latest())
        // Once we've built our client, all we need to do is set it
        // on either the shared config struct or the service config struct.
        .http_client(http_client)
        .load()
        .await;
    let client = aws_sdk_s3::Client::new(&conf);
    let buckets = client
        .list_buckets()
        .send()
        .await
        .expect("failed to list buckets");
    for bucket in buckets.buckets() {
        println!("{}", bucket.name().unwrap());
    }
}

If you encounter problems when using the Hyper 1.0 client

Adapting the SDKs to work with this new client has not been without issue, and we expect more bugs to be revealed as the new client enters wider use. As such, we do not currently recommend using the new client for critical workloads. However, the more people that try out the new client, the quicker we can identify and resolve those issues. To anyone that is willing to be an early adopter, the SDK team thanks you. If you experience issues or if you have other comments, please share them here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue
Projects
None yet
Development

No branches or pull requests

7 participants