Keychain ssh problem (bug????)

pcwulf · 11-27-2003, 08:15 AM

Hi all,

I have tried to use keychain program (version 2.0.3) from www gentoo org /projects/keychain to make passwordless logins easy and safe. After spending a couple of days with it I realized that somehow it does not work properly for me. Then I modified it a bit and now works fine. My question is whether this is a bug in the program or did I make a mistake.

To state it in brief:
Some part in this program filters all the pids of ssh-agent program under the current user and compares each with a saved number (pid) in a file. If there is a match, new ssh-agent is not started.

The program works fine when there is only one ssh-agent running. It works even when multiple ssh-agents are running IFF the FIRST pid in the output of ps command is the one saved. However, if there is any other ssh-agent pid coming before the required (saved) pid, then the test for existing pid is failed.

What happens is that there is a loop in the program driven by
for x in $mypids
apperently this needs whitespace seperated list of ssh-agent pids.
The following part of the code is supposed to provide this. But it seems that it provides only a single (first ) pid for the $mypids variable!

$ diff keychain.original keychain.i.modified
281c281
< mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"` > /dev/null 2>&1
---
> mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"|awk '{print $2}'` > /dev/null 2>&1
283,287c283,287
< if [ -n "$mypids" ]
< then
< set $mypids
< mypids=$2
< fi
---
> #if [ -n "$mypids" ]
> #then
> # set $mypids
> # mypids=$2
> #fi

Notes:
a) $psopts= 'uxw' , $grepopts='' are selected by keychain for my system
b) My OS is Redhat Linux 8.0 with 2.4.18-14 kernel
c) in RedHat (or is it all Linux) /bin/sh is simply a symbolic link to bash.

Can somebody comment on this please?

unSpawn · 11-27-2003, 10:12 AM

This isn't exactly a security related question, it's a problem with an app, or a scripting problem. Anyway, could you try this diff? I think it solves things "better".

Code:

--- keychain	Mon Apr 21 02:04:21 2003
+++ keychain	Thu Nov 27 17:10:05 2003
@@ -278,12 +278,18 @@
 	fi
 fi
 
-mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"` > /dev/null 2>&1
-#extract the second item from mypids:
-if [ -n "$mypids" ]
-then
+if [ -x /usr/bin/pgrep ]; then
+	# Select process by user, and match commandline, and separate by space.
+	# Fall back to using "ps" when no psutils are there.
+	mypids="$(/usr/bin/pgrep -u $LOGNAME -f "[s]sh-agent" -d " ")"
+else
+	mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"` > /dev/null 2>&1
+	#extract the second item from mypids:
+	if [ -n "$mypids" ]
+	then
 	set $mypids
 	mypids=$2
+	fi
 fi
 
 if [ "$myaction" = "stop" ]
@@ -328,7 +334,11 @@
 			echo $E " ${GREEN}*${OFF} Found existing ssh-agent at PID ${x}"
 		fi
 		match="yes"
-		break
+		# Don't break if we got more than one PID to handle
+		case "${#mypids}" in 1) break;; esac
+	else
+		echo $E " ${GREEN}*${OFF} Killing stale ssh-agent at PID ${x}"
+		kill "$x" > /dev/null 2>&1
 	fi
 done

pcwulf · 11-27-2003, 08:11 PM

Thanks unspawn, and sorry for posting on the wrong board! (I thought this is the board I should use since keychain is all about securtity)

Anyway, I tried your diff and it works. Now I have two problems:
1) whenever you do a su (to become root) your $LOGNAME remains <username>
So when I do that, keychain kills all my ssh-agent processes for <username> but not for root.

so is it ok to user `id -un` instead of $LOGNAME

2) Your diff works fine as long as there is pgrep in the machine. However, when I `mv /usr/bin/pgrep /usr/bin/pgrep.bak` and try, the same old problem remains.

mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"` > /dev/null 2>&1
#extract the second item from mypids:
if [ -n "$mypids" ]
then
set $mypids
mypids=$2

part DOES NOT correctly select mypids list! but something like

mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"| awk '{print $2}'` > /dev/null 2>&1
works perfectly.

unSpawn · 11-27-2003, 10:55 PM

sorry for posting on the wrong board! (I thought this is the board I should use since keychain is all about securtity)
No problem. It's kind of a opaque division for most it seems...

so is it ok to user `id -un` instead of $LOGNAME
By all means, if it's better, do use that.

something like
mypids=`ps $psopts 2>/dev/null | grep $grepopts "[s]sh-agent"| awk '{print $2}'` > /dev/null 2>&1
works perfectly.
Again, if it's better, do use that.

BTW, I hope you send a mail with the patch to the maintainers? Would be good to know you've fixed some for them.