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

Fallback to webpki certs if no native certs found, in hyper client from aws_smithy_runtime. #1167

Open
1 of 2 tasks
JustusFluegel opened this issue Jun 17, 2024 · 6 comments
Labels
feature-request A feature should be added or improved. p3 This is a minor priority issue

Comments

@JustusFluegel
Copy link

JustusFluegel commented Jun 17, 2024

Describe the feature

fallback to the webpki root certs if no native certs are found

Use Case

I like to use a project that uses this sdk in a FROM scratch docker image in which no ca-certificates is available. Falling back to webpki certs if that happens would allow this usecase.

Proposed Solution

Update hyper_rustls to at least v0.25 (from v0.24, current latest version would be v0.27.2) and check the result returned by with_native_certs() starting from that version, calling with_webpki_certs if that errors. Basically replace the following code like so:

# https://github.com/awslabs/aws-sdk-rust/blob/3a5bf4831a8d024ae0903fef0a055bfbd726b041/sdk/aws-smithy-runtime/src/client/http/hyper_014.rs#L53
rustls::ClientConfig::builder()
                    ...
                    .with_native_roots()
                    ...

to

let config_without_certs = rustls::ClientConfig::builder()
                    ...;

let config_with_certs = config_without_certs.clone()
                    .with_native_roots().unwrap_or_else(|e| { todo!("probably some trace logs here"); config_without_certs.with_webpki_roots()})

config_with_certs
                    ...

(there is probably a nicer way to write it but you should get the gist of it)

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
@JustusFluegel JustusFluegel added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Jun 17, 2024
@JustusFluegel JustusFluegel changed the title (short issue description) Fallback to webpki certs if no native certs found. Jun 17, 2024
@JustusFluegel JustusFluegel changed the title Fallback to webpki certs if no native certs found. Fallback to webpki certs if no native certs found, in hyper client from aws_smithy_runtime. Jun 17, 2024
@ysaito1001
Copy link
Collaborator

Hi, thank you for submitting a feature request! A clarifying question. Without the suggested feature, can you work around by using the code snippet mentioned in this guide (search for with_webpki_roots) (i.e. writing your own HTTP connector enabling with_webpki_roots)?

@ysaito1001 ysaito1001 removed the needs-triage This issue or PR still needs to be triaged. label Jun 17, 2024
@JustusFluegel
Copy link
Author

JustusFluegel commented Jun 18, 2024

I will test that, thanks 👍( and get back to you afterwards ) Although I think even if that works a fallback to the webpki certs would probably still make sense :)

@jmklix jmklix added p3 This is a minor priority issue response-requested Waiting on additional info and feedback. Will move to 'closing-soon' in 7 days. labels Jul 19, 2024
Copy link

Greetings! It looks like this issue hasn’t been active in longer than a week. We encourage you to check if this is still an issue in the latest release. Because it has been longer than a week since the last update on this, and in the absence of more information, we will be closing this issue soon. If you find that this is still a problem, please feel free to provide a comment or add an upvote to prevent automatic closure, or if the issue is already closed, please feel free to open a new one.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jul 19, 2024
@JustusFluegel
Copy link
Author

I'll get back to this, just haven't found the time yet :)

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to 'closing-soon' in 7 days. labels Jul 19, 2024
@JustusFluegel
Copy link
Author

Ok so I've tested your suggestion and I can confirm that the following works in our code:

aws_sdk_s3::Config::builder()
    .http_client(
        HyperClientBuilder::new().build(
            HttpsConnectorBuilder::new()
                .with_webpki_roots()
                .https_only()
                .enable_http1()
                .enable_http2()
                .build(),
        ),
    )
    .....

@JustusFluegel
Copy link
Author

Although I still think a fallback to webpki certs makes sense if no native ones are found. Is it welcome that I try to implement that in a pr or would I be wasting my time on that? :)

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. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

3 participants