Skip to content

Load system default certificates if none are given#415

Merged
liris merged 3 commits into
websocket-client:masterfrom
minus7:master
May 27, 2018
Merged

Load system default certificates if none are given#415
liris merged 3 commits into
websocket-client:masterfrom
minus7:master

Conversation

@minus7

@minus7 minus7 commented Mar 21, 2018

Copy link
Copy Markdown
Contributor

When not providing a CA file or path, load the system default certificate stores instead. This may fail (according to ssl.create_default_context)

Fixes #413. Compatibility with older Python versions untested. Should work on 3.4+, but I couldn't find info on Python 2.

minus7 added 3 commits March 21, 2018 21:34
This changes behavior. When creating a TLS connection, not specifying
cacert/capath and while on a Python version without load_default_certs,
creating the socket will not fail as before, but verifying the
connection will fail instead.
@minus7

minus7 commented Mar 21, 2018

Copy link
Copy Markdown
Contributor Author

Removed the bundled cacert.pem to make this work as intended when the Python version supports loading default certs. It will not work without explicitly passing a cert bundle on other versions.

@minus7

minus7 commented Mar 21, 2018

Copy link
Copy Markdown
Contributor Author

It looks like support for system certs was added in #88 already, but it looks like that broke. Just removing cacert.pem causes an error message cafile, capath and cadata cannot be all omitted.

Regardless, relying on the SSLContext's method to load system certs seems like a saner option than bundling CA certs. Looking at that file's history it was last updated 4 years ago!

@liris liris merged commit bd66998 into websocket-client:master May 27, 2018
@minus7

minus7 commented Sep 2, 2018

Copy link
Copy Markdown
Contributor Author

FYI this should work on Python 2.7.9+,released 2014-12-10, in which SSLContext.load_default_certs was backported

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants