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

Optional truststore support #11082

Merged
merged 2 commits into from
May 30, 2022
Merged

Optional truststore support #11082

merged 2 commits into from
May 30, 2022

Conversation

uranusjr
Copy link
Member

@uranusjr uranusjr commented May 2, 2022

This adds a --use-feature=truststore flag that, when specified on Python 3.10+ with truststore installed, switches pip to use truststore to provide HTTPS certificate validation, instead of certifi. This allows pip to verify certificates against custom certificates in the system store.

truststore is deliberately NOT vendored because it is expected the library to be under active development in the short term, and this prevents users having to wait for a pip release to get potentially vital bug fixes needed to be made in truststore.

Supplying the use-feature flag without installing truststore beforehand, or on Python versions prior to 3.10, results in a command error.

See #11038 (and other issues linked there).

@pfmoore
Copy link
Member

pfmoore commented May 2, 2022

I like this approach.

This adds a --use-feature=truststore flag that, when specified on Python
3.10+ with truststore installed, switches pip to use truststore to
provide HTTPS certificate validation, instead of certifi. This allows
pip to verify certificates against custom certificates in the system
store.

truststore is deliberately NOT vendored because it is expected the
library to be under active development in the short term, and this
prevents users having to wait for a pip release to get potentially vital
bug fixes needed to be made in truststore.

Supplying the use-feature flag without installing truststore beforehand,
or on Python versions prior to 3.10, results in a command error.
@uranusjr
Copy link
Member Author

uranusjr commented May 3, 2022

Can’t think of a very good way to test this, did what I could. It’s difficult to test against a custom cert, and this is supposed to be seamless otherwise 😅

@uranusjr uranusjr marked this pull request as ready for review May 3, 2022 17:05
@uranusjr uranusjr added this to the 22.1 milestone May 3, 2022
@pfmoore
Copy link
Member

pfmoore commented May 3, 2022

Can’t think of a very good way to test this

I guess you could install a dummy package that provided an importable "truststore" which printed a message on stdout but otherwise did nothing, and test that it got called? Sort of like a custom mock object. OTOH, maybe that's more work than this warrants - what you've got seems fine.

@uranusjr
Copy link
Member Author

uranusjr commented May 3, 2022

(I forgot whether we said we want this in 22.1 or 22.2.)

@sethmlarson
Copy link
Contributor

sethmlarson commented May 4, 2022

Adding a note on testing pip+truststore together: I've created sethmlarson/truststore#49 which once this PR is merged we'll begin testing pip with truststore mode enabled using a custom certificate loaded into the OS via mkcert so you will have more confidence that the world won't break even if this is an experimental feature.

@pradyunsg
Copy link
Member

(I forgot whether we said we want this in 22.1 or 22.2.)

I think 22.2 is a better idea. I'd like to get 22.1 out in this week.

@pradyunsg pradyunsg removed this from the 22.1 milestone May 9, 2022
@sethmlarson
Copy link
Contributor

sethmlarson commented May 18, 2022

Apologies for the ping, but in the interest of making sure this PR doesn't go stale: it looks like 22.1 was released a week ago. Are we clear to merge this into the development branch so we can start building our integration tests around pip's repository?

Also once this is merged I can take a stab at contributing documentation to pip's "User's Guide" on this feature if that's desirable from the team. Thanks again!

@uranusjr uranusjr added this to the 22.2 milestone May 19, 2022
@uranusjr
Copy link
Member Author

I’m pulling the trigger.

@uranusjr uranusjr merged commit b91dbde into pypa:main May 30, 2022
@uranusjr uranusjr deleted the truststore branch May 30, 2022 06:32
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants