Fish Running as Root Fails to Suggest All Executable Commands in PATH

[timkite]

In fish 3.6.1 as well as 3.3.1 at least, when running as root, commands (scripts or binaries) that are in $PATH, not owned by user root and/or group root, and are not world-executable will not be offered as completions, even though they are executable by root. This appears to affect Enterprise Linux (RHEL, CentOS, and Oracle Linux) 7 and 8, but does not affect macOS (any version). This happens whether ssh-ing to the Linux system via Apple Terminal or connecting directly to the machine's console. Running without third-party customizations does not affect the behavior.

How to reproduce:

1. Ensure your user account on the Linux system can sudo to root and that /usr/local/bin is in your $PATH
2. Create /usr/local/bin/testprog, using sudo if needed, containing something that could execute. An example:

#!/usr/bin/env fish
echo "Hello World"

3. Set the owner and group of testprog to something OTHER than root:root. For full effect, make sure your user account is either the owner or belongs to the group.
4. Set the permissions of testprog to 0550, 0750, or 0770
5. Attempt to auto-complete or tab-complete the command testprog (without typing it fully) and notice that fish will happily
6. Change to a root session running fish and ensure that /usr/local/bin is in your path as root
7. Attempt to auto-complete or tab-complete the command testprog (without typing it fully) and notice that fish will not
8. Run the command testprog (notice that fish will color the command as valid once it's typed fully)
9. Attempt to auto-complete or tab-complete the command testprog (without typing it fully) and notice that fish will now suggest it, since it's in your command history, but will not tab-complete it.
10. Switch to a root shell running bash
11. Attempt to tab-complete the command testprog (without typing it fully) and notice that bash will happily
12. Do any one of the following: change the owner of testprog to root, change the group of testprog to root, or set testprog's permissions to 0555, 0750, 0775, or 0777
13. Switching back to a fish root prompt, try tab-completing testprog again. Note that fish will happily tab-complete it now.

What should happen:

• If a command is executable at all, it's executable by root, and should be offered as a completion

What does happen:

• Fish only seems to offer commands for completion when the following are true: it is executable by anyone OR it is executable specifically by you OR it is executable by a group you belong to OR it's in your command history

[ridiculousfish]

Thank you for the detailed reproduction steps. I was able to reproduce this on Linux.

This bisects to 0b55f08. The issue is replacing the call to waccess with a call to fast_waccess which does a bunch of somewhat sketchy looking stuff, but mostly it tries to figure out if access would succeed without actually invoking access. I am tempted to revert.

[ridiculousfish]

Upon reflection I think 0b55f08 cannot be saved; for example it doesn't consider ACLs, plus the "Cache a list of our group memberships" is suspect as this may change during a shell session. We should not be second-guessing the kernel in this way.

Reverted in c67d77f. Thank you for the outstanding bug report.

cc @mqudsi ; I don't think 0b55f08 can be saved but maybe you have other ideas.

[mqudsi]

Thanks for cc'ing me, @ridiculousfish

for example it doesn't consider ACLs,

Yes, fair point. To be fair, no one has complained showing just how little real-world ACLs (continue to) get.

plus the "Cache a list of our group memberships" is suspect as this may change during a shell session.

I remember considering that but in practice I believe group membership is always cached anyway until you log out and log back in. You can try it yourself by creating a group and giving it permissions to access a file/directory and then add yourself to the group. You won't have permissions until you log back in after being made a member of the group (or start a new session over ssh or on another tty, but those scenarios necessarily involve a new shell instance).

I'll think over this and see if I can come up with any alternatives. The speed gain was very tangible, iirc.

[timkite]

for example it doesn't consider ACLs,

Yes, fair point. To be fair, no one has complained showing just how little real-world ACLs (continue to) get.

On our large, shared web environments we use ACLs extensively on the development tier, but almost exclusively for developer team file permissions within sites/hosted applications. I've honestly never run into an issue with fish not checking them during completions.

[mqudsi]

There are two options that I can see. The first step in both cases is to just add an euid == 0 check to fast_waccess() to address the issue in this PR - quite easily solved.

Given that code in question sonly suggests completions and doesn't affect any core functionality of the shell, I think that's the way to go. Just add the euid check and call it a day. Making 4 extra syscalls per potential file completion is expensive, especially with all the various security mitigations enabled that make the syscall boundary so much more expensive than it was.

The second option is to add the the code back as-is, add the euid check as above, but additionally fall back to an actual waccess() call when fast_waccess() returns access denied - i.e. only ask the kernel if fast_waccess() determines that a file can't be run (which should be the less common case). This will catch cases where ACL gives permission or any other kind of situation we didn't catch. The shell will never erroneously not suggest a completion, though a negative ACL rule could possibly make it suggest a non-viable completion (but no harm done).

Given that the fast_waccess() code has been in-place since fish 3.2.0 and this is literally the first regression flagged as a result, and given that workaround for this actual issue as reported is quite simple, I don't think we should be so quick to toss out that code.

[ridiculousfish]

I'm uncomfortable second-guessing the kernel here but I may be convinced. Still it's true that ACLs can also deny privileges, plus there may be other mechanisms in play such as sandboxing.

But why is it four extra syscalls per file, and not just one?

[faho]

But why is it four extra syscalls per file, and not just one?

Best I can see is that we check waccess for the executables_only flag, and then again for the description. We can of course skip the latter by passing on the information that it's definitely executable.