From: Corey Hickey <chickey@xxxxxxxxxx>
When ssh-add is used in a script like:
if ! KEY_LISTING=$(ssh-add -l 2>&1) ; then
echo "SSH agent error" >&2
exit 2
fi
...the operation fails when there is an agent but there are no keys in
the agent. This is because ssh-add exits with status of 1. If the
intent is to examine the keys in the agent, then this behavior is
undesired and not easily distinguishable from an error (e.g. no agent
running).
To address this, modify ssh-add to:
* print "The agent has no identities." to stderr instead of stdout
* exit with a status of 0 instead of 1
Also modify tests accordingly, so `make tests` passes.
---
regress/agent-restrict.sh | 4 ++--
regress/agent-timeout.sh | 5 +----
regress/agent.sh | 47 +++++++++++++++++----------------------
regress/test-exec.sh | 22 ++++++++++++++++++
ssh-add.c | 10 +++++----
5 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/regress/agent-restrict.sh b/regress/agent-restrict.sh
index 62cea8522..9d3fd71ac 100644
--- a/regress/agent-restrict.sh
+++ b/regress/agent-restrict.sh
@@ -204,8 +204,8 @@ trap "kill $AGENT_PID" EXIT
sleep 4 # Give it a chance to start
# Check that it's running.
${SSHADD} -l > /dev/null 2>&1
-if [ $? -ne 1 ]; then
- fail "ssh-add -l did not fail with exit code 1"
+if [ $? -ne 0 ]; then
+ fail "ssh-add -l did not fail with exit code 0"
fi
verbose "authentication with agent (no restrict)"
diff --git a/regress/agent-timeout.sh b/regress/agent-timeout.sh
index 6dec09285..1c11c5da6 100644
--- a/regress/agent-timeout.sh
+++ b/regress/agent-timeout.sh
@@ -28,10 +28,7 @@ else
trace "sleeping 2*${SSHAGENT_TIMEOUT} seconds"
sleep ${SSHAGENT_TIMEOUT}
sleep ${SSHAGENT_TIMEOUT}
- ${SSHADD} -l 2> /dev/null | grep 'The agent has no identities.' >/dev/null
- if [ $? -ne 0 ]; then
- fail "ssh-add -l still returns keys after timeout"
- fi
+ check_keys_absent "after timeout"
trace "kill agent"
${SSHAGENT} -k > /dev/null
diff --git a/regress/agent.sh b/regress/agent.sh
index f0022aca5..c33e8407e 100644
--- a/regress/agent.sh
+++ b/regress/agent.sh
@@ -22,8 +22,9 @@ if [ $r -ne 0 ]; then
fi
${SSHADD} -l > /dev/null 2>&1
-if [ $? -ne 1 ]; then
- fail "ssh-add -l did not fail with exit code 1"
+r=$?
+if [ $r -ne 0 ]; then
+ fail "with no keys, ssh-add -l failed: exit code $r"
fi
rm -f $OBJ/user_ca_key $OBJ/user_ca_key.pub
@@ -99,11 +100,15 @@ for t in ${SSH_KEYTYPES}; do
done
trace "agent forwarding"
-${SSH} -A -F $OBJ/ssh_proxy somehost ${SSHADD} -l > /dev/null 2>&1
+AGENT_LISTING=$(${SSH} -A -F $OBJ/ssh_proxy somehost ${SSHADD} -l 2> /dev/null)
r=$?
if [ $r -ne 0 ]; then
fail "ssh-add -l via agent fwd failed (exit code $r)"
fi
+NUM_LINES=$(printf "%s" "$AGENT_LISTING" | wc -l)
+if [ "$NUM_LINES" -eq 0 ]; then
+ fail "ssh-add -l via agent fwd has no keys"
+fi
${SSH} "-oForwardAgent=$SSH_AUTH_SOCK" -F $OBJ/ssh_proxy somehost ${SSHADD} -l > /dev/null 2>&1
r=$?
if [ $r -ne 0 ]; then
@@ -128,16 +133,20 @@ if [ $r -ne 0 ]; then
fail "ssh-add -l via agent path env fwd of different agent failed (exit code $r)"
fi
-# Remove keys from forwarded agent, ssh-add on remote machine should now fail.
+# Remove keys from forwarded agent, then test for keys on remote machine.
SSH_AUTH_SOCK=$FW_SSH_AUTH_SOCK ${SSHADD} -D > /dev/null 2>&1
r=$?
if [ $r -ne 0 ]; then
fail "ssh-add -D failed: exit code $r"
fi
-${SSH} '-oForwardAgent=$FW_SSH_AUTH_SOCK' -F $OBJ/ssh_proxy somehost ${SSHADD} -l > /dev/null 2>&1
+AGENT_LISTING=$(${SSH} '-oForwardAgent=$FW_SSH_AUTH_SOCK' -F $OBJ/ssh_proxy somehost ${SSHADD} -l 2> /dev/null)
r=$?
-if [ $r -ne 1 ]; then
- fail "ssh-add -l with different agent did not fail with exit code 1 (exit code $r)"
+if [ $r -ne 0 ]; then
+ fail "ssh-add -l via agent fwd failed (exit code $r)"
+fi
+NUM_LINES=$(printf "%s" "$AGENT_LISTING" | wc -l)
+if [ "$NUM_LINES" -ne 0 ]; then
+ fail "ssh-add -l via agent fwd still has keys"
fi
(printf 'cert-authority,principals="estragon" '; cat $OBJ/user_ca_key.pub) \
@@ -164,11 +173,7 @@ if [ $r -ne 0 ]; then
fail "ssh-add -D failed: exit code $r"
fi
# make sure they're gone
-${SSHADD} -l > /dev/null 2>&1
-r=$?
-if [ $r -ne 1 ]; then
- fail "ssh-add -l returned unexpected exit code: $r"
-fi
+check_keys_absent "after delete"
trace "readd keys"
# re-add keys/certs to agent
for t in ${SSH_KEYTYPES}; do
@@ -176,11 +181,7 @@ for t in ${SSH_KEYTYPES}; do
fail "ssh-add failed exit code $?"
done
# make sure they are there
-${SSHADD} -l > /dev/null 2>&1
-r=$?
-if [ $r -ne 0 ]; then
- fail "ssh-add -l failed: exit code $r"
-fi
+check_keys_present "after readd"
trace "delete all agent keys using SIGUSR1"
kill -s USR1 $SSH_AGENT_PID
r=$?
@@ -188,22 +189,14 @@ if [ $r -ne 0 ]; then
fail "kill -s USR1 failed: exit code $r"
fi
# make sure they're gone
-${SSHADD} -l > /dev/null 2>&1
-r=$?
-if [ $r -ne 1 ]; then
- fail "ssh-add -l returned unexpected exit code: $r"
-fi
+check_keys_absent "after USR1"
# re-add keys/certs to agent
for t in ${SSH_KEYTYPES}; do
${SSHADD} $OBJ/$t-agent-private >/dev/null 2>&1 || \
fail "ssh-add failed exit code $?"
done
# make sure they are there
-${SSHADD} -l > /dev/null 2>&1
-r=$?
-if [ $r -ne 0 ]; then
- fail "ssh-add -l failed: exit code $r"
-fi
+check_keys_present "after readd again"
check_key_absent() {
${SSHADD} -L | grep "^$1 " >/dev/null
diff --git a/regress/test-exec.sh b/regress/test-exec.sh
index 62c00fd8c..89170657e 100644
--- a/regress/test-exec.sh
+++ b/regress/test-exec.sh
@@ -495,6 +495,28 @@ stop_sshd ()
PIDFILE=""
}
+_check_keys_present() {
+ AGENT_LISTING=$(${SSHADD} -l 2> /dev/null)
+ r=$?
+ if [ $r -ne 0 ]; then
+ fail "$1: ssh-add -l returned unexpected exit code: $r"
+ fi
+ NUM_LINES=$(printf "%s" "$AGENT_LISTING" | wc -l)
+ [ "$NUM_LINES" -ne 0 ]
+}
+
+check_keys_absent() {
+ if _check_keys_present "$1" ; then
+ fail "$1: keys unexpectedly present"
+ fi
+}
+
+check_keys_present() {
+ if ! _check_keys_present "$1" ; then
+ fail "$1: keys unexpectedly absent"
+ fi
+}
+
# helper
cleanup ()
{
diff --git a/ssh-add.c b/ssh-add.c
index 0035cb84a..8194a1649 100644
--- a/ssh-add.c
+++ b/ssh-add.c
@@ -538,12 +538,14 @@ list_identities(int agent_fd, int do_fp)
size_t i;
if ((r = ssh_fetch_identitylist(agent_fd, &idlist)) != 0) {
- if (r != SSH_ERR_AGENT_NO_IDENTITIES)
+ if (r == SSH_ERR_AGENT_NO_IDENTITIES) {
+ fprintf(stderr, "The agent has no identities.\n");
+ return 0;
+ } else {
fprintf(stderr, "error fetching identities: %s\n",
ssh_err(r));
- else
- printf("The agent has no identities.\n");
- return -1;
+ return -1;
+ }
}
for (i = 0; i < idlist->nkeys; i++) {
if (do_fp) {
--
2.47.1
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
