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

Certificate loading regression with HTTPAdapters in 2.32.3 #6730

Open
ricellis opened this issue May 31, 2024 · 3 comments
Open

Certificate loading regression with HTTPAdapters in 2.32.3 #6730

ricellis opened this issue May 31, 2024 · 3 comments

Comments

@ricellis
Copy link

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 #6726 (comment).
Also related to #6710 (comment) - 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
}
pyrooka added a commit to IBM/python-sdk-core that referenced this issue Jun 3, 2024
Version `2.23.3` of the `requests` package broke our custom
`SSLHTTPAdapter`. This commit defines the maximum version of that
package, to make sure it will continue working without an issue. Note
that, this is a short term solution only. For more details see this
issue: psf/requests#6730

Signed-off-by: Norbert Biczo <[email protected]>
pyrooka added a commit to IBM/python-sdk-core that referenced this issue Jun 3, 2024
Version `2.23.3` of the `requests` package broke our custom
`SSLHTTPAdapter`. This commit defines the maximum version of that
package, to make sure it will continue working without an issue. Note
that, this is a short term solution only. For more details see this
issue: psf/requests#6730

Signed-off-by: Norbert Biczo <[email protected]>
archlinux-github pushed a commit to archlinux/aur that referenced this issue Jun 3, 2024
pyrooka added a commit to IBM/python-sdk-core that referenced this issue Jun 3, 2024
)

Version `2.23.3` of the `requests` package broke our custom
`SSLHTTPAdapter`. This commit defines the maximum version of that
package, to make sure it will continue working without an issue. Note
that, this is a short term solution only. For more details see this
issue: psf/requests#6730

Signed-off-by: Norbert Biczo <[email protected]>
idavidmcdonald added a commit to alphagov/notifications-api that referenced this issue Jun 18, 2024
We updated requests in #4108
but we saw errors in production connecting to DVLA as per
psf/requests#6730.

It looks like moving back to 2.32.2 should be unaffected, at least
according to the github issue above.

Once a new version is released that fixes it, we can drop this
pin. If it doesn't get fixed we could look at an alternative approach
like #4113
@jaraco
Copy link
Contributor

jaraco commented Jul 7, 2024

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
Copy link
Member

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
Copy link

Jamim commented Jul 12, 2024

we have a PR with the fix up already.

For those who are wondering, here it is:

Jamim added a commit to Jamim/gentoo-overlay that referenced this issue Jul 12, 2024
Note that there is no <dev-python/requests-3.32.3 for Python 3
in the portage tree at the moment, so requests' version can't
be effectively pinned like it was pinned in the PyPI package.
It leads to errors when processing HTTPS links.
Upcoming release of Requests should fix it.
Issue for tracking:
  - psf/requests#6730

Release:
  - https://github.com/httpie/cli/releases/tag/3.2.3

Changes in comparison to 3.2.2:
  - add Python 3.13 support
  - add the repository url to the HOMEPAGE
patdowney added a commit to serverless-ca/terraform-aws-ca that referenced this issue Jul 26, 2024
patdowney added a commit to serverless-ca/terraform-aws-ca that referenced this issue Jul 26, 2024
paulschwarzenberger pushed a commit to serverless-ca/terraform-aws-ca that referenced this issue Jul 26, 2024
rbignon pushed a commit to rbignon/woob that referenced this issue Sep 3, 2024
…: self-signed certificate in certificate chain" error

`requests` versions 2.32.0 to 2.32.3 (included) are affected by a bug breaking the ability to specify
custom SSLContexts in sub-classes of HTTPAdapter (psf/requests#6715) and another
breaking the ability to load certificates with HTTPAdapters (psf/requests#6730)

We skip those versions (and hope that 2.32.4 will provide a fix for psf/requests#6730)

Cf #706
AdamWill added a commit to AdamWill/httpie-cli that referenced this issue Sep 4, 2024
…e#1583)

Requests prior to 2.32.3 always loaded the default (system-wide)
set of trusted certificates into custom SSL contexts. 2.32.3 no
longer does. This has broken a lot of users, but the fix is
moving slowly upstream due to security considerations - see
psf/requests#6730 and
psf/requests#6731 .

As suggested at
psf/requests#6710 (comment)
this can be worked around by explicitly loading the default
certificates into the context. We check the method exists before
calling it just to be safe, but I'm pretty sure it's been there
as long as this interface has existed.

Signed-off-by: Adam Williamson <[email protected]>
AdamWill added a commit to AdamWill/httpie-cli that referenced this issue Sep 4, 2024
…e#1583)

Requests prior to 2.32.3 always loaded the default (system-wide)
set of trusted certificates into custom SSL contexts. 2.32.3 no
longer does. This has broken a lot of users, but the fix is
moving slowly upstream due to security considerations - see
psf/requests#6730 and
psf/requests#6731 .

As suggested at
psf/requests#6710 (comment)
this can be worked around by explicitly loading the default
certificates into the context. We check the method exists before
calling it just to be safe, but I'm pretty sure it's been there
as long as this interface has existed.

Signed-off-by: Adam Williamson <[email protected]>
AdamWill added a commit to AdamWill/httpie-cli that referenced this issue Sep 6, 2024
…e#1583)

Requests prior to 2.32.3 always loaded the default (system-wide)
set of trusted certificates into custom SSL contexts. 2.32.3 no
longer does. This has broken a lot of users, but the fix is
moving slowly upstream due to security considerations - see
psf/requests#6730 and
psf/requests#6731 .

As suggested at
psf/requests#6710 (comment)
this can be worked around by explicitly loading the default
certificates into the context. We check the method exists before
calling it just to be safe, it was added in Python 3.4.

Signed-off-by: Adam Williamson <[email protected]>
jkbrzt pushed a commit to httpie/cli that referenced this issue Nov 1, 2024
#1596)

* Explicitly load default certificates when creating SSL context (#1583)

Requests prior to 2.32.3 always loaded the default (system-wide)
set of trusted certificates into custom SSL contexts. 2.32.3 no
longer does. This has broken a lot of users, but the fix is
moving slowly upstream due to security considerations - see
psf/requests#6730 and
psf/requests#6731 .

As suggested at
psf/requests#6710 (comment)
this can be worked around by explicitly loading the default
certificates into the context. We check the method exists before
calling it just to be safe, it was added in Python 3.4.

Signed-off-by: Adam Williamson <[email protected]>

* Drop the upper bound on the requests dependency again

As we can now work with requests 2.32.3+, we no longer need this
pin.

Signed-off-by: Adam Williamson <[email protected]>

---------

Signed-off-by: Adam Williamson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants