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

[REQ] [Python] Use urllib3 default (system) CA bundle instead of certifi #6506

Closed
rparini opened this issue May 31, 2020 · 2 comments · Fixed by #8108
Closed

[REQ] [Python] Use urllib3 default (system) CA bundle instead of certifi #6506

rparini opened this issue May 31, 2020 · 2 comments · Fixed by #8108

Comments

@rparini
Copy link
Contributor

rparini commented May 31, 2020

Is your feature request related to a problem? Please describe.

Since version 1.25.3 urllib3 defaults to using the system Certificate Authority (CA) bundle:

1.25.3 (2019-05-23)

  • Change HTTPSConnection to load system CA certificates when ca_certs, ca_cert_dir, and ssl_context are unspecified. (Pull #1608, Issue #1603)

However, the Python client currently overrides this default, using a CA bundle from certifi, instead of the system bundle, if the user does not specify configuration.ssl_ca_cert.

Describe the solution you'd like

I propose removing certifi as a dependency, requiring urllib3 >= 1.25.3 and relying on urllib3 to handle the case when no CA bundle is specified by the user.

My main reasoning is that it’s up to urllib3 to handle the ssl verification and it would be a better separation of concerns to simply pass configuration.ssl_ca_cert through to urllib3.PoolManager or urllib3.ProxyManager’s ca_certs argument without additional logic. The generated client should still work "out of the box" for most people now that urllib3 has a default that allows for ssl verification using the system CA store.

My secondary concern is having certifi as a default and required dependency in a project like this that's intended to run in a lot of different environments:

  • The Windows system CA store is a registry entry, not a file that can be passed to ca_certs, so it seems quite difficult to override the certifi default with the Windows CA bundle. However, if the system CA bundle was the default then it would be easy for the user to override it with configuration.ssl_ca_cert=certifi.where() if they choose.
  • In a corporate setting the system CA store may be centrally managed to ensure frequent updates and/or company signed certificates for connecting to company services or proxies. An external bundle like certifi will not include company root certificates and its installation may itself present a security policy violation under these circumstances.

If there’s some general agreement on this I’d be happy to submit a pull request.

@aaichert
Copy link

aaichert commented Nov 9, 2020

There is currently no way to just tell the library to use the default system certificates without code change. Should this not be a bug report rather than a feature request?

Trusting a certificate should not require changes on the code level.

In addition, libraries such as requests uses environment variables, just like familiar tools such as curl do. In combination with the above issue, with urllib3, there is currently no way at all to trust a certificate from "outside python". Correct?

@rparini
Copy link
Contributor Author

rparini commented Dec 6, 2020

There is currently no way to just tell the library to use the default system certificates without code change. Should this not be a bug report rather than a feature request?

Possibly, but I assumed using certifi for the default certificate store was “working as intended” so using the system default instead felt more like a feature request to me. I have no objections if a maintainer wants to change the label, however.

In addition, libraries such as requests uses environment variables, just like familiar tools such as curl do. In combination with the above issue, with urllib3, there is currently no way at all to trust a certificate from "outside python". Correct?

Yes, you're right, that's another limitation of the current behaviour. At the moment, because urllib3's ca_certs is explicitly set by default to certifi.where() it will only look there for the CA store, not in envrionment variables. With my proposed change to let urllib3 manage its own defaults you would be able to use the SSL_CERT_DIR and SSL_CERT_FILE environment variables as for curl and documented in PEP 476 provided that the user has not explictly set configuration.ssl_ca_cert (in which case urllib3 would use that instead).

spacether pushed a commit that referenced this issue Dec 16, 2020
* Use system CA by default and remove certifi

See #6506

* Use system CA by default in asyncio client

* Update README_onlypackage.mustache

* Result of ./bin/generate-samples.sh

* Add ssl_ca_cert argument for Configuration

* Result of ./bin/generate-samples.sh

* Remove certifi, use system CA by default
therve pushed a commit to DataDog/datadog-api-client-python that referenced this issue Mar 23, 2021
* Use system CA by default and remove certifi

See OpenAPITools/openapi-generator#6506

* Use system CA by default in asyncio client

* Update README_onlypackage.mustache

* Result of ./bin/generate-samples.sh

* Add ssl_ca_cert argument for Configuration

* Result of ./bin/generate-samples.sh

* Remove certifi, use system CA by default
jirikuncar pushed a commit to DataDog/datadog-api-client-python that referenced this issue Sep 8, 2021
* Use system CA by default and remove certifi

See OpenAPITools/openapi-generator#6506

* Use system CA by default in asyncio client

* Update README_onlypackage.mustache

* Result of ./bin/generate-samples.sh

* Add ssl_ca_cert argument for Configuration

* Result of ./bin/generate-samples.sh

* Remove certifi, use system CA by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants