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

Use system TLS certificates instead of certifi #1831

Closed
legoktm opened this issue Aug 1, 2023 · 3 comments · Fixed by #1718
Closed

Use system TLS certificates instead of certifi #1831

legoktm opened this issue Aug 1, 2023 · 3 comments · Fixed by #1718
Labels

Comments

@legoktm
Copy link
Member

legoktm commented Aug 1, 2023

certifi is a dependency of requests and a redistribution of the Mozilla-approved root certificates.

Debian (and Ubuntu) already redistribute this as the ca-certificates package (https://tracker.debian.org/pkg/ca-certificates) that's installed on basically every system. Using the system store means that we can rely on Debian/Ubuntu to provide security updates to revoke bad root certs automatically instead of us needing to manually supply updates.

AFAIK requests no longer has an environment variable override to point at the system certificate store, instead they want you to override/patch the certifi.where() function. There's a certifi-debian fork on pypi that does that. (Despite the name it should also work on Fedora).

So we could either replace certifi dependencies with certifi-debian, or monkey-patch certifi.where() in Python before requests is loaded.

I believe this affects both the securedrop-sdk (which is embedded in -client) and securedrop-proxy components.

@legoktm
Copy link
Member Author

legoktm commented Aug 1, 2023

I don't think there's any straightforward way in pip nor poetry to swap out a dependency (see e.g. Cargo's patch: https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html), and monkey-patching is more explicit, so that's probably the way to go.

@rocodes
Copy link
Contributor

rocodes commented Aug 2, 2023

I admittedly don't have tons of context in this area, but my understanding that is we used certifi because we specifically wanted to rely on certifi / Mozilla certs and not on the system handling of certs, eg due to historical bugs such as this one (summarized certifi/python-certifi#35 (comment)).

That issue and discussion are ~8-10 years old, and I see there's been a fair bit of discussion in the requests community about using the system store for various reasons (lots of users with custom certs on corp networks, for example), and there are also newer projects like https://github.com/sethmlarson/truststore to facilitate using the system certificate store with requests. [Edit to add: our threat model is different from theirs, so just because there is community conversation about moving towards system store does not mean that that is the direction we should take; just noting that this conversation is happening and the alternative projects to support it do exist.]

Basically, I'm fine with making a change as long as we're aware of why we weren't doing it in the first place :), and as long as it's an informed choice and we aren't missing any historical context, since we make use of certifi in a number of fpf projects.

@legoktm
Copy link
Member Author

legoktm commented Feb 13, 2024

I had forgotten about this ticket, but I appreciate the historical input, the CA ecosystem has thankfully improved a bit.

I believe this affects both the securedrop-sdk (which is embedded in -client) and securedrop-proxy components.

The client/SDK only used certifi during local development, which is going away in proxy v2. And in proxy v2, we're switching to the Rust openssl crate, which will use system certificates (instead of like, webpki-roots, the certifi equivalent).

and as long as it's an informed choice and we aren't missing any historical context, since we make use of certifi in a number of fpf projects.

Ack. I'm going to move this into the client repo and tag it with proxy v2 so we consider it in that scope since we're effectively making this change at that time.

@legoktm legoktm transferred this issue from freedomofpress/securedrop-workstation Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants