Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Safe sudo behavior #294

Closed
isaacs opened this Issue Oct 17, 2010 · 7 comments

Comments

Projects
None yet
3 participants
Owner

isaacs commented Oct 17, 2010

The Problem

The problem with running npm (or, for that matter, most package managers) as root is that the package manager trusts the packages. In order to make sudo safe at all (and, in fact safer than running as a non-root user), we need to make that not the case.

Solution

When running as root, do the following.

Get the "user" config, which defaults to "nobody".

Before running any lifecycle scripts, recursively chown the package folder to the user config. Run the script with the powerless uid. Chown back after running.

At the end, recursively chown the package folder to the original owner.

If done properly, the running scripts will only have access to their package folder, even though npm itself is running with root privileges.

Open Questions

  1. What to do about the npm multiuser stuff? Does it still make sense for root to not read the userconfig, or dereference ~/ in dir configs?
    Idea: Control this with the --global flag. If set to true, then root behaves as it currently does. Once sudo is made safe, should --global installation perhaps require being run with root privs?
  2. Should perhaps any lifecycle scripts still do the setuid/chown stuff, and just run them in a spawned sudo shell? While I worry about the "training to be ok with entering your password at random times" issue, it would actually be safer. Maybe a little warning explaining that you're about to be asked for your password, and why, and then have a config to either disable that warning, or disable the behavior?
  3. Why isn't there a nodo type command that downgrades privileges without requiring root? It's so frustrating that you have to ask for infinite power in order to get permission to reduce your power level.
Contributor

kriskowal commented Oct 27, 2010

The opposite problem is that, as with running with administrative privileges at all times, any attacker that gains control of a process with the privileges of the user can then alter installed packages. I don't have a proposed solution, but user-level access control is broadly known to not adequately solve either of these problems.

Owner

isaacs commented Oct 27, 2010

Depends on your definition of "adequate". I'm not looking for 100% provably-safe security, and I don't believe such a thing exists. Unless you only ever run code that you wrote, on a machine with no network access. But since npm is a program you didn't write, used to install other programs you didn't write, having fetched them over a network from a computer you don't own, 100% provably safe is out the window by design.

The goal is to prevent two things:

  1. Accidentally doing something that is not harmful on the developer's machine, but is harmful on the installer's machine.
  2. Doing something that is malicious in a sneaky way, and doesn't get caught for a long time.

Having user-level access control is certainly a lot better than running arbitrary scripts with the permission level of the user or worse yet, as root. While there will always be vectors, this would cut down a significant one, and as the package manager, I think npm has a responsibility to make bad behavior as difficult as is reasonable.

Owner

isaacs commented Dec 4, 2010

Tasks/decisions:

  1. "user" config defaults to "nobody".
  2. The install command requires sudo privileges. It will do the following:
    1. mkdir 0755 {npm.dir}/{name}/{version}
    2. chown {user} {npm.dir}/{name}/{version}
    3. spawn a child process
      1. setuid to {user}
      2. unpack the tarball into {npm.dir}/{name}/{version}
      3. run preinstall in {npm.dir}/{name}/{version}
    4. do various build steps (lib/build.js) Do this as root, so that npm architecture is root-owned, and package scripts only have permission to touch their own contents.
    5. spawn a child process
      1. setuid to {user}
      2. run install scripts
      3. run postinstall scripts
  3. If non-root uid found in lib/install.js, then
    1. print message that root is required for npm install, and that this will be attempted
    2. spawn subshell with sudo npm install [args]
  4. Suppress this behavior with an --unsafe-perms flag, which will tell npm to run everything in the current process as the current user
  5. remove the sudon't warning.
  6. link will not require this behavior. However, it will typically require root access to write the various things.
  7. If a command fails with EACCESS (and --unsafe-perms is not set), then print a warning that greater permissions are required, and spawn a subshell that does sudo npm <cmd> <args>.'
    This should not happen for install, since it'll know at the start that it needs root. Also, commands that alter the npm folder architecture, or are proxies for install, or run lifecycle scripts, will need elevated perms. In particular, I'm thinking: build, update, update-dependents, link, start, stop, restart, test, activate, rebuild, uninstall, edit,
  8. In the cache:
    1. chown files and folders to env.SUDO_USER if root
    2. recursively chmod the package folder 0644
    3. chmod .cache.json, package.json, and package.tgz 0644
    4. chmod all other folders 0755 (so that tar/gzip can be run there)

Practical requirements:

  1. need a good way to spawn a subshell and then have it do things. Maybe the npm repl command could be revisited for this purpose. Strip out the require("repl") hack that's in there now, and just do a simple "read stdin, frame with \n, execute the command" kind of thing. Add a few special commands to it to downgrade UID and such. Then it could just child_process.spawn("npm", ["repl"]), write to its stdin to execute commands, and pipe between the other stdio streams.
  2. need a recursive chown/chmod util. Or, perhaps, a find-style util that recursively walks a dir tree, and can execute a function on each thing it finds. Would be easy with async map.
  3. need a "run a lifecycle script" cmd, since that's what's going to be done most often in the repl, and plus, that'd just be handy. Sometimes a build fails, and rather than re-install the whole thing, it'd be good to just do npm run-script preinstall foo.
Owner

isaacs commented Dec 4, 2010

This would be a lot easier with an option to exec that specified who to run the command as.

tsmith commented Dec 21, 2010

Perhaps you are expanding the scope of npm too far? Should npm even be aware sudo exists? sudo is not installed by default on some distros.

For example, regarding installation of npm itself, the README provides many procedures for installing npm. However, making npm installable by the port/package systems of the major OS distros would likely be less fragile and more useful. The official port/package systems of the distros have their own protocols for dealing with port/package security.

If npm was available in major distro port/package systems, the mechanism for installing npm at the machine level would be clear and the issues of root access pertaining to the installation of npm itself would be moot as that is the problem domain of the distro port/package systems. This leaves the mechanism for installing npm packages using npm and what access levels npm should have when unpacking files and executing npm package scripts.

Regarding unpacking files, in most, if not hopefully all cases, the developer does not develop as root and the app does not run in production as root. Therefore, strictly in terms of where files get unpacked, local user installation of packages is adequate for either case. npm could simply assume a user .npmrc of:

root = $HOME/.node_libraries
binroot = $HOME/bin
manroot = $HOME/man

If a system administrator really wants to install npm packages at the machine level, npm could assume a different, OS-specific .npmrc if the user is root. For example, this could be the effective .npmrc on FreeBSD if the user is root:

root=/usr/local/lib/node
binroot=/usr/local/bin
manroot/usr/local/man

The Unix security model is very clear - processes run with the access levels of the user that executed the process. It is the user's responsibility to determine whether to trust a process, and any scripts it may download and run, before executing the process. A process can be invoked with sudo, but it should not invoke sudo.

In npm's case, the chief concern regards execution of a packages's installation scripts. A package or new package version can be added to the npm repository at any time and immediately be installed by end users without any review protocol. It may be useful to consider whether npm should even support installation scripts or at least whether it should execute them directly.

Owner

isaacs commented Dec 22, 2010

Regarding different default configs based on uid=0:

No. Strong and forceful no. Been there, done that, it's what's wrong with rubygems, and it's a mistake that I refuse to make again.

The Unix security model is very clear - processes run with the access levels of the user that executed the process.

It also does not allow running with less privileges unless it is started as root.


Regarding distros that don't have sudo:

Point taken. Not sure the best workaround there. I think that sudo is common enough to be a thing that could be attempted, and then if it fails, well, it was going to fail without root access anyhow, so either put npm in insecure mode, or log in as root, or whatever.


Regarding putting npm in the "major distros":

Don't care very much about that issue at this point. Most attempts to put npm in a centrally managed opinionated distribution have resulted in an outdated and broken npm at best, and a very subtly and annoying npm at worst. Qv: debian, homebrew.

npm will be feature complete in 2011. It will be "done" (sliced and stable) probably by the end of 2012. At that time, I'll start looking at pushing adoption by other package managers, and actually care about how well that works.

Until then, that's broken, and I'm ok with it. Other fish to fry.


Regarding package installation scripts, review committees, etc.

Never. Never ever not even a little ever forever. npm will remain anarchic, free, and monitored only by a mostly hands-off dictator who is answerable to no one, forever.

I believe that this is not just the best way for software to be deployed, but also the optimal way for humans to interact with one another. The system's job is to prevent harm by default. npm currently does not do this adequately, and it's an important feature to get right. In fact, it's better to just have lots of huge warnings about it being broken, rather than pretend it's not broken when it's less than perfect. The dictator's job is to step in when the system is inadequate, or when there are annoying/confusing issues that need a human's touch. I've been filling that role with registry.npmjs.org.

If you (or anyone else) want to create a managed reviewed registry of "approved" npm packages, then you may set up your little island in that manner. I will tell you how to do it, and allow you to port all of the code in my registry if you would like. In fact, I couldn't stop you if I wanted to. It's anarchy. I can control my little dictatorship, but I can't stop you from leaving it.

But, as long as I run registry.npmjs.org, and develop npm itself, I will do it in this fashion. If everyone decides that sucks, then I'll be alone in my little island with my toy package manager, which would not be terrible.


Regarding the Unix philosophy and starting scripts:

I think that's a fine point, and certainly that is pretty much how npm runs today. However, as I stated before, the system should have some checks in place to prevent the worst and most harmful abuses.

I think most of the time it's accidents that we need to watch out for more than malicious intent. Something may be harmless on the developer's machine, but then have some awful effect elsewhere. Packagers by necessity deal in action-at-a-distance, so it's important to be a little extra careful.

Owner

isaacs commented Feb 9, 2011

Implemented in HEAD. Will be part of 0.3.0.

This issue was closed.

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