Created attachment 2781 [details] proposed patch TL;DR ./ssh-add <(echo "") asks for passphrase for empty key. The blob is passed to OpenSSL and every parser failure is reported as "wrong passphrase", which is broken. Checking the actual error from OpenSSL would solve this issue. Long story: Based on Red hat Bugzilla [1], my post on mailing list [2] and SO question [3] I went down the source code and investigated a bit communication between OpenSSH and OpenSSL in terms of private key parsing. When openSSH can not derive the key type it passes the whole blob to OpenSSL function and waits if it can parse it. PEM_read_bio_PrivateKey() # called from sshkey.c @ 3791 Only return value from this function is NULL on failure. The reason can be obtained using ERR_ functions from OpenSSL ERR_get_error(); # actual error code ERR_print_errors_fp(stderr); # prints verbose info Possible reasons for failure of the above mentioned function are described in openssl/pem.h [4]. Basically, some of the reasons are obviously not related to wrong passphrase. I tried with somehow crippled keys and I ended up with attached patch so far (currently contains also debug output from openssl on failure). The proposed patch does not solve only the issue with empty key and "crippled" one, but also problems with unknown RSA1 keys when built without SSH1 protocol: $ ./ssh-add <(echo "") 140239613359768:error:0906D06C:lib(9):func(109):reason(108):pem_lib.c:701:Expecting: ANY PRIVATE KEY Error loading key "/dev/fd/63": invalid format $ ./ssh-add /tmp/rsa 140408665470616:error:0906D066:lib(9):func(109):reason(102):pem_lib.c:809: Error loading key "/tmp/rsa": invalid format $ ./ssh-add /tmp/rsa1 140632696993432:error:0906D06C:lib(9):func(109):reason(108):pem_lib.c:701:Expecting: ANY PRIVATE KEY Error loading key "/tmp/rsa1": invalid format Let me know if there is something not clear and if it can be included in the next release. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1282423 [2] http://lists.mindrot.org/pipermail/openssh-unix-dev/2015-December/034617.html [3] http://unix.stackexchange.com/q/251194/121504 [4] https://github.com/openssl/openssl/blob/master/include/openssl/pem.h#L509
Created attachment 2806 [details] translate libcrypto error code on PEM_read_bio_PrivateKey failure This patch translates a few more error codes, making that function return either SSH_ERR_KEY_WRONG_PASSPHRASE, SSH_ERR_INVALID_FORMAT or SSH_ERR_LIBCRYPTO_ERROR. It also uses ERR_peek_last_error() which AFAIK is what we want. ERR_peek_error() might be confused by prior soft libcrypto errors in that process/thread's life.
Thanks (it was probably my first encounter with openssl API so excuse the brevity of my patch). Your looks good to me and works fine to the extent I was able to try.
path applied. Will be released in openssh-7.3 - thanks!
change was reverted as it caused regressions in pubkey auth. Needs more investigation
I saw that reverted in the upstream CVS, but quite without any details, about use case which it breaks. Is there some more information I can have a look into or help?
(In reply to Jakub Jelen from comment #5) > I saw that reverted in the upstream CVS, but quite without any > details, about use case which it breaks. Is there some more > information I can have a look into or help? The breakage seems to occur with keys that are new-format rather than PEM (ie generated with "ssh-keygen -o" or any ED25519 keys). one reproduction case was trying to read the key to convert it with ssh-keygen. $ ssh-keygen -y -f .ssh/id_rsa -vvv Load key ".ssh/id_bert": invalid format
Created attachment 2844 [details] Do not fallback to PEM parser, if only passphrase is wrong Thank you for the details. Now I see. When the key in new format have a passphrase, it is not accepted. There is assumption that sshkey_load_private returns SSH_ERR_KEY_WRONG_PASSPHRASE when the initial attempt without passphrase fails (which quite makes sense). But parsing the key in sshkey_parse_private_fileblob_type goes through waterfall from parsing openssh format to parsing PEM format. Current implementation depends on the assumption that sshkey_parse_private_pem_fileblob returns WRONG_PASSPHRASE for everything that it does not know which gets wrong with the above patch applied (sorry that I did miss that). Proper solution would be to pass-through WRONG_PASSPHRASE return value from sshkey_parse_private2, which indicates that the parser knows the key type, but the passphrase it wrong and do not let it fall-through to the PEM parser (which does not know the key format). I ran the tests from initial bug, from the last comments and the the regress and all test passed.
retarget unfinished bugs to next release
OpenSSH 7.4 release is closing; punt the bugs to 7.5
Comment on attachment 2844 [details] Do not fallback to PEM parser, if only passphrase is wrong Thanks for tracking this down. The fix looks fine to me and, with the error code translation re-applied, passes the test-cases that were failing previously. I'd like to commit this and the original error code translation.
Comment on attachment 2844 [details] Do not fallback to PEM parser, if only passphrase is wrong third time's a charm?
Applied and will be in openssh-7.5. Thanks! commit 155d540d00ff55f063421ec182ec8ff2b7ab6cbe Author: djm@openbsd.org <djm@openbsd.org> Date: Fri Feb 10 04:34:50 2017 +0000 upstream commit bring back r1.34 that was backed out for problems loading public keys: translate OpenSSL error codes to something more meaninful; bz#2522 reported by Jakub Jelen, ok dtucker@ with additional fix from Jakub Jelen to solve the backout. bz#2525 bz#2523 re-ok dtucker@ Upstream-ID: a9d5bc0306f4473d9b4f4484f880e95f3c1cc031
*** Bug 2523 has been marked as a duplicate of this bug. ***
Close all resolved bugs after release of OpenSSH 7.7.