openssh bugs - Sep 2003 - [Bug 684] ssh cannot access keys stored in agent

http://bugzilla.mindrot.org/show_bug.cgi?id=684

           Summary: ssh cannot access keys stored in agent
           Product: Portable OpenSSH
           Version: 3.7.1p1
          Platform: UltraSparc
        OS/Version: Solaris
            Status: NEW
          Severity: major
          Priority: P2
         Component: ssh
        AssignedTo: openssh-bugs at mindrot.org
        ReportedBy: philipp at open.ch


It seems the ssh command in 3.7.1p1 can no longer access SSH keys stored in the
ssh-agent. The exact same configuration has worked in 3.6.1p2 and any previous
version.

When connecting to a host, the key-passphrase is always requested, even when the
key was previously added to the agent.

Other observations:
- this is a client only issue.
- authentication works for user root, but not for normal users
- ssh-keysign is setuid root and is enabled in ssh_config
- I can reproduce this on all hosts running OpenSSH 3.7.1p1

$ ssh-agent bash
$ ssh-add /home/phk/.ssh/id_dsa
$ ssh -vvv myhost
[...]
debug3: authmethod_lookup publickey
debug3: remaining preferred: keyboard-interactive,password
debug3: authmethod_is_enabled publickey
debug1: Next authentication method: publickey
debug1: Offering public key: /home/phk/.ssh/id_dsa
debug3: send_pubkey_test
debug2: we sent a publickey packet, wait for reply
debug1: Server accepts key: pkalg ssh-dss blen 433
debug2: input_userauth_pk_ok: fp a1:04:99:61:03:22:7e:79:20:fd:57:57:2c:7c:a8:98
debug3: sign_and_send_pubkey
debug1: PEM_read_PrivateKey failed
debug1: read PEM private key done: type <unknown>
Enter passphrase for key '/home/phk/.ssh/id_dsa':



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684

mdelan at computer.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mdelan at computer.org



------- Additional Comments From mdelan at computer.org  2003-09-20 03:38
-------
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.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684

philipp at open.ch changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |normal



------- Additional Comments From philipp at open.ch  2003-09-20 05:12 -------
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?



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684





------- Additional Comments From mdelan at computer.org  2003-09-20 22:15
-------
Created an attachment (id=440)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=440&action=view)
patch to remove duplicate identity file keys




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684





------- Additional Comments From mdelan at computer.org  2003-09-20 23:17
-------
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 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.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684





------- Additional Comments From markus at openbsd.org  2003-09-21 00:36 -------
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))





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684





------- Additional Comments From philipp at open.ch  2003-09-21 02:03 -------
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.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684





------- Additional Comments From mdelan at computer.org  2003-09-21 12:02
-------
Markus's patch also works - tested on Solaris & Linux.
His is probably the cleaner fix.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684

mdelan at computer.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #440 is|0                           |1
           obsolete|                            |



------- Additional Comments From mdelan at computer.org  2003-09-22 10:01
-------
Created an attachment (id=441)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=441&action=view)
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.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684





------- Additional Comments From djm at mindrot.org  2003-09-22 10:08 -------
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).



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684

markus at openbsd.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED





------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=684

dtucker at zip.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED



------- Additional Comments From dtucker at zip.com.au  2003-10-15 17:54 -------
Fixed in OpenBSD and Portable (HEAD and 3.7):
   - markus at cvs.openbsd.org 2003/10/11 08:26:43
     [sshconnect2.c]
     search keys in reverse order; fixes #684



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.