uxpgnt: correct control flow in find_key().

If we try to interpret a string argument as the name of a key file,
sometimes we it's in circumstances where we _know_ it's a key file, so
we must print an error message and return failure if the file can't be
loaded. Other times it's not, and we just fall back to interpreting
the argument in some other way (e.g. as a pattern match against the
comment or fingerprint of a key already in the agent).

My code dealing with failure returns from the public-key loading
functions were mishandling the latter case, if they identified a file
as existing and looking more or less like some kind of key file but
then it turned out to have a format error; they would try to copy and
return a public key that they didn't actually have. Even if
pageant_pubkey_copy avoided crashing as a result, this would still
inhibit the fallback to treating the input string as some other kind
of pattern match.

diff --git a/unix/uxpgnt.c b/unix/uxpgnt.c
index a4bc6b9389b0ff09e7b7bc7fdcc5dc9fa3ebf087..50f6516860370cef6b8f14639b34a0275368800c 100644 (file)
--- a/unix/uxpgnt.c
+++ b/unix/uxpgnt.c
@@ -543,19 +543,19 @@ struct pageant_pubkey *find_key(const char *string, char **retstr)
                     filename_free(fn);
                     return NULL;
                 }
+            } else {
+                /*
+                 * If we've successfully loaded the file, stop here - we
+                 * already have a key blob and need not go to the agent to
+                 * list things.
+                 */
+                key_in.ssh_version = 1;
+                key_in.comment = NULL;
+                key_ret = pageant_pubkey_copy(&key_in);
+                sfree(key_in.blob);
+                filename_free(fn);
+                return key_ret;
             }
-
-            /*
-             * If we've successfully loaded the file, stop here - we
-             * already have a key blob and need not go to the agent to
-             * list things.
-             */
-            key_in.ssh_version = 1;
-            key_in.comment = NULL;
-            key_ret = pageant_pubkey_copy(&key_in);
-            sfree(key_in.blob);
-            filename_free(fn);
-            return key_ret;
         } else if (keytype == SSH_KEYTYPE_SSH2 ||
                    keytype == SSH_KEYTYPE_SSH2_PUBLIC_RFC4716 ||
                    keytype == SSH_KEYTYPE_SSH2_PUBLIC_OPENSSH) {
@@ -570,19 +570,19 @@ struct pageant_pubkey *find_key(const char *string, char **retstr)
                     filename_free(fn);
                     return NULL;
                 }
+            } else {
+                /*
+                 * If we've successfully loaded the file, stop here - we
+                 * already have a key blob and need not go to the agent to
+                 * list things.
+                 */
+                key_in.ssh_version = 2;
+                key_in.comment = NULL;
+                key_ret = pageant_pubkey_copy(&key_in);
+                sfree(key_in.blob);
+                filename_free(fn);
+                return key_ret;
             }
-
-            /*
-             * If we've successfully loaded the file, stop here - we
-             * already have a key blob and need not go to the agent to
-             * list things.
-             */
-            key_in.ssh_version = 2;
-            key_in.comment = NULL;
-            key_ret = pageant_pubkey_copy(&key_in);
-            sfree(key_in.blob);
-            filename_free(fn);
-            return key_ret;
         } else {
             if (file_errors) {
                 *retstr = dupprintf("unable to load key file '%s': %s",