Created attachment 1767 [details] Uses access(2) to check readability of identity file before loading When using publickey authentication, the private key is loaded from an IdentityFile using ssh2connect.c's load_identity_file() function. A stat(2) is used to ensure that the file exists, and then the key is loaded. If the file exists, but the process does not have read permissions on that file, then ssh will prompt the user for a passphrase. This is particularly confusing if the private key in question is not passphrase-protected. The fact that the ssh client prompts for the passphrase is misleading; the underlying inability to load the private key stems not from the need for a passphrase, but from the lack of readability. It would be better to display a "Permission denied" message to the user, and/or move on to the next authentication method, rather than needlessly prompting the user for passphrases in this case. The attached patch attempts to remedy this situation by using access(2) after the stat(2) call. If the file is not readable, then the private key cannot be loaded, and thus the call to key_load_private_type() is skipped.
Look at this for 5.4
Created attachment 1774 [details] log (to debug) the reason for failing to load a private key OK, so this is interesting: it doesn't happen on OpenBSD, but I do see it on Linux. sshconnect2.c:load_identity_file() calls key_load_private_type. If that fails and if we're either or in batchmode or if the key has bad perms then we quit, otherwise we prompt for a passphrase. Or at least that's what it's supposed to do. So why doesn't this actually happen on Linux? perm_ok is on the stack, and if the open() in key_load_private_type fails then it returns early without setting (or unsetting) perm_ok. Thus, whether or not you get prompted depends on the initial value of perm_ok from whatever happened to be on the stack. Nasty. This patch reports the reason if the open() fails and sets perms_ok appropriately, no need for the extra access() calls. It also initializes perms_ok to the safe default.
Patch has been applied and will be in 5.4. Thanks for the report.
Thanks for the analysis; I'd missed the reliance upon the stack-based value of perm_ok when trying to figure this out. Cheers!
With the release of 5.4p1, this bug is now considered closed.