sudo-project / sudo

Utility to execute a command as another user
https://www.sudo.ws
Other
1.15k stars 208 forks source link

Sudo not allowing execution of symlink to white-listed command #333

Open r-vdp opened 7 months ago

r-vdp commented 7 months ago

Version used

Version of sudo: 1.9.15p2. I tried recompiling sudo with 8dd29677666f987237a2aa2ee457b49a457c21b7 and 44f0908e73266a595860084778ecd6cefa135317 cherry-picked, but that didn't make a difference.

This used to work correctly with version 1.9.13p3.

Issue description

I have a sudoers file which has a rule like this one:

robot     ALL=(root)    SETENV:NOPASSWD: !ALL, SETENV:NOPASSWD: /nix/store/i0sdqs34r68if9s4sfmpixnnj36npiwj-systemd-254.6/bin/systemctl --system start nixos-upgrade.service

The user's PATH contains /run/current-system/sw/bin which contains a symlink pointing to the same path:

$ which systemctl
/run/current-system/sw/bin/systemctl

$ realpath $(which systemctl)
/nix/store/i0sdqs34r68if9s4sfmpixnnj36npiwj-systemd-254.6/bin/systemctl

However, when this user runs sudo systemctl --system start nixos-upgrade.service, the command gets refused with

Sorry, user robot is not allowed to execute '/run/current-system/sw/bin/systemctl --system start nixos-upgrade.service' as root on host.

In the sudo debug logs, I see this:

Dec  6 12:15:27.466 sudo[1263] -> canon_path @ ./canon_path.c:124
Dec  6 12:15:27.466 sudo[1263] -> rbfind @ ./redblack.c:282
Dec  6 12:15:27.466 sudo[1263] <- rbfind @ ./redblack.c:286 := 0x5abedc4ee580
Dec  6 12:15:27.466 sudo[1263] canon_path: path /nix/store/i0sdqs34r68if9s4sfmpixnnj36npiwj-systemd-254.6/bin -> /nix/store/i0sdqs34r68if9s4sfmpixnnj36npiwj-systemd-254.6/bin (cache hit)
Dec  6 12:15:27.466 sudo[1263] <- canon_path @ ./canon_path.c:199 := /nix/store/i0sdqs34r68if9s4sfmpixnnj36npiwj-systemd-254.6/bin
Dec  6 12:15:27.466 sudo[1263] -> canon_path_free @ ./canon_path.c:89
Dec  6 12:15:27.466 sudo[1263] -> canon_path_free_item @ ./canon_path.c:74
Dec  6 12:15:27.466 sudo[1263] <- canon_path_free_item @ ./canon_path.c:79
Dec  6 12:15:27.466 sudo[1263] <- canon_path_free @ ./canon_path.c:92
Dec  6 12:15:27.466 sudo[1263] <- command_matches_normal @ ./match_command.c:711 := 181786330
Dec  6 12:15:27.466 sudo[1263] user command "/run/current-system/sw/bin/systemctl --system start nixos-upgrade.service" matches sudoers command "/nix/store/i0sdqs34r68if9s4sfmpixnnj36npiwj-systemd-254.6/bin/systemctl --system start nixos-upgrade.service": DENY @ command_matches() ./match_command.c:892

Shouldn't sudo resolve the symlink, see that these are the same binary and allow the user to execute it?

millert commented 7 months ago

This is a result of the fix for #228. One option may be to make the behavior configurable.

r-vdp commented 7 months ago

Yeah I saw that issue and the commits.

So if I understand it correctly, we canonicalize the parent dirs of both the white-listed command and of the actual command, but if those canonicalized parent dirs are different while the actual executables are the same (because one of them is a symlink to the other) then we don't get a match. And we don't want to simply canonicalize the full path to the executables since this could potentially change argv[0].

Could we compare the canonicalized paths to the executables, but still execute the path like we do now (canonicalized parent dir + original executable name)? This would allow for a successful match when two executables are identical after resolving symlinks, but would also preserve argv[0]. Are there any security implications if we would do this?

millert commented 7 months ago

@r-vdp I don't think that would still solve #228. The problem is when you have multiple commands that resolve to the same executable in a sudoers rule. If we canonicalize both the sudoers path and the user path, there is no way to tell which is which and we just match the last one (which was the behavior I was trying to change). That is why I did not canonicalize the entire path, just the parent directory. The simplest thing is probably to provide a setting in sudoers to choose the method to use when matching. There are other, more complicated, potential solutions but I'm not sure they are worth the complexity. I'll continue thinking about it, though.