requests icon indicating copy to clipboard operation
requests copied to clipboard

SSLV3_ALERT_HANDSHAKE_FAILURE after upgrade from 2.31.0 to 2.32.2

Open luisvicenteatprima opened this issue 1 year ago • 13 comments

After upgrading to requests 2.32.* our custom SSL adapter doesn't seem to working anymore. This is how it looks like

class CustomSSLAdapter(HTTPAdapter):
    def __init__(self):
        self.context = ssl.create_default_context()

        certfile = ...
        keyfile = ...
        password = ...
        self.context.load_cert_chain(certfile=certfile, keyfile=keyfile, password=password)
        
        super().__init__()

    def init_poolmanager(self, *args, **kwargs):
        kwargs['ssl_context'] = self.context
        return super().init_poolmanager(*args, **kwargs)

We have tried to mount this only for the target url but also for "http://". The code was working find with version 2.31.0.

Expected Result

The adapter should still work.

Actual Result

We got the following error:

SSLError: HTTPSConnectionPool(host=..., port=543): Max retries exceeded with url: ... (Caused by SSLError(SSLError(1, '[SSL: SSLV3_ALERT_HANDSHAKE_FAILURE] sslv3 alert handshake failure (_ssl.c:1007)')))

luisvicenteatprima avatar May 22 '24 10:05 luisvicenteatprima

This is running on databricks using python 3.10.12.

luisvicenteatprima avatar May 22 '24 10:05 luisvicenteatprima

@nateprewitt this is a different side-effect of #6655. I think to enable this particular use case we need to expose a way for sub-classes to muck with the pool kwargs, something like

class HTTPAdapter(BaseAdapter):
    def build_pool_kwargs(self, request, verify, cert=None):
        return _urllib3_request_context(request, verify, cert)

That way a subclass like this could do something like:

class CustomSSLAdapter(HTTPAdapter):
    def build_pool_kwargs(self, request, verify, cert=None):
        params, kwargs = super().build_pool_kwargs(request, verify, cert)
        params["ssl_context"] = self.context

Thoughts? I'm not particularly fond of this but it seems like the least intrusive way to preserve the fix for the CVE while giving people the ability to use custom SSLContexts the way they already were

sigmavirus24 avatar May 22 '24 11:05 sigmavirus24

We have a different error, but probably the same issue. We inject a ssl_context with a custom cipher selection. If there is a more "correct" way to do this I can also update our code, but a fix in the library would be appreciated.

Anyway, thanks for the quick response.

Marco-Kaulea avatar May 22 '24 12:05 Marco-Kaulea

You will need to fix your code but right now it's not easily doable

sigmavirus24 avatar May 22 '24 12:05 sigmavirus24

when do you expect releasing v2.32.3 which is suggested at #6716?

DanSIntel avatar May 23 '24 05:05 DanSIntel

@DanSIntel See the conversation in the MR: https://github.com/psf/requests/pull/6716#pullrequestreview-2072332359

Marco-Kaulea avatar May 23 '24 06:05 Marco-Kaulea

On requests 2.32.x with truststore we are seeing a maximum recursion error with verify=True on windows Jupyter labs and jupyter notebook environments.

import requests
with requests.Session() as session:
   
     print(session.get("https://www.arcgis.com/sharing/rest/portals/self?f=json").text)

achapkowski avatar May 23 '24 08:05 achapkowski

@achapkowski I don't see trust store being used there or any kind of traceback

sigmavirus24 avatar May 23 '24 10:05 sigmavirus24

I also can't reproduce your claim @achapkowski but I don't have windows available:

>>> import truststore; truststore.inject_into_ssl()
>>> import requests
>>> requests.get('https://google.com')
<Response [200]>
>>> requests.get('https://www.arcgis.com/sharing/rest/portals/self?f=json')
<Response [200]>

sigmavirus24 avatar May 23 '24 11:05 sigmavirus24

image

It recursively occurs here.

full stack attached:

full-stack-error.txt

achapkowski avatar May 23 '24 12:05 achapkowski

Dug in a bit more:

import requests
print(requests.get("https://www.arcgis.com/sharing/rest/portals/self?f=json").text)
with requests.Session() as session:
    print(session.get("https://www.arcgis.com/sharing/rest/portals/self?f=json").text)

The above works on windows

Below will fails on windows. Works on Mac/Linux

import truststore
truststore.inject_into_ssl()
with requests.Session() as session:
    print(session.get("https://www.arcgis.com/sharing/rest/portals/self?f=json").text)

achapkowski avatar May 23 '24 13:05 achapkowski

I posted a response to the recursion issue in https://github.com/psf/requests/pull/6716#issuecomment-2127152349. This is already known behavior that was reported to be broken in arcgis previously due to misuse of truststore (https://github.com/Esri/arcgis-python-api/issues/1698). It will need to be fixed there. That's unrelated to the topic of this issue.

For the minimal repro, you're producing this because your repro is also misusing truststore in the same way as arcgis. truststore MUST be imported before any networking code for either urllib3 or Requests. inject_into_ssl() is not an intended entry point for any library or package code. You'll find a large warning at the top of the user guide directing users to avoid this.

You can swap the order of your imports and the above repro works fine on Windows:

# Must be done first
import truststore
truststore.inject_into_ssl()

import requests
with requests.Session() as session:
    print(session.get("https://www.arcgis.com/sharing/rest/portals/self?f=json").text)

nateprewitt avatar May 23 '24 13:05 nateprewitt

I had a similar issue. Try setting the REQUESTS_CA_BUNDLE= environment variable if possible to point to your CA bundle

baolsen avatar May 24 '24 10:05 baolsen

We used pip-system-certs to trust the os bundle which has custom CAs in. I dont fully understand why but since 2.32.0 we get what I think is the same issue. Someone has raised an issue over at gitab https://gitlab.com/alelec/pip-system-certs/-/issues/27

awalker125 avatar May 24 '24 12:05 awalker125

https://github.com/psf/requests/pull/6721 is staged for release on Tuesday. We're going to hold releasing over the weekend to avoid any unnecessary pain for ops teams and will release after the holiday in the US on Monday. You can follow along for the merge of that PR to track the release time.

nateprewitt avatar May 24 '24 21:05 nateprewitt

@nateprewitt

For the adapter issue with 2.32.x do developers need to modify their logic? Is that documented anywhere if so?

Also thank you for the quick fix!

achapkowski avatar May 29 '24 00:05 achapkowski

@achapkowski If you're using the init_poolmanager setup now like what's linked in the example, you shouldn't need to make any additional changes. 2.32.3 should work the same as 2.31.0 and earlier. If you want to do anything more than what's described in the first post of this issue, the new build_connection_pool_key_attributes will be the escape hatch to do any additional customization.

nateprewitt avatar May 29 '24 02:05 nateprewitt

Alright, 2.32.3 is out. Thanks everyone for your patience, please let us know if you're still hitting this failure case after upgrading.

nateprewitt avatar May 29 '24 15:05 nateprewitt

Alright, 2.32.3 is out. Thanks everyone for your patience, please let us know if you're still hitting this failure case after upgrading.

With 2.32.0-2.32.2 I've been seeing the same issue when using requests_pkcs12; now with 2.32.3, the issue did unfortunately not disappear but rather move to HTTPSConnectionPool(host='some.system', port=443): Max retries exceeded with url: /foo/bar (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1006)'))).

I've opened an issue with requests_pkcs12, but since you specifically asked to point out problems where things do not behave like in 2.31.0...

pc-coholic avatar May 29 '24 16:05 pc-coholic

We also use requests_pkcs12 and I can confirm that 2.32.3 doesn't work with it. Stack trace I saw:

Traceback (most recent call last): File "/workspace/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 715, in urlopen httplib_response = self._make_request( ^^^^^^^^^^^^^^^^^^^ File "/workspace/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 404, in _make_request self._validate_conn(conn) File "/workspace/.venv/lib/python3.11/site-packages/urllib3/connectionpool.py", line 1058, in _validate_conn conn.connect() File "/workspace/.venv/lib/python3.11/site-packages/urllib3/connection.py", line 419, in connect self.sock = ssl_wrap_socket( ^^^^^^^^^^^^^^^^ File "/workspace/.venv/lib/python3.11/site-packages/urllib3/util/ssl_.py", line 449, in ssl_wrap_socket ssl_sock = _ssl_wrap_socket_impl( ^^^^^^^^^^^^^^^^^^^^^^ File "/workspace/.venv/lib/python3.11/site-packages/urllib3/util/ssl_.py", line 493, in _ssl_wrap_socket_impl return ssl_context.wrap_socket(sock, server_hostname=server_hostname) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/ssl.py", line 517, in wrap_socket return self.sslsocket_class._create( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/lib/python3.11/ssl.py", line 1108, in _create self.do_handshake() File "/usr/lib/python3.11/ssl.py", line 1379, in do_handshake self._sslobj.do_handshake() ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1006)

Heraldk avatar May 30 '24 09:05 Heraldk

I am seeing a similar behaviour with 2.32.3 and build_connection_pool_key_attributeswith a self-signed certificate. I need a custom ssl context since otherwise the connection will faile due to weak certificate strength.

Here is my code:

import ssl
import requests
from requests.adapters import HTTPAdapter


class SSLAdapter(HTTPAdapter):
    """An HTTPAdapter that uses an arbitrary SSL context."""

    def __init__(self, ssl_context: ssl.SSLContext = None, **kwargs):
        """Initialize the SSLAdapter."""
        super().__init__(**kwargs)
        self.ssl_context = ssl_context

    def build_connection_pool_key_attributes(
        self,
        request: requests.PreparedRequest,
        verify: bool | str,
        cert: str | tuple[str, str] | None = None,
    ) -> tuple[dict, dict]:
        host_params, ssl_params = super().build_connection_pool_key_attributes(
            request, verify, cert
        )
        if verify is True and self.ssl_context:
            ssl_params["ssl_context"] = self.ssl_context

        return host_params, ssl_params


if __name__ == "__main__":
    # Create a custom SSL context
    ssl_context = ssl._create_unverified_context()
    ssl_context.set_ciphers("DEFAULT@SECLEVEL=2")  # Adjusting the security level to support 2048 bit keys

    # Example API call setup
    username = "<admin>"
    password = "<password>"
    protocol = "https"
    api_url = f"{protocol}://<host>/"
    action = "<action>"
    headers = {"Content-Type": "application/json"}

    # Create a session with the SSLAdapter
    session = requests.Session()
    session.auth = (username, password)
    session.mount(f"{protocol}://", SSLAdapter(ssl_context = ssl_context))

    try:
        response = session.get(api_url + action, timeout=15, headers=headers)
        response.raise_for_status()  # Raise an exception for HTTP errors
        print("Response:", response.json())
    except requests.exceptions.RequestException as e:
        print(f"An error occurred: {e}")

which results in this error:

An error occurred: HTTPSConnectionPool(host='<host>', port=443): Max retries exceeded with url: /<action>/ (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: self-signed certificate (_ssl.c:1000)')))

What am I doing wrong? Shouldn't _create_unverified_context exacly allow for self-signed certs?

suaveolent avatar Nov 17 '24 11:11 suaveolent

suaveolent @suaveolent did you resolve this? same problem with 2.32.3

CraftedCat avatar Dec 24 '24 06:12 CraftedCat

suaveolent @suaveolent did you resolve this? same problem with 2.32.3

Unfortunately not. Only workaround seems to additionally disable certificate verification by setting verify to false.

suaveolent avatar Dec 24 '24 08:12 suaveolent