[openssl-commits] [openssl] OpenSSL_1_0_2-stable update

Andy Polyakov appro at openssl.org
Thu Jun 1 20:24:34 UTC 2017


The branch OpenSSL_1_0_2-stable has been updated
       via  9a2a0617e5b042ae5d5b53886e30dc47fe778f7f (commit)
      from  44191de234b061145a6ed14221927ec0c9c7b7bf (commit)


- Log -----------------------------------------------------------------
commit 9a2a0617e5b042ae5d5b53886e30dc47fe778f7f
Author: Diego Santa Cruz <Diego.SantaCruz at spinetix.com>
Date:   Tue May 16 16:05:19 2017 +0200

    Fix srp app missing NULL termination with password callback
    
    The password_callback() function does not necessarily NULL terminate
    the password buffer, the caller must use the returned length but the
    srp app uses this function as if it was doing NULL termination.
    
    This made the -passin and -passout options of "openssl srp"
    fail inexpicably and randomly or even crash.
    
    Fixed by enlarging the buffer by one, so that the maximum password length
    remains unchanged, and adding NULL termination upon return.
    
    [Rearrange code for coding style compliance in process.]
    
    This backport of 0e83981d61fc435f42d4bb4d774272b69556b7bc.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    Reviewed-by: Andy Polyakov <appro at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/3579)

-----------------------------------------------------------------------

Summary of changes:
 apps/srp.c | 66 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/apps/srp.c b/apps/srp.c
index ce01a24..491445d 100644
--- a/apps/srp.c
+++ b/apps/srp.c
@@ -123,13 +123,14 @@ static int get_index(CA_DB *db, char *id, char type)
     int i;
     if (id == NULL)
         return -1;
-    if (type == DB_SRP_INDEX)
+    if (type == DB_SRP_INDEX) {
         for (i = 0; i < sk_OPENSSL_PSTRING_num(db->db->data); i++) {
             pp = sk_OPENSSL_PSTRING_value(db->db->data, i);
             if (pp[DB_srptype][0] == DB_SRP_INDEX
                 && !strcmp(id, pp[DB_srpid]))
                 return i;
-    } else
+        }
+    } else {
         for (i = 0; i < sk_OPENSSL_PSTRING_num(db->db->data); i++) {
             pp = sk_OPENSSL_PSTRING_value(db->db->data, i);
 
@@ -137,6 +138,7 @@ static int get_index(CA_DB *db, char *id, char type)
                 && !strcmp(id, pp[DB_srpid]))
                 return i;
         }
+    }
 
     return -1;
 }
@@ -177,8 +179,8 @@ static int update_index(CA_DB *db, BIO *bio, char **row)
     char **irow;
     int i;
 
-    if ((irow =
-         (char **)OPENSSL_malloc(sizeof(char *) * (DB_NUMBER + 1))) == NULL) {
+    irow = (char **)OPENSSL_malloc(sizeof(char *) * (DB_NUMBER + 1));
+    if (irow == NULL) {
         BIO_printf(bio_err, "Memory allocation failure\n");
         return 0;
     }
@@ -205,30 +207,32 @@ static char *srp_verify_user(const char *user, const char *srp_verifier,
                              char *srp_usersalt, const char *g, const char *N,
                              const char *passin, BIO *bio, int verbose)
 {
-    char password[1024];
+    char password[1025];
     PW_CB_DATA cb_tmp;
     char *verifier = NULL;
     char *gNid = NULL;
+    int len;
 
     cb_tmp.prompt_info = user;
     cb_tmp.password = passin;
 
-    if (password_callback(password, 1024, 0, &cb_tmp) > 0) {
+    len = password_callback(password, sizeof(password)-1, 0, &cb_tmp);
+    if (len > 0) {
+        password[len] = 0;
         VERBOSE BIO_printf(bio,
                            "Validating\n   user=\"%s\"\n srp_verifier=\"%s\"\n srp_usersalt=\"%s\"\n g=\"%s\"\n N=\"%s\"\n",
                            user, srp_verifier, srp_usersalt, g, N);
-        BIO_printf(bio, "Pass %s\n", password);
+        VVERBOSE BIO_printf(bio, "Pass %s\n", password);
 
-        if (!
-            (gNid =
-             SRP_create_verifier(user, password, &srp_usersalt, &verifier, N,
-                                 g))) {
+        if (!(gNid = SRP_create_verifier(user, password, &srp_usersalt,
+                                         &verifier, N, g))) {
             BIO_printf(bio, "Internal error validating SRP verifier\n");
         } else {
             if (strcmp(verifier, srp_verifier))
                 gNid = NULL;
             OPENSSL_free(verifier);
         }
+        OPENSSL_cleanse(password, len);
     }
     return gNid;
 }
@@ -237,24 +241,27 @@ static char *srp_create_user(char *user, char **srp_verifier,
                              char **srp_usersalt, char *g, char *N,
                              char *passout, BIO *bio, int verbose)
 {
-    char password[1024];
+    char password[1025];
     PW_CB_DATA cb_tmp;
     char *gNid = NULL;
     char *salt = NULL;
+    int len;
     cb_tmp.prompt_info = user;
     cb_tmp.password = passout;
 
-    if (password_callback(password, 1024, 1, &cb_tmp) > 0) {
+    len = password_callback(password, sizeof(password)-1, 1, &cb_tmp);
+    if (len > 0) {
+        password[len] = 0;
         VERBOSE BIO_printf(bio,
                            "Creating\n user=\"%s\"\n g=\"%s\"\n N=\"%s\"\n",
                            user, g, N);
-        if (!
-            (gNid =
-             SRP_create_verifier(user, password, &salt, srp_verifier, N,
-                                 g))) {
+        if (!(gNid = SRP_create_verifier(user, password, &salt,
+                                         srp_verifier, N, g))) {
             BIO_printf(bio, "Internal error creating SRP verifier\n");
-        } else
+        } else {
             *srp_usersalt = salt;
+        }
+        OPENSSL_cleanse(password, len);
         VVERBOSE BIO_printf(bio, "gNid=%s salt =\"%s\"\n verifier =\"%s\"\n",
                             gNid, salt, *srp_verifier);
 
@@ -314,9 +321,9 @@ int MAIN(int argc, char **argv)
     argc--;
     argv++;
     while (argc >= 1 && badops == 0) {
-        if (strcmp(*argv, "-verbose") == 0)
+        if (strcmp(*argv, "-verbose") == 0) {
             verbose++;
-        else if (strcmp(*argv, "-config") == 0) {
+        } else if (strcmp(*argv, "-config") == 0) {
             if (--argc < 1)
                 goto bad;
             configfile = *(++argv);
@@ -328,15 +335,15 @@ int MAIN(int argc, char **argv)
             if (--argc < 1)
                 goto bad;
             dbfile = *(++argv);
-        } else if (strcmp(*argv, "-add") == 0)
+        } else if (strcmp(*argv, "-add") == 0) {
             add_user = 1;
-        else if (strcmp(*argv, "-delete") == 0)
+        } else if (strcmp(*argv, "-delete") == 0) {
             delete_user = 1;
-        else if (strcmp(*argv, "-modify") == 0)
+        } else if (strcmp(*argv, "-modify") == 0) {
             modify_user = 1;
-        else if (strcmp(*argv, "-list") == 0)
+        } else if (strcmp(*argv, "-list") == 0) {
             list_user = 1;
-        else if (strcmp(*argv, "-gn") == 0) {
+        } else if (strcmp(*argv, "-gn") == 0) {
             if (--argc < 1)
                 goto bad;
             gN = *(++argv);
@@ -366,8 +373,9 @@ int MAIN(int argc, char **argv)
             BIO_printf(bio_err, "unknown option %s\n", *argv);
             badops = 1;
             break;
-        } else
+        } else {
             break;
+        }
 
         argc--;
         argv++;
@@ -388,7 +396,7 @@ int MAIN(int argc, char **argv)
                    "Need at least one user for options -add, -delete, -modify. \n");
         badops = 1;
     }
-    if ((passin || passout) && argc != 1) {
+    if ((passargin || passargout) && argc != 1) {
         BIO_printf(bio_err,
                    "-passin, -passout arguments only valid with one user.\n");
         badops = 1;
@@ -706,9 +714,9 @@ int MAIN(int argc, char **argv)
                 doupdatedb = 1;
             }
         }
-        if (--argc > 0)
+        if (--argc > 0) {
             user = *(argv++);
-        else {
+        } else {
             user = NULL;
             list_user = 0;
         }


More information about the openssl-commits mailing list