search

trifectatechfoundation / sudo-rs

A memory safe implementation of sudo and su.
Other
2.87k stars 76 forks source link

SUDO_PROMPT implementation request #842

Open id3v1669 opened 4 months ago

id3v1669 commented 4 months ago

I've seen in #129 that creation of this functionality is in Doubtful section, but asking for its implementation due to this issue.

squell commented 4 months ago

If I understand that issue correcty, it is enough to support -p "" to disable the prompt (although it's just as easy to support any other fixed string consisting of printable characters), and we don't need to add all the % expansions that c-sudo does. The interplay between the prompt and the -S option does sound like it's not unreasonable to allow some control over it.

Of course we should then also add the passprompt_override sudoers setting.

squell commented 4 months ago

Note that that sudo-rs does indeed use stderr if -S is used, as c-sudo does.

rnijveld commented 4 months ago

The reason for not implementing this previously was that we don't necessarily have a password prompt, given that such prompts are being generated by PAM. You could have a system where asking for input could mean entering any kind of information, some other piece of information such as a username or email address, or more likely something like a pin, or some security or TOTP code. PAM modules could even just output a URL and hang until you visit that URL on some other device and do some authentication ritual over there.

Sudo works around this for PAM authentication by using a regex for matching the password prompt and then replacing that prompt with the one provided via -p or SUDO_PROMPT, which at least to me feels very fragile. But I'd also consider such a feature to allow misleading the user into inputting something that they did not intent to input at all (and probably confusing them as to why that piece of information didn't work when they were supposed to enter their password). And (with the -S flag set), sudo might still output stuff from any number of PAM modules to stderr anyway even with the SUDO_PROMPT set to a specific value.

For the Veracrypt GUI, I think they should really be implementing this using something like pkexec, which is intended to be used in a GUI context and doesn't touch stdout/err/in if in the right context, instead of relying on parsing CLI output based on very specific behavior. If the worry is that some weird input could be given via SUDO_PROMPT, why is that environment value even passed at all? Alternatively, it might be better to open a PTY at the very least, to prevent having to multiplex everything through stderr?

LeChatP commented 3 months ago

Ansible uses a pseudorandomly generated prompt to ensure the terminal prompts for the password. The PAM default prompt is "Password:" which could be wrongly matched with many other stdout strings. I understand it isn't a great solution, but adding a mandatory way to deactivate it by default could solve the misleading issue. And so, Ansible and VeraCrypt users could use sudo-rs by doing an additional configuration step.