psf / requests

A simple, yet elegant, HTTP library.
https://requests.readthedocs.io/en/latest/
Apache License 2.0
51.97k stars 9.29k forks source link

Certificate loading regression with HTTPAdapters in 2.32.3 #6730

Open ricellis opened 3 months ago

ricellis commented 3 months ago

It appears that in version 2.32.3 default certificates are no longer loaded for custom HTTPAdapter contexts when they were previously.

I guess this might be a duplicate/related to https://github.com/psf/requests/issues/6726#issuecomment-2138406456. Also related to https://github.com/psf/requests/pull/6710#issuecomment-2137802782 - adding load_default_certs() resolves the issue, but this wasn't required in previous versions and thus makes upgrading to 2.32.3 breaking.

Expected Result

With the code below using requests version 2.32.2 I get the URL content with no error.

Actual Result

Using 2.32.3 I get:

requests.exceptions.SSLError: HTTPSConnectionPool(host='raw.githubusercontent.com', port=443): Max retries exceeded with url: /psf/requests/main/MANIFEST.in (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate (_ssl.c:1006)')))

Reproduction Steps

import requests
import ssl
from requests.adapters import HTTPAdapter, DEFAULT_POOLBLOCK
from urllib3.util.ssl_ import create_urllib3_context

# adapted from https://github.com/IBM/python-sdk-core/blob/1c207385de627df5d12fd0a0ebd04717ce5bb29d/ibm_cloud_sdk_core/utils.py#L34
class SSLHTTPAdapter(HTTPAdapter):
    """Wraps the original HTTP adapter and adds additional SSL context."""

    def init_poolmanager(self, connections, maxsize, block=DEFAULT_POOLBLOCK, **pool_kwargs):
        """Create and use custom SSL configuration."""

        ssl_context = create_urllib3_context()
        ssl_context.minimum_version = ssl.TLSVersion.TLSv1_2
        # ssl_context.load_default_certs() # Adding this resolves the certificate issue but it was not required before

        super().init_poolmanager(connections, maxsize, block, ssl_context=ssl_context, **pool_kwargs)

session = requests.Session()
http_adapter = SSLHTTPAdapter()
session.mount('https://', http_adapter)

print(session.get(url='https://raw.githubusercontent.com/psf/requests/main/MANIFEST.in').text)

System Information

$ python -m requests.help
{
  "chardet": {
    "version": null
  },
  "charset_normalizer": {
    "version": "3.2.0"
  },
  "cryptography": {
    "version": ""
  },
  "idna": {
    "version": "3.4"
  },
  "implementation": {
    "name": "CPython",
    "version": "3.11.9"
  },
  "platform": {
    "release": "23.5.0",
    "system": "Darwin"
  },
  "pyOpenSSL": {
    "openssl_version": "",
    "version": null
  },
  "requests": {
    "version": "2.32.3"
  },
  "system_ssl": {
    "version": "30300000"
  },
  "urllib3": {
    "version": "2.2.1"
  },
  "using_charset_normalizer": true,
  "using_pyopenssl": false
}
jaraco commented 2 months ago

This issue appears to be leading to widespread breakage. Have you considered yanking the release? It's personally cost me a good deal of time troubleshooting, distilling, and reporting the issue in httpie/cli#1581, to the point that users are suggesting to move away from requests (feels drastic, admittedly). Would the maintainers at least consider acknowledging the issue and giving some insight into the plan?

nateprewitt commented 2 months ago

Hi @jaraco, we have a PR with the fix up already. We've been evaluating if there are any other breakages because this series of releases has been problematic.

Applying the patch or downgrading is the immediate fix. The reason it's not yanked is because this was part of a change for a CVE fix in 2.32.x.

Jamim commented 1 month ago

we have a PR with the fix up already.

For those who are wondering, here it is: