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

Add option to use platform default certificates for making HTTPS calls #10961

Closed
1 task done
notatallshaw opened this issue Mar 11, 2022 · 10 comments
Closed
1 task done
Labels
project: vendored dependency Related to a vendored dependency type: feature request Request for a new feature

Comments

@notatallshaw
Copy link
Contributor

notatallshaw commented Mar 11, 2022

What's the problem this feature will solve?

Specifically on Windows you can not get Pip to easily use the platforms Certificate Store for verifying the Certificate Authority. Other platforms are not as affected because it is possible to provide an environmental variable to point to a Certificate Store file, but Windows does not provide the Certificate Store as a file.

Particularly in organizations that use their own certificates for internal repositories this causes Pip to fail connecting.

Describe the solution you'd like

Since Python 3.4 the ssl module has provided a way to get the Platforms default cert store which on Windows will use the Certificate Store: https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_default_certs

Here is an example of how you get requests to use it: https://stackoverflow.com/a/50215614

Alternative Solutions

Keep as is

Additional context

Should reduce the number of use cases where --trusted-host is currently required.

Code of Conduct

@notatallshaw notatallshaw added S: needs triage Issues/PRs that need to be triaged type: feature request Request for a new feature labels Mar 11, 2022
@uranusjr
Copy link
Member

I wonder if it would be worthwhile to start a transition to use the system store by default (and make the requests certs an opt-in).

@uranusjr uranusjr removed the S: needs triage Issues/PRs that need to be triaged label Mar 11, 2022
@pradyunsg
Copy link
Member

I'd much rather that we mirror what requests does by default (which is also the behaviour users will see in other places) and defer to it for making the decision for such a migration.

@notatallshaw
Copy link
Contributor Author

I wonder if it would be worthwhile to start a transition to use the system store by default (and make the requests certs an opt-in).

I have two concerns about making it default and why I suggested it as optional:

  1. No doubt lots of pip users would be disrupted in someway that is tricky to test or predict ahead of time
  2. I think on Linux the location of certificate paths is not standardized and defined at compile time? Could be tricky to test across lots of distros. Or at least I seem to remember reading Christian Heimes say something like that (can't find the source right now I'll post later if I find what I was thinking of)

@pradyunsg
Copy link
Member

What would "platform default" mean on Linux? As you've said, there's no real standard location for where the certificates are kept.

@notatallshaw
Copy link
Contributor Author

What would "platform default" mean on Linux? As you've said, there's no real standard location for where the certificates are kept.

Well as I mentioned in the original post this largely solves a problem on Windows because on Linux I can set the env variable REQUESTS_CA_BUNDLE and point it to wherever my specific distro / platform places the Certificate Authority file.

Otherwise I think you're depending on the distro and OpenSSL compilation to correctly provide the value: https://docs.python.org/3/library/ssl.html#ssl.SSLContext.set_default_verify_paths

@pfmoore
Copy link
Member

pfmoore commented Mar 11, 2022

I'd much rather that we mirror what requests does by default

I agree. IMO, one point of using a library for this is so that we can rely on their expertise for how to handle stuff they specialise in. I'd be happy to see the system store get used, but I think the choice should be made by requests, not by us.

@notatallshaw
Copy link
Contributor Author

notatallshaw commented Mar 11, 2022

I'd much rather that we mirror what requests does by default

I agree. IMO, one point of using a library for this is so that we can rely on their expertise for how to handle stuff they specialise in. I'd be happy to see the system store get used, but I think the choice should be made by requests, not by us.

I agree that a simple supported option from the underlying HTTP library that pip could expose would be best.

However it seems that Python HTTP libraries expect users to use their APIs to insert SSL contexts when they want to do anything other than use certifi. With is fine for most Python libraries because you can usually somehow insert your own HTTPAdapter in to the requests calls, but Pip is special 🙂 .

Perhaps it could be revisited in a few years if requests starts adding features again and it can be added there or if pip moves to httpx and if the feature is added there.

@pradyunsg pradyunsg added the project: vendored dependency Related to a vendored dependency label Mar 12, 2022
@uranusjr
Copy link
Member

uranusjr commented Mar 12, 2022

Linking to the (pretty old) requests issue on this for reference: psf/requests#2966

@notatallshaw
Copy link
Contributor Author

notatallshaw commented Mar 12, 2022

Linking to the (pretty old) requests issue on this for reference: psf/requests#2966

In fact it seems the very last comment is someone who has this exact use case and they currently have written their own package to monkey patch Pip's session object so that it uses the system trust store.

Reading through that thread though I also see there an important edge case to using my originally linked solution. That is the Certificate Store on Windows is lazily loaded and this code does not force certificates that are available but not loaded to load. It seems a program must use Schannel or an API that calls it to ensure certificate is loaded.

I don't know of a publicly available way to get requests to use Schannel in a well tested non-deprecated form, I do have a working sample script that uses oscrypto (wbond/oscrypto#10 (comment)) but it doesn't work for my use case because I think that client authentication isn't supported (wbond/oscrypto#4)

The only way I know to get Schannel to work with Python (without relying on platform specific libraries like pythonnet or win32api) is to use pycurl as libcurl supports Schannel. But pycurl is far too low level to be used by Pip (and would maybe require shipping compiled C code which is presumably also a non-starter for Pip).

tl;dr: After reading through that thread I agree this is probably more hassle than it's worth for Pip until the HTTP library Pip uses explicitly supports this option.

@pradyunsg
Copy link
Member

Looks like we're in agreement that this needs to happen in requests and then we'll pull in that behaviour from there.

I'm going to go ahead and close this then, since I don't think there's much value in tracking whether requests has added this functionality or not.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
project: vendored dependency Related to a vendored dependency type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

4 participants