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

Incompatibility with requests 2.32.3 - custom SSLContext via a Transport Adapter #55

Open
pc-coholic opened this issue May 27, 2024 · 10 comments

Comments

@pc-coholic
Copy link

This might be a false positive - but from preliminary experimentation, it seems like a recent change in requests might cause issues with requests_pkcs12.

With requests 2.32.0, a breaking change was introduced that returned every call with that involved requests_pkcs12 with SSLContext via a Transport Adapter.

requests 2.32.3 (still unreleased as of right now, apparently scheduled for Tuesday) aims to fix this issue (Allow for overriding of specific pool key params #6716), but now all calls involving requests_pkcs12 fail with SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: unable to get local issuer certificate for me.

I'll try to dig a little deeper and see if this is indeed something that requires changes here - I just wanted to provide a heads-up in case 2.32.3 is indeed shipping a breaking change.

Minimal example:

import requests
from requests_pkcs12 import Pkcs12Adapter

client = requests.Session()
client.mount(
    'https://some.system/',
    Pkcs12Adapter(
        pkcs12_data=self.cert.read(),
        pkcs12_password=self.cert_password,
)

client.get('https://some.system/foo/bar')

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)')))

@pc-coholic
Copy link
Author

This issue is now being hopefully fixed in psf/requests#6731

A very quick fix would be to follow mikf/gallery-dl@6cfbc10 suit and place this within _create_sslcontext(). But since an update for requests is probably imminent, we could also just close this issue and wait for the issue to disappear by itself :-)

@vog vog closed this as completed Jun 6, 2024
@vog
Copy link
Member

vog commented Jun 6, 2024

Ok

@prontsevychev
Copy link

@vog Sorry, but this problem is still current with requests 2.32.0:

image

Could you reopen it?

@dbaumgarten
Copy link

Hi,
the issue still persists with requests 2.32.3.
I think this issue should be reopened

@vog
Copy link
Member

vog commented Jun 7, 2024

Ok, reopened. Pull requests are welcome.

@vog vog reopened this Jun 7, 2024
@pc-coholic
Copy link
Author

@vog I'll be happy to provide a one-line pull request to fix the issue.

But I was wondering if you'd prefer putting in a workaround (which will be superfluous once 2.32.4 is released/the above linked PR is merged) or just wait for said PR to land in requests....

@vog
Copy link
Member

vog commented Jun 7, 2024

Never mind, we quickly added the fix of

on our own, and released requests_pkcs12 version 1.25:

Our local test suite runs fine with it.

Please check whether this solves the issue for you as well.

@pc-coholic
Copy link
Author

Works for me, was exactly what I was experimenting with, too :)

@vog
Copy link
Member

vog commented Jun 7, 2024

Thanks for your quick feedback. 👍

@vog vog closed this as completed Jun 7, 2024
@vog
Copy link
Member

vog commented Jun 7, 2024

Leaving this issue open as a reminder to revert that workaround as soon as

  • requests 2.32.4 is released, and
  • we verified that "make check" works fine with the reverted workaround

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