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

Secure Pex against sha1 collision attacks. #1662

Closed
jsirois opened this issue Mar 11, 2022 · 1 comment · Fixed by #1661
Closed

Secure Pex against sha1 collision attacks. #1662

jsirois opened this issue Mar 11, 2022 · 1 comment · Fixed by #1661
Assignees
Labels
resolver tech-debt Issue that addresses technical debt.

Comments

@jsirois
Copy link
Member

jsirois commented Mar 11, 2022

Although lockfiles introduce protection against sha1 collision attacks when resolving artifacts since PyPI is de-facto all sha256 fingerprinted, Pex internal caches all use sha1.

@jsirois
Copy link
Member Author

jsirois commented Mar 11, 2022

It seems like protection against external (downloaded) artifacts is enough here. The other inputs to a PEX file are the local interpreter and local --source added to the PEX. Both of these are implicitly trusted. Switching the built_wheels and installed_wheels caches to use sha256 should be enough. The unzipped_pexes and venvs caches are composed from those and guarded by the pex_hash in PEX-INFO which is a sha1 hash of the PEX-INFO json which includes the sha256 hashes of all contained dependency artifacts. I'm no security expert, but to create an attack on that sha1 the only attack surface is the local code hash (which is sha1) and other PEX-INFO settings. All of these items are under local control and so implicitly trusted again.

@jsirois jsirois mentioned this issue Mar 12, 2022
2 tasks
jsirois added a commit that referenced this issue Mar 12, 2022
Previously Pex used Pip's `--target` scheme which had both known bugs
(pypa/pip#7658) and unknown quirks that Pex
was failing to fully be able to work around. Switch to the `--prefix`
scheme which exactly mirrors the scheme venvs use so that venvs can be
created with content of all sorts placed where it belongs.

This removes fragile parsing and interpretation of the RECORD; now Pex
only creates a RECORD, which is much more straight forward, when
building a venv.

Partially addresses #1655 by switching to sha256 for all external
artifact hashing. Only internal hashing remains for:
1. `interpreters` / INTERP-INFO
2. `venvs` and `unzipped_pexes` / PEX-INFO pex_hash (but this is a hash
   that includes all distributions' sha256 hashes).

Fixes #1656
Closes #1662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolver tech-debt Issue that addresses technical debt.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant