Bug 684

It seems that the problem isn't that ssh can't use the agent, but rather
that it's trying to use the keys listed in ssh_config directly first,
then falling back to the keys in the agent.

Hitting enter at the passphrase prompt enough times to cycle through the
valid ssh_config listed keys causes the login to succeed.  If there are
no valid keys listed in ssh_config, but there are keys in the agent we
get the expected behavior.

It's as though ssh is trying keys in the exact opposite order it's supposed to.

Hm, you're right. In my local client configuration $HOME/.ssh/config, I have
keys defined like this:

  IdentityFile ~/.ssh/id_dsa
  IdentityFile ~/.ssh/identity

These are the same entries I have in the global ssh_config. When I comment out
the lines above in $HOME/.ssh/config authentication works fine. 

Now, here's the interesting part: if *both* config files ($HOME/.ssh/config and
ssh_config) list the IdentityFile lines involving a "~" as a placeholder for
$HOME, I get the effect with the passphrase request. If one of the files refers
to the IdentityFile using "$HOME" for the homedirectory, such as 

  IdentityFile $HOME/.ssh/id_dsa
  IdentityFile $HOME/.ssh/identity 

it works fine. It may be as simple as to say: Don't use tilde as a placeholder
for $HOME in the 3.7.1p1 config files anymore?

Created attachment 440 [details]
patch to remove duplicate identity file keys

Actually, it's $HOME that isn't expanding, not ~:
debug2: key: /home/mdelan/.ssh/id_rsa (65630)
debug2: key: $HOME/.ssh/identity (0)
debug2: key: $HOME/.ssh/id_rsa (0)
debug2: key: $HOME/.ssh/id_dsa (0)
debug2: key: /home/mdelan/.ssh/identity (0)
debug2: key: /home/mdelan/.ssh/id_dsa (0)

The real problem is that if the same key is loaded 
more than once through IdentityFile statements, either
because different listed files contain the same key, 
or the same file has been listed more than once (e.g.
global and user ssh_config files contain the same
IdentityFile statements), and that key also exists
in the agent, then we end up with multiple copies of
that key in the authctx list - one tagged as being an
agent key, the other(s) not.

userauth_pubkey() does offer the public key for the
copy in the agent first, but due to the way it manipulates
the authctx list, when input_userauth_pk_ok() goes to do
the private key side of the exchange, it finds one of
the non-agent copies first and uses that - hence the
passphrase prompt.

The patch in attachment #440 [details] is a fairly quick hack to
eliminate duplicate keys obtained from IdentityFiles
during the setup phase of pubkey_prepare(), prior to
the routine that orders the identities.

Just checking for extra duplicates of agent keys in the
next loop down might be a slicker solution, but at
the moment, I can't quite figure out how to manipulate
the TAILQ_* macros correctly to do that.

this should work, but i don't know how
portable TAILQ_FOREACH_REVERSE IS:

Index: sshconnect2.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sshconnect2.c,v
retrieving revision 1.124
diff -u -r1.124 sshconnect2.c
--- sshconnect2.c       25 Aug 2003 10:33:33 -0000      1.124
+++ sshconnect2.c       20 Sep 2003 14:25:30 -0000
@@ -445,7 +445,7 @@
        debug2("input_userauth_pk_ok: fp %s", fp);
        xfree(fp);
 
-       TAILQ_FOREACH(id, &authctxt->keys, next) {
+       TAILQ_FOREACH_REVERSE(id, &authctxt->keys, next, idlist) {
                if (key_equal(key, id->key)) {
                        sent = sign_and_send_pubkey(authctxt, id);
                        break;
folly% 


#define TAILQ_FOREACH_REVERSE(var, head, field, headname)               \
        for((var) = TAILQ_LAST(head, headname);                         \
            (var) != TAILQ_END(head);                                   \
            (var) = TAILQ_PREV(var, headname, field))

I have applied Mike's patch (#440) and it works great, thanks a lot!

Your explanation about the duplicate keys makes perfect sense.

Until you have found the most suitable solution to put into the next OpenSSH
version, I will run with the patch for now and forget about the $HOME and tilde
stuff.

Markus's patch also works - tested on Solaris & Linux.
His is probably the cleaner fix.

Created attachment 441 [details]
patch to test that identity has been offered

On further reflection, aside from Markus's concern about the portability of
TAILQ_FOREACH_REVERSE, I see another problem with that fix: it's still assuming

a particular order of &authctxt->keys.	Some future change to the way
userauth_pubkey() manipulates that list could re-introduce the bug.

Checking that id->tried is set for the identity in addition to key_equal()
within the loop seems the more robust solution.

We don't need to worry about the portability of TAILQ_FOREACH_REVERSE. For
portable OpenSSH we always use a local copy of OpenBSD's sys/queue.h (and
sys/tree.h too).

Fixed in OpenBSD and Portable (HEAD and 3.7):
   - markus@cvs.openbsd.org 2003/10/11 08:26:43
     [sshconnect2.c]
     search keys in reverse order; fixes #684

Mass change of RESOLVED bugs to CLOSED