<p dir="auto">I'm requesting we reconsider the unconditional use of "urllib3.contrib.pyopenssl.inject_into_urllib3()".</p>
<p dir="auto">Added 7 years ago, the goal was to "add SNI support for Python 2" <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="5979220" data-permission-text="Title is private" data-url="https://github.com/psf/requests/issues/749" data-hovercard-type="issue" data-hovercard-url="/psf/requests/issues/749/hovercard" href="https://github.com/psf/requests/issues/749">#749</a></p>
<p dir="auto">"urllib3.contrib.pyopenssl.inject_into_urllib3()" is described to "Monkey-patch urllib3 with PyOpenSSL-backed SSL-support.": (<a href="https://github.com/urllib3/urllib3/blob/master/src/urllib3/contrib/pyopenssl.py">https://github.com/urllib3/urllib3/blob/master/src/urllib3/contrib/pyopenssl.py</a>)</p>
<p dir="auto">Request justification:<br>
1: Security/stability: Monkey patching to swap out standard library use for a 3rd party library should arguably be more surgical, especially for ssl. If the patch is for support in Python2, then at least check for a major version. Or even better, check if SNI support is already present ssl.HAS_SNI.</p>
<p dir="auto">2: Unnecessary: As of Dec. 10, 2014, the entirety of Python 3.4's ssl module has been backported for Python 2.7.9. See PEP 466 for justification. <a href="https://www.python.org/downloads/release/python-279/" rel="nofollow">https://www.python.org/downloads/release/python-279/</a>. This enables SNI support in the standard library for &gt; v2.7.9</p>
<p dir="auto">3: Inflexible: As implemented, there is no way to disable this behavior. The only option to prevent requests use of pyopenssl context is to uninstall pyopenssl for my entire python environment.<br>
Summary.</p>
<p dir="auto"><a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/WhyNotHugo/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/WhyNotHugo">@WhyNotHugo</a></p>
<p dir="auto">Requests added the <code class="notranslate">requests[security]</code> dependency many, many years ago because the ssl module in Python's stdlib was lacking some features. Since PEP 466 has been implemented in Python 2.7.9, the ssl module supports hostname verification and SNI. 2.7.9 was released in 2014. Python 3.7.0 improved the situation even further by delegating hostname verification to OpenSSL. The PyOpenSSL compatibility layer in urllib3 uses the deprecated <code class="notranslate">ssl.match_hostname</code> function or a backport.</p>
<p dir="auto">Unconditional monkey-patching of urllib3 with <code class="notranslate">urllib3.contrib.pyopenssl.inject_into_urllib3</code> is causing issues, too.</p>
<p dir="auto">PyOpenSSL uses dynamic libffi callbacks (also known as cffi old-style callbacks). The callbacks are implemented with trampolines and dynamic creation of native machine code. This either uses executable and writeable memory pages or some hacks with shared mmap() regions. Dynamic callbacks are a security risk and blocked by security frameworks like SELinux for good reasons (deny_execmem). Armin Rigo and I explored various ways to work around these problems, but there is simply solution. Eventually Armin implemented a new callback system for cffi. Some of the issues with old callbacks are documented at <a href="https://cffi.readthedocs.io/en/latest/using.html#callbacks-old-style" rel="nofollow">https://cffi.readthedocs.io/en/latest/using.html#callbacks-old-style</a> .</p>
<p dir="auto">PyOpenSSL currently pulls in a versions of cryptography, which itself depends on asn1crypto. A problem with asn1crypto and ctypes causes Python to segfault on recent macOS, <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="524066080" data-permission-text="Title is private" data-url="https://github.com/pyca/pyopenssl/issues/874" data-hovercard-type="issue" data-hovercard-url="/pyca/pyopenssl/issues/874/hovercard" href="https://github.com/pyca/pyopenssl/issues/874">pyca/pyopenssl#874</a> and <a class="issue-link js-issue-link" data-error-text="Failed to load title" data-id="509309071" data-permission-text="Title is private" data-url="https://github.com/wbond/asn1crypto/issues/158" data-hovercard-type="issue" data-hovercard-url="/wbond/asn1crypto/issues/158/hovercard" href="https://github.com/wbond/asn1crypto/issues/158">wbond/asn1crypto#158</a> .</p>
<p dir="auto">Due to bugs like <a href="https://bugzilla.redhat.com/show_bug.cgi?id=1535689" rel="nofollow">https://bugzilla.redhat.com/show_bug.cgi?id=1535689</a> I convinced the Fedora and RHEL maintainers to patch requests and drop <code class="notranslate">inject_into_urllib3</code> a while ago. The distros have been running with patch <a href="https://src.fedoraproject.org/rpms/python-requests/blob/master/f/Don-t-inject-pyopenssl-into-urllib3.patch" rel="nofollow">https://src.fedoraproject.org/rpms/python-requests/blob/master/f/Don-t-inject-pyopenssl-into-urllib3.patch</a> for over a year without any reported issues.</p>
<p dir="auto">I propose to:</p>
<ol dir="auto">
<li>Remove <code class="notranslate">requests[security]</code> extra requires.</li>
<li>Remove unconditionally monkey-patch of urllib3 from requests.</li>
</ol>