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

Use the system-native TLS CAs instead of rustls's built-in CAs #824

Merged
merged 2 commits into from
Oct 2, 2024

Conversation

josh-berry
Copy link
Contributor

Closes #818.

@josh-berry josh-berry requested a review from a team as a code owner September 27, 2024 21:22
@josh-berry
Copy link
Contributor Author

Note: This still needs to be manually tested, but I'm posting here anyway to get a CI run and because I don't think that testing has to block the (trivial) review.

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

LGTM. Deprecation error in CI is due to brand new tonic change release yesterday in 0.12.3 and we don't use lock files in this repo so it downloads the latest on each CI. Will have to change the deprecated method name to the new one and commit to get CI to pass.

@josh-berry
Copy link
Contributor Author

Did some manual testing:

  1. Created an nginx server to serve the temporal CLI (/dev-server) locally
  2. Created a custom CA for nginx
  3. Verified that cargo test --test=integ_tests temporal_cli_default failed with an SSL error
  4. Added the CA to my system's trusted CAs
  5. Verified that cargo test --test=integ_tests temporal_cli_default succeded (well, actually, failed with a different error since it was expecting JSON and I gave it a binary by mistake)

So I can confirm we are actually paying attention to the system CA roots with this feature flag changed. Merging accordingly.

@josh-berry josh-berry merged commit 8691fed into master Oct 2, 2024
6 checks passed
@josh-berry josh-berry deleted the josh/use-native-tls-roots-818 branch October 2, 2024 22:46
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.

[Bug] Broken download for ephemeral_server due to wrong CA
2 participants