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

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

Open
intgr opened this issue Jan 24, 2017 · 14 comments · May be fixed by #5816
Open

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

intgr opened this issue Jan 24, 2017 · 14 comments · May be fixed by #5816
Milestone

Comments

@intgr
Copy link

intgr commented Jan 24, 2017

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)
@Lukasa
Copy link
Member

Lukasa commented Jan 24, 2017

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 Lukasa added this to the 3.0.0 milestone Jan 24, 2017
@intgr
Copy link
Author

intgr commented Jan 24, 2017

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

@Lukasa
Copy link
Member

Lukasa commented Jan 24, 2017

"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.

@intgr
Copy link
Author

intgr commented Jan 27, 2017

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)

@megahall
Copy link

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?

@Lukasa
Copy link
Member

Lukasa commented May 16, 2017

No change.

@Justsoos
Copy link

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)

@schlenk
Copy link

schlenk commented Jul 7, 2018

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

avishaymahluf pushed a commit to rhos-infra/reportportal that referenced this issue Nov 19, 2018
Due to open bug for requests python package:
psf/requests#3829

Remove the "REQUESTS_CA_BUNDLE" env param when ssl verify is false.

Change-Id: Ifc47937bdc5839b08af93851f012a1a67075c453
@mback2k
Copy link

mback2k commented Jun 5, 2020

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. 😞

@fopina
Copy link

fopina commented Feb 20, 2021

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...

@hardenchant
Copy link

hardenchant commented Mar 5, 2021

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

SteveMcGrath added a commit to librestfly/restfly that referenced this issue Mar 9, 2021
Akasurde added a commit to Akasurde/requests that referenced this issue May 14, 2021
When session.verify=False, session.trust_env=True and
REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE is defined as environment
variables then, notify user that requests will use environment variables
rather than silently failing.

partially fixes: psf#3829

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
Akasurde added a commit to Akasurde/requests that referenced this issue May 14, 2021
When session.verify=False, session.trust_env=True and
REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE is defined as environment
variables then, notify user that requests will use environment variables
rather than silently failing.

partially fixes: psf#3829

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
@Akasurde Akasurde linked a pull request May 14, 2021 that will close this issue
@Akasurde
Copy link

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 added a commit to Akasurde/requests that referenced this issue May 14, 2021
When session.verify=False, session.trust_env=True and
REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE is defined as environment
variables then, notify user that requests will use environment variables
rather than silently failing.

partially fixes: psf#3829

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
inetAnt added a commit to inetAnt/napalm-servertech-pro2 that referenced this issue Jul 1, 2021
Due to psf/requests#3829, the `verify` flag
has no effect when the `REQUESTS_CA_BUNDLE` environment variable is set.
inetAnt added a commit to napalm-automation-community/napalm-servertech-pro2 that referenced this issue Jul 1, 2021
Due to psf/requests#3829, the `verify` flag
has no effect on Sssions when the `REQUESTS_CA_BUNDLE` env variable is set.
inetAnt added a commit to napalm-automation-community/napalm-servertech-pro2 that referenced this issue Jul 1, 2021
Due to psf/requests#3829, the `verify` flag
has no effect on Sssions when the `REQUESTS_CA_BUNDLE` env variable is set.
BenGale93 added a commit to BenGale93/confluencebuilder that referenced this issue Dec 11, 2021
Due to this issue: psf/requests#3829
the confluence_disable_ssl_validation setting does not work in all cases.
As described in the requests issue, passing the verify variable
into the method call does work.

Signed-off-by: Ben Gale <bengale2007@googlemail.com>
Akasurde added a commit to Akasurde/requests that referenced this issue Jun 2, 2022
When session.verify=False, session.trust_env=True and
REQUESTS_CA_BUNDLE or CURL_CA_BUNDLE is defined as environment
variables then, notify user that requests will use environment variables
rather than silently failing.

partially fixes: psf#3829

Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
openstack-mirroring pushed a commit to openstack/sushy that referenced this issue Jan 26, 2023
if the env REQUESTS_CA_BUNDLE is set the requests.Session() ignores the
verify parameter. Therefore the verify parameter is moved directly into
the function call of request.

Workaround for psf/requests#3829

Change-Id: I66dc7c4d90e5bd5e3d1b331cf1728f27dece6dd4
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Jan 26, 2023
* Update sushy from branch 'master'
  to 98c899997f4f7b7aaccf911c41655aad6548fe6c
  - workaround: requests verify handling if env is set
    
    if the env REQUESTS_CA_BUNDLE is set the requests.Session() ignores the
    verify parameter. Therefore the verify parameter is moved directly into
    the function call of request.
    
    Workaround for psf/requests#3829
    
    Change-Id: I66dc7c4d90e5bd5e3d1b331cf1728f27dece6dd4
openstack-mirroring pushed a commit to openstack/sushy that referenced this issue May 1, 2023
if the env REQUESTS_CA_BUNDLE is set the requests.Session() ignores the
verify parameter. Therefore the verify parameter is moved directly into
the function call of request.

Workaround for psf/requests#3829

Change-Id: I66dc7c4d90e5bd5e3d1b331cf1728f27dece6dd4
(cherry picked from commit 98c8999)
elfosardo pushed a commit to elfosardo/openstack-sushy that referenced this issue May 3, 2023
if the env REQUESTS_CA_BUNDLE is set the requests.Session() ignores the
verify parameter. Therefore the verify parameter is moved directly into
the function call of request.

Workaround for psf/requests#3829

Change-Id: I66dc7c4d90e5bd5e3d1b331cf1728f27dece6dd4
@a-belhadj
Copy link

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

rbikar added a commit to rbikar/ubi-population-tool that referenced this issue Oct 29, 2023
Due to bug psf/requests#3829,
setting 'verify' on session doesn't work if REQUESTS_CA_BUNDLE
is set on environment. The bundle defined via REQUESTS_CA_BUNDLE
will take precedence and the custom bundle provided via 'verify'
is ignored.

Let's now set 'verify' for each request.

This can be revetrted when the bug is fixed, likely in
python-requests-v3.
@psf psf locked and limited conversation to collaborators Feb 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

11 participants