requests icon indicating copy to clipboard operation
requests copied to clipboard

Session.verify=False ignored when REQUESTS_CA_BUNDLE environment variable is set

Open intgr opened this issue 7 years ago • 14 comments

One would expect that when the caller explicitly asks to make unverified requests, then the REQUESTS_CA_BUNDLE environment variable doesn't affect it. The reality is different, however.

import os
import requests

os.environ['REQUESTS_CA_BUNDLE'] = 'asd.pem'  # Must be an existing file

r = requests.get('https://self-signed.badssl.com/', verify=False)
print(r)
# Prints: <Response [200]>

session = requests.Session()
session.verify = False

r = session.get('https://self-signed.badssl.com/')
print(r)
# Fails: requests.exceptions.SSLError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:749)

intgr avatar Jan 24 '17 13:01 intgr

Thanks for raising this issue! This is a related issue to #2018: specifically, we prefer the environment to the Session value assuming the environment specifies an auth value. The fix is easy, but we've unfortunately ossified around this value, so we can't fix it until v3.

Given that we fail-closed here (that is, it's not possible to use this arrangement to force us not to verify when we should), this isn't a security vulnerability, so there is no way we can justify bringing it forward to before v3.

Lukasa avatar Jan 24 '17 13:01 Lukasa

OK, thanks for the quick response. Is 3.0.0 coming some time soon or is it just a plan for now?

intgr avatar Jan 24 '17 14:01 intgr

"soon" is a bit strong. However, there's a branch that code can be committed to, and there is active work being done on urllib3 v2, which once done will be the catalyst for us to actually ship requests v3.

Lukasa avatar Jan 24 '17 14:01 Lukasa

For whoever else is struggling with this problem, I created a wrapper class as workaround:

class WrappedSession(requests.Session):
    """A wrapper for requests.Session to override 'verify' property, ignoring REQUESTS_CA_BUNDLE environment variable.

    This is a workaround for https://github.com/kennethreitz/requests/issues/3829 (will be fixed in requests 3.0.0)
    """
    def merge_environment_settings(self, url, proxies, stream, verify, *args, **kwargs):
        if self.verify is False:
            verify = False

        return super(WrappedSession, self).merge_environment_settings(url, proxies, stream, verify, *args, **kwargs)

intgr avatar Jan 27 '17 07:01 intgr

This issue hit me today. I had to debug a good amount of code to track it down.

Where are we now relative to when it was identified back in January?

megahall avatar May 16 '17 06:05 megahall

No change.

Lukasa avatar May 16 '17 06:05 Lukasa

as my test in win10 python 3.6.4, requests (2.18.3) (not until v3 ??) urllib3 (1.22) the

session = requests.Session()
session.verify = False

worked, and will prompt a warning message:

\appdata\local\programs\python\python36\lib\site-packages\urllib3\connectionpool.py:858: InsecureRequestWarning: Unverified HTTPS request is being made. Adding certificate verification is strongly advised. See: https://urllib3.readthedocs.io/en/latest/advanced-usage.html#ssl-warnings
  InsecureRequestWarning)

to disable the warning message, just:

from requests.packages.urllib3.exceptions import InsecureRequestWarning
requests.packages.urllib3.disable_warnings(InsecureRequestWarning)

Justsoos avatar Mar 10 '18 22:03 Justsoos

The 'fail close, so no security issue' argument is only correct if the verify is set to False.

If the verify is set to a subset of the global CAs for access to some systems (as a poor mans form of certificate pinning) this fails open. So when used in a library that allows specifiying certificate authorities to allow for e.g. authentication backends that use sessions to allow easier cookie flows, the globally set environments (e.g. to allow access to internet sites in other parts of the program) invalidates this unless the library takes extra precautions.

For example: I allow the usual CAs via the curl/mozilla ca bundle for normal internet sites. Authentication is done via OAuth2 against Azure AD and that should be limited to only allow the Baltimore Cybertrust CA used by Azure. Now i cannot use session.verify for this and have to propagate the flag to all individual requests calls (or use a wrapper around Session as seen above).

schlenk avatar Jul 07 '18 10:07 schlenk

It would be nice if requests could at least throw a warning about this. I just spend 6 hours trying to figure out what is going on, then found out that REQUESTS_CA_BUNDLE is set by the daemon executing my test script and to then only find this issue. :disappointed:

mback2k avatar Jun 05 '20 12:06 mback2k

It's funny that this "fix" is delayed to v3 because it's considered "breaking change", yet a huge number of people keep burning hours pinning down this behavior. I'd say that calls it a bug, if no one expects it, and bug fixing would not be breaking changes...

fopina avatar Feb 20 '21 22:02 fopina

We spent 3 hours on this, third party library was set env REQUESTS_CA_BUNDLE ... It really would be nice to have a warning.

hardenchant avatar Mar 05 '21 09:03 hardenchant

Since this is fixed in requests 3.* version, I added a mechanism to warn the user via #5816. Please provide feedback on the PR.

Akasurde avatar May 14 '21 08:05 Akasurde

Hello, Is this documented ? Official documentation did not mention this behaviour. I lost 2 hours..

a-belhadj avatar May 16 '23 13:05 a-belhadj