| msg160392 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-11 00:12 |
| OpenSSL provides a method, SSL_CTX_set_default_verify_paths(), for loading a default certificate store, which is used by many distributions. In openSUSE, the default store is not a bundle, but a directory-based store, which is not supported at all by the SSL module in Python 2.7. A bug related to this was assigned to me here: https://bugzilla.novell.com/show_bug.cgi?id=761501 I created patches for the Python 2.7.3 and 3.2.3 SSL modules that will load the distribution-specific store if ca_certs is omitted. |
|
|
| msg160393 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-11 00:13 |
| Here's the patch for Python 3. |
|
|
| msg160397 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-05-11 04:42 |
| Well, this is basically a change in behaviour, so it could only go in the default branch (3.3). But 3.3 already has SSLContext.set_default_verify_paths(), so it's not obvious why the patch is needed. (furthermore, automatically selecting the system-wide certificates could be surprising. There may be applications where you don't want to trust them.) |
|
|
| msg160398 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-05-11 04:44 |
| By the way, I see that the original bug is against the python-requests library? Perhaps it would be better to advocate the use of load_verify_locations() there, since it's a more adequate place for a policy decision. (that said, Python's own urllib.request could perhaps grow a similar option) |
|
|
| msg160421 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-11 16:42 |
| load_verify_locations() is not available in Python 2.x. It was added in 3.x. Also, there is no way to load a directory-based certificate store at all in Python 2.x, which is why the bug was opened. |
|
|
| msg160422 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2012-05-11 16:44 |
| Sorry, by our policy this change is a new feature and cannot go into stable versions. |
|
|
| msg160429 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-11 17:24 |
| Fair enough. What about a patch to handle a directory store passed through the ca_certs parameter? As it stands now, it's impossible to load the distribution-supplied cert store on openSUSE. |
|
|
| msg160431 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-05-11 17:34 |
| > What about a patch to handle a directory store passed through the > ca_certs parameter? As it stands now, it's impossible to load the > distribution-supplied cert store on openSUSE. I'm afraid it would still be a new feature, unsuitable for a bugfix release. Other distros simply have both a directory-based cert store and a cert bundle. In Mageia I see both /etc/pki/tls/rootcerts/ (a directory-based cert store) and /etc/pki/tls/certs/ca-bundle.crt (a single file cert bundle). (yes, I hope they're synchronized :)) Generally, the only reason we would add a new feature in a bugfix release is if it's necessary to fix a security issue (such as the hash randomization feature). Here it's not necessary: you could simply ship a cert bundle in addition to the cert store. I suppose its generation is easily automated with a script. (and, yes, the ssl module has long lacked important features; its history is a bit bumpy) Again, for 3.3, a patch allowing urllib.request to call load_default_verify_locations() could be a good idea. |
|
|
| msg160437 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-11 18:06 |
| Something like this perhaps? --- a/Lib/urllib/request.py Fri May 11 13:11:02 2012 -0400 +++ b/Lib/urllib/request.py Fri May 11 11:03:02 2012 -0700 @@ -135,16 +135,19 @@ _opener = None def urlopen(url, data=None, timeout=socket._GLOBAL_DEFAULT_TIMEOUT, - *, cafile=None, capath=None): + *, cafile=None, capath=None, cadefault=True): global _opener if cafile or capath: if not _have_ssl: raise ValueError('SSL support not available') context = ssl.SSLContext(ssl.PROTOCOL_SSLv23) context.options |= ssl.OP_NO_SSLv2 - if cafile or capath: + if cafile or capath or cadefault: context.verify_mode = ssl.CERT_REQUIRED - context.load_verify_locations(cafile, capath) + if cafile or capath: + context.load_verify_locations(cafile, capath) + else: + context.load_default_verify_locations() check_hostname = True else: check_hostname = False |
|
|
| msg160824 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-05-16 10:14 |
| > Something like this perhaps? For example, yes. Now we need to find a way of testing this... |
|
|
| msg160919 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-16 19:20 |
| Ok, here's a patch with a test and documentation updates. |
|
|
| msg160921 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-05-16 19:24 |
| > Ok, here's a patch with a test and documentation updates. Ok, thanks! The only change I would make is that cadefault needs to be False by default; particularly because some platforms don't have a OpenSSL-compatible default CA store (Windows comes to mind) and all HTTPS requests would then start failing. You don't have to upload a new patch; I can change it when committing. |
|
|
| msg160922 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-05-16 19:26 |
| Oh, by the way, could you sign and send a contributor agreement? See http://www.python.org/psf/contrib/ (it is not a copyright assignment, just a formal licensing agreement) |
|
|
| msg160923 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-05-16 19:44 |
| New changeset f2ed5de1c568 by Antoine Pitrou in branch 'default': Issue #14780: urllib.request.urlopen() now has a `cadefault` argument to use the default certificate store. http://hg.python.org/cpython/rev/f2ed5de1c568 |
|
|
| msg160924 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-05-16 19:47 |
| Patch committed. I propose to close the issue, unless further enhancements are suggested. |
|
|
| msg160927 - (view) |
Author: James Oakley (jfunk) * |
Date: 2012-05-16 20:16 |
| Ok, perfect. I submitted a copy of the agreement. |
|
|
| msg245360 - (view) |
Author: Martin Panter (martin.panter) * |
Date: 2015-06-15 03:02 |
| The documentation of the default value “cadefault=False” was fixed in Issue 17977. A later change seems to have made this paramter redundant. Anyway I think this can be closed. |
|
|