Relevant BUGIDs: 440107

Purpose of commit: bugfix/cleanup

Commit summary:
---------------
Removed superfluous use of static variables in md5 and bigcrypt routines,
bringing us a step closer to thread-safeness.  Eliminated some variable
indirection along the way.

diff --git a/CHANGELOG b/CHANGELOG
index 9e1bd0b91102d00c14e8b648d02021df0982ba83..091a7387be4735aea7a019615bddbf33933586ed 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -49,6 +49,9 @@ bug report - outstanding bugs are listed here:
 0.76: please submit patches for this section with actual code/doc
       patches!
 
+* pam_unix: removed superfluous use of static variables in md5 and bigcrypt
+  routines, bringing us a step closer to thread-safeness.  Eliminated
+  some variable indirection along the way.  (Bug 440107 - vorlon)
 * pam_tally: remove #include of stdlib.h, which isn't needed by anything
   found in this module.  Can be readded if we find a real need for it at
   a later date. (Bug 436432 - vorlon)

diff --git a/modules/pam_unix/bigcrypt.c b/modules/pam_unix/bigcrypt.c
index b1568d6b340a69874bcc447db21bbb2dc311d9fd..6b73f3d254e9455059148f8b900d115d0dda5e67 100644 (file)
--- a/modules/pam_unix/bigcrypt.c
+++ b/modules/pam_unix/bigcrypt.c
@@ -25,6 +25,7 @@
  */
 
 #include <string.h>
+#include <stdlib.h>
 #include <security/_pam_macros.h>
 
 char *crypt(const char *key, const char *salt);
@@ -45,7 +46,7 @@ char *bigcrypt(const char *key, const char *salt);
 
 char *bigcrypt(const char *key, const char *salt)
 {
-       static char dec_c2_cryptbuf[CBUF_SIZE];         /* static storage area */
+       char *dec_c2_cryptbuf;
 
        unsigned long int keylen, n_seg, j;
        char *cipher_ptr, *plaintext_ptr, *tmp_ptr, *salt_ptr;
@@ -54,6 +55,10 @@ char *bigcrypt(const char *key, const char *salt)
        D(("called with key='%s', salt='%s'.", key, salt));
 
        /* reset arrays */
+       dec_c2_cryptbuf = malloc(CBUF_SIZE);
+       if (!dec_c2_cryptbuf) {
+               return NULL;
+       }
        memset(keybuf, 0, KEYBUF_SIZE + 1);
        memset(dec_c2_cryptbuf, 0, CBUF_SIZE);
 

diff --git a/modules/pam_unix/md5_crypt.c b/modules/pam_unix/md5_crypt.c
index a7243a2e1e84220a2e4afac850cdc6c4dcd8938a..53972fccefcbaf80f4fcc92c321a16c04b8679d0 100644 (file)
--- a/modules/pam_unix/md5_crypt.c
+++ b/modules/pam_unix/md5_crypt.c
@@ -13,6 +13,7 @@
  */
 
 #include <string.h>
+#include <stdlib.h>
 #include "md5.h"
 
 static unsigned char itoa64[] =        /* 0 ... 63 => ascii - 64 */
@@ -37,8 +38,8 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt)
        const char *magic = "$1$";
        /* This string is magic for this algorithm.  Having
         * it this way, we can get get better later on */
-       static char passwd[120], *p;
-       static const char *sp, *ep;
+       char *passwd, *p;
+       const char *sp, *ep;
        unsigned char final[16];
        int sl, pl, i, j;
        MD5_CTX ctx, ctx1;
@@ -47,6 +48,10 @@ char *MD5Name(crypt_md5)(const char *pw, const char *salt)
        /* Refine the Salt first */
        sp = salt;
 
+       /* TODO: now that we're using malloc'ed memory, get rid of the
+          strange constant buffer size. */
+       passwd = malloc(120);
+
        /* If it starts with the magic string, then skip that */
        if (!strncmp(sp, magic, strlen(magic)))
                sp += strlen(magic);

diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c
index c85305e424fce57f73c64cf4a2c866642b3ea997..85c0a44d0d6be510bb812b2ea2c38196c1df9a88 100644 (file)
--- a/modules/pam_unix/pam_unix_passwd.c
+++ b/modules/pam_unix/pam_unix_passwd.c
@@ -153,7 +153,7 @@ static char *crypt_md5_wrapper(const char *pass_new)
        char *cp = (char *) result;
        unsigned char tmp[16];
        int i;
-       char *x, *e = NULL;
+       char *x = NULL;
 
        GoodMD5Init(&ctx);
        gettimeofday(&tv, (struct timezone *) 0);
@@ -171,9 +171,7 @@ static char *crypt_md5_wrapper(const char *pass_new)
        *cp = '\0';
 
        /* no longer need cleartext */
-       e = Goodcrypt_md5(pass_new, (const char *) result);
-       x = x_strdup(e);        /* put e in malloc()ed memory */
-       _pam_overwrite(e);      /* clean up */
+       x = Goodcrypt_md5(pass_new, (const char *) result);
 
        return x;
 }
@@ -227,11 +225,14 @@ static int check_old_password(const char *forwho, const char *newpass)
                        s_npas = strtok(NULL, ":,");
                        s_pas = strtok(NULL, ":,");
                        while (s_pas != NULL) {
-                               if (!strcmp(Goodcrypt_md5(newpass, s_pas), s_pas)) {
+                               char *md5pass = Goodcrypt_md5(newpass, s_pas);
+                               if (!strcmp(md5pass, s_pas)) {
+                                       _pam_delete(md5pass);
                                        retval = PAM_AUTHTOK_ERR;
                                        break;
                                }
                                s_pas = strtok(NULL, ":,");
+                               _pam_delete(md5pass);
                        }
                        break;
                }
@@ -287,6 +288,7 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman
                                sprintf(nbuf, "%s:%s:%d:%s\n", s_luser, s_uid, npas, pass);
                        else
                                sprintf(nbuf, "%s:%s:%d:%s,%s\n", s_luser, s_uid, npas, s_pas, pass);
+                       _pam_delete(pass);
                        if (fputs(nbuf, pwfile) < 0) {
                                retval = PAM_AUTHTOK_ERR;
                                err = 1;
@@ -308,6 +310,7 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman
                } else {
                        pass = crypt_md5_wrapper(oldpass);
                        sprintf(nbuf, "%s:%d:1:%s\n", forwho, pwd->pw_uid, pass);
+                       _pam_delete(pass);
                        if (fputs(nbuf, pwfile) < 0) {
                                retval = PAM_AUTHTOK_ERR;
                                err = 1;
@@ -928,7 +931,6 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
                                 * function we truncate the newly entered password
                                 */
                                char *temp = malloc(9);
-                               char *e;
 
                                if (temp == NULL) {
                                        _log_err(LOG_CRIT, pamh,
@@ -944,19 +946,11 @@ PAM_EXTERN int pam_sm_chauthtok(pam_handle_t * pamh, int flags,
                                temp[8] = '\0';
 
                                /* no longer need cleartext */
-                               e = bigcrypt(temp, salt);
-                               tpass = x_strdup(e);
+                               tpass = bigcrypt(temp, salt);
 
-                               _pam_overwrite(e);
                                _pam_delete(temp);      /* tidy up */
                        } else {
-                               char *e;
-
-                               /* no longer need cleartext */
-                               e = bigcrypt(pass_new, salt);
-                               tpass = x_strdup(e);
-
-                               _pam_overwrite(e);
+                               tpass = bigcrypt(pass_new, salt);
                        }
                }
 

diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c
index 690714083307beecfb19c13d5f87cf123640d189..964d1a46267ae1873c73297232a395f176d8530b 100644 (file)
--- a/modules/pam_unix/support.c
+++ b/modules/pam_unix/support.c
@@ -570,6 +570,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name
                        if (!strncmp(salt, "$1$", 3)) {
                                pp = Goodcrypt_md5(p, salt);
                                if (strcmp(pp, salt) != 0) {
+                                       _pam_delete(pp);
                                        pp = Brokencrypt_md5(p, salt);
                                }
                        } else {
@@ -661,7 +662,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name
        if (salt)
                _pam_delete(salt);
        if (pp)
-               _pam_overwrite(pp);
+               _pam_delete(pp);
 
        D(("done [%d].", retval));
 
@@ -682,7 +683,6 @@ int _unix_read_password(pam_handle_t * pamh
 {
        int authtok_flag;
        int retval;
-       const char *item;
        char *token;
 
        D(("called"));
@@ -704,16 +704,14 @@ int _unix_read_password(pam_handle_t * pamh
         */
 
        if (on(UNIX_TRY_FIRST_PASS, ctrl) || on(UNIX_USE_FIRST_PASS, ctrl)) {
-               retval = pam_get_item(pamh, authtok_flag, (const void **) &item);
+               retval = pam_get_item(pamh, authtok_flag, (const void **) pass);
                if (retval != PAM_SUCCESS) {
                        /* very strange. */
                        _log_err(LOG_ALERT, pamh
                                 ,"pam_get_item returned error to unix-read-password"
                            );
                        return retval;
-               } else if (item != NULL) {      /* we have a password! */
-                       *pass = item;
-                       item = NULL;
+               } else if (*pass != NULL) {     /* we have a password! */
                        return PAM_SUCCESS;
                } else if (on(UNIX_USE_FIRST_PASS, ctrl)) {
                        return PAM_AUTHTOK_RECOVER_ERR;         /* didn't work */
@@ -812,9 +810,10 @@ int _unix_read_password(pam_handle_t * pamh
                _pam_delete(token);     /* clean it up */
                if (retval != PAM_SUCCESS
                    || (retval = pam_get_item(pamh, authtok_flag
-                                             ,(const void **) &item))
+                                             ,(const void **) pass))
                    != PAM_SUCCESS) {
 
+                       *pass = NULL;
                        _log_err(LOG_CRIT, pamh, "error manipulating password");
                        return retval;
 
@@ -833,13 +832,10 @@ int _unix_read_password(pam_handle_t * pamh
                        _pam_delete(token);
                        return retval;
                }
-               item = token;
+               *pass = token;
                token = NULL;   /* break link to password */
        }
 
-       *pass = item;
-       item = NULL;            /* break link to password */
-
        return PAM_SUCCESS;
 }
 

diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c
index 5b9ed43efb69e48fc7939fa3a1bacb5895cf41d3..b0509e47355ab75a3b63c02a975cf6ea0da26a4b 100644 (file)
--- a/modules/pam_unix/unix_chkpwd.c
+++ b/modules/pam_unix/unix_chkpwd.c
@@ -155,6 +155,7 @@ static int _unix_verify_password(const char *name, const char *p, int opt)
                if (pp != NULL) {
                        while (tp && *tp)
                                *tp++ = '\0';
+                       free(pp);
                }
                pp = tp = NULL;
        }