Pinentry fallback mechanism is poorly constructed

[ultimatespirit]

Checked the issue history and saw several pinentry issues that basically went over this exact same issue and never really got handled in any longterm manner.

Issue

As noted here on line 491 tomb rolls its own fallback mechanism for /usr/bin/pinentry, and does so poorly.

Current algorithm:

• Check if a DISPLAY is set
• If so, check if a specific command happens to exist, and if it does use it unconditionally
• Else, try more commands
• Else, use pinentry-curses else pinentry-tty (one of those virtually guaranteed to succeed if they exist)
• Else, fail

Algorithm used by /usr/bin/pinentry wrapping script

Note, this is from my Arch Linux installation with pinentry 1.3.1-5.

# Guess preferred backend based on environment.
backends=(curses tty)
if [[ -n "$DISPLAY" || -n "$WAYLAND_DISPLAY" ]]; then
	case "$XDG_CURRENT_DESKTOP" in
	KDE|LXQT|LXQt)
		backends=(qt qt5 gnome3 gtk curses tty)
		;;
	*)
		backends=(gnome3 gtk qt qt5 curses tty)
		;;
	esac
fi

for backend in "${backends[@]}"
do
	lddout=$(ldd "/usr/bin/pinentry-$backend" 2>/dev/null) || continue
#       ^~~~ most important part here, emphasis mine
	[[ "$lddout" == *'not found'* ]] && continue
	exec "/usr/bin/pinentry-$backend" "$@"
done

Which importantly checks that the found executable would even run before deciding on it.

Proposed solution

Several fold solution:

1. Just adopt that wrapper script's implementation. Checking an array of potential executables is clearly more maintainable than the current hand-unrolled implementation, and ldd is guaranteed to be on any functional system, or at least any sane one. On the off chance it's a musl-only system and they didn't make a ldd symlink, consider having support for ld-musl-$ARCH.so.
2. Provide a user-facing option to just specify the pinentry they want to use at command invocation time
3. Also, add pinentry-gtk to the list of potential applications, with gtk2 gone Arch Linux at least has removed pinentry-gtk2 and made pinentry-gtk3 -> pinentry-gtk

The implementation

# Guess preferred backend based on environment.
backends=(curses tty)
if [[ -n "$DISPLAY" || -n "$WAYLAND_DISPLAY" ]]; then
    _verbose "Graphical display system detected"
	case "$XDG_CURRENT_DESKTOP" in
	KDE|LXQT|LXQt)
		backends=(qt qt5 gnome3 gtk gtk2 gtk3 curses tty)
		;;
	*)
		backends=(gnome3 gtk gtk2 gtk3 qt qt5 curses tty)
		;;
	esac
    _verbose "Checking backends '${backends[@]}'"
fi

for backend in "${backends[@]}"
do
	lddout=$(ldd "/usr/bin/pinentry-$backend" 2>/dev/null) || continue
	[[ "$lddout" == *'not found'* ]] && continue
    _verbose "using $backend"
    output=$(pinentry_assuan_getpass | /usr/bin/pinentry-$backend)
    break
done
# Add a check that we asked for a password and then '_failure "Cannot find any viable pinentry command"'

Added in gtk{2,3} fallback, the logging statements tomb has, and changed the logic to match what tomb needs. Importantly, I did not address the "musl system that doesn't have ldd" case, as I can't test that, nor do I know if it's even a case distros can have. Additionally it does not handle alternative paths, I didn't check if tomb takes a which dependency, considering it uses zsh though it should have a which builtin. So can just augment the backend checking code to do lddout=$(ldd $(which $backend) 2>/dev/null) || continue

It's over all a trivial enough fix that I didn't bother making a pull for it. Though, that's also mostly because it doesn't address item 2, which I sincerely believe should be added regardless of this script change, and I'm not sure the exact testing across distributions required. The only real caveats I can see are which (which should be handled by the hard zsh dependency) and ldd (which you have the musl alternative, not sure there's anything else even remotely viable enough to need to be worried about here).

Conclusion

Finally, thanks y'all for making this tool. It's great, warts and all, let's make it greater.

[Narrat]

Thank you for this summary.
Indeed a place which could see some improvements. And great that you still opened the PR, even if you were intially hesistant :)