python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.9k stars 2.28k forks source link

poetry.lock hashes not checked when running poetry install #2422

Closed slai closed 3 years ago

slai commented 4 years ago

Issue

It appears that Poetry does not check the hashes of the packages in poetry.lock when installing packages using poetry install.

The Dockerfile in this gist (https://gist.github.com/slai/9d0d442fe7e4f4ea04e8f658b675192a) demonstrates the issue. It runs poetry lock, mangles the hashes in poetry.lock, then runs poetry install. This succeeds, with no errors.

I would expect poetry install to fail in this case, or at least print a warning for packages where the hash does not match.

signed8bit commented 3 years ago

It's a bit concerning that this issue has been open for nearly a year now. Is this something the core team is looking to address?

FlorianLudwig commented 3 years ago

It doesn't seem so. I opened a new MR here: https://github.com/python-poetry/poetry-core/pull/113

Since the code moved into a different repo.

signed8bit commented 3 years ago

Thanks for the pointer @FlorianLudwig!

pietrodn commented 3 years ago

List of PRs related to this issue:

Can we merge one of the valid ones? Not checking the hashes in poetry.lock poses a significant security problem.

pietrodn commented 3 years ago

In particular python-poetry/poetry-core#159 + #3885 are meant to be fixed together from what I understand. The other PRs can be closed.

remram44 commented 3 years ago

I recommend we don't close anything until we manage to catch the eye of a maintainer. It looks like if anything we should make more noise about this.

cburgess commented 3 years ago

I very much agree we need to get something merged and released to address this. With recent supply chain attacks in the media, a number of enterprises are launching efforts to harden their build and release processes and hash checking is one of the key features they are looking for, an assume poetry provides.

myoung34 commented 3 years ago

Anything upcoming soon?

As per our security requirements its hard to justify losing hashes or shimming our poetry.lock to requirements.txt to keep them

waitsfornone commented 3 years ago

I am wanting to move to Poetry in production, but I would prefer to have hash checking before I do so. With global Data protection laws being stricter, I need to know that I am not vulnerable through my dependency management.

rabadin commented 3 years ago

Same here, this issue is the one big blocker that is preventing us from adopting poetry as our standard to manage Python projects.

JonZeolla commented 3 years ago

This issue was the primary reason why my company standardized on pipenv instead.

dpippenger commented 3 years ago

A CVE should be filed for this issue as it presents a supply chain attack vector that puts any poetry user relying on hash validations at risk of compromise.

finswimmer commented 3 years ago

Hello everyone,

unfortunately my knowledge about this part of code of poetry is to limited to review the existing PRs. I can see that this issue is serious for many people, so I pinged @sdispater and @abn directly to have a look on it.

fin swimmer

pietrodn commented 3 years ago

I confirm that with the latest master version the issue is solved (I tried to change the hashes in poetry.lock and the installation errored). Thank you! 🚀

fredrikaverpil commented 3 years ago

I suspect this was not released in a formal release yet. It would be really good if we can have this included the next time a release is being cut. @finswimmer @abn @sdispater do you intend to include this in a 1.1.x release?

pietrodn commented 3 years ago

This fix is not yet included in the latest bugfix release (1.1.7). It would be great if it was included in the next one, as it's an important security issue.

pietrodn commented 3 years ago

I made a PR to backport this fix to Poetry 1.1: https://github.com/python-poetry/poetry/pull/4420

slai commented 3 years ago

I've just tested this using the gist in the description and the newly released poetry 1.1.9, and can confirm poetry install fails if the hashes do not match those in poetry.lock.

> poetry install --ansi

 Installing dependencies from lock file

 Package operations: 1 install, 0 updates, 0 removals

   • Installing chardet (3.0.4): Failed

   RuntimeError

   Retrieved digest for link chardet-3.0.4.tar.gz(sha256:84ab92ed1c4d4f16916e05906b6b75a6c0fb5db821cc65e70cbd64a3e2a5eaae) not in poetry.lock metadata ['sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef', 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef']

   at ~/.poetry/lib/poetry/installation/chooser.py:115 in _get_links
       111│
       112│         if links and not selected_links:
       113│             raise RuntimeError(
       114│                 "Retrieved digest for link {}({}) not in poetry.lock metadata {}".format(
     → 115│                     link.filename, h, hashes
       116│                 )
       117│             )
       118│
       119│         return selected_links

Thanks @pietrodn @sdispater and everyone else involved!

fredrikaverpil commented 3 years ago

I haven't tested this yet but huge thanks for everyone involved in this! 🎉

jowparks commented 3 years ago

This hash system is broken for me, we have an internal pypi repo and the one package we have installed from there, we the poetry.lock is outputting a md5 hash when I run poetry install and regenerate the lock file:

snowflake-rdp-broker = [
    {file = "snowflake-rdp-broker-1.5.9.tar.gz", hash = "md5:fa3d9fc394bd06488e07a54fc788b1db"},
]

Then when I try to run poetry install, I get:

  Invalid hash for snowflake-rdp-broker (1.5.9) using archive snowflake-rdp-broker-1.5.9.tar.gz

  at /usr/local/lib/python3.9/site-packages/poetry/installation/executor.py:613 in _download_link
      609│ 
      610│         if package.files:
      611│             archive_hash = "sha256:" + FileDependency(package.name, archive).hash()
      612│             if archive_hash not in {f["hash"] for f in package.files}:
    → 613│                 raise RuntimeError(
      614│                     "Invalid hash for {} using archive {}".format(package, archive.name)
      615│                 )
      616│ 
      617│         return archive
remram44 commented 3 years ago

@jowparks Do you have to use md5 instead of sha256? Is that even supported by pip?

In any case, this seems unrelated to this (closed) issue.

jowparks commented 3 years ago

@jowparks Do you have to use md5 instead of sha256? Is that even supported by pip?

In any case, this seems unrelated to this (closed) issue.

I don't do anything special for that package, I am fine with using sha256 and in fact the nexus pypi repo lists all the different hashes in the UI.

The relevant sections of the pyproject.toml are:

[[tool.poetry.source]]
name = "private-pypi"
url = "https://nexus.ourdomain.net/repository/invitae-pypi/simple/"

[tool.poetry.dependencies]
snowflake_rdp_broker = "*"

then we just run

pip install poetry && poetry install

which fails. This succeeds:

pip install poetry==1.0.10 && poetry install

Also I will note that all the other dependencies (which are from public pypi) have sha256 hashes in the generatedpoetry.lock, only the internal pypi is getting an md5 hash, I have no idea why 🤷

remram44 commented 3 years ago

This seems to be #4523

github-actions[bot] commented 9 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.