classification
Title: urllib.request could use the default CA store
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: open Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.araujo, jfunk, orsenthil, pitrou, python-dev
Priority: normal Keywords: patch

Created on 2012-05-11 00:12 by jfunk, last changed 2012-05-16 20:16 by jfunk.

Files
File name Uploaded Description Edit
python-2.7.3-ssl_default_certs.patch jfunk, 2012-05-11 00:12 Patch for Python 2.7.3 to load distribution-specific certificate store if ca_certs is omitted
python-3.2.3-ssl_default_certs.patch jfunk, 2012-05-11 00:13 Patch for Python 3.2 to load distribution-specific certificate store if ca_certs is omitted
cpython-urllib_urlopen_cadefault.patch jfunk, 2012-05-16 19:20 Add a cadefault parameter to urllib.urlopen() review
Messages (16)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2012-05-16 20:16:22jfunksetmessages: + msg160927
2012-05-16 19:47:02pitrousetresolution: fixed
messages: + msg160924
stage: resolved
2012-05-16 19:44:39python-devsetnosy: + python-dev
messages: + msg160923
2012-05-16 19:26:13pitrousetmessages: + msg160922
2012-05-16 19:24:44pitrousetmessages: + msg160921
2012-05-16 19:20:31jfunksetfiles: + cpython-urllib_urlopen_cadefault.patch

messages: + msg160919
2012-05-16 10:14:48pitrousetnosy: + orsenthil

messages: + msg160824
title: SSL should use OpenSSL-defined default certificate store if ca_certs parameter is omitted -> urllib.request could use the default CA store
2012-05-11 18:06:25jfunksetmessages: + msg160437
2012-05-11 17:34:02pitrousetmessages: + msg160431
2012-05-11 17:24:00jfunksetmessages: + msg160429
2012-05-11 16:44:04eric.araujosetnosy: + eric.araujo

messages: + msg160422
versions: + Python 3.3
2012-05-11 16:42:01jfunksetmessages: + msg160421
2012-05-11 04:44:23pitrousetmessages: + msg160398
2012-05-11 04:42:37pitrousetnosy: + pitrou
messages: + msg160397
2012-05-11 00:13:12jfunksetfiles: + python-3.2.3-ssl_default_certs.patch

messages: + msg160393
2012-05-11 00:12:13jfunkcreate