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

DeprecationWarning forurllib3.contrib.pyopenssl #40

Closed
yonting opened this issue May 14, 2023 · 8 comments
Closed

DeprecationWarning forurllib3.contrib.pyopenssl #40

yonting opened this issue May 14, 2023 · 8 comments

Comments

@yonting
Copy link
Contributor

yonting commented May 14, 2023

I am getting a DeprecationWarning from urllib3 as described here: urllib3/urllib3#2691. Is it possible to fix this?

On a related note, are there plans to allow urllib3>=2.0.0?

@vog
Copy link
Member

vog commented May 15, 2023

Any pull request making requests_pkcs2 work with urllib3 2.x is welcome - or just testing whether just removing that restriction in setup.py is already sufficient. If you succeed, please file a pull request for that change.

Having said that, the urllib3 stuff is a leftover from the Python2 compatible implementation. If there is a way to implement this via Python3's plain urllib, it'd be even happier. Here, again, any pull request is welcome.

@yonting
Copy link
Contributor Author

yonting commented May 15, 2023

I'll try make a PR, but I'm sure no expert in this area.

What if we change this line

self.ssl_context = create_pyopenssl_sslcontext(pkcs12_data, pkcs12_password_bytes, ssl_protocol)

to use create_ssl_sslcontext instead. Then delete create_pyopenssl_sslcontext and remove all dependency on urllib3 and pyopenssl?

Or is there a reason we need to use pyopenssl or provide that option?

@yonting
Copy link
Contributor Author

yonting commented May 16, 2023

I made a PR to allow urllib3<2.1. I can make one to remove it entirely if you agree that is all fine.

@vog
Copy link
Member

vog commented May 17, 2023

Sorry for the late response.

I believe that your proposal:

to use create_ssl_sslcontext instead. Then delete create_pyopenssl_sslcontext and remove all dependency on urllib3 and pyopenssl?

is the way to go. It doesn't make sense to support cryptography and pyOpenSSL at the same time. Along the way, it would be great if you could rename

create_ssl_sslcontext

to

create_sslcontext

as well.

@vog
Copy link
Member

vog commented May 22, 2023

Thanks for your contribution! We just released version 1.16 which contains your improvement:

https://pypi.org/project/requests-pkcs12/

@vog vog closed this as completed May 22, 2023
@vog
Copy link
Member

vog commented May 23, 2023

Our solution appears to have an unintended side effect, see #44.

@bobleckelibm
Copy link

Could be wrong, but is it possible that this change also had the side affect of no longer allowing verify=False? (Caveat, I know that verify=False is a bad idea... We require it for internal use)

This is the error that we get with verify=False:

Cannot set verify_mode to CERT_NONE when check_hostname is enabled.

@yonting
Copy link
Contributor Author

yonting commented May 25, 2023

Could be wrong, but is it possible that this change also had the side affect of no longer allowing verify=False? (Caveat, I know that verify=False is a bad idea... We require it for internal use)

This is the error that we get with verify=False:

Cannot set verify_mode to CERT_NONE when check_hostname is enabled.

I am seeing the same problem. I do not know how to make a fix for this. You can change ssl_protocol, so Pkcs12Adapter(..., ssl_protocol=ssl.PROTOCOL_SSLv23), maybe, but ssl.PROTOCOL_SSLv23 is deprecated.

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

3 participants