Fixes and memory leak elimination in SRP authentication.

4 files changed, 81 insertions, 40 deletions

diff --git a/NEWS b/NEWS
index a11f02b459..c7fe581d48 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,8 @@ Version 2.12.17 (unreleased)
 ** libgnutls: Corrections in record packet parsing.
 Reported by Matthew Hall.
 
+** libgnutls: Fixes in SRP authentication.
+
 ** libgnutls: Added function to force explicit reinitialization
 of PKCS #11 modules. This is required on the child process after
 a fork.
diff --git a/lib/auth_srp_passwd.c b/lib/auth_srp_passwd.c
index 370f845243..cd6c7fdbcd 100644
--- a/lib/auth_srp_passwd.c
+++ b/lib/auth_srp_passwd.c
@@ -209,6 +209,7 @@ pwd_read_conf (const char *pconf_file, SRP_PWD_ENTRY * entry, int idx)
   char line[2 * 1024];
   unsigned i, len;
   char indexstr[10];
+  int ret;
 
   snprintf (indexstr, sizeof(indexstr), "%u", (unsigned int)idx);
 
@@ -231,14 +232,22 @@ pwd_read_conf (const char *pconf_file, SRP_PWD_ENTRY * entry, int idx)
       if (strncmp (indexstr, line, MAX (i, len)) == 0)
         {
           if ((idx = pwd_put_values2 (entry, line)) >= 0)
-            return 0;
+            {
+              ret = 0;
+              goto cleanup;
+            }
           else
             {
-              return GNUTLS_E_SRP_PWD_ERROR;
+              ret = GNUTLS_E_SRP_PWD_ERROR;
+              goto cleanup;
             }
         }
     }
-  return GNUTLS_E_SRP_PWD_ERROR;
+  ret = GNUTLS_E_SRP_PWD_ERROR;
+  
+cleanup:
+  fclose(fd);
+  return ret;
 
 }
 
@@ -247,12 +256,12 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
                             SRP_PWD_ENTRY ** _entry)
 {
   gnutls_srp_server_credentials_t cred;
-  FILE *fd;
+  FILE *fd = NULL;
   char line[2 * 1024];
   unsigned i, len;
   int ret;
   int idx, last_idx;
-  SRP_PWD_ENTRY *entry;
+  SRP_PWD_ENTRY *entry = NULL;
 
   *_entry = gnutls_calloc (1, sizeof (SRP_PWD_ENTRY));
   if (*_entry == NULL)
@@ -267,8 +276,8 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
   if (cred == NULL)
     {
       gnutls_assert ();
-      _gnutls_srp_entry_free (entry);
-      return GNUTLS_E_INSUFFICIENT_CREDENTIALS;
+      ret = GNUTLS_E_INSUFFICIENT_CREDENTIALS;
+      goto cleanup;
     }
 
   /* if the callback which sends the parameters is
@@ -287,8 +296,7 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
               if (ret < 0)
                 {
                   gnutls_assert ();
-                  _gnutls_srp_entry_free (entry);
-                  return ret;
+                  goto cleanup;
                 }
               return 0;
             }
@@ -302,8 +310,8 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
       if (ret < 0)
         {
           gnutls_assert ();
-          _gnutls_srp_entry_free (entry);
-          return GNUTLS_E_SRP_PWD_ERROR;
+          ret = GNUTLS_E_SRP_PWD_ERROR;
+          goto cleanup;
         }
 
       return 0;
@@ -315,7 +323,8 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
   if (cred->password_file == NULL)
     {
       gnutls_assert ();
-      return GNUTLS_E_SRP_PWD_ERROR;
+      ret = GNUTLS_E_SRP_PWD_ERROR;
+      goto cleanup;
     }
 
   /* Open the selected password file.
@@ -324,8 +333,8 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
   if (fd == NULL)
     {
       gnutls_assert ();
-      _gnutls_srp_entry_free (entry);
-      return GNUTLS_E_SRP_PWD_ERROR;
+      ret = GNUTLS_E_SRP_PWD_ERROR;
+      goto cleanup;
     }
 
   last_idx = 1;                 /* a default value */
@@ -351,20 +360,20 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
               last_idx = idx;
               if (pwd_read_conf (cred->password_conf_file, entry, idx) == 0)
                 {
-                  return 0;
+                  goto found;
                 }
               else
                 {
                   gnutls_assert ();
-                  _gnutls_srp_entry_free (entry);
-                  return GNUTLS_E_SRP_PWD_ERROR;
+                  ret = GNUTLS_E_SRP_PWD_ERROR;
+                  goto cleanup;
                 }
             }
           else
             {
               gnutls_assert ();
-              _gnutls_srp_entry_free (entry);
-              return GNUTLS_E_SRP_PWD_ERROR;
+              ret = GNUTLS_E_SRP_PWD_ERROR;
+              goto cleanup;
             }
         }
     }
@@ -378,17 +387,21 @@ _gnutls_srp_pwd_read_entry (gnutls_session_t state, char *username,
       if (ret < 0)
         {
           gnutls_assert ();
-          _gnutls_srp_entry_free (entry);
-          return ret;
+          goto cleanup;
         }
 
-      return 0;
+      goto found;
     }
 
+cleanup:
   gnutls_assert ();
+  if (fd) fclose(fd);
   _gnutls_srp_entry_free (entry);
   return GNUTLS_E_SRP_PWD_ERROR;
 
+found:
+  if (fd) fclose(fd);
+  return 0;
 }
 
 /* Randomizes the given password entry. It actually sets the verifier
diff --git a/lib/ext_srp.c b/lib/ext_srp.c
index e77be77bc0..2d39ba98f7 100644
--- a/lib/ext_srp.c
+++ b/lib/ext_srp.c
@@ -111,7 +111,9 @@ _gnutls_srp_send_params (gnutls_session_t session, opaque * data,
 {
   unsigned len;
   extension_priv_data_t epriv;
-  srp_ext_st *priv;
+  srp_ext_st *priv = NULL;
+  char *username = NULL, *password = NULL;
+  int ret;
 
   if (_gnutls_kx_priority (session, GNUTLS_KX_SRP) < 0 &&
       _gnutls_kx_priority (session, GNUTLS_KX_SRP_DSS) < 0 &&
@@ -131,6 +133,13 @@ _gnutls_srp_send_params (gnutls_session_t session, opaque * data,
       if (cred == NULL)
         return 0;
 
+      priv = gnutls_malloc (sizeof (*priv));
+      if (priv == NULL)
+        {
+          gnutls_assert ();
+          return GNUTLS_E_MEMORY_ERROR;
+        }
+
       if (cred->username != NULL)
         {                       /* send username */
           len = MIN (strlen (cred->username), 255);
@@ -141,6 +150,23 @@ _gnutls_srp_send_params (gnutls_session_t session, opaque * data,
               return GNUTLS_E_SHORT_MEMORY_BUFFER;
             }
 
+          priv->username = strdup(cred->username);
+          if (priv->username == NULL)
+            {
+              gnutls_assert();
+              goto cleanup;
+            }
+
+          priv->password = strdup(cred->password);
+          if (priv->password == NULL)
+            {
+              gnutls_assert();
+              goto cleanup;
+            }
+
+          epriv.ptr = priv;
+          _gnutls_ext_set_session_data (session, GNUTLS_EXTENSION_SRP, epriv);
+
           data[0] = (uint8_t) len;
           memcpy (&data[1], cred->username, len);
           return len + 1;
@@ -149,7 +175,6 @@ _gnutls_srp_send_params (gnutls_session_t session, opaque * data,
         {
           /* Try the callback
            */
-          char *username = NULL, *password = NULL;
 
           if (cred->get_function (session, &username, &password) < 0
               || username == NULL || password == NULL)
@@ -162,17 +187,9 @@ _gnutls_srp_send_params (gnutls_session_t session, opaque * data,
 
           if (data_size < len + 1)
             {
-              gnutls_free (username);
-              gnutls_free (password);
               gnutls_assert ();
-              return GNUTLS_E_SHORT_MEMORY_BUFFER;
-            }
-
-          priv = gnutls_malloc (sizeof (*priv));
-          if (priv == NULL)
-            {
-              gnutls_assert ();
-              return GNUTLS_E_MEMORY_ERROR;
+              ret = GNUTLS_E_SHORT_MEMORY_BUFFER;
+              goto cleanup;
             }
 
           priv->username = username;
@@ -187,6 +204,13 @@ _gnutls_srp_send_params (gnutls_session_t session, opaque * data,
         }
     }
   return 0;
+
+cleanup:
+  gnutls_free(username);
+  gnutls_free(password);
+  gnutls_free(priv);
+  
+  return ret;
 }
 
 static void
diff --git a/lib/gnutls_errors.c b/lib/gnutls_errors.c
index 37e5b1e4e4..7dd649916c 100644
--- a/lib/gnutls_errors.c
+++ b/lib/gnutls_errors.c
@@ -491,11 +491,13 @@ _gnutls_mpi_log (const char *prefix, bigint_t a)
   char *hexbuf;
   int res;
 
+  if (_gnutls_log_level < 2) return;
+
   res = _gnutls_mpi_print (a, NULL, &binlen);
-  if (res != 0)
+  if (res < 0 && res != GNUTLS_E_SHORT_MEMORY_BUFFER)
     {
       gnutls_assert ();
-      _gnutls_hard_log ("MPI: can't print value (%d/%d)\n", res,
+      _gnutls_hard_log ("MPI: %s can't print value (%d/%d)\n", prefix, res,
                         (int) binlen);
       return;
     }
@@ -503,7 +505,7 @@ _gnutls_mpi_log (const char *prefix, bigint_t a)
   if (binlen > 1024 * 1024)
     {
       gnutls_assert ();
-      _gnutls_hard_log ("MPI: too large mpi (%d)\n", (int) binlen);
+      _gnutls_hard_log ("MPI: %s too large mpi (%d)\n", prefix, (int) binlen);
       return;
     }
 
@@ -511,7 +513,7 @@ _gnutls_mpi_log (const char *prefix, bigint_t a)
   if (!binbuf)
     {
       gnutls_assert ();
-      _gnutls_hard_log ("MPI: out of memory (%d)\n", (int) binlen);
+      _gnutls_hard_log ("MPI: %s out of memory (%d)\n", prefix, (int) binlen);
       return;
     }
 
@@ -519,7 +521,7 @@ _gnutls_mpi_log (const char *prefix, bigint_t a)
   if (res != 0)
     {
       gnutls_assert ();
-      _gnutls_hard_log ("MPI: can't print value (%d/%d)\n", res,
+      _gnutls_hard_log ("MPI: %s can't print value (%d/%d)\n", prefix, res,
                         (int) binlen);
       gnutls_free (binbuf);
       return;
@@ -531,7 +533,7 @@ _gnutls_mpi_log (const char *prefix, bigint_t a)
   if (!hexbuf)
     {
       gnutls_assert ();
-      _gnutls_hard_log ("MPI: out of memory (hex %d)\n", (int) hexlen);
+      _gnutls_hard_log ("MPI: %s out of memory (hex %d)\n", prefix, (int) hexlen);
       gnutls_free (binbuf);
       return;
     }