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 'native-tls' instead of rustls for reqwest #38

Closed
wants to merge 4 commits into from

Conversation

SteiMi
Copy link

@SteiMi SteiMi commented Oct 24, 2023

This aims to solve #36.

Instead of using 'rustls-tls' feature of reqwest, this uses 'native-tls'. This checks the certificates trusted by the operating system, which allows the user to easily trust custom self-signed certificates.

This built fine for me locally on Windows. Let's hope that the CI can build this successfully for all platforms.

Note: I also tried the feature 'rustls-tls-native-roots' which should theoretically also take certificates trusted by the OS into consideration, but I could not get it to trust my certificate.

@McPatate
Copy link
Member

McPatate commented Feb 9, 2024

Is what you're looking to achieve not possible with rust-tls?

@FileMagic
Copy link

Before reading anything here just know I am a rust noob. I am just curious.

Is what you're looking to achieve not possible with rust-tls?

I think it is possible with rust-tls after some cursory glances at some docs.

I was just coming around to this for I saw that this repo is the one related to this issue here in llm-vscode. I think after reading this could instead be usage of rustls-native-certs instead of using native-tls.

Reading through the reqwests docs here tells me that the usage of native-tls will probs give us the best by platform import of tls certs trusted by the os.

However, what is the reasoning behind using rust-tls over native-tls? I don't see a reason to not use native-tls outside of the openssl compiling dependency?

TLDR: I think this MR is just to support using tls certs trusted by the host os, and I wonder if the reason they want to use native-tls is to just support tls certs by default on this host instead of having to disable tls which is what most people are doing.

@FileMagic
Copy link

Responding to myself here, I realized that you said in the #36 (comment) that you had issues with compiling using openssl.

So I think it would be more reasonable to switch to the usage of rustls-native-certs instead which exposes pub fn load_native_certs() -> Result<Vec<pki_types::CertificateDer<'static>>, std::io::Error>. I may write up a PR based on this here soon then to get a fix out for this issue.

@McPatate
Copy link
Member

Closing in favor of #77

@McPatate McPatate closed this Feb 13, 2024
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.

3 participants