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

corporate proxy got in the way and cannot specify a certificate bundle using en vars like such as CARGO_HTTP_CAINFO, SSL_CERT_FILE & SSL_CERT_PATH #759

Closed
NobodyXu opened this issue Feb 2, 2023 · 13 comments · Fixed by #774
Assignees
Labels
good first issue Good for newcomers Report: feature request New feature request

Comments

@NobodyXu
Copy link
Member

NobodyXu commented Feb 2, 2023

I found out about cargo-binstall through this awesome introductory blog post about cargo-dist. I am admittedly late to the party. Unfortunately, when I went to try this out at work, the corporate proxy got in the way and I can't yet figure out how to specify a certificate bundle like I can with other apps. I already have the customary environment variables set properly such as CARGO_HTTP_CAINFO, SSL_CERT_FILE, and SSL_CERT_PATH. This is a bit frustrating but doesn't have to be the end. Excited to learn more.

Originally posted by @yonkeltron in #19 (comment)

@NobodyXu NobodyXu changed the title I found out about cargo-binstall through [this awesome introductory blog post](https://blog.axo.dev/2023/02/cargo-dist) about cargo-dist. I am admittedly late to the party. Unfortunately, when I went to try this out at work, the corporate proxy got in the way and I can't yet figure out how to specify a certificate bundle like I can with other apps. I already have the customary environment variables set properly such as CARGO_HTTP_CAINFO, SSL_CERT_FILE, and SSL_CERT_PATH. This is a bit frustrating but doesn't have to be the end. Excited to learn more. corporate proxy got in the way and cannot specify a certificate bundle using en vars like such as CARGO_HTTP_CAINFO, SSL_CERT_FILE & SSL_CERT_PATH Feb 2, 2023
@NobodyXu NobodyXu added Report: feature request New feature request good first issue Good for newcomers labels Feb 2, 2023
@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 2, 2023

@yonkeltron It should be possible since we use reqwest for making http requests and it has ClientBuilder::add_root_certificate for adding a custom root certificate.

Not sure whether this will fix your issue since I do not have the experience of corporate proxy, but it does work for you, I am more than happy to add that feature.

@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 4, 2023

@passcod I'm not very familiar with corp proxy env, will using ClientBuilder::add_root_certificate to add root certificate help here?

@passcod
Copy link
Member

passcod commented Feb 4, 2023

Not familiar either. That sounds like it's intercepting TLS? Weird. Probably need to check out how cargo does it.

@yonkeltron
Copy link

yonkeltron commented Feb 5, 2023 via email

@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 8, 2023

@yonkeltron If that is the case, then can you check if ClientBuilder::add_root_certificate solve the issue based on how your corp proxy works?

If the answer is yes, then I can open a PR to add this feature and let you test it out.

@NobodyXu
Copy link
Member Author

NobodyXu commented Feb 8, 2023

Hmm, I will go ahead and create a PR so that you can test this.

@yonkeltron
Copy link

Hey I'm ready to give this a shot whenever you give me the go ahead. I'll happily provide all the feedback and experience info I can muster!

@NobodyXu
Copy link
Member Author

Hey I'm ready to give this a shot whenever you give me the go ahead. I'll happily provide all the feedback and experience info I can muster!

The PR linked with this issue is already ready for review and it also has pre-built artifacts for you to try out.

@yonkeltron
Copy link

Ok, after a false start owing to an error on my part, it seems to be working fine now! I love this! Can't wait to tell others once it's out. Is there any other info I can provide?

@NobodyXu
Copy link
Member Author

@yonkeltron Good to hear that this resolves this issue!

Since you are in corp env, is there anything else that we can make it easier to use binstall in that kinds of env or could potentially block usage of binstall?

@yonkeltron
Copy link

is there anything else that we can make it easier to use binstall in that kinds of env or could potentially block usage of binstall?

Right now, no. My biggest issues are a proxy and the cert it requires, both of which seem to work just fine right now. I think we're good to go but I'll keep my eyes open. Thanks so much for all of your help and support. It means a lot and your hard work really matters!

Thanks,
+Jonathan

@NobodyXu
Copy link
Member Author

Thank you Jonathan!

We would probably make a release this weekend or next Monday along with other fixes for http conns.
There are quite some issues due to the auto discovery mechanism creating quite a lot requests in a short time that we are now trying to resolve by increasing the interval in rate limiting and add more wait and retry logic to binstalk-downloader.

@NobodyXu
Copy link
Member Author

@yonkeltron #820 adds --root-certificates option and read from BINSTALL_HTTPS_ROOT_CERTS if the option is not specified.

I removed CARGO_HTTP_CAINFO, SSL_CERT_FILE and SSL_CERT_PATH since these env variables seem to be set in our CI somehow and it might accidentally break cargo-binstall for our users, so we decide to use a new env variable name that is specific for cargo-binstall.

I also added file-format as a dependency for more robust pem/der file detection and several cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers Report: feature request New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants