Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changes related to SSL context #489

Merged
merged 3 commits into from
Jun 3, 2022

Conversation

notEvil
Copy link

@notEvil notEvil commented May 12, 2022

Fixes #486 (introduced with python 3.10)

  • SocketStream.ssl_connect now handles all arguments correctly (ca_certs and ciphers were incorrectly passed to context.wrap_socket)
  • SSLAuthenticator now also uses SSLContext which enables hostname check (see https://docs.python.org/3/library/ssl.html#ssl.SSLContext.check_hostname)
  • Both now rely on ssl.create_default_context to set reasonable defaults for ssl_version and cert_reqs. Former defaults led to inconsistencies (issue 486 is due to deprecation of PROTOCOL_TLS in python 3.10; check_hostname is enabled by default in python 3.10 conflicting with former default CERT_NONE)

I ran the ssl unittest with python 3.6 to check backwards compat

edit:
I left the docstrings untouched because I expect opinions regarding the switch to create_default_context. When the changes are accepted I don't mind updating the PR with changes to docstrings and the doc

@comrumino
Copy link
Collaborator

At a glance, things look good. I would be cool w/ you updating the docstrings. I will most likely merge after that. Thanks!

@comrumino comrumino self-assigned this May 17, 2022
@comrumino comrumino added the Triage Investigation by a maintainer has started label May 17, 2022
@notEvil
Copy link
Author

notEvil commented May 29, 2022

In case you missed the commit, its there. Feel free to adjust

@comrumino
Copy link
Collaborator

I did miss the commit. Thanks for posting a comment! I'm working on getting more active in open-source again. Thanks for being a gem!

@comrumino comrumino merged commit 5e50641 into tomerfiliba-org:master Jun 3, 2022
@comrumino comrumino added Done The issue discussion is exhausted and is closed w/ comment and removed Triage Investigation by a maintainer has started labels Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Done The issue discussion is exhausted and is closed w/ comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_ssl.py: ssl.SSLError: [SSL: NO_CIPHERS_AVAILABLE] no ciphers available (_ssl.c:1001)
2 participants