PID reuse; confirmation screen confusion

[runxiyu]

// peerChain
ucred, err = unix.GetsockoptUcred(int(fd), unix.SOL_SOCKET, unix.SO_PEERCRED)

// procStatus
b, err := os.ReadFile(fmt.Sprintf("/proc/%d/status", pid))

There's a TOCTOU window between GetsockoptUcred and ReadFile where the original process can exit and the PID can be reused.

I thought that pidfd would trivially solve this by holding on to the pidfd, doing a pidfd_getpid, and using /proc. However, holding the pidfd doesn't actually guarantee the does not get reused.

I believe that the race-free option is sort of like this past Linux 6.13, although these need further verification. In pseudo-C-ish (omitting sizeof(info) and such extensibility fields, not real C syntax, but hopefully illustrating the concepts):

• ioctl(pidfd, PIDFD_GET_INFO, info = &pidfd_info{ .request_mask = PIDFD_INFO_PID })
pid = info.pid (or replace this step with something like pidfd_getpid)
• statx(pidfd, "", AT_EMPTY_PATH, STATX_INO, &sx0) /* pidfs inode is never reused */
• procfs_dirfd = open("/proc/%d", O_PATH | O_DIRECTORY | O_CLOEXEC)
• v = pidfd_open(pid, 0)
• statx(v, "", AT_EMPTY_PATH, STATX_INO, &sx1)
• if (sx0.stx_ino != sx1.stx_ino) error;
• Then use the procfs_dirfd normally.
  Magic links might ESRCH if the process dies though.

See also https://www.corsix.org/content/what-is-a-pidfd.

Now, I think the more central issue is the framing of the UI/UX itself.

Allow use of key $name?
Key fingerprint $fp.
Requested by ssh (1234) ← git ← zsh ← kitty

This reads to a non-expert user as "we have authoritatively identified the application", in a way that any mechanism on the same UNIX user without using containers/namespaces/flatpak/etc, could not support: the comm field is settable by the process itself via prctl, or iirc by writing to /proc/self/comm. I think we need at least something like "Note: any program running as your user can present arbitrary information here."

Note that same-UID Linux has several ways to completely defeat any inter-process introspection scheme:

• LD_PRELOAD, so checking proc_exe carefully won't work;
• ptrace can inject arbitrary code
• pidfd_getfd
• Some other prctl schenanigans

cc @Foxboron @Mic92

Disclosed publicly with permission

[runxiyu]

<Foxboron> I think this is a bit borderline becuase the confirmation prompt should sorta be understood as a "best effort thing"
<Foxboron> it's not really as secure as the hardware bound confirmation in mac/secure enclave. But having something is atleast informative for the user
<runxiyu> yeah, i think we should explicitly mention that on the prompt
<runxiyu> or at least in the documentation
<runxiyu> informative is great and i think we shouldn't be removing that
<runxiyu> however the user should not rely on it to make authorization decisions
<Foxboron> I suspect having it in the prompt would be verbose, but the manpage and/or docs somewhere would be fine
<runxiyu> right

[Mic92]

Sure. It won't protect you from a targeted attack. But it's still a safety feature and it adds an additional hoop that an attacker has to also consider. If the chain looks suspicious, it also provides the pid on top. If an attacker has ptrace, than ssh-tpm-agent is doomed anyway. The latter one makes me wonder if we should run these deamons as a different uid?

[runxiyu]

@Mic92 Good point on ssh-tpm-agent being manipulable by processes on the same UID, although I believe that implementing this securely requires some rethink of the UNIX userspace's security model.

I do insist that the user should be made well aware that the information presented to them is trivially manipulable, instead of presenting it as something authorized/determined properly?

[Mic92]

Than add it to the docs. /proc/self/exe might be harder to spoof but also less useful for interpreted languages.

[runxiyu]

@Mic92 /proc/self/exe is also just a heuristic anyway; dynamically linked programs and ptrace'd ones are stilly easily spoofable