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

feat: use native TLS roots along webpki #1772

Merged
merged 1 commit into from
Mar 27, 2024

Conversation

rvolosatovs
Copy link
Member

@rvolosatovs rvolosatovs commented Mar 27, 2024

Feature or Problem

Include both webpki and native OS root certificates by default for TLS. This ensures that outgoing connections work everywhere - be it a FROM scratch Docker container, consumer OS behind a corporate firewall, an embedded device or anything in-between.

Eventually we should standardize TLS setup in providers and have a way to switch between the CA pools, but for now we should focus on getting things working.

The specified reqwest feature set ensures both bundles are included https://github.com/seanmonstar/reqwest/blob/14e46ff8cb7550473c950f3471d049ad2e139d3f/src/async_impl/client.rs#L501-L529 this fails when the OS bundle is missing, so simply load the native certs ourselves and log if we failed to load the native bundle

Related Issues

May fix - #1433

Release Information

Consumer Impact

Testing

Unit Test(s)

Acceptance or Integration

Manual Verification

@rvolosatovs rvolosatovs requested review from a team as code owners March 27, 2024 15:07
@rvolosatovs rvolosatovs enabled auto-merge (rebase) March 27, 2024 15:07
@rvolosatovs rvolosatovs force-pushed the feat/native-tls branch 2 times, most recently from fb86510 to 4a329e0 Compare March 27, 2024 15:12
@rvolosatovs rvolosatovs marked this pull request as draft March 27, 2024 15:18
auto-merge was automatically disabled March 27, 2024 15:18

Pull request was converted to draft

@rvolosatovs rvolosatovs marked this pull request as ready for review March 27, 2024 15:25
@rvolosatovs rvolosatovs enabled auto-merge (rebase) March 27, 2024 15:25
@rvolosatovs rvolosatovs marked this pull request as draft March 27, 2024 15:37
auto-merge was automatically disabled March 27, 2024 15:37

Pull request was converted to draft

Copy link
Contributor

@thomastaylor312 thomastaylor312 left a comment

Choose a reason for hiding this comment

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

I think this approach is straightforward and unblocks any cert issue we're seeing, so once this is ready for final review I can go ahead and approve

@rvolosatovs rvolosatovs force-pushed the feat/native-tls branch 10 times, most recently from bd1093f to 2f6849a Compare March 27, 2024 17:22
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
@rvolosatovs rvolosatovs marked this pull request as ready for review March 27, 2024 17:45
@rvolosatovs rvolosatovs enabled auto-merge (rebase) March 27, 2024 17:45
@rvolosatovs rvolosatovs merged commit 07b5e70 into wasmCloud:main Mar 27, 2024
59 checks passed
@rvolosatovs rvolosatovs deleted the feat/native-tls branch March 27, 2024 18:10
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.

None yet

2 participants