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

Document the security extra for SNI support with legacy Python #4825

Closed
wants to merge 3 commits into from

Conversation

browniebroke
Copy link
Contributor

@browniebroke browniebroke commented Oct 15, 2018

This was added back in #2195.

This started as I was trying to understand why we had PyOpenSSL installed on the project I joined recently. After a bit of digging, it seems that it came from the time when we were using Python 2 and was for Requests/urllib3.

However, looking at Requests' documentation, I couldn't find a clear explanation about this feature, so this is my attempt at fixing this.

Disclaimer: I'm by no means an expert at SNI, so please correct me on anything wrong that follows.

Here are what I could find:

I'm not sure if this is the best place to document this, adding it to the "install" section feels overwhelming for beginners, and I felt like it belongs close to the SSL & certificates verification of the advanced section. Happy to move it somewhere else.

Edit:

(*) Actually, it seems it was implemented in PEP 476 which covers Python >=2.7.9 or >=3.4.3, so not any Python 3 versions.

SNI support with legacy Python
------------------------------

.. versionadded:: 2.4.1
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've put this versionadded directive, but since it's quoting a very old version, it might not make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was originally added for SNI support but further is really just a way of providing modern OpenSSL for old versions of Python too. It allows them to communicate with modern websites regardless of SNI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback. I've updated the change to be less focused on SNI, and direct to the upstream documentation for urllib3.

@codecov-io
Copy link

codecov-io commented Oct 15, 2018

Codecov Report

Merging #4825 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4825   +/-   ##
=======================================
  Coverage   66.89%   66.89%           
=======================================
  Files          15       15           
  Lines        1577     1577           
=======================================
  Hits         1055     1055           
  Misses        522      522

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c501ec9...ee76291. Read the comment docs.

Copy link
Contributor

@sigmavirus24 sigmavirus24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a single facet of the issue

SNI support with legacy Python
------------------------------

.. versionadded:: 2.4.1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was originally added for SNI support but further is really just a way of providing modern OpenSSL for old versions of Python too. It allows them to communicate with modern websites regardless of SNI.

@sethmlarson
Copy link
Member

Closing this as we're not recommending the security extra any longer.

@browniebroke browniebroke deleted the doc/sni-install branch August 19, 2020 08:07
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants