Repository navigation
Make SSLContext.set_default_verify_paths() work on Windows #63491
Description
| BPO | 19292 |
|---|---|
| Nosy | @gvanrossum, @pitrou, @giampaolo, @tiran, @intgr |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2015-04-13.17:28:31.735>
created_at = <Date 2013-10-18.23:18:54.207>
labels = ['type-feature']
title = 'Make SSLContext.set_default_verify_paths() work on Windows'
updated_at = <Date 2015-04-13.17:28:31.734>
user = 'https://github.com/gvanrossum'bugs.python.org fields:
activity = <Date 2015-04-13.17:28:31.734>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2015-04-13.17:28:31.735>
closer = 'pitrou'
components = []
creation = <Date 2013-10-18.23:18:54.207>
creator = 'gvanrossum'
dependencies = []
files = ['32768', '32793']
hgrepos = []
issue_num = 19292
keywords = ['patch']
message_count = 26.0
messages = ['200328', '200335', '200355', '200358', '200392', '200394', '200407', '200430', '200432', '200436', '200441', '200442', '200443', '200444', '200446', '200455', '200493', '203711', '203715', '203719', '203795', '203991', '203994', '213003', '240674', '240675']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'janssen', 'pitrou', 'giampaolo.rodola', 'christian.heimes', 'intgr', 'python-dev']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19292'
versions = ['Python 3.4']
Activity
gvanrossum commentedon Oct 18, 2013
See discussion on https://groups.google.com/forum/#!topic/python-tulip/c_lqdFjPEbE .
If you set sslcontext.verify_mode = ssl.CERT_REQUIRED and call sslcontext.set_default_verify_paths(), the stdlib ought to have enough smarts to use the system root certificates.
I understand this is difficult, as the location of the root certificates may vary between Windows versions or installations. But if we leave this up to the app developer they are much more likely to disable certificate verification by setting verify_mode to CERT_NONE than to provide secure root certs (or do even less secure things, like using plain HTTP :-).
gvanrossum commentedon Oct 18, 2013
Maybe once this is addressed we could also change urllib.request.urlopen() to default to cadefault=True?
pitrou commentedon Oct 19, 2013
I don't think it's ok to change the default and break compatibility. Passing True manually is easy enough.
gvanrossum commentedon Oct 19, 2013
Why is this not a security patch? Because it's not a "vulnerability" in the narrow technical sense? I expect that it will greatly increase the actual practical security, by making it easier to do the right thing.
tiran commentedon Oct 19, 2013
I've implemented most of the necessarily bindings in bpo-17134. It's still missing trust setting checks and bpo-16487 to load certs from memory or file object.
tiran commentedon Oct 19, 2013
http://www.python.org/dev/peps/pep-0453/#bundling-ca-certificates-with-cpython proposes that ensurepip comes with a default CA cert bundle, too. I see two issues with the proposal:
We must have a way to update the cert bundle outside the release cycle, e.g. with a download-able package from PyPI
CA certs can have an implicit purpose that is not part of the X.509 cert. A cert may only apply to server certs, client certs, S/MIME and/or other purposes like software signing. I have found a couple of issues in NSS certdata parsers and cert bundles like curl, Egenix OpenSSL (both fixed) and Ubuntu (not fixed yet). In order to get it right we need a separate bundle for every purpose. See https://bugs.launchpad.net/ubuntu/+source/ca-certificates/+bug/1207004
A while ago I started a PEP about the topic but it's not done yet. https://bitbucket.org/tiran/peps/src/tip/pep-9999.txt
pitrou commentedon Oct 19, 2013
IMO it's not a vulnerability. It's not a security hole in Python: the
flag is there for people to turn on or off, and the whole thing is
documented (with a highly visible red warning). The situation is
actually much better than in 2.7.
I would also like to point out Python isn't a Web browser: its use cases
are wider, and there's no default interactive UI to allow the user to
bypass certificate issues (which are still common nowadays on the
Internet). I think it makes it much less appropriate to be "strict by
default".
gvanrossum commentedon Oct 19, 2013
@christian: What is holding up those patches? I don't believe we should be
in the business of distributing certificates -- we should however make it
easy to use the system certificates.
@antoine: I still claim that a flag that defaults to no security is a
vulnerability -- nobody reads warnings in docs until *after* they've been
bitten. It should be an explicit choice in the script or app to disable
certificate checking. If you can't access a server because its certificate
is expired, how is that different than any other misconfiguration that
makes a server inaccessible until its administrator fixes it?
On Sat, Oct 19, 2013 at 4:53 AM, Antoine Pitrou <[email protected]>wrote:
tiran commentedon Oct 19, 2013
Am 19.10.2013 18:02, schrieb Guido van Rossum:
The usual issues: lack of time and too much to do.
It would be nice to add a feature to the SSL module that behaves like
browsers: white list a cert's SPKI (subject private key info) for a FQDN
+ Port.
Christian
pitrou commentedon Oct 19, 2013
If we were introducing urllib right now, I agree it would be better to
be "secure by default". But urlopen() has been there for a long time,
it's used a lot and changing the default will break some of the scripts
or apps that have been working fine for ages.
It's different because the server isn't inaccessible. Its HTTPS service
works fine, it's just that the certificate is untrustable according to
some chosen security settings.
(besides, an expired certificate is not a "misconfiguration"; it worked
fine until it expired; not all systems have dedicated paid
administrators, so this situation is not uncommon)
Again, I think it may be reasonable to change in 3.4, but not in bugfix
versions.
By the way, I seem to remember even svn.python.org used a CACert-emitted
certificate during a long time... a cert that wasn't validated by major
browsers.
gvanrossum commentedon Oct 19, 2013
So you agree that we should change the urllib default in 3.4? I'm all for
that.
Tools like svn and hg have extensive configurations for this purpose, and
(at least hg) secure defaults; I certainly remember having to deal with hg
complaining about the security of some repo site, where the fix was
something I had to put in my .hgrc. That's interactive enough.
These days, I wouldn't trust a site that lets its certificate expire with
anything important.
On Sat, Oct 19, 2013 at 9:22 AM, Antoine Pitrou <[email protected]>wrote:
pitrou commentedon Oct 19, 2013
It is not about svn and hg, it was just an example that even in the
python.org realm we have used certificates that were (deliberately, in a
way) untrusted by major software.
3 remaining items
gvanrossum commentedon Oct 19, 2013
No, please let's not get in the business of shipping certs. Please not.
There should be only *one* place per system where sysadmins have to update
certs. It would not scale if every language implementation were to have its
own set of certs.
Trusting only certs already on the system sounds fine.
Reading certs from memory sounds like a good start no matter whether we
manage to get the rest working, so please prioritize that.
The next step should be fixing set_default_verify_paths() for Windows (at
least for somewhat recent versions).
On OS X it becomes a priority once the default build no longers use the
system openssl.
tiran commentedon Oct 19, 2013
Can somebody step in for bpo-16487 please? For my stuff I just need to load DER as bytes and maybe PEM as str.
tiran commentedon Nov 22, 2013
The patch implements a new method SSLContext.load_default_certs(). A new method is a required because set_default_verify_paths() doesn't have a way to specify a purpose. Every cert store allows the user to specify the purpose of a certificate (e.g. suitable for every purpose or just for serverAuth and clientAuth). The feature is supported by NSS certdata.txt, Windows API and Apple's crypto API.
The patch is rather simple and uses features implemented in issues
bpo-17134 Use Windows' certificate store for CA certs
bpo-18138 ctx.load_verify_locations(cadata)
bpo-19448 SSL: add OID / NID lookup
gvanrossum commentedon Nov 22, 2013
Can you also add a patch to asyncio (I suppose to the code where it calls
set_default_verify_paths())?
tiran commentedon Nov 22, 2013
I have slightly different plans to make it even easier, bpo-19689
gvanrossum commentedon Nov 22, 2013
So do you need anything on *this* issue?
(And are you asking me to review/approve the other issue? I haven't kept
track carefully enough for that, and the beta is looming.)
tiran commentedon Nov 23, 2013
New patch with enum (for Antoine), tests and documentation.
python-dev commentedon Nov 23, 2013
New changeset dfd33140a2b5 by Christian Heimes in branch 'default':
Issue bpo-19292: Add SSLContext.load_default_certs() to load default root CA
http://hg.python.org/cpython/rev/dfd33140a2b5
python-dev commentedon Mar 10, 2014
New changeset 35a5284d5388 by R David Murray in branch 'default':
whatsnew: SSLContext.load_default_certs (bpo-19292).
http://hg.python.org/cpython/rev/35a5284d5388
tiran commentedon Apr 13, 2015
I think we can close this bug for good.
pitrou commentedon Apr 13, 2015
Great! Thank you!