Skip to content

fix: verify TLS against certifi instead of a vendored cacert.pem - #272

Open
dominicroessner wants to merge 1 commit into
pusher:masterfrom
dominicroessner:use-certifi-ca-bundle
Open

dominicroessner wants to merge 1 commit into
pusher:masterfrom
dominicroessner:use-certifi-ca-bundle

Conversation

@dominicroessner

Copy link
Copy Markdown

What does this PR do?

Stops shipping pusher/cacert.pem and verifies TLS against certifi.where() instead.

The vendored bundle was last updated in 2021 and still trusts emSign ECC Root CA - G3 and AAA Certificate Services. Those are the roots that the 8 compromised eMudhra intermediate CAs chain to (see #271 for details and links). Because RequestsBackend forces verify to that bundle, a client on any released version accepts a forged certificate for the Pusher API from those keys, regardless of which certifi is installed.

Changes:

  • pusher/cacert.pem: removed
  • pusher/requests.py: CERT_PATH = certifi.where() (name kept so existing imports keep working)
  • setup.py: add certifi to install_requires; drop the package_data entry for cacert.pem
  • pusher_tests/test_requests_adapter.py: tests that https requests use certifi.where() and that http requests set no verify

Fixes #271. Complementary to #270, which bumps certifi in the dev/test pins in requirements.txt only; that does not affect what the library trusts at runtime, which is why this change is needed.

Testing

  • python -m unittest discover -s pusher_tests --top-level-directory . passes on Python 3.10, 3.11 and 3.12 (68 tests, 1 skipped; 66 before this change).
  • The new verify test fails against the old requests.py and cacert.pem.
  • Built a wheel: it no longer contains cacert.pem and declares Requires-Dist: certifi.
  • Installed only that wheel in a clean venv and called trigger against the public test hosts from https://emudhra.we-have-a-problem.com/ (dv-g3b-emudhra and dv-g3b-aaa-emudhra). Both are now rejected with an SSLError. On 3.3.4 they are accepted.

Notes

  • RequestsBackend still overwrites any caller-supplied verify option, as before. Letting callers override it (e.g. setdefault) would be a reasonable follow-up, but I kept this PR to the bundle itself.

  • If you have changed dependencies, ensure both requirements.txt and setup.py have been updated

CHANGELOG

  • [FIXED] Stop shipping a vendored cacert.pem and verify TLS against certifi.where(). The bundled roots still trusted the compromised eMudhra intermediate CAs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Vendored cacert.pem still trusts the compromised eMudhra intermediate CAs and cannot be overridden

1 participant