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.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:30 | admin | set | github: 58985 |
2015-06-15 03:02:23 | martin.panter | set | status: open -> closed nosy:
+ martin.panter messages:
+ msg245360
|
2012-05-16 20:16:22 | jfunk | set | messages:
+ msg160927 |
2012-05-16 19:47:02 | pitrou | set | resolution: fixed messages:
+ msg160924 stage: resolved |
2012-05-16 19:44:39 | python-dev | set | nosy:
+ python-dev messages:
+ msg160923
|
2012-05-16 19:26:13 | pitrou | set | messages:
+ msg160922 |
2012-05-16 19:24:44 | pitrou | set | messages:
+ msg160921 |
2012-05-16 19:20:31 | jfunk | set | files:
+ cpython-urllib_urlopen_cadefault.patch
messages:
+ msg160919 |
2012-05-16 10:14:48 | pitrou | set | nosy:
+ 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:25 | jfunk | set | messages:
+ msg160437 |
2012-05-11 17:34:02 | pitrou | set | messages:
+ msg160431 |
2012-05-11 17:24:00 | jfunk | set | messages:
+ msg160429 |
2012-05-11 16:44:04 | eric.araujo | set | nosy:
+ eric.araujo
messages:
+ msg160422 versions:
+ Python 3.3 |
2012-05-11 16:42:01 | jfunk | set | messages:
+ msg160421 |
2012-05-11 04:44:23 | pitrou | set | messages:
+ msg160398 |
2012-05-11 04:42:37 | pitrou | set | nosy:
+ pitrou messages:
+ msg160397
|
2012-05-11 00:13:12 | jfunk | set | files:
+ python-3.2.3-ssl_default_certs.patch
messages:
+ msg160393 |
2012-05-11 00:12:13 | jfunk | create | |