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

Newest version cause DeprecationWarning: 'urllib3.contrib.pyopenssl' module is deprecated #2744

Closed
ddzialak opened this issue Aug 22, 2022 · 7 comments
Labels
dependencies This issue is a problem in a dependency.

Comments

@ddzialak
Copy link

ddzialak commented Aug 22, 2022

Describe the bug

Fresh installation cause warning:

DeprecationWarning: 'urllib3.contrib.pyopenssl' module is deprecated and will be removed in a future release of urllib3 2.x. Read more in this issue: https://github.com/urllib3/urllib3/issues/2680

That warning somehow cause, that pytest does not start tests.

Expected Behavior

import of botocore should not issue that warning

Current Behavior

DeprecationWarning is issued

Reproduction Steps

after fresh installation:

(sf-new) [1]17:00:19:~/workspace/$ python -m botocore.httpsession
/home/darek/workspace/sf-new/lib/python3.10/site-packages/botocore/httpsession.py:41: DeprecationWarning: 'urllib3.contrib.pyopenssl' module is deprecated and will be removed in a future release of urllib3 2.x. Read more in this issue: https://github.com/urllib3/urllib3/issues/2680
  from urllib3.contrib.pyopenssl import orig_util_SSLContext as SSLContext

Possible Solution

avoid import of urllib3.contrib.pyopenssl, for more see:
urllib3/urllib3#2680

Additional Information/Context

Workaround: enforcing version urllib3==1.26.9 solve that issue

SDK version used

Python 3.10.4
botocore 1.27.56

Environment details (OS name and version, etc.)

Linux darek-dell 5.4.0-122-generic #138-Ubuntu SMP Wed Jun 22 15:00:31 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

@ddzialak ddzialak added bug This issue is a confirmed bug. needs-triage This issue or PR still needs to be triaged. labels Aug 22, 2022
@nateprewitt
Copy link
Contributor

Hi @ddzialak,

Botocore doesn't use pyopenssl, we have intentionally patched it to always use the standard library version of SSLContext. There's a lingering import from the pyopenssl module for backwards compatibility which appears to be triggering the warning.

If we were impacted we're already pinned to urllib3<1.27 but this deprecation won't take effect until urllib3==2.0. We'll take a look at getting the warning suppressed, thanks for letting us know!

@nateprewitt nateprewitt added dependencies This issue is a problem in a dependency. and removed bug This issue is a confirmed bug. needs-triage This issue or PR still needs to be triaged. labels Aug 22, 2022
@akx
Copy link
Contributor

akx commented Sep 5, 2022

@ddzialak You likely have

filterwarnings =
    error

or similar in your Py.test configuration, causing deprecation warnings (such as this one) at test-dependency module import time to become errors.

You could configure that to something like

filterwarnings = 
    error
    ignore:.*urllib3.contrib.pyopenssl.*:DeprecationWarning

to let this particular warning slip.

@miketheman
Copy link
Contributor

miketheman commented Sep 17, 2022

Botocore doesn't use pyopenssl, we have intentionally patched it to always use the standard library version of SSLContext. There's a lingering import from the pyopenssl module for backwards compatibility which appears to be triggering the warning.

@nateprewitt code is not importing ssl.SSLContext directly as far as I can see, using urillib3.util.ssl_.SSLContext as a fallback which either resolves to an ssl.SSLContext or None

Seems to me that this could be start to be solved by reversing the logic here:

try:
# Always import the original SSLContext, even if it has been patched
from urllib3.contrib.pyopenssl import orig_util_SSLContext as SSLContext
except ImportError:
from urllib3.util.ssl_ import SSLContext

to something like

from urllib3.util.ssl_ import SSLContext
if SSLContext is None:
     from urllib3.contrib.pyopenssl import orig_util_SSLContext as SSLContext 

of if preserving the try structure,

try:
    from urllib3.util.ssl_ import SSLContext
    if SSLContext is None:
        raise ImportError
except ImportError:
    from urllib3.contrib.pyopenssl import orig_util_SSLContext as SSLContext 

Thus providing the non-deprecated context if it's available from urllib3, and fall back to pyopenssl for backwards compatibility if SSLContext is None in the rare event the end user is using a Python version compiled without SSL and is trying to communicate with an AWS API endpoint.
urllib3 has a test method to simulate the lack of SSL support compiled into Python, might be helpful.

@nateprewitt
Copy link
Contributor

Hi @miketheman, the import order is intentional for handling the original injection mechanism used for pyopenssl. The value of urllib3.util.ssl_.SSLContext changes depending on the environment urllib3 is imported into. Reversing this order unfortunately leads to pyopenssl leaking into botocore, compromising our current security posture.

You can do a quick check to validate this:

from urllib3.contrib.pyopenssl import orig_util_SSLContext as orig_SSLContext
from ssl import SSLContext

assert orig_SSLContext == SSLContext

import urllib3
urllib3.contrib.pyopenssl.inject_into_urllib3()

from urllib3.util.ssl_ import SSLContext as urllib3_SSLContext
assert urllib3_SSLContext == SSLContext, f"SSLContext is {urllib3_SSLContext}"

This is due to the value being patched out with old versions of Requests and urllib3 where pyopenssl is present in the environment. Our best option is likely to suppress the specific warning with something like catch_warnings as our exception logic will handle its removal in the future.

Once the pyopenssl injection mechanism is removed in 2.0, we'll consistently fallback to the unpatched from urllib3.util.ssl_ import SSLContext import. Eventually when the dependency floor for Botocore is raised to urllib3>=2.0, we can remove the import entirely.

@dlm6693
Copy link
Contributor

dlm6693 commented Sep 26, 2022

Hi all this was addressed in #2763. Closing this out.

@dlm6693 dlm6693 closed this as completed Sep 26, 2022
@n1ngu
Copy link

n1ngu commented Oct 6, 2022

Why is this lingering import kept at all? AFAIU the urllib3.contrib.pyopenssl module is only good for python 2 compatibility but botocore 1.21.0 already dropped support for that platform.

Also, since when suppressing a deprecation warning fixes anything?

@nateprewitt
Copy link
Contributor

Why is this lingering import kept at all?

Apologies if my last comment was unclear. This import is specifically to not use pyopenssl and avoid its import if present. We are working around a behavior of older versions of requests/urllib3, botocore has never used pyopenssl.

A simplified example of how this happens in production systems:

$ python -m pip install requests==2.22.0
$ python -m pip install pyopenssl
$ python
>>> import requests
>>> import urllib3
>>> urllib3.util.ssl_.SSLContext
<class 'urllib3.contrib.pyopenssl.PyOpenSSLContext'>

urllib3 patches the value of SSLContext when inject_into_urllib3 is called to use PyOpenSSL if present. This has been fixed in newer versions of requests/urllib3, but not everyone has upgraded. Until urllib3 2.0 is the minimum supported version, this needs to stay in place, otherwise we will be increasing the usage of PyOpenSSL. That's not an acceptable security regression for botocore.

Also, since when suppressing a deprecation warning fixes anything?

From the urllib3 team's perspective, we actively test botocore in the urllib3 test suite itself. This is a known import that is safe behavior, so the warning doesn't apply. Suppressing it is the best option to avoid misleading noise. The code already handles the upcoming removal gracefully.

jjyao pushed a commit to ray-project/ray that referenced this issue Oct 25, 2022
Looks like we are using PyOpenSSL < 22.0 which can cause issues with the newer version of cryptography module that causes AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK' . Check boto/botocore#2744
urllib3/urllib3#2680
for more details.

The problem was that when we call install-dependencies, it downloads requirements.txt and then requirement_test.txt. When we download requirement_test.txt, we don't have -U flag, and then this means some of dependencies are not upgraded as stated inside requirement_test.txt. I tried adding -U flag to the PR

Signed-off-by: SangBin Cho <rkooo567@gmail.com>
WeichenXu123 pushed a commit to WeichenXu123/ray that referenced this issue Dec 19, 2022
Looks like we are using PyOpenSSL < 22.0 which can cause issues with the newer version of cryptography module that causes AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK' . Check boto/botocore#2744
urllib3/urllib3#2680
for more details.

The problem was that when we call install-dependencies, it downloads requirements.txt and then requirement_test.txt. When we download requirement_test.txt, we don't have -U flag, and then this means some of dependencies are not upgraded as stated inside requirement_test.txt. I tried adding -U flag to the PR

Signed-off-by: SangBin Cho <rkooo567@gmail.com>
Signed-off-by: Weichen Xu <weichen.xu@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies This issue is a problem in a dependency.
Projects
None yet
Development

No branches or pull requests

6 participants