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

Add support for reading custom cert paths #104

Conversation

Owen-CH-Leung
Copy link
Contributor

@Owen-CH-Leung Owen-CH-Leung commented Nov 6, 2023

Fixes #103

As per discussed in #103 , this PR adds support to use custom ca cert via setting the env variable REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE that points to the custom cert file

@Owen-CH-Leung Owen-CH-Leung marked this pull request as ready for review November 6, 2023 14:46
@Jake-Shadle
Copy link
Owner

I assume these variables are commonly set by those who use custom certs? If it is I guess this is fine, but personally I find them quite confusing since lib/curl nor requests (I guess python?) is used in any way by this library.

@Owen-CH-Leung
Copy link
Contributor Author

I assume these variables are commonly set by those who use custom certs? If it is I guess this is fine, but personally I find them quite confusing since lib/curl nor requests (I guess python?) is used in any way by this library.

Yes. To my knowledge, REQUESTS_CA_BUNDLE is commonly used in python while CURL_CA_BUNDLE is commonly used when using the curl command line tool.

https://stackoverflow.com/questions/42982143/python-requests-how-to-use-system-ca-certificates-debian-ubuntu
https://curl.se/docs/sslcerts.html

I guess we can also use SSL_CERT_FILE which is commonly used for openssl. What do you think ?

https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_default_verify_paths.html

@Jake-Shadle
Copy link
Owner

I guess as long as they are documented it doesn't really matter

@Jake-Shadle Jake-Shadle changed the title Add support to env variables REQUESTS_CA_BUNDLE and CURL_CA_BUNDLE Add support for reading custom cert paths Nov 7, 2023
@Jake-Shadle Jake-Shadle merged commit eee7f11 into Jake-Shadle:main Nov 7, 2023
8 checks passed
@@ -16,7 +16,7 @@ default = ["rustls-tls"]
rustls-tls = ["ureq/tls"]
# If this feature is enabled we instead use the native TLS implementation for the
# target platform
native-tls = ["ureq/native-tls", "native-tls-crate/vendored"]
native-tls = ["ureq/native-tls", "native-tls-crate/vendored", "rustls-pemfile", "rustls"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe rustls isn't required here, only rustls-pemfile is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants