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

Refactor: replace requests with urllib3 #1040

Open
woodruffw opened this issue Jun 7, 2024 · 5 comments
Open

Refactor: replace requests with urllib3 #1040

woodruffw opened this issue Jun 7, 2024 · 5 comments
Assignees
Labels
chore enhancement New feature or request refactoring Refactoring tasks.

Comments

@woodruffw
Copy link
Member

urllib3 has everything we need from requests (and is the library beneath it), including connection pooling. It's much more actively maintained and has a more consistent API, so we should switch to it.

@jku
Copy link
Member

jku commented Jan 8, 2025

linking to theupdateframework/python-tuf#2762 FYI (a related PR for python-tuf).

One thing to note that already came up:

  • urllib3 defaults to system certificates while requests defaults to certifi

@woodruffw
Copy link
Member Author

  • urllib3 defaults to system certificates while requests defaults to certifi

Yep, this is a "plus" IMO: the fact that PyPI is a CA bundle delivery mechanism (for certifi) has always been one of those "don't look but this is very scary" realities about HTTPS in Python 🙂

woodruffw added a commit to di/id that referenced this issue Jan 8, 2025
This simplifies our dependency tree a bit,
since requests is built on urllib3 internally.

It also chips away at a larger plan to replace
requests with urllib3 in sigstore-python, per
sigstore/sigstore-python#1040.

Signed-off-by: William Woodruff <[email protected]>
@jku
Copy link
Member

jku commented Jan 9, 2025

Yeah I definitely agree with your concerns -- what I'm not sure is how to combine the two views:

  • scary-http-security-bootstrap-situation you mentioned + some users would definitely like to use system certificates
  • python ecosystem seems to like certifi for platform-compat concerns. I am not familiar with the compatibility issue in question but I guess it's windows?

Basically, can we just switch to using system certificates for https without collateral damage? is there a way for downstream (applications/distributors/users) to change that or is that unnecessary (I remember requests had a REQUESTS_CA_BUNDLE hack)?

I see Seth has a potential solution https://github.com/sethmlarson/truststore -- I suppose this means we could just use system certs and point people to truststore (and potentially use truststore in the sigstore-python CLI ourselves)

@woodruffw
Copy link
Member Author

  • python ecosystem seems to like certifi for platform-compat concerns. I am not familiar with the compatibility issue in question but I guess it's windows?

Windows may have been a concern, but (IIRC) the original motivation for certifi was that Linux distributions didn't do a consistently good job of updating/maintaining their CA bundles, nor did users do a consistently good job of retrieving distribution-pushed updates. The Mozilla root program was (and is) the best at the time, so certifi just redistributed that.

In practice, I think this problems have been mostly ameliorated: the Web PKI is in much better shape than it was a decade+ ago, and root program/OS providers tend to be much more stringent about security updates.

With urllib3 we could allow an override via urllib3.PoolManager(ca_certs="..."), although IMO we shouldn't expose that as a public interface until a user actually explicitly requests it (and presents a good justification -- 99% of the time they should be configuring their system roots instead). Thoughts on that?

@jku
Copy link
Member

jku commented Jan 11, 2025

I think that sounds very reasonable.

@sethmlarson if you have the time, would you mind commenting on this one?

  • are we overlooking any potential problems when switching from certifi to system certs
  • should we (as library) mention truststore in docs?
  • should we (as CLI app) use truststore explicitly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore enhancement New feature or request refactoring Refactoring tasks.
Projects
None yet
Development

No branches or pull requests

2 participants