From 4e4d6bb78e3bd6430838d854832c58f104d5f559 Mon Sep 17 00:00:00 2001 From: "Andrew G. Morgan" Date: Sun, 11 Feb 2001 06:33:53 +0000 Subject: Relevant BUGIDs: 112540 Purpose of commit: minor security bugfix Commit summary: --------------- Fixes for the password helper binaries. Before, there was no check that the password entered was actually that of the intended user being authenticated. Instead, the password was checked for the requesting user. While this disstinction sounds like a security hole, its actually not been a problem in practice. The helper binaries have only been used in the case that the application is not setuid-0 and as such even if an improper authentication succeeded, the application could not change its uid from that of the requesting user. --- CHANGELOG | 5 +++++ modules/pam_pwdb/pwdb_chkpwd.c | 24 ++++++++++++++++++++---- modules/pam_pwdb/support.-c | 13 ++++++++----- modules/pam_unix/Makefile | 3 ++- modules/pam_unix/pam_unix_passwd.c | 4 ++-- modules/pam_unix/support.c | 9 ++++++--- modules/pam_unix/unix_chkpwd.c | 34 ++++++++++++++-------------------- 7 files changed, 57 insertions(+), 35 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ce7eb20e..6ec9c485 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -50,6 +50,11 @@ libpam. Prior versions were buggy - see bugfix for Bug 129775. ** WARNING ** +* fixed a small security hole (more of a user confusion issue) with + the unix and pwdb password helper binaries. The beef is described in + the bug report, but no uid change was possible so no-one should + think they need to issue a security bulletin over this one! (Bug + 112540 - agmorgan) * pam_lastlog needs to be linked with -lutil (Bug 131549 - agmorgan) * pam_cracklib needs to be linked with -lcrypt (old password checking) (Bug 131601 - agmorgan) diff --git a/modules/pam_pwdb/pwdb_chkpwd.c b/modules/pam_pwdb/pwdb_chkpwd.c index fdd3cfa3..36cf0984 100644 --- a/modules/pam_pwdb/pwdb_chkpwd.c +++ b/modules/pam_pwdb/pwdb_chkpwd.c @@ -83,12 +83,12 @@ static int _unix_verify_passwd(const char *salt, const char *p) return retval; } -int main(void) +int main(int argc, char **argv) { const struct pwdb *pw=NULL; const struct pwdb_entry *pwe=NULL; char pass[MAXPASS+1]; - int npass; + int npass, force_failure=0; int retval=UNIX_FAILED; /* @@ -120,14 +120,26 @@ int main(void) retval = UNIX_FAILED; } if (retval != UNIX_FAILED) { - retval = pwdb_locate("user", PWDB_DEFAULT, PWDB_NAME_UNKNOWN - , getuid(), &pw); + retval = pwdb_locate("user", PWDB_DEFAULT, PWDB_NAME_UNKNOWN, + getuid(), &pw); } if (retval != PWDB_SUCCESS) { _log_err(LOG_ALERT, "could not identify user"); while (pwdb_end() != PWDB_SUCCESS); exit(UNIX_FAILED); } + if (argc == 2) { + if (pwdb_get_entry(pw, "user", &pwe) == PWDB_SUCCESS) { + if (pwe == NULL) { + force_failure = 1; + } else { + if (strcmp((const char *) pwe->value, argv[1])) { + force_failure = 1; + } + pwdb_entry_delete(&pwe); + } + } + } /* read the password from stdin (a pipe from the pam_pwdb module) */ @@ -158,6 +170,10 @@ int main(void) memset(pass, '\0', MAXPASS); /* clear memory of the password */ while (pwdb_end() != PWDB_SUCCESS); + if ((retval != UNIX_FAILED) && force_failure) { + retval = UNIX_FAILED; + } + /* return pass or fail */ exit(retval); diff --git a/modules/pam_pwdb/support.-c b/modules/pam_pwdb/support.-c index d43e0554..bbaa51ac 100644 --- a/modules/pam_pwdb/support.-c +++ b/modules/pam_pwdb/support.-c @@ -345,7 +345,8 @@ static void _cleanup_failures(pam_handle_t *pamh, void *fl, int err) #include #include -static int pwdb_run_helper_binary(pam_handle_t *pamh, const char *passwd) +static int pwdb_run_helper_binary(pam_handle_t *pamh, const char *passwd, + const char *user) { int retval, child, fds[2]; @@ -359,7 +360,7 @@ static int pwdb_run_helper_binary(pam_handle_t *pamh, const char *passwd) /* fork */ child = fork(); if (child == 0) { - static char *args[] = { NULL, NULL }; + static char *args[] = { NULL, NULL, NULL }; static char *envp[] = { NULL }; /* XXX - should really tidy up PAM here too */ @@ -371,6 +372,8 @@ static int pwdb_run_helper_binary(pam_handle_t *pamh, const char *passwd) /* exec binary helper */ args[0] = x_strdup(CHKPWD_HELPER); + args[1] = x_strdup(user); + execve(CHKPWD_HELPER, args, envp); /* should not get here: exit with error */ @@ -398,8 +401,8 @@ static int pwdb_run_helper_binary(pam_handle_t *pamh, const char *passwd) return retval; } -static int _unix_verify_password(pam_handle_t *pamh, const char *name - , const char *p, unsigned int ctrl) +static int _unix_verify_password(pam_handle_t *pamh, const char *name, + const char *p, unsigned int ctrl) { const struct pwdb *pw=NULL; const struct pwdb_entry *pwe=NULL; @@ -465,7 +468,7 @@ static int _unix_verify_password(pam_handle_t *pamh, const char *name if (geteuid()) { /* we are not root perhaps this is the reason? Run helper */ D(("running helper binary")); - retval = pwdb_run_helper_binary(pamh, p); + retval = pwdb_run_helper_binary(pamh, p, name); } else { retval = PAM_AUTHINFO_UNAVAIL; _log_err(LOG_ALERT, "get passwd; %s", pwdb_strerror(retval)); diff --git a/modules/pam_unix/Makefile b/modules/pam_unix/Makefile index dc0b6ac2..e627d728 100644 --- a/modules/pam_unix/Makefile +++ b/modules/pam_unix/Makefile @@ -148,7 +148,8 @@ ifdef DYNAMIC for x in pam_unix_auth pam_unix_acct pam_unix_passwd pam_unix_session;\ do ln -sf $(LIBSHARED) $(FAKEROOT)$(SECUREDIR)/$$x.so ; done endif - install $(CHKPWD) $(FAKEROOT)$(SUPLEMENTED) + $(MKDIR) $(FAKEROOT)$(SUPLEMENTED) + install -m 4555 $(CHKPWD) $(FAKEROOT)$(SUPLEMENTED) remove: rm -f $(FAKEROOT)$(SECUREDIR)/$(LIBSHARED) diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 5d8d2d7d..3fe8a27a 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -328,7 +328,7 @@ static int save_old_password(const char *forwho, const char *oldpass, int howman return retval; } -static int _update_passwd(const char *forwho, char *towhat) +static int _update_passwd(const char *forwho, const char *towhat) { struct passwd *tmpent = NULL; FILE *pwfile, *opwfile; @@ -588,7 +588,7 @@ static int _pam_unix_approve_pass(pam_handle_t * pamh ,const char *pass_new) { const char *user; - char *remark = NULL; + const char *remark = NULL; int retval = PAM_SUCCESS; D(("&new=%p, &old=%p", pass_old, pass_new)); diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 87a5d938..69071408 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -394,7 +394,8 @@ int _unix_blankpasswd(unsigned int ctrl, const char *name) #include #include -static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, unsigned int ctrl) +static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, + unsigned int ctrl, const char *user) { int retval, child, fds[2]; @@ -408,8 +409,8 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, unsig /* fork */ child = fork(); if (child == 0) { - static char *args[] = { NULL, NULL }; static char *envp[] = { NULL }; + char *args[] = { NULL, NULL, NULL }; /* XXX - should really tidy up PAM here too */ @@ -419,6 +420,8 @@ static int _unix_run_helper_binary(pam_handle_t *pamh, const char *passwd, unsig /* exec binary helper */ args[0] = x_strdup(CHKPWD_HELPER); + args[1] = x_strdup(user); + execve(CHKPWD_HELPER, args, envp); /* should not get here: exit with error */ @@ -530,7 +533,7 @@ int _unix_verify_password(pam_handle_t * pamh, const char *name if (geteuid()) { /* we are not root perhaps this is the reason? Run helper */ D(("running helper binary")); - retval = _unix_run_helper_binary(pamh, p, ctrl); + retval = _unix_run_helper_binary(pamh, p, ctrl, name); if (pwd == NULL && !on(UNIX_AUDIT,ctrl) && retval != PAM_SUCCESS) { diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index 6e7d3b28..5b9ed43e 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -165,22 +165,6 @@ static int _unix_verify_password(const char *name, const char *p, int opt) static char *getuidname(uid_t uid) { struct passwd *pw; -#if 0 - char *envname; - - envname = getenv("LOGNAME"); - if (envname == NULL) - return NULL; - - pw = getpwuid(uid); - if (pw == NULL) - return NULL; - - if (strcmp(envname, pw->pw_name)) - return NULL; - - return envname; -#else static char username[32]; pw = getpwuid(uid); @@ -192,7 +176,6 @@ static char *getuidname(uid_t uid) username[31] = '\0'; return username; -#endif } int main(int argc, char *argv[]) @@ -200,6 +183,7 @@ int main(int argc, char *argv[]) char pass[MAXPASS + 1]; char option[8]; int npass, opt; + int force_failure = 0; int retval = UNIX_FAILED; char *user; @@ -228,12 +212,18 @@ int main(int argc, char *argv[]) sleep(10); /* this should discourage/annoy the user */ return UNIX_FAILED; } + /* * determine the current user's name is - * 1. supplied as a environment variable as LOGNAME - * 2. the uid has to match the one associated with the LOGNAME. */ user = getuidname(getuid()); + if (argc == 2) { + /* if the caller specifies the username, verify that user + matches it */ + if (strcmp(user, argv[1])) { + force_failure = 1; + } + } /* read the nollok/nonull option */ @@ -281,7 +271,11 @@ int main(int argc, char *argv[]) /* return pass or fail */ - return retval; + if ((retval != UNIX_PASSED) || force_failure) { + return UNIX_FAILED; + } else { + return UNIX_PASSED; + } } /* -- cgit v1.2.3