requests should ignore REQUESTS_CA_BUNDLE when session.verify is False

[brmzkw]

The argument session.verify set to False is ignored when the environment variable REQUESTS_CA_BUNDLE is set.

Reproduction Steps

✅ empty REQUESTS_CA_BUNDLE and session.verify = True : exception is raised

$> REQUESTS_CA_BUNDLE=  python -c 'import requests; session = requests.Session(); session.verify = True; session.get("https://expired.badssl.com")'
...
requests.exceptions.SSLError: HTTPSConnectionPool(host='expired.badssl.com', port=443): Max retries exceeded with url: / (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: certificate has expired (_ssl.c:1131)')))

✅ empty REQUESTS_CA_BUNDLE and session.verify = False: no exception raised

$> REQUESTS_CA_BUNDLE=  python -c 'import requests; session = requests.Session(); session.verify = False; session.get("https://expired.badssl.com")'

✅ REQUESTS_CA_BUNDLE is invalid, and no verify is given to requests.get (defaults to True): exception is raised because REQUESTS_CA_BUNDLE is invalid

$> REQUESTS_CA_BUNDLE=invalid  python -c 'import requests; requests.get("https://expired.badssl.com")'
...
OSError: Could not find a suitable TLS CA certificate bundle, invalid path: invalid

✅ REQUESTS_CA_BUNDLE is invalid and verify=False is given to requests.get: no exception raised

$> REQUESTS_CA_BUNDLE=invalid  python -c 'import requests; requests.get("https://expired.badssl.com", verify=False)'

✅ REQUESTS_CA_BUNDLE is invalid and session.verify = True : exception is raised

$> REQUESTS_CA_BUNDLE=invalid python -c 'import requests; session = requests.Session(); session.verify = True; session.get("https://expired.badssl.com")'
...
OSError: Could not find a suitable TLS CA certificate bundle, invalid path: invalid

❌ REQUESTS_CA_BUNDLE is invalid and session.verify = False : exception is raised

$> REQUESTS_CA_BUNDLE=invalid python -c 'import requests; session = requests.Session(); session.verify = False; session.get("https://expired.badssl.com")'
...
OSError: Could not find a suitable TLS CA certificate bundle, invalid path: invalid

The problem comes from

requests/requests/sessions.py

Line 717 in b0e025a

if verify is True or verify is None:

which ignores the value of self.verify.

[nateprewitt]

Hey @brmzkw, thanks for the detailed rundown! I agree this behavior seems wrong and I'd be inclined to treat it as a bug. I've got some general feedback on the PR, but before we start that, I'd like to make sure we map this out completely.

Session has two interfaces for sending requests, either the request method (base for get, post, etc.) and the send method. It looks like you have request functionality well mapped, but I'd like to make sure we understand how send works in these cases. If you have a moment to put together a table your test cases above for both methods, that would be greatly appreciated. Otherwise, I'll work on getting one together.

Once we have that, it'll help us inform the eventual change. In general we try to adhere to a precedence order of method kwargs > Request > Session > Environment. We'll want to make sure we're maintaining that to the best of our ability.

[brmzkw]

Hello and thanks for your quick review!

The tests below should cover what you asked. They have been run with and without the patch from my pull request, and all the results are similar in both cases.

✅ REQUESTS_CA_BUNDLE is empty, using prepped request: exception is raised

$> REQUESTS_CA_BUNDLE= python -c "from requests import Request, Session; s = Session(); req = Request('GET', 'https://expired.badssl.com'); prepped = req.prepare(); s.send(prepped)"
...
requests.exceptions.SSLError: HTTPSConnectionPool(host='expired.badssl.com', port=443): Max retries exceeded with url: / (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: certificate has expired (_ssl.c:1131)')))

⁉️ REQUESTS_CA_BUNDLE is invalid, using prepped request:

$> REQUESTS_CA_BUNDLE=invalid python -c "from requests import Request, Session; s = Session(); req = Request('GET', 'https://expired.badssl.com'); prepped = req.prepare(); s.send(prepped)"
...
requests.exceptions.SSLError: HTTPSConnectionPool(host='expired.badssl.com', port=443): Max retries exceeded with url: / (Caused by SSLError(SSLCertVerificationError(1, '[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: certificate has expired (_ssl.c:1131)')))

I don't understand why this exception. REQUESTS_CA_BUNDLE isn't used. This is likely a problem, but not related to this issue.

✅ REQUESTS_CA_BUNDLE is invalid, using prepped request, set verify=False in session.send(): no exception

$> REQUESTS_CA_BUNDLE=invalid python -c "from requests import Request, Session; s = Session(); req = Request('GET', 'https://expired.badssl.com'); prepped = req.prepare(); s.send(prepped, verify=False)"

✅ REQUESTS_CA_BUNDLE is invalid, using prepped request, set session.verify = False: no exception

$> REQUESTS_CA_BUNDLE=invalid python -c "from requests import Request, Session; s = Session(); req = Request('GET', 'https://expired.badssl.com'); prepped = req.prepare(); s.verify = False; s.send(prepped)"

[nateprewitt]

Thanks for checking on those, @brmzkw! I agree the invalid bundle for send is likely wrong behavior, but this helps us get a better view of current behavior.

Unfortunately, I started leaving feedback on the PR and realized I'd oversimplified the problem yesterday. Fixing this introduces some breakages into merge_environment_settings that I'm not sure we can do outside of a major version bump. As a standalone interface it has long standing precedent for how it should behave with Session.verify. That will mean we have to push off changing this for the time being.

When we do come around to fixing this, I'll try to layout what things should look like with an invalid REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE:

Requests 2.26.0 (Wrong):

	verify : True	verify : None	verify : False
self.verify : True	❌	❌	✅
self.verify : None	❌	❌	✅
self.verify : False	❌	❌	✅

#5922 PR (Better):

	verify : True	verify : None	verify : False
self.verify : True	❌	❌	✅
self.verify : None	❌	❌	✅
self.verify : False	✅	✅	✅

Desired Behavior (method kwargs > Session):

	verify : True	verify : None	verify : False
self.verify : True	❌	❌	✅
self.verify : None	❌	❌	✅
self.verify : False	❌	✅	✅

This gets a bit more nuanced when we look at Session.send and Session.request because send ignores those environment variables entirely (which is the behavior you saw). I wrote up a quick test suite here that we could probably code against in the future to make sure we're getting the desired behavior. These methods have seen considerable drift over the years and neither does what it actually should. There's a larger discussion to be had about what role each fills and a in-depth rework to minimize future drift.

So to summarize, I'll mark this as a bug but it may be a considerable amount of time before it can be addressed in a new major version. Thanks for the report though, we do truly appreciate it!

[nateprewitt]

It looks like this was actually identified several years ago in #3829 but has been sitting since with the same conclusion. I think we've got some more useful information here that wasn't part of the initial ticket. We'll leave this here as a bread crumb on the initial ticket but I'm going to resolve this in favor of the original to consolidate tracking. Thanks again, @brmzkw!