Sad outcome

Posted Nov 27, 2024 19:55 UTC (Wed) by adobriyan (subscriber, #30858)
In reply to: Sad outcome by Wol
Parent article: The kernel's command-line commotion

Yes, but all lookup info gets tossed once final inode has been located.

Sad outcome

Posted Nov 27, 2024 20:26 UTC (Wed) by Wol (subscriber, #4433) [Link]

Then don't toss it?

Cheers,
Wol

Sad outcome

Posted Nov 28, 2024 14:42 UTC (Thu) by NYKevin (subscriber, #129325) [Link] (9 responses)

When exactly does it get tossed? Suppose we construct a file descriptor with open(..., O_NOFOLLOW|O_PATH) (or openat or what have you). Then it probably can't get tossed until after we're in execveat, at which point the kernel should be able to preserve it if it so desires.

Sad outcome

Posted Nov 28, 2024 16:40 UTC (Thu) by adobriyan (subscriber, #30858) [Link] (8 responses)

See terminate_walk() in path_lookupat().

Descriptor pins "struct file" which pins dentry which pins inode.
You can walk upwards to the root and get _some_ name, that's what readlink(/proc/*/fd/*) does.

Now _some_ history must kept for loop detection purposes and too-deep-recursion detection but it surely won't exist once system call exits.

In theory, the name of the first symlink which started last pathname resolution chain could be kept to use as argv[0] but I don't want to be the one sending such patch. :-)

Sad outcome

Posted Nov 28, 2024 16:54 UTC (Thu) by NYKevin (subscriber, #129325) [Link] (7 responses)

But if we pass O_NOFOLLOW, then we should end up with a path fd that refers to the symlink, not an fd that refers to the executable. Given that (as you say) the fd pins the dentry of the requested file, and the requested file is a symlink and not its target, I don't understand how the kernel would avoid pinning the symlink dentry in that case.

Is the problem that execveat fails to dereference the symlink afterwards?

Sad outcome

Posted Nov 28, 2024 18:50 UTC (Thu) by adobriyan (subscriber, #30858) [Link] (6 responses)

Apparently you can't start lookup from a symlink.

readlink("symlink", "/bin/false", 4096) = 10
openat(AT_FDCWD, "symlink", O_RDONLY|O_NOFOLLOW|O_PATH) = 3
execveat(3, "", NULL, NULL, AT_EMPTY_PATH) = -1 ELOOP

Sad outcome

Posted Nov 28, 2024 19:54 UTC (Thu) by dskoll (subscriber, #1630) [Link] (2 responses)

The first argument of execveat needs to be a directory file descriptor. Unless /bin/false is a directory on your system, this shouldn't work even without O_NOFOLLOW.

As to why you get an ELOOP error return, I guess that's just a strange detail of the implementation. I would have thought ENOTDIR would be the appropriate error return.

Sad outcome

Posted Nov 28, 2024 20:15 UTC (Thu) by intelfx (subscriber, #130118) [Link] (1 responses)

> The first argument of execveat needs to be a directory file descriptor. Unless /bin/false is a directory on your system, this shouldn't work even without O_NOFOLLOW.

AT_EMPTY_PATH was used, though.

Sad outcome

Posted Nov 28, 2024 22:24 UTC (Thu) by dskoll (subscriber, #1630) [Link]

Ah, ok, missed that... sorry.

Sad outcome

Posted Nov 29, 2024 15:46 UTC (Fri) by NYKevin (subscriber, #129325) [Link] (2 responses)

I would tend to assume that there is no existing userspace reliance on this behavior (it would be quite insane to use execveat to test whether a path is a symlink, when fstat is right there). Perhaps there are security issues, but OTOH execveat already supports AT_SYMLINK_NOFOLLOW for non-null pathname, and it would (presumably) be trivial to extend that to the AT_EMPTY_PATH case.

Unfortunately, this whole discussion is probably moot for fexecve(3), considering this passage in the man page:

> The idea behind fexecve() is to allow the caller to verify (checksum) the contents of an executable before executing it. Simply opening the file, checksumming the contents, and then doing an execve(2) would not suffice, since, between the two steps, the filename, or a directory prefix of the pathname, could have been exchanged (by, for example, modifying the target of a symbolic link).

If the whole point of the function is to disallow symlink shenanigans, then it is obviously a non-starter to deliberately reintroduce those semantics, so libc would presumably just start passing AT_SYMLINK_NOFOLLOW (if it does not already), and we would be right back where we started.

Sad outcome

Posted Dec 3, 2024 15:14 UTC (Tue) by stevie-oh (subscriber, #130795) [Link] (1 responses)

> If the whole point of the function is to disallow symlink shenanigans

Note that symlinks aren't necessary to this. The operative word here is _shenanigans_: fexecve is designed to prevent this sort of scenario:

1. Guard launcher opens path "/bin/foo"
2. Guard launcher proceeds to read the file contents and verifies the checksum. (It probably also verifies the checksum on all SOs that are /usr/bin/foo are linked to)
3. While #2 is happening, rogue user with sufficient access deletes "/bin/foo" and replaces it with a modified version.
4. Guard launcher finishes verifying checksum on /bin/foo and the checksum passes, so it execve's "/bin/foo". Since that path now refers to a different file/inode, the guard launcher executes the wrong program. Whoops!

By using fexecve instead of execve in step 4, the guard launcher can guarantee that the executable it launches is the _exact same file that it originally opened_.

I see three primary goals here, which currently don't work well together:

1. Some people want/need to be able to prevent certain kinds of shenanigans, which can only be done by using fexecve
2. Other people who aren't as concerned about those shenanigans want the utility of /proc/fd/comm
3. Developers writing launchers such as systemd don't want to have to write separate code paths to satisfy both groups.

Sad outcome

Posted Dec 3, 2024 15:29 UTC (Tue) by intelfx (subscriber, #130118) [Link]

> 1. Some people want/need to be able to prevent certain kinds of shenanigans, which can only be done by using fexecve
> 2. Other people who aren't as concerned about those shenanigans want the utility of /proc/fd/comm
> 3. Developers writing launchers such as systemd don't want to have to write separate code paths to satisfy both groups.

I'd have rather said that everyone wants the utility of /proc/fd/comm. However, while people that specifically have a goal of preventing shenanigans (i.e. those operating secure environments), are probably willing to pay the cost of reduced convenience for security, the people in charge of systemd have a goal of "security by default". And security by default only works if it does not inflict misery elsewhere.