Improve auto public key authentication.

git-svn-id: svn+ssh://svn.berlios.de/svnroot/repos/libssh/trunk@636 7dcaeef0-15fb-0310-b436-a5af3365683c

include/libssh/libssh.h

@@ -82,6 +82,7 @@ typedef struct channel_struct CHANNEL;
 typedef struct agent_struct AGENT;
 typedef struct ssh_session SSH_SESSION;
 typedef struct ssh_kbdint SSH_KBDINT;
+struct keys_struct;
 
 /* integer values */
 typedef uint32_t u32;
@@ -255,8 +256,9 @@ PUBLIC_KEY *publickey_from_privatekey(PRIVATE_KEY *prv);
 void privatekey_free(PRIVATE_KEY *prv);
 STRING *publickey_from_file(SSH_SESSION *session, const char *filename,
     int *type);
-STRING *publickey_from_next_file(SSH_SESSION *session, const char **pub_keys_path,
-    const char **keys_path, char **privkeyfile, int *type, int *count);
+STRING *publickey_from_next_file(SSH_SESSION *session,
+    struct keys_struct *keytab, size_t keytab_size,
+    char **privkeyfile, int *type, unsigned int *count);
 int ssh_is_server_known(SSH_SESSION *session);
 int ssh_write_knownhost(SSH_SESSION *session);
 

include/libssh/priv.h

@@ -299,6 +299,11 @@ struct agent_struct {
   unsigned int count;
 };
 
+struct keys_struct {
+  const char *private;
+  const char *public;
+};
+
 struct ssh_session {
     struct error_struct error;
     struct socket *socket;
@@ -719,6 +724,9 @@ int match_hostname(const char *host, const char *pattern, unsigned int len);
 /** Zero a structure given a pointer to the structure */
 #define ZERO_STRUCTP(x) do { if ((x) != NULL) memset((char *)(x), 0, sizeof(*(x))); } while(0)
 
+/** Get the size of an array */
+#define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
+
 #ifdef HAVE_LIBGCRYPT
 /* gcrypt_missing.c */
 int my_gcry_dec2bn(bignum *bn, const char *data);

libssh/auth.c

@@ -687,61 +687,71 @@ error:
     return rc;
 }
 
-static const char *keys_path[] = {
-  NULL,
-  "%s/.ssh/identity",
-  "%s/.ssh/id_dsa",
-  "%s/.ssh/id_rsa",
-  NULL
-};
-
-static const char *pub_keys_path[] = {
-  NULL,
-  "%s/.ssh/identity.pub",
-  "%s/.ssh/id_dsa.pub",
-  "%s/.ssh/id_rsa.pub",
-  NULL
+static struct keys_struct keytab[] = {
+  {
+    .private = "%s/.ssh/identity",
+    .public = "%s/.ssh/identity.pub"
+  },
+  {
+    .private = "%s/.ssh/id_dsa",
+    .public = "%s/.ssh/id_dsa.pub",
+  },
+  {
+    .private = "%s/.ssh/id_rsa",
+    .public = "%s/.ssh/id_rsa.pub",
+  },
+  {
+    .private = NULL,
+    .public = NULL
+  }
 };
 
 /* this function initialy was in the client */
 /* but the fools are the ones who never change mind */
 
-/** it may fail, for instance it doesn't ask for a password and uses a default
- * asker for passphrases (in case the private key is encrypted)
- * \brief Tries to automaticaly authenticate with public key and "none"
- * \param session ssh session
- * \param passphrase use this passphrase to unlock the privatekey. Use
- *                   NULL if you don't want to use a passphrase or the
- *                   user should be asked.
- * \returns SSH_AUTH_ERROR : a serious error happened\n
- * SSH_AUTH_DENIED : Authentication failed : use another method\n
- * SSH_AUTH_PARTIAL : You've been partially authenticated, you still have to use another method\n
- * SSH_AUTH_SUCCESS : Authentication success
- * \see ssh_userauth_kbdint()
- * \see ssh_userauth_password()
- * \see ssh_options_set_identity()
+/**
+ * @brief Tries to automaticaly authenticate with public key and "none"
+ *
+ * It may fail, for instance it doesn't ask for a password and uses a default
+ * asker for passphrases (in case the private key is encrypted).
+ *
+ * @param session       The ssh session to authenticate with.
+ *
+ * @param passphrase    Use this passphrase to unlock the privatekey. Use NULL
+ *                      if you don't want to use a passphrase or the user
+ *                      should be asked.
+ *
+ * @returns SSH_AUTH_ERROR:   A serious error happened\n
+ *          SSH_AUTH_DENIED:  Authentication failed: use another method\n
+ *          SSH_AUTH_PARTIAL: You've been partially authenticated, you still
+ *                            have to use another method\n
+ *          SSH_AUTH_SUCCESS: Authentication success
+ *
+ * @see ssh_userauth_kbdint()
+ * @see ssh_userauth_password()
+ * @see ssh_options_set_identity()
  */
-
 int ssh_userauth_autopubkey(SSH_SESSION *session, const char *passphrase) {
-    int count=1; /* bypass identity */
-    int type=0;
-    int err;
-    STRING *pubkey;
   struct public_key_struct *publickey;
-    char *privkeyfile=NULL;
+  STRING *pubkey;
   PRIVATE_KEY *privkey;
+  char *privkeyfile = NULL;
   char *id = NULL;
+  size_t size;
+  unsigned int count = 0;
+  int type = 0;
+  int rc;
 
   enter_function();
 
-    // always testing none
-    err=ssh_userauth_none(session,NULL);
-    if(err==SSH_AUTH_ERROR || err==SSH_AUTH_SUCCESS){
+  /* Always test none authentication */
+  rc = ssh_userauth_none(session, NULL);
+  if (rc == SSH_AUTH_ERROR || rc == SSH_AUTH_SUCCESS) {
     leave_function();
-        return err;
+    return rc;
   }
 
-    /* try ssh-agent keys first */
+  /* Try authentication with ssh-agent first */
 #ifndef _WIN32
   if (agent_is_running(session)) {
     ssh_log(session, SSH_LOG_RARE,
@@ -755,16 +765,16 @@ int ssh_userauth_autopubkey(SSH_SESSION *session, const char *passphrase) {
 
       pubkey = publickey_to_string(publickey);
       if (pubkey) {
-          err = ssh_userauth_offer_pubkey(session, NULL, publickey->type, pubkey);
+        rc = ssh_userauth_offer_pubkey(session, NULL, publickey->type, pubkey);
         string_free(pubkey);
-          if (err == SSH_AUTH_ERROR) {
+        if (rc == SSH_AUTH_ERROR) {
           SAFE_FREE(id);
           SAFE_FREE(privkeyfile);
           publickey_free(publickey);
           leave_function();
 
-            return err;
-          } else if (err != SSH_AUTH_SUCCESS) {
+          return rc;
+        } else if (rc != SSH_AUTH_SUCCESS) {
           ssh_log(session, SSH_LOG_PACKET, "Public key refused by server\n");
           SAFE_FREE(id);
           SAFE_FREE(privkeyfile);
@@ -773,15 +783,15 @@ int ssh_userauth_autopubkey(SSH_SESSION *session, const char *passphrase) {
         }
         ssh_log(session, SSH_LOG_RARE, "Public key accepted");
         /* pubkey accepted by server ! */
-          err = ssh_userauth_agent_pubkey(session, NULL, publickey);
-          if (err == SSH_AUTH_ERROR) {
+        rc = ssh_userauth_agent_pubkey(session, NULL, publickey);
+        if (rc == SSH_AUTH_ERROR) {
           SAFE_FREE(id);
           SAFE_FREE(privkeyfile);
           publickey_free(publickey);
           leave_function();
 
-            return err;
-          } else if (err != SSH_AUTH_SUCCESS) {
+          return rc;
+        } else if (rc != SSH_AUTH_SUCCESS) {
           ssh_log(session, SSH_LOG_RARE,
               "Server accepted public key but refused the signature\n"
               "It might be a bug of libssh\n");
@@ -808,101 +818,109 @@ int ssh_userauth_autopubkey(SSH_SESSION *session, const char *passphrase) {
   } /* if agent is running */
 #endif
 
-    if(session->options->identity){
+  size = ARRAY_SIZE(keytab);
+  if (session->options->identity) {
     ssh_log(session, SSH_LOG_RARE,
         "Trying identity file %s\n", session->options->identity);
-        keys_path[0]=session->options->identity;
-        /* let's hope alloca exists */
-        id=malloc(strlen(session->options->identity)+1 + 4);
+
+    id = malloc(strlen(session->options->identity) + 1 + 4);
     if (id == NULL) {
-          keys_path[0] = NULL;
       leave_function();
       return SSH_AUTH_ERROR;
     }
-        sprintf(id,"%s.pub",session->options->identity);
-        pub_keys_path[0]=id;
-        count =0;
+    sprintf(id, "%s.pub", session->options->identity);
+
+    keytab[size - 1].private = session->options->identity;
+    keytab[size - 1].public = id;
   }
-    while((pubkey=publickey_from_next_file(session,pub_keys_path,keys_path, &privkeyfile,&type,&count))){
-        err=ssh_userauth_offer_pubkey(session,NULL,type,pubkey);
-        if(err==SSH_AUTH_ERROR){
-            if(id){
-                pub_keys_path[0]=NULL;
-                keys_path[0]=NULL;
-                free(id);
+
+  while ((pubkey = publickey_from_next_file(session, keytab, size,
+          &privkeyfile, &type, &count))) {
+    rc = ssh_userauth_offer_pubkey(session, NULL, type, pubkey);
+    if (rc == SSH_AUTH_ERROR){
+      if (id != NULL) {
+        keytab[size - 1].private = NULL;
+        keytab[size - 1].public  = NULL;
+        SAFE_FREE(id);
       }
-            free(pubkey);
-            free(privkeyfile);
+      string_free(pubkey);
+      SAFE_FREE(privkeyfile);
       leave_function();
-            return err;
-        } else
-        if(err != SSH_AUTH_SUCCESS){
-            ssh_log(session, SSH_LOG_RARE, "Public key refused by server\n");
-            free(pubkey);
-            pubkey=NULL;
-            free(privkeyfile);
-            privkeyfile=NULL;
+      return rc;
+    } else {
+      if (rc != SSH_AUTH_SUCCESS){
+        ssh_log(session, SSH_LOG_RARE, "Public key refused by server");
+        string_free(pubkey);
+        pubkey = NULL;
+        SAFE_FREE(privkeyfile);
+        privkeyfile = NULL;
         continue;
       }
-        /* pubkey accepted by server ! */
-        privkey=privatekey_from_file(session,privkeyfile,type,passphrase);
-        if(!privkey){
+    }
+
+    /* Public key accepted by server! */
+    privkey = privatekey_from_file(session, privkeyfile, type, passphrase);
+    if (privkey == NULL) {
       ssh_log(session, SSH_LOG_FUNCTIONS,
-                "Reading private key %s failed (bad passphrase ?)\n",
+          "Reading private key %s failed (bad passphrase ?)",
           privkeyfile);
-            free(pubkey);
-            pubkey=NULL;
-            free(privkeyfile);
-            privkeyfile=NULL;
+      string_free(pubkey);
+      pubkey = NULL;
+      SAFE_FREE(privkeyfile);
+      privkeyfile = NULL;
       continue; /* continue the loop with other pubkey */
     }
-        err=ssh_userauth_pubkey(session,NULL,pubkey,privkey);
-        if(err==SSH_AUTH_ERROR){
-            if(id){
-                pub_keys_path[0]=NULL;
-                keys_path[0]=NULL;
-                free(id);
+
+    rc = ssh_userauth_pubkey(session, NULL, pubkey, privkey);
+    if (rc == SSH_AUTH_ERROR) {
+      if (id != NULL) {
+        keytab[size - 1].private = NULL;
+        keytab[size - 1].public  = NULL;
+        SAFE_FREE(id);
       }
-            free(pubkey);
-            free(privkeyfile);
+      string_free(pubkey);
+      SAFE_FREE(privkeyfile);
       privatekey_free(privkey);
       leave_function();
-            return err;
-        } else
-        if(err != SSH_AUTH_SUCCESS){
+      return rc;
+    } else {
+      if (rc != SSH_AUTH_SUCCESS){
         ssh_log(session, SSH_LOG_FUNCTIONS,
-                "Weird : server accepted our public key but refused the signature\n"
-                "it might be a bug of libssh\n");
-            free(pubkey);
-            pubkey=NULL;
-            free(privkeyfile);
-            privkeyfile=NULL;
+            "The server accepted the public key but refused the signature");
+        string_free(pubkey);
+        pubkey = NULL;
+        SAFE_FREE(privkeyfile);
+        privkeyfile = NULL;
         privatekey_free(privkey);
         continue;
       }
+    }
+
     /* auth success */
     ssh_log(session, SSH_LOG_RARE,
-            "Authentication using %s success\n", privkeyfile);
-        free(pubkey);
+        "Successfully authenticated using %s", privkeyfile);
+    string_free(pubkey);
     privatekey_free(privkey);
-        free(privkeyfile);
-        if(id){
-            pub_keys_path[0]=NULL;
-            keys_path[0]=NULL;
-            free(id);
+    SAFE_FREE(privkeyfile);
+    if (id != NULL) {
+      keytab[size - 1].private = NULL;
+      keytab[size - 1].public  = NULL;
+      SAFE_FREE(id);
     }
+
     leave_function();
     return SSH_AUTH_SUCCESS;
   }
   /* at this point, pubkey is NULL and so is privkeyfile */
   ssh_log(session, SSH_LOG_FUNCTIONS,
-        "Tried every public key, none matched\n");
-    ssh_set_error(session,SSH_NO_ERROR,"no public key matched");
-    if(id){
-        pub_keys_path[0]=NULL;
-        keys_path[0]=NULL;
-        free(id);
+      "Tried every public key, none matched");
+  ssh_set_error(session,SSH_NO_ERROR,"No public key matched");
+  if (id) {
+    keytab[size - 1].private = NULL;
+    keytab[size - 1].public  = NULL;
+    SAFE_FREE(id);
   }
+
   leave_function();
   return SSH_AUTH_DENIED;
 }

libssh/keyfiles.c

@@ -920,51 +920,91 @@ STRING *publickey_from_file(SSH_SESSION *session, const char *filename,
 }
 
 
-/* why recursing ? i'll explain. on top, publickey_from_next_file will be executed until NULL returned */
-/* we can't return null if one of the possible keys is wrong. we must test them before getting over */
-STRING *publickey_from_next_file(SSH_SESSION *session, const char **pub_keys_path,
-    const char **keys_path, char **privkeyfile, int *type, int *count) {
-    static char *home=NULL;
-    char public[256];
-    char private[256];
+/*
+ * Why a recursive function?
+ *
+ * publickey_from_next_file() will be executed until NULL is returned
+ * We can't return NULL if one of the possible keys is wrong. We want to
+ * test them before getting over
+ */
+STRING *publickey_from_next_file(SSH_SESSION *session,
+    struct keys_struct *keytab, size_t keytab_size,
+    char **privkeyfile, int *type,
+    unsigned int *count) {
+  static char *home = NULL;
+
+  char public[256] = {0};
+  char private[256] = {0};
   const char *priv;
   const char *pub;
+  char *new;
   STRING *pubkey;
-    if(!home)
-        home=ssh_get_user_home_dir();
-    if(home==NULL) {
+
+  if (home == NULL) {
+    home = ssh_get_user_home_dir();
+    if (home == NULL) {
       ssh_set_error(session,SSH_FATAL,"User home dir impossible to guess");
       return NULL;
     }
-    ssh_set_error(session,SSH_REQUEST_DENIED,"no public key matched");
-    if((pub=pub_keys_path[*count])==NULL)
+  }
+
+  if (*count >= keytab_size) {
     return NULL;
-    if((priv=keys_path[*count])==NULL)
+  }
+
+  pub = keytab[*count].public;
+  if (pub == NULL) {
     return NULL;
-    ++*count;
+  }
+  priv = keytab[*count].private;
+  if (priv == NULL) {
+    return NULL;
+  }
+
+  (*count)++;
+
   /* are them readable ? */
-    snprintf(public,256,pub,home);
-    ssh_log(session,SSH_LOG_PACKET,"Trying to open public key %s",public);
-    if(!ssh_file_readaccess_ok(public)){
-        ssh_log(session,SSH_LOG_PACKET,"Failed");
-        return publickey_from_next_file(session,pub_keys_path,keys_path,privkeyfile,type,count);
+  snprintf(public, sizeof(public), pub, home);
+  ssh_log(session, SSH_LOG_PACKET, "Trying to open public key %s", public);
+  if (!ssh_file_readaccess_ok(public)) {
+    ssh_log(session, SSH_LOG_PACKET, "Failed");
+    return publickey_from_next_file(session, keytab, keytab_size,
+        privkeyfile, type, count);
   }
-    snprintf(private,256,priv,home);
-    ssh_log(session,SSH_LOG_PACKET,"Trying to open private key %s",private);
-    if(!ssh_file_readaccess_ok(private)){
-        ssh_log(session,SSH_LOG_PACKET,"Failed");
-        return publickey_from_next_file(session,pub_keys_path,keys_path,privkeyfile,type,count);
+
+  snprintf(private, sizeof(private), priv, home);
+  ssh_log(session, SSH_LOG_PACKET, "Trying to open private key %s", private);
+  if (!ssh_file_readaccess_ok(private)) {
+    ssh_log(session, SSH_LOG_PACKET, "Failed");
+    return publickey_from_next_file(session, keytab, keytab_size,
+        privkeyfile, type, count);
   }
-    ssh_log(session,SSH_LOG_PACKET,"Success reading public and private key");
-    /* ok, we are sure both the priv8 and public key files are readable : we return the public one as a string,
-        and the private filename in arguments */
-    pubkey=publickey_from_file(session,public,type);
-    if(!pubkey){
-        ssh_log(session,SSH_LOG_PACKET,"Wasn't able to open public key file %s : %s",public,ssh_get_error(session));
-        return publickey_from_next_file(session,pub_keys_path,keys_path,privkeyfile,type,count);
+
+  ssh_log(session, SSH_LOG_PACKET, "Success reading public and private key");
+
+  /*
+   * We are sure both the private and public key file is readable. We return
+   * the public as a string, and the private filename as an argument
+   */
+  pubkey = publickey_from_file(session, public, type);
+  if (pubkey == NULL) {
+    ssh_log(session, SSH_LOG_PACKET,
+        "Wasn't able to open public key file %s: %s",
+        public,
+        ssh_get_error(session));
+    return publickey_from_next_file(session, keytab, keytab_size,
+        privkeyfile, type, count);
   }
-    *privkeyfile=realloc(*privkeyfile,strlen(private)+1);
-    strcpy(*privkeyfile,private);
+
+  new = realloc(*privkeyfile, strlen(private) + 1);
+  if (new == NULL) {
+    string_free(pubkey);
+    return NULL;
+  }
+
+  strcpy(new, private);
+  *privkeyfile = new;
+
   return pubkey;
 }