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

Ability to run in FIPS compliant mode using aws-lc-rs #966

Open
2 tasks
adnan-shoreline opened this issue Nov 22, 2023 · 11 comments
Open
2 tasks

Ability to run in FIPS compliant mode using aws-lc-rs #966

adnan-shoreline opened this issue Nov 22, 2023 · 11 comments
Labels
feature-request A feature should be added or improved. p2 This is a standard priority issue

Comments

@adnan-shoreline
Copy link

Describe the feature

aws-sdk-rust can be configured to connect to AWS FIPS endpoints. However, the client side is still not FIPS compliant. AWS now has a FIPS compliant crypto rust library: https://github.com/aws/aws-lc-rs. It would be very useful to have an option to use this library when FIPS is enabled.

Use Case

Needed for FIPS compliance.

Proposed Solution

No response

Other Information

No response

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
@adnan-shoreline adnan-shoreline added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 22, 2023
@Velfi
Copy link
Contributor

Velfi commented Nov 22, 2023

Hey @adnan-shoreline. Thanks for submitting this. We've looked at using aws-lc-rs instead of ring but the fips feature requires Go and cmake to build it. We don't think it's reasonable to require our customers to install CMake and Go just to use Rust, hence why we haven't made the switch yet.

Once those dependencies are removed, then we should be able to "drop in" aws-lc-rs very easily. However, I don't think our team or the aws-lc-rs team will be prioritizing that soon.

@Velfi Velfi removed the needs-triage This issue or PR still needs to be triaged. label Nov 22, 2023
@adnan-shoreline
Copy link
Author

Thanks @Velfi

In the meantime, is there a workaround that we can follow to achieve fips compliance? We are fine putting in the effort (we have no choice). Thanks!

@adnan-shoreline
Copy link
Author

For example, the ability to use native-tls could really help. We can have fips enabled openssl in our environment. If we can make aws-rust-sdk use that for https, that would be really great. Thanks!

@Velfi
Copy link
Contributor

Velfi commented Nov 24, 2023

It should be possible for you to create your own connector based on the native-tls crate. We used to include this option but removed it b/c the native-tls crate provides poor forwards compatibility so we didn't want to risk depending on it.

We have a test that creates a native-tls connector here.

The code would look something like this:

use aws_smithy_runtime::client::http::hyper_014::HyperClientBuilder;
use some_aws_sdk::{Client, Config};

/// A `hyper` connector that uses the `native-tls` crate for TLS. To use this in a Smithy client,
/// wrap with a [`HyperClientBuilder`].
pub type NativeTlsConnector = hyper_tls::HttpsConnector<hyper::client::HttpConnector>;

fn native_tls_connector() -> NativeTlsConnector {
    // You'll likely have to load and configure some certs here.
    let tls_connector = hyper_tls::native_tls::TlsConnector::builder()
        .min_protocol_version(Some(hyper_tls::native_tls::Protocol::Tlsv12))
        .build()
        .unwrap_or_else(|e| panic!("error while creating TLS connector: {}", e));
    let mut http_connector = hyper::client::HttpConnector::new();
    http_connector.enforce_http(false);
    hyper_tls::HttpsConnector::from((http_connector, tls_connector.into()))
}

#[tokio::main]
async fn main() {
    let config = Config::builder()
        .http_client(HyperClientBuilder::new().build(native_tls_connector()))
        .endpoint_url(format!("https://{DEFAULT_DOMAIN}:{DEFAULT_PORT}"))
        .build();
    let client = Client::from_conf(config);
    
    // use the SDK
}

@adnan-shoreline
Copy link
Author

@Velfi we got it working with native tls. thanks for sharing the example! we're just wondering about the forward compatibility concern you mentioned. does it just mean that if we upgrade aws rust sdk, the native tls related code might not compile and so we'll have to make some code changes? or does it mean that we can have runtime issues? thanks!

@samster25
Copy link

Hi @Velfi, we would also love to be able to continue using native-tls (and have been stuck on an older version of the SDK aws-smithy-client=0.55.3) or switch to aws-lc-rs. When running with rustls / ring we hit Sending fatal alert BadRecordMac errors running large scale workloads on S3. However I'll give your workaround a shot with a newer version of the SDK!

@jmklix jmklix added the p2 This is a standard priority issue label Dec 8, 2023
@Velfi
Copy link
Contributor

Velfi commented Feb 15, 2024

One of the limitations that prevents us from using aws-lc-rs is being worked on: aws/aws-lc-rs#317

@samster25
Copy link

It looks like rustls default provider is now aws-lc-rs

Does that make this integration a bit easier?

https://github.com/rustls/rustls/releases/tag/v%2F0.23.0

@jdisanti
Copy link
Contributor

jdisanti commented Mar 6, 2024

Yeah, that will definitely make it easier. Beyond that, I think there's still some remaining work to do in our other uses of ring that are not TLS to make things FIPS compliant.

@zinid
Copy link

zinid commented Mar 12, 2024

@Velfi any thoughts on how to perform certificate revocation checks using native-tls? In the openssl wrapper I can use the callbacks, but I cannot find anything relevant for native-tls. Note that revocation checks are the requirement for FedRAMP environments.

@rcoh
Copy link
Contributor

rcoh commented Apr 24, 2024

this is now supported if you use our experimental support for Hyper 1.0: https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/s3/tests/hyper-10.rs

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