This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Unsupported provider

classification
Title: Make SSLContext.set_default_verify_paths() work on Windows
Type: enhancement Stage: resolved
Components: Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, giampaolo.rodola, gvanrossum, intgr, janssen, pitrou, python-dev
Priority: high Keywords: patch

Created on 2013-10-18 23:18 by gvanrossum, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
load_default_certs.patch christian.heimes, 2013-11-22 01:43 review
load_default_certs2.patch christian.heimes, 2013-11-23 12:20 review
Messages (26)
msg200328 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-18 23:18
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 :-).
msg200335 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-18 23:58
Maybe once this is addressed we could also change urllib.request.urlopen() to default to cadefault=True?
msg200355 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-19 01:35
> Maybe once this is addressed we could also change urllib.request.urlopen() to default to cadefault=True?

I don't think it's ok to change the default and break compatibility. Passing True manually is easy enough.
msg200358 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-19 02:07
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.
msg200392 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-10-19 10:00
I've implemented most of the necessarily bindings in #17134. It's still missing trust setting checks and #16487 to load certs from memory or file object.
msg200394 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-10-19 10:13
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:

1) We must have a way to update the cert bundle outside the release cycle, e.g. with a download-able package from PyPI

2) 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
msg200407 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-19 11:53
> 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.

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".
msg200430 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-19 16:02
@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 <report@bugs.python.org>wrote:

>
> Antoine Pitrou added the comment:
>
> > 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.
>
> 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".
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue19292>
> _______________________________________
>
msg200432 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-10-19 16:09
Am 19.10.2013 18:02, schrieb Guido van Rossum:
> @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.

The usual issues: lack of time and too much to do.

> 
> @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?

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
msg200436 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-19 16:22
> @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 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.

> 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?

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.
msg200441 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-19 16:48
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 <report@bugs.python.org>wrote:

>
> Antoine Pitrou added the comment:
>
> > @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 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.
>
> > 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?
>
> 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.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue19292>
> _______________________________________
>
msg200442 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-19 16:51
> 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.

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.
msg200443 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-19 16:53
But do you agree that the urllib default should be changed?
msg200444 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-10-19 16:55
> But do you agree that the urllib default should be changed?

Well, I'm fine for 3.4, even though I'm not particularly
enthusiastic :-)
msg200446 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-10-19 17:08
I fear it's a bit too late in the release cycle to get it right. Feature freeze is in about a month and this is a major change. 

The set_default_verify_paths() works only on some Unix platforms when OpenSSL configured with the distribution-specific paths to CAfile or CApath. A user installation of OpenSSL will most probably not work correctly. And there is Mac OS X ... Apple has deprecated OpenSSL and doesn't provide certificates as files. Apple's build of OpenSSL is patched and re-uses the keychain API.

My Windows patch only offers certificates that already exist in Windows' cert stores. IE can trigger background downloads of yet unknown
CA certs...

IMHO we should add root CA certs for every purpose with Python and implement a way to replace the shipped certs with update packages.
msg200455 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-10-19 18:01
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.
msg200493 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-10-19 20:11
Can somebody step in for #16487 please? For my stuff I just need to load DER as bytes and maybe PEM as str.
msg203711 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-22 01:43
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

#17134 Use Windows' certificate store for CA certs
#18138 ctx.load_verify_locations(cadata)
#19448 SSL: add OID / NID lookup
msg203715 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-11-22 02:44
Can you also add a patch to asyncio (I suppose to the code where it calls
set_default_verify_paths())?
msg203719 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-22 03:25
I have slightly different plans to make it even easier, #19689
msg203795 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2013-11-22 16:12
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.)
msg203991 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-11-23 12:20
New patch with enum (for Antoine), tests and documentation.
msg203994 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-11-23 12:57
New changeset dfd33140a2b5 by Christian Heimes in branch 'default':
Issue #19292: Add SSLContext.load_default_certs() to load default root CA
http://hg.python.org/cpython/rev/dfd33140a2b5
msg213003 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-03-10 01:35
New changeset 35a5284d5388 by R David Murray in branch 'default':
whatsnew: SSLContext.load_default_certs (#19292).
http://hg.python.org/cpython/rev/35a5284d5388
msg240674 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2015-04-13 17:28
I think we can close this bug for good.
msg240675 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2015-04-13 17:28
Great! Thank you!
History
Date User Action Args
2022-04-11 14:57:52adminsetgithub: 63491
2015-04-13 17:28:31pitrousetstatus: open -> closed
resolution: fixed
messages: + msg240675

stage: patch review -> resolved
2015-04-13 17:28:04christian.heimessetmessages: + msg240674
2015-04-13 17:15:50pitrousetassignee: pitrou ->
2014-03-10 01:35:01python-devsetmessages: + msg213003
2013-11-23 12:57:07python-devsetnosy: + python-dev
messages: + msg203994
2013-11-23 12:20:48christian.heimessetfiles: + load_default_certs2.patch
assignee: pitrou
messages: + msg203991
2013-11-22 16:12:30gvanrossumsetmessages: + msg203795
2013-11-22 03:25:13christian.heimessetmessages: + msg203719
2013-11-22 02:44:23gvanrossumsetmessages: + msg203715
2013-11-22 01:43:09christian.heimessetfiles: + load_default_certs.patch
priority: normal -> high


keywords: + patch
nosy: + janssen, giampaolo.rodola
messages: + msg203711
stage: needs patch -> patch review
2013-10-21 10:00:43intgrsetnosy: + intgr
2013-10-19 20:11:17christian.heimessetmessages: + msg200493
2013-10-19 18:01:06gvanrossumsetmessages: + msg200455
2013-10-19 17:08:10christian.heimessetmessages: + msg200446
2013-10-19 16:55:18pitrousetmessages: + msg200444
2013-10-19 16:53:56gvanrossumsetmessages: + msg200443
2013-10-19 16:51:52pitrousetmessages: + msg200442
2013-10-19 16:48:20gvanrossumsetmessages: + msg200441
2013-10-19 16:22:48pitrousetmessages: + msg200436
2013-10-19 16:09:50christian.heimessetmessages: + msg200432
2013-10-19 16:02:19gvanrossumsetmessages: + msg200430
2013-10-19 11:53:07pitrousetmessages: + msg200407
2013-10-19 10:13:35christian.heimessetmessages: + msg200394
2013-10-19 10:00:25christian.heimessetmessages: + msg200392
2013-10-19 02:07:11gvanrossumsetmessages: + msg200358
2013-10-19 01:35:04pitrousetnosy: + pitrou
messages: + msg200355
2013-10-19 01:29:48pitrousetnosy: + christian.heimes

type: security -> enhancement
versions: - Python 3.3
2013-10-18 23:58:15gvanrossumsetmessages: + msg200335
2013-10-18 23:18:54gvanrossumcreate