Repository navigation
system/term: Lock the controlling tty device instead of /dev/tty - #1682
Conversation
/dev/tty is a single device node shared by all processes, so taking a flock() on it serializes the password prompts of every sudo invocation in the system, not only of the ones sharing the same controlling tty. Resolve the controlling tty path using the same device-based logic we use for PAM_TTY and lock that one instead, so that only the processes on the same terminal contend for the prompt. In case the path can't be resolved, still fall back to lock /dev/tty, since serializing the prompts across all the terminals is better than not serializing them at all. Follow-up of: trifectatechfoundation#1651
|
Having now looked at this a bit more closely: @3v1n0 Could you rebase #1684 on top of this PR? I feel the issue this PR addresses (which I will create) and trying to de-duplicate the TTY-resolving bits in sudo-rs are two orthogonal things. I would like to merge this first and then be able to judge #1684 separately. The reason is also that I think the changes this PR introduce are very easily acceptable (it fixes a clear problem), whereas #1684, I think the sudo-rs team should feel freedom to decide that the added complexity it introduces isn't worth the benefits (conforming our criteria for inclusion). |
|
I've created issue #1693 to be able to track this PR separately. |
|
Thanks for this bugfix! |
|
Oh, thanks for merging, for some reasons I had lost your comments before, so I'll rebase the other one. And indeed, this was supposed to be a minimal fix, while other one could have been either the base or the followup of this one. |
Commit 44d9512 locks /dev/tty directly, bypassing the controlling tty
selection we do in
ttyname_from_dev().However /dev/tty is a single device node shared by all processes, so taking
a flock() on it serializes the password prompts of every sudo invocation in
the system, not only of the ones sharing the same controlling tty: a sudo
prompting on one terminal blocks any other sudo on a different terminal
until it is done.
Resolve the controlling tty path using the same device-based logic we use
for PAM_TTY and lock that one instead, so that only the processes on the
same terminal contend for the prompt.
In case the path can't be resolved, still fall back to locking /dev/tty,
since serializing the prompts across all the terminals is better than not
serializing them at all.
Add tests for both cases (that are also passing in sudo.ws):
prompt_on_another_tty_does_not_block: fails without this change, asthe second sudo waits for the first one's prompt on another tty
prompt_on_same_tty_is_serialized: covers the behavior intended byLock tty in sudo before the password prompt shows #1651, i.e. sudo invocations in a pipeline don't prompt concurrently
Follow-up of: #1651