Bug 684 - ssh cannot access keys stored in agent
Summary: ssh cannot access keys stored in agent
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh (show other bugs)
Version: 3.7.1p1
Hardware: UltraSPARC Solaris
: P2 normal
Assignee: OpenSSH Bugzilla mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-09-19 04:20 AEST by Philipp Koller
Modified: 2004-04-14 12:24 AEST (History)
1 user (show)

See Also:


Attachments
patch to remove duplicate identity file keys (1019 bytes, patch)
2003-09-20 22:15 AEST, Mike Delaney
no flags Details | Diff
patch to test that identity has been offered (338 bytes, patch)
2003-09-22 10:01 AEST, Mike Delaney
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Koller 2003-09-19 04:20:38 AEST
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':
Comment 1 Mike Delaney 2003-09-20 03:38:19 AEST
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.
Comment 2 Philipp Koller 2003-09-20 05:12:00 AEST
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?
Comment 3 Mike Delaney 2003-09-20 22:15:03 AEST
Created attachment 440 [details]
patch to remove duplicate identity file keys
Comment 4 Mike Delaney 2003-09-20 23:17:02 AEST
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.
Comment 5 Markus Friedl 2003-09-21 00:36:23 AEST
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))

Comment 6 Philipp Koller 2003-09-21 02:03:53 AEST
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.
Comment 7 Mike Delaney 2003-09-21 12:02:09 AEST
Markus's patch also works - tested on Solaris & Linux.
His is probably the cleaner fix.
Comment 8 Mike Delaney 2003-09-22 10:01:38 AEST
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.
Comment 9 Damien Miller 2003-09-22 10:08:23 AEST
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).
Comment 10 Darren Tucker 2003-10-15 17:54:04 AEST
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
Comment 11 Damien Miller 2004-04-14 12:24:19 AEST
Mass change of RESOLVED bugs to CLOSED