diff --git a/ChangeLog b/ChangeLog index f5a86486..fc063179 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2013-08-03 Nicolas François + + * Changelog: Update documentation of 2013-07-28 mancha entry. + * lib/prototypes.h, lib/encrypt.c: Update splint marker, + pw_encrypt can return NULL. + * lib/encrypt.c: Fix outdated statement on GNU crypt. + * src/chgpasswd.c: Improve diagnostic to user when pw_encrypt + fails and use fail_exit() instead of exit(). + * src/chpasswd.c: Likewise. + * src/newusers.c: Likewise. + * src/passwd.c: Likewise when new password is encrypted. + * src/newgrp.c: Improve diagnostic to user and syslog when + pw_encrypt fails. Do not apply 1s penalty as this is not an + invalid password issue. + * src/passwd.c: Likewise when password is checked. + 2013-08-02 Nicolas François * libmisc/setupenv.c: xstrdup the static char* temp_pw_dir / @@ -52,19 +68,24 @@ 2013-07-28 mancha - * lib/encrypt.c: crypt() in glibc/eglibc 2.17 now fails if passed - a salt that violates specs. On Linux, crypt() also fails with - DES/MD5 salts in FIPS140 mode. Rather than exit() on NULL returns - we send them back to the caller for appropriate handling. - Closes: alioth#314234 - * lib/pwauth.c: Handle NULL return from crypt(). - * libmisc/valid.c: Likewise. - * src/chgpasswd.c: Likewise. - * src/chpasswd.c: Likewise. - * src/gpasswd.c: Likewise. - * src/newgrp.c: Likewise. - * src/newusers.c: Likewise. - * src/passwd.c: Likewise. + * lib/encrypt.c (pw_encrypt): crypt() in glibc/eglibc 2.17 now + fails if passed a salt that violates specs. On Linux, crypt() also + fails with DES/MD5 salts in FIPS140 mode. Rather than exit() on + NULL returns we send them back to the caller for appropriate + handling (instead of exiting). Closes: alioth#314234 + * lib/pwauth.c: Handle NULL return from pw_crypt(), return non + zero (as in case of failure). + * libmisc/valid.c: Likewise. + * src/chgpasswd.c: Handle NULL return from pw_crypt(), report + crypt error to stderr and exit. + * src/chpasswd.c: Likewise. + * src/gpasswd.c: Likewise. + * src/newusers.c: Likewise. + * src/passwd.c: Likewise when new password is encrypted. + * src/newgrp.c: Handle NULL return from pw_crypt(), report crypt + error to stderr and syslog and return to report unchanged + password. + * src/passwd.c: Likewise when password is checked. 2013-07-28 Christian Perrier diff --git a/lib/encrypt.c b/lib/encrypt.c index 7ff8b506..53c99c94 100644 --- a/lib/encrypt.c +++ b/lib/encrypt.c @@ -40,22 +40,22 @@ #include "prototypes.h" #include "defines.h" -/*@exposed@*/char *pw_encrypt (const char *clear, const char *salt) +/*@exposed@*//*@null@*/char *pw_encrypt (const char *clear, const char *salt) { static char cipher[128]; char *cp; cp = crypt (clear, salt); - if (!cp) { + if (NULL == cp) { /* * Single Unix Spec: crypt() may return a null pointer, * and set errno to indicate an error. In this case return * the NULL so the caller can handle appropriately. */ - return cp; + return NULL; } - /* The GNU crypt does not return NULL if the algorithm is not + /* Some crypt() do not return NULL if the algorithm is not * supported, and return a DES encrypted password. */ if ((NULL != salt) && (salt[0] == '$') && (strlen (cp) <= 13)) { diff --git a/lib/prototypes.h b/lib/prototypes.h index eb26d4d4..737e40cd 100644 --- a/lib/prototypes.h +++ b/lib/prototypes.h @@ -124,7 +124,7 @@ extern int copy_tree (const char *src_root, const char *dst_root, gid_t old_gid, gid_t new_gid); /* encrypt.c */ -extern /*@exposed@*/char *pw_encrypt (const char *, const char *); +extern /*@exposed@*//*@null@*/char *pw_encrypt (const char *, const char *); /* entry.c */ extern void pw_entry (const char *, struct passwd *); diff --git a/lib/pwauth.c b/lib/pwauth.c index 086a72e2..9e24fbf2 100644 --- a/lib/pwauth.c +++ b/lib/pwauth.c @@ -179,10 +179,11 @@ int pw_auth (const char *cipher, */ encrypted = pw_encrypt (input, cipher); - if (encrypted!=NULL) + if (NULL != encrypted) { retval = strcmp (encrypted, cipher); - else + } else { retval = -1; + } #ifdef SKEY /* diff --git a/src/chgpasswd.c b/src/chgpasswd.c index 6c42a096..4dd5fbab 100644 --- a/src/chgpasswd.c +++ b/src/chgpasswd.c @@ -459,6 +459,7 @@ int main (int argc, char **argv) && ( (NULL == crypt_method) || (0 != strcmp (crypt_method, "NONE")))) { void *arg = NULL; + const char *salt; if (md5flg) { crypt_method = "MD5"; } @@ -467,12 +468,14 @@ int main (int argc, char **argv) arg = &sha_rounds; } #endif - cp = pw_encrypt (newpwd, - crypt_make_salt (crypt_method, arg)); - if (cp == NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); - } + salt = crypt_make_salt (crypt_method, arg); + cp = pw_encrypt (newpwd, salt); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + fail_exit (1); + } } /* diff --git a/src/chpasswd.c b/src/chpasswd.c index 4968b0d2..78436d6a 100644 --- a/src/chpasswd.c +++ b/src/chpasswd.c @@ -482,6 +482,7 @@ int main (int argc, char **argv) && ( (NULL == crypt_method) || (0 != strcmp (crypt_method, "NONE")))) { void *arg = NULL; + const char *salt; if (md5flg) { crypt_method = "MD5"; } @@ -490,11 +491,13 @@ int main (int argc, char **argv) arg = &sha_rounds; } #endif - cp = pw_encrypt (newpwd, - crypt_make_salt(crypt_method, arg)); - if (cp == NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); + salt = crypt_make_salt (crypt_method, arg); + cp = pw_encrypt (newpwd, salt); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + fail_exit (1); } } diff --git a/src/gpasswd.c b/src/gpasswd.c index 00436106..8959a35a 100644 --- a/src/gpasswd.c +++ b/src/gpasswd.c @@ -898,6 +898,7 @@ static void change_passwd (struct group *gr) char *cp; static char pass[BUFSIZ]; int retries; + const char *salt; /* * A new password is to be entered and it must be encrypted, etc. @@ -938,10 +939,13 @@ static void change_passwd (struct group *gr) exit (1); } - cp = pw_encrypt (pass, crypt_make_salt (NULL, NULL)); - if (cp==NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); + salt = crypt_make_salt (NULL, NULL); + cp = pw_encrypt (pass, salt); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + exit (1); } memzero (pass, sizeof pass); #ifdef SHADOWGRP diff --git a/src/newgrp.c b/src/newgrp.c index 6b877618..49dd1512 100644 --- a/src/newgrp.c +++ b/src/newgrp.c @@ -184,8 +184,17 @@ static void check_perms (const struct group *grp, cpasswd = pw_encrypt (cp, grp->gr_passwd); strzero (cp); - if (cpasswd == NULL || - grp->gr_passwd[0] == '\0' || + if (NULL == cpasswd) { + fprintf (stderr, + _("%s: failed to crypt password with previous salt: %s\n"), + Prog, strerror (errno)); + SYSLOG ((LOG_INFO, + "Failed to crypt password with previous salt of group '%s'", + groupname)); + goto failure; + } + + if (grp->gr_passwd[0] == '\0' || strcmp (cpasswd, grp->gr_passwd) != 0) { #ifdef WITH_AUDIT snprintf (audit_buf, sizeof(audit_buf), diff --git a/src/newusers.c b/src/newusers.c index 5f83a6a5..4117a45f 100644 --- a/src/newusers.c +++ b/src/newusers.c @@ -98,7 +98,7 @@ static int add_group (const char *, const char *, gid_t *, gid_t); static int get_user_id (const char *, uid_t *); static int add_user (const char *, uid_t, gid_t); #ifndef USE_PAM -static void update_passwd (struct passwd *, const char *); +static int update_passwd (struct passwd *, const char *); #endif /* !USE_PAM */ static int add_passwd (struct passwd *, const char *); static void process_flags (int argc, char **argv); @@ -384,7 +384,12 @@ static int add_user (const char *name, uid_t uid, gid_t gid) } #ifndef USE_PAM -static void update_passwd (struct passwd *pwd, const char *password) +/* + * update_passwd - update the password in the passwd entry + * + * Return 0 if successful. + */ +static int update_passwd (struct passwd *pwd, const char *password) { void *crypt_arg = NULL; char *cp; @@ -399,14 +404,18 @@ static void update_passwd (struct passwd *pwd, const char *password) if ((crypt_method != NULL) && (0 == strcmp(crypt_method, "NONE"))) { pwd->pw_passwd = (char *)password; } else { - cp=pw_encrypt (password, crypt_make_salt (crypt_method, - crypt_arg)); - if (cp == NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); + const char *salt = crypt_make_salt (crypt_method, crypt_arg); + cp = pw_encrypt (password, salt); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + return 1; } pwd->pw_passwd = cp; } + + return 0; } #endif /* !USE_PAM */ @@ -435,8 +444,7 @@ static int add_passwd (struct passwd *pwd, const char *password) * harder since there are zillions of things to do ... */ if (!is_shadow) { - update_passwd (pwd, password); - return 0; + return update_passwd (pwd, password); } #endif /* USE_PAM */ @@ -455,9 +463,11 @@ static int add_passwd (struct passwd *pwd, const char *password) const char *salt = crypt_make_salt (crypt_method, crypt_arg); cp = pw_encrypt (password, salt); - if (cp == NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + return 1; } spent.sp_pwdp = cp; } @@ -477,8 +487,7 @@ static int add_passwd (struct passwd *pwd, const char *password) * the password set someplace else. */ if (strcmp (pwd->pw_passwd, "x") != 0) { - update_passwd (pwd, password); - return 0; + return update_passwd (pwd, password); } #else /* USE_PAM */ /* @@ -504,9 +513,11 @@ static int add_passwd (struct passwd *pwd, const char *password) } else { const char *salt = crypt_make_salt (crypt_method, crypt_arg); cp = pw_encrypt (password, salt); - if (cp == NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + return 1; } spent.sp_pwdp = cp; } diff --git a/src/passwd.c b/src/passwd.c index ae266661..3424f3bf 100644 --- a/src/passwd.c +++ b/src/passwd.c @@ -218,6 +218,7 @@ static int new_password (const struct passwd *pw) { char *clear; /* Pointer to clear text */ char *cipher; /* Pointer to cipher text */ + const char *salt; /* Pointer to new salt */ char *cp; /* Pointer to getpass() response */ char orig[200]; /* Original password */ char pass[200]; /* New password */ @@ -242,7 +243,19 @@ static int new_password (const struct passwd *pw) } cipher = pw_encrypt (clear, crypt_passwd); - if ((cipher == NULL) || (strcmp (cipher, crypt_passwd) != 0)) { + + if (NULL == cipher) { + strzero (clear); + fprintf (stderr, + _("%s: failed to crypt password with previous salt: %s\n"), + Prog, strerror (errno)); + SYSLOG ((LOG_INFO, + "Failed to crypt password with previous salt of user '%s'", + pw->pw_name)); + return -1; + } + + if (strcmp (cipher, crypt_passwd) != 0) { strzero (clear); strzero (cipher); SYSLOG ((LOG_WARN, "incorrect password for %s", @@ -348,13 +361,17 @@ static int new_password (const struct passwd *pw) /* * Encrypt the password, then wipe the cleartext password. */ - cp = pw_encrypt (pass, crypt_make_salt (NULL, NULL)); - if (cp == NULL) { - perror ("crypt"); - exit (EXIT_FAILURE); - } + salt = crypt_make_salt (NULL, NULL); + cp = pw_encrypt (pass, salt); memzero (pass, sizeof pass); + if (NULL == cp) { + fprintf (stderr, + _("%s: failed to crypt password with salt '%s': %s\n"), + Prog, salt, strerror (errno)); + return -1; + } + #ifdef HAVE_LIBCRACK_HIST HistUpdate (pw->pw_name, crypt_passwd); #endif /* HAVE_LIBCRACK_HIST */