From: Corey Hickey <chickey at tagged.com> 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, add a new option -p to make ssh-add behavior more friendly to parsing. --- ssh-add.1 | 4 ++++ ssh-add.c | 13 +++++++++---- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/ssh-add.1 b/ssh-add.1 index c31de4dd9..80fdfcbcc 100644 --- a/ssh-add.1 +++ b/ssh-add.1 @@ -223,6 +223,10 @@ Lists public key parameters of all identities currently represented by the agent. .It Fl l Lists fingerprints of all identities currently represented by the agent. +.It Fl p +Behave in a more parsing-friendly fashion: when listing public keys or +fingerprints, and there are none in the agent, print nothing and exit with a +status of 0. .It Fl q Be quiet after a successful operation. .It Fl S Ar provider diff --git a/ssh-add.c b/ssh-add.c index 0035cb84a..889decc0d 100644 --- a/ssh-add.c +++ b/ssh-add.c @@ -529,7 +529,7 @@ test_key(int agent_fd, const char *filename) } static int -list_identities(int agent_fd, int do_fp) +list_identities(int agent_fd, int do_fp, int parse_friendly) { char *fp; int r; @@ -541,6 +541,8 @@ list_identities(int agent_fd, int do_fp) if (r != SSH_ERR_AGENT_NO_IDENTITIES) fprintf(stderr, "error fetching identities: %s\n", ssh_err(r)); + else if (parse_friendly) + return 0; /* no identities; nothing to do */ else printf("The agent has no identities.\n"); return -1; @@ -814,7 +816,7 @@ main(int argc, char **argv) char **dest_constraint_strings = NULL, **hostkey_files = NULL; int r, i, ch, deleting = 0, ret = 0, key_only = 0, cert_only = 0; int do_download = 0, xflag = 0, lflag = 0, Dflag = 0; - int qflag = 0, Tflag = 0; + int pflag = 0, qflag = 0, Tflag = 0; SyslogFacility log_facility = SYSLOG_FACILITY_AUTH; LogLevel log_level = SYSLOG_LEVEL_INFO; struct sshkey *k, **certs = NULL; @@ -846,7 +848,7 @@ main(int argc, char **argv) skprovider = getenv("SSH_SK_PROVIDER"); - while ((ch = getopt(argc, argv, "vkKlLCcdDTxXE:e:h:H:M:m:qs:S:t:")) != -1) { + while ((ch = getopt(argc, argv, "vkKlLCcdDTxXE:e:h:H:M:m:pqs:S:t:")) != -1) { switch (ch) { case 'v': if (log_level == SYSLOG_LEVEL_INFO) @@ -935,6 +937,9 @@ main(int argc, char **argv) case 'T': Tflag = 1; break; + case 'p': + pflag = 1; + break; default: usage(); ret = 1; @@ -950,7 +955,7 @@ main(int argc, char **argv) ret = 1; goto done; } else if (lflag) { - if (list_identities(agent_fd, lflag == 'l' ? 1 : 0) == -1) + if (list_identities(agent_fd, lflag == 'l' ? 1 : 0, pflag) == -1) ret = 1; goto done; } else if (Dflag) { -- 2.47.1
On 2025-01-09 15:27, Corey Hickey wrote:> From: Corey Hickey <chickey at tagged.com> > > 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, add a new option -p to make ssh-add behavior more > friendly to parsing.I took the approach of preserving current behavior by default, but another approach would be to: * print "The agent has no identities." to stderr instead of stdout * exit with a status of 0 instead of 1 If that alternate approach would be better, please let me know and I will send a new patch. Thank you, Corey
Corey Hickey
2025-Jan-10 03:09 UTC
[PATCH v2] ssh-add: support external parsing of key listing
From: Corey Hickey <chickey at tagged.com> 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
Apparently Analagous Threads
- [PATCH v2] ssh-add: support external parsing of key listing
- [PATCH v2] ssh-add: support external parsing of key listing
- extracting values from txt file that follow user-supplied quote
- [PATCH] ssh-agent: Add support to load additional certificates
- [PATCH] ssh-add: support parser-friendly operation