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

Remove rustls-native-roots dependency #110

Merged
merged 3 commits into from
Jun 25, 2021
Merged

Remove rustls-native-roots dependency #110

merged 3 commits into from
Jun 25, 2021

Conversation

jsha
Copy link
Collaborator

@jsha jsha commented Jun 14, 2021

There are some issues integrating with trust stores: rustls/rustls-native-certs#16, and also some issues with regards to what gets run in forked processes: kornelski/rust-security-framework#136.

@jsha jsha requested a review from tgeoghegan June 14, 2021 05:56
Copy link
Collaborator

@tgeoghegan tgeoghegan left a comment

Choose a reason for hiding this comment

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

This breaks the tests on macOS, because /etc/ssl/certs/ca-certificates.crt doesn't exist there. I don't have it on my Fedora 34 install either; instead I have /etc/ssl/certs/ca-bundle.crt. And I doubt that bundle exists on Windows. curl hosts the Mozilla CA bundle at https://curl.se/ca/cacert.pem, so perhaps we could at least make CI tests behave reliably by making the makefile fetch the cert bundle before running crustls-demo.

@tgeoghegan
Copy link
Collaborator

Or an alternative that wouldn't depend on httpbin.org's certain chain being trusted by the CA bundle currently hosted by curl.se would be to fetch the cert chain for httpbin.org and then pass that to crustls-demo as a trust store, since the point of crustls's tests is to exercise making connections over HTTPS rather than certificate trust evaluation. Continuing on that line of thought, the most robust way forward might be to have the tests spin up a local HTTPS server with a self-signed certificate, which would remove all external dependencies and even make it possible to run crustls tests without reaching the internet.

@jsha
Copy link
Collaborator Author

jsha commented Jun 24, 2021

Merged latest main, which includes a switch to talking to a local server and using a locally trusted root.

@jsha jsha dismissed tgeoghegan’s stale review June 24, 2021 23:56

Addressed changes.

@jsha jsha merged commit ea868de into main Jun 25, 2021
@jsha jsha deleted the remove-native-roots branch June 25, 2021 01:45
@jsha jsha mentioned this pull request Sep 30, 2021
5 tasks
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.

2 participants