ossf / scorecard

OpenSSF Scorecard - Security health metrics for Open Source
https://scorecard.dev
Apache License 2.0
4.45k stars 487 forks source link

pip install shouldn't be banned #1306

Closed evverx closed 2 years ago

evverx commented 2 years ago

From https://github.com/ossf/scorecard/pull/611#discussion_r752910507

I think we should just recommend pipenv instead, and consider all direct pip install commands as unpinned

I think it's overkill though when all I need is to install a package in a test script. I'm planning to switch to -r in systemd/systemd#21438 and I can theoretically add --require-hashes there since Dependabot supports it but switching to pipenv or any other tool creating virtual python environments is absolutely unwarranted in that particular case I think

laurentsimon commented 2 years ago

Thanks for sharing. Tests are typically run on pull requests, in which case getting pwned is less serious. So this is again an instance of test false positive, it seems?

We need to start a doc around best security practice for pipy in general, because there's no general doc or agreement so it's hard for scorecard to implement something that does not exists. We've started one for npm but it's WIP.

evverx commented 2 years ago

Tests are typically run on pull requests, in which case getting pwned is less serious

Scripts like that can be run on push events or be "scheduled" as well and in some repositories they can have access to GITHUB_TOKEN with the "write" permissions granted to GHActions by default: https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token so in theory they can pwn repositories.

evverx commented 2 years ago

Just to clarify, I think in this case scorecard rightfully complains and pip should be changed there one way or another but it isn't obvious how it can be done without turning it into an unmaintainable mess

laurentsimon commented 2 years ago

cc @di who may know what pipy are planning to improve tooling

di commented 2 years ago

There's not really anything functionally different between using pipenv's lockfile and using requirements.txt with --require-hashes, a pip install with --require-hashes should be allowed IMO.

laurentsimon commented 2 years ago

thanks @di. One limitation of requirements.txt is that it does not include transitive dependencies. Is this correct? If so, any plans to change this behavior?

di commented 2 years ago

A requirements.txt file without hashes does not need to include transitive dependencies, but using --require-hashes enables strict hash checking mode, in which hashes must be included for all packages to be installed, including transitive dependencies, so it has the effect of requiring transitive dependencies to be included.

Example:

$ cat requirements.txt
sampleproject==2.0.0 \
    --hash=sha256:2b0c55537193b792098977fdb62f0acbaeb2c3cfc56d0e24ccab775201462e04 \
    --hash=sha256:d99de34ffae5515db43916ec47380d3c603e9dead526f96581b48c070cc816d3

$ pip install -r requirements.txt --require-hashes
Collecting sampleproject==2.0.0
  Using cached sampleproject-2.0.0-py3-none-any.whl (4.2 kB)
Collecting peppercorn
ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
    peppercorn from https://files.pythonhosted.org/packages/14/84/d8d9c3f17bda2b6f49406982546d6f6bc0fa188a43d4e3ba9169a457ee04/peppercorn-0.6-py3-none-any.whl#sha256=46125cad688a9cf3b08e463bcb797891ee73ece93602a8ea6f14e40d1042d454 (from sampleproject==2.0.0->-r requirements.txt (line 1))
laurentsimon commented 2 years ago

thank you, that's great. So it would be fair to add the requirement that --require-hashes is always used in scripts, correct?

laurentsimon commented 2 years ago

reminds me of this conversation https://github.com/ossf/scorecard/pull/611#discussion_r660203476

evverx commented 2 years ago

I tried to actually install meson with --require-hashes and judging by

Collecting install
Collecting meson==0.60.1
  Using cached meson-0.60.1-py3-none-any.whl (837 kB)
ERROR: In --require-hashes mode, all requirements must have their versions pinned with ==. These do not:
    install from https://files.pythonhosted.org/packages/f0/a5/fd2eb807a9a593869ee8b7a6bcb4ad84a6eb31cef5c24d1bfbf7c938c13f/install-1.3.4-py3-none-any.whl#sha256=dfec614e0cc90f00659067a078bc2ec032d9f0758f3df230c073eeaffd0cefa1

it isn't ready to be pinned to any hashes. Looks like at this point in practice it isn't possible to install any non-trivial package that way unfortunately

laurentsimon commented 2 years ago

I think you need to follow this procedure https://github.com/ossf/scorecard/pull/611#discussion_r660203476. That's where tooling improvement may help?

di commented 2 years ago

reminds me of this conversation #611 (comment)

I think that comment is not quite 100% accurate. Installs with --require-hashes will either require all direct and indirect dependencies, or will fail. We don't need to call PyPI's API or use pip-compile to verify this.

I tried to actually install meson with --require-hashes and judging by... it isn't ready to be pinned to any hashes. Looks like at this point in practice it isn't possible to install any non-trivial package that way unfortunately

I think that's because you didn't specify hashes? This works:

$ cat requirements.txt
meson==0.60.1 \
    --hash=sha256:5add789c953d984b500858b2851ee3d7add0460cf1a6f852f0a721af17384e13 \
    --hash=sha256:79a07aabd43f5934472d71921ef3470acbf6a719ba538743c505156fb27d62f6

$ pip install -r requirements.txt --require-hashes
Collecting meson==0.60.1
  Using cached meson-0.60.1-py3-none-any.whl (837 kB)
Installing collected packages: meson
Successfully installed meson-0.60.1
evverx commented 2 years ago

I'm pretty sure I did. Here's the file I passed to pip

meson==0.60.1 \
    --hash=sha256:5add789c953d984b500858b2851ee3d7add0460cf1a6f852f0a721af17384e13 \
    --hash=sha256:79a07aabd43f5934472d71921ef3470acbf6a719ba538743c505156fb27d62f6
ninja==1.10.2.2 \
    --hash=sha256:23b01780debf1fd8977ddcec3639364a414b2c6ed0e32cb29d1395ab00aa38d9 \
    --hash=sha256:325798418e0e3daa327ec98b5eb9ae37762dbdb34799720233e21aa95c8388b5 \
    --hash=sha256:3f8a75acd929abb9f003d3aa5bc299cea30b9db0dfa18669877e9c02ddcf530d \
    --hash=sha256:510d14aaf1fa10c48cae012a31408982158478620384b31961977a0402a76f62 \
    --hash=sha256:6dd57dd01bfb73f3b52c760c10d89f19d95b1216f9afdc37d9c36179213b5172 \
    --hash=sha256:7aa5bcdcef993869f0431b3203679313ee09883e4c80c0098fc93094f4385929 \
    --hash=sha256:83c1f38996b8b0446fcfefff4cd2d0c88fb0a691f41de1422bf65f1bd8c2f562 \
    --hash=sha256:b51e9c2049a9773a7a037597ca49a3883650f7505bb28bed733c6d5139c94cbf \
    --hash=sha256:b8eba6f2b075098761a7396355af33075a8e2cbbe70203089c4af2346b824da3 \
    --hash=sha256:bee21e3ae56ad79b17170947d5dc2cec19e2aee83b3c09d7054696d17ee11ded \
    --hash=sha256:bf03193cd34d4afcb0040da65715cd176958325eac7580e5de67577ae3be7b6f \
    --hash=sha256:c94f62d74aff38c5a7b9ee533597a7365d5f5b683b548df60bc1ffae94b1a86d \
    --hash=sha256:ddc7319a705f352aca53783af60d719b18beee6dce4450873c2cf3fc2a431858 \
    --hash=sha256:ea58fe7dbb15425e839185dd2b62898cc623fde81342a50ed5dce99344e74a34

Could it be pip 20.2.2 is too old?

di commented 2 years ago

@evverx Looking more closely at the error message, looks like you're also trying to install https://pypi.org/project/install/, which doesn't have hashes.

evverx commented 2 years ago

I'm not sure where it came from though. I'll try to reproduce it in a clean environment and report back if it's still there. Thanks!

evverx commented 2 years ago

I was able to install meson and ninja with pip install --require-hashes. jinja2 fails but that seems to be OSS-Fuzz forcing me to use pip instead of apt-get for no apparent reason: https://github.com/google/oss-fuzz/issues/6868 :-)

laurentsimon commented 2 years ago

how do you create the requirements.txt to contain all dependencies, including the transitive ones?

@rsprabery FYI

evverx commented 2 years ago

As far as I can tell, it was fixed. Closing. Thanks!