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 in some logic to select a CA bundle #10

Closed
wants to merge 2 commits into from
Closed

Add in some logic to select a CA bundle #10

wants to merge 2 commits into from

Conversation

erinn
Copy link

@erinn erinn commented Jan 23, 2013

This was mainly inspired by https://github.com/google/signet, but I didn't think having to hard code in the path to the CA file was worth it, so I added in a check for the SSL_CERT_FILE environment variable and used a default set of locations to try and find the system CA bundle.

@Kami
Copy link
Owner

Kami commented Jan 23, 2013

@erinn Thanks.

I actually need to update httplib_ssl.py with a new version from Libcloud (https://github.com/apache/libcloud/blob/trunk/libcloud/httplib_ssl.py), because version which is currently bundled is old and not robust enough.

I will try to do this later today.

@erinn
Copy link
Author

erinn commented Jan 23, 2013

I sent a pull request for libcloud to allow an override, so you can pull it from there. I think I will expand the CA list given in security in libcloud and that should be about it.

@Kami
Copy link
Owner

Kami commented Jan 24, 2013

@erinn I've pushed a branch with a bunch of fixes - #11.

I've decided to use requests library instead of Libcloud HTTP code. Code which handles HTTP and SSL certificate validation in Libcloud is overly complex. One of the main reasons for this is that Libcloud needs to support a wide range of Python versions. That's not the case for this library so I just went with simpler approach and used requests library.

In this new branch you can specify a path to a custom CA certificates bundle file by setting yubico.CA_CERTS_BUNDLE_PATH variable.

If there are no major objections to changes in the new branch and plan to merge it and push new version to PyPi in a day or two.

@erinn
Copy link
Author

erinn commented Jan 24, 2013

Using requests is certainly a lot nicer than using urllib. However, it still ends up at the same place, I was trying to remove a value that has to be coded in, with something that looks in common locations for CA bundles and loads them if found.

I would say that having the CA bundle found and loaded for you would be a great convenience, so the basic premise is still there. Let me know if you would like this done for your branch. However, my guess is that you prefer having the location set by hand.

@Kami
Copy link
Owner

Kami commented Jan 24, 2013

@erinn I'm fine with looking for CA bundle path in common places (excluding locations in home directory) and using the first bundle found.

Is this what you had in mind or you want to do something else?

@erinn
Copy link
Author

erinn commented Jan 24, 2013

Yep that is what I had in mind, you can easily take what I have above and put it into place in your branch. You will want to remove the last 3-6 lines in the list of possible locations (not sure how valuable windows locations would be) to fit your needs.

As well as we were sort of discussing in libcloud using os.getenv may or may not be valuable.

Kami added a commit that referenced this pull request Jan 24, 2013
@Kami
Copy link
Owner

Kami commented Jan 24, 2013

Added in f8187e8. Thanks!

@Kami Kami closed this Jan 24, 2013
@Kami
Copy link
Owner

Kami commented Jan 24, 2013

Just wanted to let you know that a new version with this and a bunch of other improvements and fixes has been published to PyPi - http://pypi.python.org/pypi/yubico/1.6.0

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