Move _get_connection to get_connection_with_tls_context

[nateprewitt]

This PR is meant to provide a new publicly supported api get_connection_with_tls_context to better facilitate custom adapters with our recent fix for CVE-2024-35195.

As described here, we will be deprecating the existing get_connection API, and custom adapters will need to migrate their existing get_connection implementations to use get_connection_with_tls_context. Below is a very simple implementation that will get projects relying on a custom get_connection implementation unblocked.

However, we strongly recommend maintainers evaluate if their custom get_connection is subject to the same issues as the CVE above and make appropriate changes as needed.

Minimum new code to work as a pass-through
This will be backwards compatible between versions of Requests.

def get_connection_with_tls_context(self, request, verify, proxies=None, cert=None):
    return self.get_connection(request.url, proxies)

[nateprewitt]

cc @felixfontein

[sethmlarson]

Looks good, only had one suggestion that might require a new test.

[kdymov]

A small update about the minimum pass-through code for someone who will stumble upon this - don’t forget to return the value, as you will face a different exception if you do 🙂

def get_connection_with_tls_context(self, request, verify, proxies=None, cert=None):
    return self.get_connection(request.url, proxies)

[nateprewitt]

Thank you @kdymov! I've fixed the initial example as well.

[WhyNotHugo]

I'm having some issues upgrading to requests 2.32 which I think are related to this change, but I can't quite figure out how I'd need to upgrade my code.

My HTTPAdapter subclass basically overrides the TLS context to allow weak DH ciphers:

class AFIPAdapter(HTTPAdapter):
    """An adapter with reduced security so it'll work with AFIP."""

    def init_poolmanager(self, *args, **kwargs) -> PoolManager:
        context = create_urllib3_context(ciphers=CIPHERS)
        kwargs["ssl_context"] = context
        return super().init_poolmanager(*args, **kwargs)

    def proxy_manager_for(self, *args, **kwargs) -> ProxyManager:
        context = create_urllib3_context(ciphers=CIPHERS)
        kwargs["ssl_context"] = context
        return super().proxy_manager_for(*args, **kwargs)

With requests==2.31.0 this works, but with requests==2.32.0 it doesn't. I keep re-reading the changes in this patchset and can't fully grasp how they're affecting me.

Do you have any guidance?

[WhyNotHugo]

I'm also a bit confused by get_connection_with_tls_context, because it reads Returns a urllib3 connection for the given request and TLS settings, but no TLS settings are given.

[nateprewitt]

@WhyNotHugo You may be hitting a separate issue with init_poolmanager. We just released 2.32.3 to address that problem with custom adapters, you may want to give that a try.

[WhyNotHugo]

2.32.3 raises an entirely different exception:

requests.exceptions.SSLError: HTTPSConnectionPool(host='wswhomo.afip.gov.ar', port=443): Max retries exceeded with url: /wsfev1/service.asmx (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1000)')))

[WhyNotHugo]

Looks like I need to add context.load_default_certs() and it works fine. It wasn't required before tho.

[nateprewitt]

We may not be loading the cert bundle the same as we were previously. Would you mind opening a new issue with a minimal reproduction (the adapter you have above is likely fine). If you can also include the info we request from help, and if you're working behind a proxy when this happens. That'd be helpful, thanks!