@samueldr points out this would break sudo on binaries fetched by nix-shell, which seems pretty annoying (nix-shell -p mtr anyone?) so maybe we don't want this by default? Hmm

Not a bad workaround, though:

[grahamc@Petunia:~]$ nix-shell -p vim

[nix-shell:~]$ sudo vim
sudo: vim : commande introuvable

[nix-shell:~]$ sudo $(which vim)

(worked fine)

cc @cransom

It sounds like a good idea but I am not sure we have a clear picture of "valid" uses.

For the mtr problem you should be using programs.mtr.enable anyway….

I have the feeling that we might be able to accept this since we are already breaking so many "standard" assumptions when compared to other Distros (Debian, Fedora, …). It might still be an infinite source of user confusion (paradoxically; it should have been the other way) as it used to work on likely works everywhere else. I am also thinking about stuff like ruby, python, … "virtualenvs" where sudo might be used seldomly and we do not know about it.

As with any change: We can try and roll back if things are too broken. I would definitely prefer some kind of A/B-Testing for these but we don't have that.
(nixos.experiements.enable = true; anyone? :-))

Nix allows any user to install software in to their PATH. A crafty
person with malicious intent could use Nix to alter the installed
software of a given user without editing their PATH, and without any
"writable" user directories in the PATH.

I’m not sure I understand how they would do that without already having root?

Yeah, that wasn't worded well, @alyssais. I don't mean an arbitrary user, but the current user. Any program run as a given user could install new programs for that same user.

@andir

I have the feeling that we might be able to accept this since we are already breaking so many "standard" assumptions when compared to other Distros (Debian, Fedora, …).

RHEL and Debian lookalikes already set this to something like:

Defaults        secure_path="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin"

:)

Does sudo nix-shell work out of the box? I imagine it does, right?

It does!

$ sudo nix-shell -p hello --run hello
Bonjour, le monde !

@grahamc I assume the threat model is:

A user has some restricted sudo access (eg. only sudo foo --bar), and could use $PATH to make foo execute arbitrary programs as root if foo does a system() or similar.

In which case it's AFAIK not nix-specific (anyone could just set $PATH to point to ~/bin), but the generic attack against sudo, which is as you mention mitigated by secure_path.

@alyssais If I'm not mistaken with the above, then I think this is the threat model you were wondering about -- and in this situation sudo nix-shell is not a possible bypass :)

@grahamc I assume the threat model is:

A user has some restricted sudo access (eg. only sudo foo --bar), and could use $PATH to make foo execute arbitrary programs as root if foo does a system() or similar.

With Nix in the path, there is no illusion of restricting what programs a user can run with sudo. So, this makes no attempt to help with that. This is more about a problem with an attacker transparently replacing executables on the PATH. For example, replacing vim with a pwnd vim in the user's profile, they might never know because the $PATH looks right and they "never" use nix-env. As it currently stands, next time they run sudo vim they'll give this pwnd vim root access. With this PR, sudo vim would use the root-installed vim.

@grahamc IMO your example is not a realistic threat model: if the attacking program can replace vim, it can replace sudo with myevilsudo just as well (ie. something that would be a wrapper around sudo but also installs a backdoor or similar).

OTOH, for a user with restricted sudo access (including, presumedly, no nix, otherwise it wouldn't be restricted), the patch does totally make sense to me: the user with something like [...] = (ALL) /run/current-system/sw/bin/something --foobar, assuming something does system("foobar"), could execute whatever they want as root by sending in a $PATH that includes their own evil foobar without this patch.

This patch has nothing to do with explicitly setting the PATH for a restricted sudo user, it is just a Defaults clause. More specific restrictions should be set with extraRules. This specific option should not be used for general access restriction.

I asked in #debian why they set the secure_path the way they do, and they said indeed it is to make carefully restricted users' PATHs safe. I'm not sure we can claim that value, though, since nix-shell will still be in PATH and that is a lot of power.

Note: I'm requesting reviews and ccing people who tell me they combine sudo and nix-shell.

Not a bad workaround, though:

My feeling is that sudo nix-shell -p ... would be preferable over nix-shell -p ... --run '...' is preferable anyway as in the former case the entire environment is being set up as the privileged user and the latter is possibly being influenced by ~/.config/nixpkgs and other user-specific configuration.

@grahamc I think debian's threat model, as well as the one I'm envisioning, is basically something like:

#!/bin/sh
# (in /whatever/foo.sh)
exec bar

And then the user U having access to sudo /whatever/foo.sh.

In this model, with $PATH user-controlled, U would be able to execute anything as root by naming it bar and prepending its dir to $PATH. While, with $PATH root-controlled (ie. with this patch), U can execute only the root-expected bar.

Now, if /whatever/foo.sh were calling nix-shell, then it would potentially open up some more ways to bypass things by U, but the point is so long as /whatever/foo.sh isn't explicitly calling nix-shell, it won't allow the user to run whatever-they-want :)

So if a malicious program could just take over sudo anyways, what are the upsides? You can't really install suftware as non-root anyway on debian, so there are not much downsides there. But there are for nixos.

what are the upsides?

Yes, that is the question.

@timokau The upsides are to defend root against the malicious (or compromised) restricted-sudo user U described in #64209 (comment) :)

That does seem a bit contrieved to me. Its basically the setuid scenario, and the same security concerns apply. This change sounds like it could be a significant inconvenience (I manage most of my packages with home-manager, sometimes use nix-shell to query hardware info or something that needs a one-off sudo package). The "restricted sudo" case doesn't seem worth it to me.

It is exactly the setuid scenario indeed. OTOH, sudo is actually designed to work around the security shortcomings of setuid.

BTW, this “cleaning $PATH” behavior is very similar to the “cleaning $HOME” behavior we already have, and has similar inconvenience level: user-level config (vs. user-level packages) don't get in, because they would be dangerous as they'd allow unexpected software to run as root (eg. .vimrc or similar).

Arguably managing all one's packages through home-manager is thus a broken workflow (it installs packages for root as a user), and it makes sense to make it no longer work. OTOH, it is convenient.

I'm personally in favor of this change, mostly because people expect sudo to clean $PATH, while they don't expect setuid to, as it's the way all other distros do it.

Well "cleaning $HOME" is actually a convenience in my view, since it keeps all my home from being cluttered by root-owned files.

Arguably managing all one's packages through home-manager is thus a broken workflow (it installs packages for root as a user), and it makes sense to make it no longer work. OTOH, it is convenient.

I don't understand that point. Why would that be a broken workflow?

mostly because people expect sudo to clean $PATH

Do they though? Because I don't. Debian may do it, but I don't think most people will have noticed because most debian binaries will live in one of the whiltelisted paths anyway.

This will be my last message on this topic, as we're clearly running around in circles. (for the “broken workflow”, it's in the parenthesis that just follow these two words, in your quote)

I'm sorry in advance if my tone gets heated along the message (which is another reason for this to be my last message), I've tried to tone it down by re-reading but may have missed places.

Here is a scenario:

1. I'm a sysadmin.
2. I allow my apache2 user to run sudo my_database_cleanup.sh, which will be called by some kind of webcron or whatever.
3. my_database_cleanup.sh is some kind of shell script, written like any usual shell script, that just performs a database cleanup.
4. (Note: this is exactly how restricted-sudo is used just about everywhere)
5. Poof, apache2 now has rights equivalent to those of root, by using the attack listed at sudo: set secure_path with root-installed software only #64209 (comment). Welcome privilege escalation.

NixOS currently encourages behavior that unfortunately doesn't work with the fix to this vulnerability, by having user-only-installed packages propagate to root. However, this behavior is not right, just like a user being able to install packages in root's environment would be wrong (though it's not the same threat model).

The workaround @grahamc suggested at #64209 (comment) works when you already are with user-installed packages (nix-shell or home-manager) and really want to do that, and other than that just enter a sudo and then enter a nix-shell.

But having the user able to make it look to other programs like it's installed software as root is exactly what causes the vulnerability I described above.

Now, your opposition to this is based on usability. To which my answer will be: why are you even installing sysadmin tools via home-manager, and not via /etc/nixos or via root's home-manager? There is no semantic reason for them to be there, and they could shadow things installed as root.

Your user being an all-privileged sudo user should be considered the exception, not the rule. Sudo is not just a way to have a keyword that you need to type to brick your system, it's a tool that 1. switches to another user, 2. runs a command, and 3. switches back to the original user. When you see it in these 3 steps, it becomes obvious that what you have installed as your local user should not affect execution of the command in step 2 -- if it did (and for the time being it does), then it'd be a security vulnerability.

One way to easily preserve the UX and improve automated script security is to make wheel an exempt group.

This will be my last message on this topic, as we're clearly running around in circles. (for the “broken workflow”, it's in the parenthesis that just follow these two words, in your quote)

I'm sorry in advance if my tone gets heated along the message (which is another reason for this to be my last message), I've tried to tone it down by re-reading but may have missed places.

I certainly don't want to step on anyones toes, sorry if it seems that way :)

Here is a scenario:

1. I'm a sysadmin.

2. I allow my `apache2` user to run `sudo my_database_cleanup.sh`, which will be called by some kind of webcron or whatever.

3. `my_database_cleanup.sh` is some kind of shell script, written like any usual shell script, that just performs a database cleanup.

4. (Note: this is _exactly_ how restricted-sudo is used just about everywhere)

5. Poof, `apache2` now has rights equivalent to those of root, by using the attack listed at [#64209 (comment)](https://github.com/NixOS/nixpkgs/pull/64209#issuecomment-508123221). Welcome privilege escalation.

Okay now the scenario doesn't seem contrieved anymore. For me sudo is just a convenient tool to elevate my privileges temporarily, for that usecase I would use setuid. I also think in that case authors should use absolute paths anyway. But I know that "I think people should be doing" is not what people actually will be doing. And its always good to remove footguns.

NixOS currently encourages behavior that unfortunately doesn't work with the fix to this vulnerability, by having user-only-installed packages propagate to root. However, this behavior is not right, just like a user being able to install packages in root's environment would be wrong (though it's not the same threat model).

I disagree here. sudo (for me) is about executing something with the privileges of another user, while keeping the environment. Just because a package is in my PATH while I run sudo, I wouln't consider it to "propagate to root" -- its just part of my current environment.

The workaround @grahamc suggested at #64209 (comment) works when you already are with user-installed packages (nix-shell or home-manager) and really want to do that, and other than that just enter a sudo and then enter a nix-shell.

In my opinion that would break user expectations too much. I would certainly have been surprised, and it wouldn't be obvious why its not working as I expect.

Now, your opposition to this is based on usability. To which my answer will be: why are you even installing sysadmin tools via home-manager, and not via /etc/nixos or via root's home-manager? There is no semantic reason for them to be there, and they could shadow things installed as root.

Actually looking into my shell history, I have to admit I don't actually do that very often. Still, not every tool run with sudo has to strictly be a "sysadmin tool", it could be a tool that is only executed with elevated permissions as an exception. The best example I can come up with off the top of my head is an editor. That's not ideal of course (sudoedit), but you get the idea.

I like @grahamc's idea. If the main thread model here is the restricted-sudo case, why not just apply to that one. Only downside is that it could be even more confusing because of the inconsistency.

(disclaimer: I only got pinged because I participated in the twitter poll).

For me personally the sudo $(which foobar) workaround is fine (if I understood everything correctly 😅 ), I don't have any strong opinions about adding further options for this.

For the author, reviewers, and committers: this PR was scanned and appears to add a use of the deprecated types.string, which emits a warning as of #66346. Before merging, please change this to another type, possibly:

• types.str for a single string where merging does not make sense, or cannot work
• types.lines for multi-line configuration or scripts where merging is possible
• types.listOf types.str for a mergeable list of strings

Looks good to me. Let's make it the default.

@grahamc why was this not merged? Is it not worth it anymore, has the problem been solved by any other issue ? Could you explains what happened for this to be forgotten?