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

Enhancement: Do not allow pip to upgrade python core packages by default #4527

Closed
GadgetSteve opened this issue Jun 2, 2017 · 15 comments
Closed
Labels
auto-locked Outdated issues that have been locked by automation resolution: invalid Invalid issue/PR

Comments

@GadgetSteve
Copy link
Contributor

  • Pip version: 9+
  • Python version: All
  • Operating system: All

Description:

As reported on: Hacker Noon it is possible for people to maliciously or accidentally register packages on pypi or other locations that would overwrite the default packages installed with python. These could be downloaded directly or via requires in other packages.

Some installations on some OSs do provide a degree of protection via the user permissions on the packages.

While it is sometimes necessary to upgrade the packages shipped by default with python, (such as pip or ssl recently), it would be an enhancement for pip to refuse to work, with a nice clear error message on any package that overwrites the default packages unless the user specifies a specific override at invocation.

This could be achieved by having a list of the packages that are present by default for each current version of python and filtering processing based on the list for the version being run against.

Possibly --allow-core-overwrite might be a suitable flag to disable this behaviour.

@pradyunsg
Copy link
Member

Hey @GadgetSteve! Thanks for filing this issue but I think this be a duplicate of #585?

@pfmoore
Copy link
Member

pfmoore commented Jun 2, 2017

@pradyunsg I don't think it is. From the linked article, and some discussion on distutils-sig, I think this is more about shutting down a potential attack vector for malicious code, registering a hostile package as something like "tkinter" on PyPI and then if a naive user runs "pip install tkinter" not realising it's in the stdlib, they get infected.

This suggestion seems to me like a reasonable precaution, but IMO it would have a fairly limited benefit, while requiring a certain level of maintenance cost (we'd have to maintain the list of blacklisted packages, and consider other Python implementations like PyPy and Jython).

I'm not against the idea, but personally, I think any resource devoted to the issue is likely best spent assisting in looking at a solution from the PyPI end (as was noted on distutils-sig, PyPI is critically short of support resources, and needs community support to look at this).

@pradyunsg
Copy link
Member

pradyunsg commented Jun 2, 2017

Whoops! I intended to link pypi/legacy#585.

I think any resource devoted to the issue is likely best spent assisting in looking at a solution from the PyPI end (as was noted on distutils-sig, PyPI is critically short of support resources, and needs community support to look at this).

+1

FWIW, I too think so - this is something that is better fixed on the PyPI end.

@pfmoore
Copy link
Member

pfmoore commented Jun 2, 2017

Ah yes, it is effectively a duplicate of that one (and better addressed at that end).

@GadgetSteve
Copy link
Contributor Author

While I agree that a part of the problem is that PyPI allows core packages to be registered, possibly by someone with malicious intent and that this should be fixed at that end this would only close that attack vector as pip can be used, directly by the user or indirectly via requires to install software from just about anywhere.

One possible attack vector, that would bypass any PyPI changes, would be for someone to register a useful or useful sounding library on PyPI that in its requires specifies a malicious location for an overwrite of a core package, (lets say SSL since this could be subverted in a manner that the user was not even aware of). PyPI would not be able to do much about this short of vetting requires.txt and setup.py plus any code that might modify the same and it has no control over the content of other locations.

Attempting to install anything on the black-list, from any of PyPI, GitHub, other servers & local files or anywhere else would be aborted with an error message informing the user that they needed to give permission to overwrite a core package. We could then, depending on the option used, either allow a single install or white-list the specific location, (e.g. a specific core package being available from a specific github repository for the running version of python only. If this white-list was implemented in a cascading manner, (like a lot of ini files are), then it should be possible for developers of core components to have options such as allow pip to install from the local file system when running in a specific virtual environment but not when running the system python, etc.

Naming each list for the specific version of python that it is generated for should also address the point of other python implementations, they would also benefit from this change.

@pfmoore My thoughts to minimise, to just about zero, the support issue of having to maintain a black-list of items (those that should not be pip install-able) is to have pip on start-up look for a default packages list, (the black list), named for the version of python that it is currently being run under, (possibly in any of the locations that pip.ini would be found or in the python libraries location), and if it is not found to generate one for the current python, using the same algorithms/mechanisms that virtual-env or venv uses to include only the core packages. I would suggest having a default white-list item of pip itself white-listing PyPI.

@pfmoore
Copy link
Member

pfmoore commented Jun 3, 2017

I'm somewhat skeptical - after all, if you pip install anything, you allow arbitrary code to be executed on your system. That's simply the nature of setup.py installs.

However, if someone wants to try to move this proposal forward, that's fine.

@pradyunsg
Copy link
Member

@pfmoore @dstufft Should this be tagged as invalid and closed or tagged as an enhancement proposal?

@pfmoore
Copy link
Member

pfmoore commented Jun 26, 2017

"Enhancement proposal that no-one is willing to take forward". But I'm against leaving things open that will just languish indefinitely. So how about making it an enhancement proposal and closing it on the basis that the OP didn't progress it after the last round of discussion?

OTOH, it's only 24 days since the last comment - do we have a policy on how long an idle enhancement proposal should remain open? I don't want to be too quick in shutting down proposals. But it feels to me that although @GadgetSteve has a strong opinion on this issue, he isn't expecting to take on the work to write the code and push it through in the face of opposition/limited interest from the core devs. If that's the case, is there any value in leaving the issue open?

@GadgetSteve
Copy link
Contributor Author

GadgetSteve commented Jun 27, 2017

If there is no violent opposition from the core developers I would be happy to take this forward, just didn't wish to spend a lot of time on it if it was never going to go anywhere. I would address #583 at the same time, simply by always adding python to the forbidden list.

I am thinking that the allow core installs flag should be an environmental variable, maybe PIP_ALLOW_CORE, and the "forbidden" error message should mention it.

@pfmoore @pradyunsg

@pfmoore
Copy link
Member

pfmoore commented Jun 27, 2017

I don't know what constitutes "violent" opposition, but I still haven't had a response on my point that people have to trust/check the code they install because pip install can invoke arbitrary code. Your examples seem to me to simply be "if you can trick people into installing code that does something bad, you win", and we're saying that the fix for this is in PyPI to make it harder to trick people that way, not in pip to remove one of many things that malicious code could do (as the author of the malicious code just chooses a different way to attach the user).

@GadgetSteve
Copy link
Contributor Author

I am not saying that PyPI should not address this but rather suggesting "Defence In Depth" to make it harder for this specific threat vector - I very much doubt that anything but the eventual heat depth of the Universe will ever stop malicious actions from finding targets.

While personal responsibility does come into play with the growth of buy in to python from non-technical people there are an ever growing number of people installing python packages who do not have the background to check for such issues. Also, I personally would rather spend a little time putting this in place than every time I go to type pip install -U... instead having to do a pip download and vet the dependencies in any requires.txt & setup.py files.

@pradyunsg pradyunsg added the type: enhancement Improvements to functionality label Jun 27, 2017
@pradyunsg
Copy link
Member

@GadgetSteve I understand that you're trying to reduce the possibility that pip and PyPI are used as a delivery channel for malicious code to a target system.

My suggestion would be that you try to make a patch for Warehouse as well for doing that. Warehouse will eventually serve as the codebase powering PyPI and this is something that should really be fixed on the PyPI side too.

Other than that, my position is - I won't do it but I won't oppose if some else did.

@pradyunsg
Copy link
Member

I am thinking that the allow core installs flag should be an environmental variable, maybe PIP_ALLOW_CORE, and the "forbidden" error message should mention it.

Adding a command line option should be enough to implement this. The current setup that pip has to determine the value of arguments takes environment variables and configuration files into account.

@pradyunsg
Copy link
Member

@dstufft Your thoughts?

@dstufft
Copy link
Member

dstufft commented Jun 27, 2017

I don't believe the pip side of this can meaningfully be fixed. At the end of the day you're allowing arbitrary Python code to be invoked (either as part of the build process, or whenever you use the thing you just installed) and it is ~impossible for us to protect against those doing bad things. This aligns with the fact that traditionally our threat model has been largely that it's up to you to vet the bits you're asking to have installed, but that we're going to ensure you get the bits you ask for.

Thus I don't think this makes sense as a pip enhancement issue. However I do think it makes sense as an enhancement in Warehouse (which will be the only way to upload files to PyPI on July 3rd). However, even there we can't protect against malicious package contents automatically, only against malicious names.

@dstufft dstufft closed this as completed Jun 27, 2017
@pradyunsg pradyunsg added resolution: invalid Invalid issue/PR and removed type: enhancement Improvements to functionality labels Jun 27, 2017
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation resolution: invalid Invalid issue/PR
Projects
None yet
Development

No branches or pull requests

4 participants