From 849f9712703119e7b9924baae9949eeacffacd71 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 13 Nov 2023 14:21:50 +0100 Subject: pam_unix: avoid printing NULL values The value of pp can potentially be NULL. This handles this case when printing debug output. Signed-off-by: Benny Baumann --- modules/pam_unix/passverify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 81b10d88..9d4ce232 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -157,7 +157,7 @@ PAMH_ARG_DECL(int verify_pwd_hash, p = NULL; /* no longer needed here */ /* the moment of truth -- do we agree with the password? */ - D(("comparing state of pp[%s] and hash[%s]", pp, hash)); + D(("comparing state of pp[%s] and hash[%s]", pp ? pp : "(null)", hash)); if (pp && strcmp(pp, hash) == 0) { retval = PAM_SUCCESS; -- cgit v1.2.3 From e5e0985f2a479232ff723e3c67b43accaf167785 Mon Sep 17 00:00:00 2001 From: Benny Baumann Date: Mon, 13 Nov 2023 14:24:08 +0100 Subject: pam_unix: avoid integer truncation in debug output When printing the current day and when the password was last changed, a truncation of the value could happen due to incorrect data types used in the format string. Signed-off-by: Benny Baumann --- modules/pam_unix/passverify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 9d4ce232..930c7d3c 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -285,7 +285,7 @@ PAMH_ARG_DECL(int check_shadow_expiry, long int curdays; *daysleft = -1; curdays = (long int)(time(NULL) / (60 * 60 * 24)); - D(("today is %d, last change %d", curdays, spent->sp_lstchg)); + D(("today is %ld, last change %ld", curdays, spent->sp_lstchg)); if ((curdays >= spent->sp_expire) && (spent->sp_expire != -1)) { D(("account expired")); return PAM_ACCT_EXPIRED; -- cgit v1.2.3 From b8429cc8036cd23d075174d13eedc6d857e2b454 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 10 Dec 2023 14:20:32 +0000 Subject: pam_unix: check str to integer conversions Print an error in syslog if an integer could not be converted. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 10 ++++++-- modules/pam_unix/support.c | 60 ++++++++++++++++++++++++++++++++----------- 2 files changed, 53 insertions(+), 17 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 930c7d3c..98f997d5 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -5,6 +5,7 @@ #include #include #include "support.h" +#include #include #include #include @@ -703,7 +704,8 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, while (fgets(buf, 16380, opwfile)) { if (!strncmp(buf, forwho, len) && strchr(":,\n", buf[len]) != NULL) { - char *sptr = NULL; + char *ep, *sptr = NULL; + long value; found = 1; if (howmany == 0) continue; @@ -724,7 +726,11 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, continue; } s_pas = strtok_r(NULL, ":", &sptr); - npas = strtol(s_npas, NULL, 10) + 1; + value = strtol(s_npas, &ep, 10); + if (value < 0 || value >= INT_MAX || s_npas == ep || *ep != '\0') + npas = 0; + else + npas = (int)value + 1; while (npas > howmany && s_pas != NULL) { s_pas = strpbrk(s_pas, ","); if (s_pas != NULL) diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index ec9a5725..9cc39ad7 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -46,6 +46,18 @@ int _make_remark(pam_handle_t * pamh, unsigned long long ctrl, return retval; } +static int _unix_strtoi(const char *str, int minval, int *result) +{ + char *ep; + long value = strtol(str, &ep, 10); + if (value < minval || value > INT_MAX || str == ep || *ep != '\0') { + *result = minval; + return -1; + } + *result = (int)value; + return 0; +} + /* * set the control flags for the UNIX module. */ @@ -126,9 +138,11 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, "option remember not allowed for this module type"); continue; } - *remember = strtol(str, NULL, 10); - if ((*remember == INT_MIN) || (*remember == INT_MAX)) - *remember = -1; + if (_unix_strtoi(str, -1, remember)) { + pam_syslog(pamh, LOG_ERR, + "option remember invalid [%s]", str); + continue; + } if (*remember > 400) *remember = 400; } else if (j == UNIX_MIN_PASS_LEN) { @@ -137,14 +151,22 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, "option minlen not allowed for this module type"); continue; } - *pass_min_len = atoi(str); + if (_unix_strtoi(str, 0, pass_min_len)) { + pam_syslog(pamh, LOG_ERR, + "option minlen invalid [%s]", str); + continue; + } } else if (j == UNIX_ALGO_ROUNDS) { if (rounds == NULL) { pam_syslog(pamh, LOG_ERR, "option rounds not allowed for this module type"); continue; } - *rounds = strtol(str, NULL, 10); + if (_unix_strtoi(str, 0, rounds)) { + pam_syslog(pamh, LOG_ERR, + "option rounds invalid [%s]", str); + continue; + } } ctrl &= unix_args[j].mask; /* for turning things off */ @@ -166,16 +188,24 @@ unsigned long long _set_ctrl(pam_handle_t *pamh, int flags, int *remember, /* Read number of rounds for sha256, sha512 and yescrypt */ if (off(UNIX_ALGO_ROUNDS, ctrl) && rounds != NULL) { - val = NULL; - if (on(UNIX_YESCRYPT_PASS, ctrl)) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "YESCRYPT_COST_FACTOR"); - } else if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) { - val = pam_modutil_search_key(pamh, LOGIN_DEFS, "SHA_CRYPT_MAX_ROUNDS"); - } - if (val) { - *rounds = strtol(val, NULL, 10); - set(UNIX_ALGO_ROUNDS, ctrl); - free (val); + const char *key = NULL; + if (on(UNIX_YESCRYPT_PASS, ctrl)) + key = "YESCRYPT_COST_FACTOR"; + else if (on(UNIX_SHA256_PASS, ctrl) || on(UNIX_SHA512_PASS, ctrl)) + key = "SHA_CRYPT_MAX_ROUNDS"; + else + key = NULL; + + if (key != NULL) { + val = pam_modutil_search_key(pamh, LOGIN_DEFS, key); + if (val) { + if (_unix_strtoi(val, 0, rounds)) + pam_syslog(pamh, LOG_ERR, + "option %s invalid [%s]", key, val); + else + set(UNIX_ALGO_ROUNDS, ctrl); + free (val); + } } } -- cgit v1.2.3 From 2c711ce57ced9f97c2cf4c8d59c1730447a7bd7f Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 10 Dec 2023 12:46:26 +0000 Subject: pam_unix: fix possible shadow signed overflows It is possible to trigger signed integer overflows in check_shadow_expiry if /etc/shadow contains very large values. Since these values have to be set by a system administrator, it would already count as a configuration error. Yet, avoid overflows which would consider accounts which are supposed to be valid for a veeery long time as already invalid. Also, it would be undefined behavior for almost all C standards. Also consider every negative value as invalid, not just -1. The shadow project has different ways of handling these values, but this approach is in sync with its lib/isexpired.c implementation. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 61 +++++++++++++++++++++++++++++-------------- 1 file changed, 41 insertions(+), 20 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 98f997d5..366f244c 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -280,14 +280,29 @@ PAMH_ARG_DECL(int get_pwd_hash, return PAM_SUCCESS; } +/* + * invariant: 0 <= num1 + * invariant: 0 <= num2 + */ +static int +subtract(long num1, long num2) +{ + long value = num1 - num2; + if (value < INT_MIN) + return INT_MIN; + if (value > INT_MAX) + return INT_MAX; + return (int)value; +} + PAMH_ARG_DECL(int check_shadow_expiry, struct spwd *spent, int *daysleft) { - long int curdays; + long int curdays, passed; *daysleft = -1; curdays = (long int)(time(NULL) / (60 * 60 * 24)); D(("today is %ld, last change %ld", curdays, spent->sp_lstchg)); - if ((curdays >= spent->sp_expire) && (spent->sp_expire != -1)) { + if (spent->sp_expire >= 0 && curdays >= spent->sp_expire) { D(("account expired")); return PAM_ACCT_EXPIRED; } @@ -302,25 +317,31 @@ PAMH_ARG_DECL(int check_shadow_expiry, spent->sp_namp); return PAM_SUCCESS; } - if ((curdays - spent->sp_lstchg > spent->sp_max) - && (curdays - spent->sp_lstchg > spent->sp_inact) - && (curdays - spent->sp_lstchg > spent->sp_max + spent->sp_inact) - && (spent->sp_max != -1) && (spent->sp_inact != -1)) { - *daysleft = (int)((spent->sp_lstchg + spent->sp_max) - curdays); - D(("authtok expired")); - return PAM_AUTHTOK_EXPIRED; - } - if ((curdays - spent->sp_lstchg > spent->sp_max) && (spent->sp_max != -1)) { - D(("need a new password 2")); - return PAM_NEW_AUTHTOK_REQD; - } - if ((curdays - spent->sp_lstchg > spent->sp_max - spent->sp_warn) - && (spent->sp_max != -1) && (spent->sp_warn != -1)) { - *daysleft = (int)((spent->sp_lstchg + spent->sp_max) - curdays); - D(("warn before expiry")); + passed = curdays - spent->sp_lstchg; + if (spent->sp_max >= 0) { + if (spent->sp_inact >= 0) { + long inact = spent->sp_max < LONG_MAX - spent->sp_inact ? + spent->sp_max + spent->sp_inact : LONG_MAX; + if (passed > inact) { + *daysleft = subtract(inact, passed); + D(("authtok expired")); + return PAM_AUTHTOK_EXPIRED; + } + } + if (passed > spent->sp_max) { + D(("need a new password 2")); + return PAM_NEW_AUTHTOK_REQD; + } + if (spent->sp_warn >= 0) { + long warn = spent->sp_warn > spent->sp_max ? -1 : + spent->sp_max - spent->sp_warn; + if (passed > warn) { + *daysleft = subtract(spent->sp_max, passed); + D(("warn before expiry")); + } + } } - if ((curdays - spent->sp_lstchg < spent->sp_min) - && (spent->sp_min != -1)) { + if (spent->sp_min >= 0 && passed < spent->sp_min) { /* * The last password change was too recent. This error will be ignored * if no password change is attempted. -- cgit v1.2.3 From 525a62a643f0bf18b12643dd4e8a3dc3d4c63fcd Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 12 Dec 2023 15:51:13 +0100 Subject: pam_unix: simplify save_old_password The combination of snprintf and fputs is not needed. It is possible to call fprintf directly. The previously ignored return value of snprintf is covered this way as well. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 366f244c..d5155b4c 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -647,7 +647,6 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, #endif { static char buf[16384]; - static char nbuf[16384]; char *s_luser, *s_uid, *s_npas, *s_pas, *pass; int npas; FILE *pwfile, *opwfile; @@ -760,16 +759,14 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, } pass = crypt_md5_wrapper(oldpass); if (s_pas == NULL) - snprintf(nbuf, sizeof(nbuf), "%s:%s:%d:%s\n", - s_luser, s_uid, npas, pass); + err = fprintf(pwfile, "%s:%s:%d:%s\n", + s_luser, s_uid, npas, pass) < 0; else - snprintf(nbuf, sizeof(nbuf),"%s:%s:%d:%s,%s\n", - s_luser, s_uid, npas, s_pas, pass); + err = fprintf(pwfile, "%s:%s:%d:%s,%s\n", + s_luser, s_uid, npas, s_pas, pass) < 0; _pam_delete(pass); - if (fputs(nbuf, pwfile) < 0) { - err = 1; + if (err) break; - } } else if (fputs(buf, pwfile) < 0) { err = 1; break; @@ -783,12 +780,9 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, err = 1; } else { pass = crypt_md5_wrapper(oldpass); - snprintf(nbuf, sizeof(nbuf), "%s:%lu:1:%s\n", - forwho, (unsigned long)pwd->pw_uid, pass); + err = fprintf(pwfile, "%s:%lu:1:%s\n", + forwho, (unsigned long)pwd->pw_uid, pass) < 0; _pam_delete(pass); - if (fputs(nbuf, pwfile) < 0) { - err = 1; - } } } -- cgit v1.2.3 From 51a06bc8cc2278c6e81c9c08a9381c9eb0d2de96 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 12 Dec 2023 20:09:45 +0100 Subject: pam_unix: sync expiry checks with shadow The shadow library uses "greater than or equal to" checks instead of current "greater than" checks in pam_unix. The account expiry check is already "greater than or equal to" so this adjustment can even be argued without making references to other projects. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index d5155b4c..a842b70d 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -322,20 +322,20 @@ PAMH_ARG_DECL(int check_shadow_expiry, if (spent->sp_inact >= 0) { long inact = spent->sp_max < LONG_MAX - spent->sp_inact ? spent->sp_max + spent->sp_inact : LONG_MAX; - if (passed > inact) { + if (passed >= inact) { *daysleft = subtract(inact, passed); D(("authtok expired")); return PAM_AUTHTOK_EXPIRED; } } - if (passed > spent->sp_max) { + if (passed >= spent->sp_max) { D(("need a new password 2")); return PAM_NEW_AUTHTOK_REQD; } if (spent->sp_warn >= 0) { long warn = spent->sp_warn > spent->sp_max ? -1 : spent->sp_max - spent->sp_warn; - if (passed > warn) { + if (passed >= warn) { *daysleft = subtract(spent->sp_max, passed); D(("warn before expiry")); } -- cgit v1.2.3 From 9ebc14085a3ba253598cfaa0d3f0d76ea5ee8ccb Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Wed, 13 Dec 2023 00:37:29 +0100 Subject: pam_unix: allow disabled password aging According to shadow(5) manual page, an empty sp_lstchg field implies that password aging is disabled. This indeed is in sync with shadow's isexpired function. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index a842b70d..7993737c 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -311,6 +311,11 @@ PAMH_ARG_DECL(int check_shadow_expiry, *daysleft = 0; return PAM_NEW_AUTHTOK_REQD; } + if (spent->sp_lstchg < 0) { + D(("password aging disabled")); + *daysleft = 0; + return PAM_SUCCESS; + } if (curdays < spent->sp_lstchg) { pam_syslog(pamh, LOG_DEBUG, "account %s has password changed in future", -- cgit v1.2.3 From 297f0b554e497e95c891fd6a77f7ac5591649f26 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Wed, 13 Dec 2023 00:46:06 +0100 Subject: pam_unix: sp_min and sp_warn must be at least 1 If sp_min or sp_warn are set to 0 or empty (-1), then their respective features are disabled according to shadow(5). Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 7993737c..9d365dc9 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -337,7 +337,7 @@ PAMH_ARG_DECL(int check_shadow_expiry, D(("need a new password 2")); return PAM_NEW_AUTHTOK_REQD; } - if (spent->sp_warn >= 0) { + if (spent->sp_warn > 0) { long warn = spent->sp_warn > spent->sp_max ? -1 : spent->sp_max - spent->sp_warn; if (passed >= warn) { @@ -346,7 +346,7 @@ PAMH_ARG_DECL(int check_shadow_expiry, } } } - if (spent->sp_min >= 0 && passed < spent->sp_min) { + if (spent->sp_min > 0 && passed < spent->sp_min) { /* * The last password change was too recent. This error will be ignored * if no password change is attempted. -- cgit v1.2.3 From 1d2814a8a9262444b4c30d1cb320448728aa10ab Mon Sep 17 00:00:00 2001 From: Solar Designer Date: Fri, 29 Dec 2023 01:56:58 +0100 Subject: unix_chkpwd, unix_update: Use exit codes 128+ on signals --- modules/pam_unix/passverify.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 9d365dc9..3bcfed7f 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -1154,7 +1154,7 @@ su_sighandler(int sig) } #endif if (sig > 0) { - _exit(sig); + _exit(128 + (sig & 127)); } } -- cgit v1.2.3 From 73d009e9ea8edafc18c7fe3650b25dd6bdce88c1 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Tue, 2 Jan 2024 22:42:58 +0100 Subject: pam_unix: use getline Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/pam_unix_passwd.c | 6 ++++-- modules/pam_unix/passverify.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 9947f12a..7c141c3b 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -339,17 +339,18 @@ static int _unix_run_update_binary(pam_handle_t *pamh, unsigned long long ctrl, static int check_old_password(const char *forwho, const char *newpass) { - static char buf[16384]; + char *buf = NULL; char *s_pas; int retval = PAM_SUCCESS; FILE *opwfile; + size_t n = 0; size_t len = strlen(forwho); opwfile = fopen(OLD_PASSWORDS_FILE, "r"); if (opwfile == NULL) return PAM_ABORT; - while (fgets(buf, 16380, opwfile)) { + while (getline(&buf, &n, opwfile) != -1) { if (!strncmp(buf, forwho, len) && (buf[len] == ':' || buf[len] == ',')) { char *sptr; @@ -371,6 +372,7 @@ static int check_old_password(const char *forwho, const char *newpass) break; } } + free(buf); fclose(opwfile); return retval; diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 3bcfed7f..2474fa7a 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -651,7 +651,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, int howmany) #endif { - static char buf[16384]; + char *buf = NULL; char *s_luser, *s_uid, *s_npas, *s_pas, *pass; int npas; FILE *pwfile, *opwfile; @@ -660,6 +660,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, int found = 0; struct passwd *pwd = NULL; struct stat st; + size_t bufsize = 0; size_t len = strlen(forwho); #ifdef WITH_SELINUX char *prev_context_raw = NULL; @@ -727,7 +728,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, goto done; } - while (fgets(buf, 16380, opwfile)) { + while (getline(&buf, &bufsize, opwfile) == -1) { if (!strncmp(buf, forwho, len) && strchr(":,\n", buf[len]) != NULL) { char *ep, *sptr = NULL; long value; @@ -777,6 +778,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, break; } } + free(buf); fclose(opwfile); if (!found) { -- cgit v1.2.3 From b3020da7da384d769f27a8713257fbe1001878be Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Mon, 1 Jan 2024 12:00:00 +0000 Subject: pam_unix/passverify: always run the helper to obtain shadow password file entries Initially, when pam_unix.so verified the password, it used to try to obtain the shadow password file entry for the given user by invoking getspnam(3), and only when that didn't work and the effective uid was nonzero, pam_unix.so used to invoke the helper as a fallback. When SELinux support was introduced by commit 67aab1ff5515054341a438cf9804e9c9b3a88033, the fallback was extended also for the case when SELinux was enabled. Later, commit f220cace205332a3dc34e7b37a85e7627e097e7d extended the fallback conditions for the case when pam_modutil_getspnam() failed with EACCES. Since commit 470823c4aacef5cb3b1180be6ed70846b61a3752, the helper is invoked as a fallback when pam_modutil_getspnam() fails for any reason. The ultimate solution for the case when pam_unix.so does not have permissions to obtain the shadow password file entry is to stop trying to use pam_modutil_getspnam() and to invoke the helper instead. Here are two recent examples. https://github.com/linux-pam/linux-pam/pull/484 describes a system configuration where libnss_systemd is enabled along with libnss_files in the shadow entry of nsswitch.conf, so when libnss_files is unable to obtain the shadow password file entry for the root user, e.g. when SELinux is enabled, NSS falls back to libnss_systemd which returns a synthesized shadow password file entry for the root user, which in turn locks the root user out. https://bugzilla.redhat.com/show_bug.cgi?id=2150155 describes essentially the same problem in a similar system configuration. This commit is the final step in the direction of addressing the issue: for password verification pam_unix.so now invokes the helper instead of making the pam_modutil_getspnam() call. * modules/pam_unix/passverify.c (get_account_info) [!HELPER_COMPILE]: Always return PAM_UNIX_RUN_HELPER instead of trying to obtain the shadow password file entry. Complements: https://github.com/linux-pam/linux-pam/pull/386 Resolves: https://github.com/linux-pam/linux-pam/pull/484 Link: https://github.com/authselect/authselect/commit/1e78f7e048747024a846fd22d68afc6993734e92 --- modules/pam_unix/passverify.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 2474fa7a..c48e3c5a 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -238,20 +238,21 @@ PAMH_ARG_DECL(int get_account_info, return PAM_UNIX_RUN_HELPER; #endif } else if (is_pwd_shadowed(*pwd)) { +#ifdef HELPER_COMPILE /* - * ...and shadow password file entry for this user, + * shadow password file entry for this user, * if shadowing is enabled */ - *spwdent = pam_modutil_getspnam(pamh, name); - if (*spwdent == NULL) { -#ifndef HELPER_COMPILE - /* still a chance the user can authenticate */ - return PAM_UNIX_RUN_HELPER; -#endif - return PAM_AUTHINFO_UNAVAIL; - } - if ((*spwdent)->sp_pwdp == NULL) + *spwdent = getspnam(name); + if (*spwdent == NULL || (*spwdent)->sp_pwdp == NULL) return PAM_AUTHINFO_UNAVAIL; +#else + /* + * The helper has to be invoked to deal with + * the shadow password file entry. + */ + return PAM_UNIX_RUN_HELPER; +#endif } } else { return PAM_USER_UNKNOWN; -- cgit v1.2.3 From d3b8c0723d0d691585474b0e14982f62b115a672 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Mon, 8 Jan 2024 21:59:23 +0100 Subject: pam_unix: do not truncate user names This could allow users with very long names to impersonate a user with a 255 characters long name. The check if the argument argv[1] actually matches the user name implies that "user" can unconditionally be set to argv[1]: If they are equal, the strings are obviously equal. If they are not or if null is returned by getuidname, "user" is set to argv[1] anyway. This way, the static buffer can be safely removed because the result of getpwuid() is not stored, which means that subsequent calls to such functions can safely overwrite their internal buffers. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 6 +----- modules/pam_unix/unix_chkpwd.c | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index c48e3c5a..c6515a65 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -1190,16 +1190,12 @@ char * getuidname(uid_t uid) { struct passwd *pw; - static char username[256]; pw = getpwuid(uid); if (pw == NULL) return NULL; - strncpy(username, pw->pw_name, sizeof(username)); - username[sizeof(username) - 1] = '\0'; - - return username; + return pw->pw_name; } #endif diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index 556a2e2c..50570dbc 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -138,11 +138,11 @@ int main(int argc, char *argv[]) /* if the caller specifies the username, verify that user matches it */ if (user == NULL || strcmp(user, argv[1])) { - user = argv[1]; /* no match -> permanently change to the real user and proceed */ if (setuid(getuid()) != 0) return PAM_AUTH_ERR; } + user = argv[1]; } option=argv[2]; -- cgit v1.2.3 From a695a03b9d0e15aa3f7e2eafe8e2ad47ab4c7e00 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Mon, 15 Jan 2024 15:02:58 +0100 Subject: pam_unix: annotate declaration with format attribute Instead of annotating the function definition with the format attribute annotate the declaration, so the annotation is visible at call sites. --- modules/pam_unix/passverify.c | 1 - modules/pam_unix/passverify.h | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index c6515a65..045ea785 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -1132,7 +1132,6 @@ helper_verify_password(const char *name, const char *p, int nullok) } void -PAM_FORMAT((printf, 2, 3)) helper_log_err(int err, const char *format, ...) { va_list args; diff --git a/modules/pam_unix/passverify.h b/modules/pam_unix/passverify.h index 463ef185..9276347c 100644 --- a/modules/pam_unix/passverify.h +++ b/modules/pam_unix/passverify.h @@ -37,6 +37,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, #ifdef HELPER_COMPILE void +PAM_FORMAT((printf, 2, 3)) helper_log_err(int err, const char *format,...); int -- cgit v1.2.3 From e3309f060a67805e679fee55b1101b91991ea824 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Mon, 15 Jan 2024 20:05:28 +0100 Subject: pam_unix: fix regressions The returned value stored in pwd from _unix_getpwnam is inserted into pam handler through pam_set_data. Do not manually free the value. Also check getline return value for != -1 instead of == -1. Fixes 8f2ca5919b26843ef774ef0aeb9bf261dec943a0 and 73d009e9ea8edafc18c7fe3650b25dd6bdce88c1. No release affected. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/pam_unix_passwd.c | 1 - modules/pam_unix/passverify.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 7c141c3b..fe3f566a 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -660,7 +660,6 @@ pam_sm_chauthtok(pam_handle_t *pamh, int flags, int argc, const char **argv) user); return PAM_USER_UNKNOWN; } - _pam_drop(pwd); /* * This is not an AUTH module! diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 045ea785..60d9ceca 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -729,7 +729,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, goto done; } - while (getline(&buf, &bufsize, opwfile) == -1) { + while (getline(&buf, &bufsize, opwfile) != -1) { if (!strncmp(buf, forwho, len) && strchr(":,\n", buf[len]) != NULL) { char *ep, *sptr = NULL; long value; -- cgit v1.2.3 From 9f19e7da7a014e022cdbba06accca171adef27e0 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 4 Jan 2024 18:23:59 +0100 Subject: pam_unix: set close-on-exec Since the module operates on sensitive files set the close-on-exec flag, to avoid file descriptor leaks if there is ever any sibling thread. The fopen(3) mode "e" is supported in glibc since version 2.7 (released in 2007), and ignored prior, see: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=65d834b0add966dbbdb5ed1e916c60b2b2d87f10 --- modules/pam_unix/lckpwdf.-c | 17 +++-------------- modules/pam_unix/pam_unix_passwd.c | 2 +- modules/pam_unix/passverify.c | 16 ++++++++-------- modules/pam_unix/support.c | 2 +- 4 files changed, 13 insertions(+), 24 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/lckpwdf.-c b/modules/pam_unix/lckpwdf.-c index c3e63155..4d0f0ad3 100644 --- a/modules/pam_unix/lckpwdf.-c +++ b/modules/pam_unix/lckpwdf.-c @@ -35,15 +35,6 @@ static int lockfd = -1; -static int set_close_on_exec(int fd) -{ - int flags = fcntl(fd, F_GETFD, 0); - if (flags == -1) - return -1; - flags |= FD_CLOEXEC; - return fcntl(fd, F_SETFD, flags); -} - static int do_lock(int fd) { struct flock fl; @@ -70,7 +61,7 @@ static int lckpwdf(void) #ifdef WITH_SELINUX if(is_selinux_enabled()>0) { - lockfd = open(LOCKFILE, O_WRONLY); + lockfd = open(LOCKFILE, O_WRONLY | O_CLOEXEC); if(lockfd == -1 && errno == ENOENT) { char *create_context_raw; @@ -82,18 +73,16 @@ static int lckpwdf(void) freecon(create_context_raw); if(rc) return -1; - lockfd = open(LOCKFILE, O_CREAT | O_WRONLY, 0600); + lockfd = open(LOCKFILE, O_CREAT | O_WRONLY | O_CLOEXEC, 0600); if(setfscreatecon_raw(NULL)) return -1; } } else #endif - lockfd = open(LOCKFILE, O_CREAT | O_WRONLY, 0600); + lockfd = open(LOCKFILE, O_CREAT | O_WRONLY | O_CLOEXEC, 0600); if (lockfd == -1) return -1; - if (set_close_on_exec(lockfd) == -1) - goto cleanup_fd; memset(&act, 0, sizeof act); act.sa_handler = alarm_catch; diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index fe3f566a..3a223949 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -346,7 +346,7 @@ static int check_old_password(const char *forwho, const char *newpass) size_t n = 0; size_t len = strlen(forwho); - opwfile = fopen(OLD_PASSWORDS_FILE, "r"); + opwfile = fopen(OLD_PASSWORDS_FILE, "re"); if (opwfile == NULL) return PAM_ABORT; diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 60d9ceca..303929a4 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -400,7 +400,7 @@ crypt_make_salt(char *where, int length) int fd; int rv; - if ((rv = fd = open(PAM_PATH_RANDOMDEV, O_RDONLY)) != -1) { + if ((rv = fd = open(PAM_PATH_RANDOMDEV, O_RDONLY | O_CLOEXEC)) != -1) { while ((rv = read(fd, where, length)) != length && errno == EINTR); close (fd); } @@ -557,7 +557,7 @@ unix_selinux_confined(void) } /* let's try opening shadow read only */ - if ((fd=open("/etc/shadow", O_RDONLY)) != -1) { + if ((fd=open("/etc/shadow", O_RDONLY | O_CLOEXEC)) != -1) { close(fd); confined = 0; return confined; @@ -695,14 +695,14 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, freecon(passwd_context_raw); } #endif - pwfile = fopen(OPW_TMPFILE, "w"); + pwfile = fopen(OPW_TMPFILE, "we"); umask(oldmask); if (pwfile == NULL) { err = 1; goto done; } - opwfile = fopen(OLD_PASSWORDS_FILE, "r"); + opwfile = fopen(OLD_PASSWORDS_FILE, "re"); if (opwfile == NULL) { fclose(pwfile); err = 1; @@ -858,14 +858,14 @@ PAMH_ARG_DECL(int unix_update_passwd, freecon(passwd_context_raw); } #endif - pwfile = fopen(PW_TMPFILE, "w"); + pwfile = fopen(PW_TMPFILE, "we"); umask(oldmask); if (pwfile == NULL) { err = 1; goto done; } - opwfile = fopen("/etc/passwd", "r"); + opwfile = fopen("/etc/passwd", "re"); if (opwfile == NULL) { fclose(pwfile); err = 1; @@ -983,14 +983,14 @@ PAMH_ARG_DECL(int unix_update_shadow, freecon(shadow_context_raw); } #endif - pwfile = fopen(SH_TMPFILE, "w"); + pwfile = fopen(SH_TMPFILE, "we"); umask(oldmask); if (pwfile == NULL) { err = 1; goto done; } - opwfile = fopen("/etc/shadow", "r"); + opwfile = fopen("/etc/shadow", "re"); if (opwfile == NULL) { fclose(pwfile); err = 1; diff --git a/modules/pam_unix/support.c b/modules/pam_unix/support.c index 546ef820..d391973f 100644 --- a/modules/pam_unix/support.c +++ b/modules/pam_unix/support.c @@ -352,7 +352,7 @@ int _unix_getpwnam(pam_handle_t *pamh, const char *name, if (!matched && files && strchr(name, ':') == NULL) { FILE *passwd; - passwd = fopen("/etc/passwd", "r"); + passwd = fopen("/etc/passwd", "re"); if (passwd != NULL) { size_t n = 0, userlen; ssize_t r; -- cgit v1.2.3 From 05d50c9f29ef1a1c897feb604c0595142840a93e Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 4 Jan 2024 18:24:03 +0100 Subject: pam_unix: use more appropriate types --- modules/pam_unix/bigcrypt.c | 2 +- modules/pam_unix/passverify.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/bigcrypt.c b/modules/pam_unix/bigcrypt.c index c1028668..f7c35a47 100644 --- a/modules/pam_unix/bigcrypt.c +++ b/modules/pam_unix/bigcrypt.c @@ -55,7 +55,7 @@ char *bigcrypt(const char *key, const char *salt) #ifdef HAVE_CRYPT_R struct crypt_data *cdata; #endif - unsigned long int keylen, n_seg, j; + size_t keylen, n_seg, j; char *cipher_ptr, *plaintext_ptr, *tmp_ptr, *salt_ptr; char keybuf[KEYBUF_SIZE + 1] = {}; diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 303929a4..2c95bba2 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -53,7 +53,7 @@ static void strip_hpux_aging(char *hash) { - static const char valid[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + static const char *const valid = "ABCDEFGHIJKLMNOPQRSTUVWXYZ" "abcdefghijklmnopqrstuvwxyz" "0123456789./"; if ((*hash != '$') && (strlen(hash) > 13)) { @@ -657,7 +657,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, int npas; FILE *pwfile, *opwfile; int err = 0; - int oldmask; + mode_t oldmask; int found = 0; struct passwd *pwd = NULL; struct stat st; @@ -834,7 +834,7 @@ PAMH_ARG_DECL(int unix_update_passwd, struct stat st; FILE *pwfile, *opwfile; int err = 1; - int oldmask; + mode_t oldmask; #ifdef WITH_SELINUX char *prev_context_raw = NULL; #endif @@ -957,7 +957,7 @@ PAMH_ARG_DECL(int unix_update_shadow, struct stat st; FILE *pwfile, *opwfile; int err = 0; - int oldmask; + mode_t oldmask; int wroteentry = 0; #ifdef WITH_SELINUX char *prev_context_raw = NULL; -- cgit v1.2.3 From d5c01cfd6e47503fb597c5568f43cdf079a30719 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 4 Jan 2024 18:24:05 +0100 Subject: pam_unix: clean additional possible sensitive buffers --- modules/pam_unix/bigcrypt.c | 3 +++ modules/pam_unix/pam_unix_passwd.c | 3 ++- modules/pam_unix/passverify.c | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/bigcrypt.c b/modules/pam_unix/bigcrypt.c index f7c35a47..be7cdb93 100644 --- a/modules/pam_unix/bigcrypt.c +++ b/modules/pam_unix/bigcrypt.c @@ -107,6 +107,7 @@ char *bigcrypt(const char *key, const char *salt) tmp_ptr = crypt(plaintext_ptr, salt); /* libc crypt() */ #endif if (tmp_ptr == NULL) { + pam_overwrite_array(keybuf); free(dec_c2_cryptbuf); #ifdef HAVE_CRYPT_R free(cdata); @@ -136,6 +137,7 @@ char *bigcrypt(const char *key, const char *salt) tmp_ptr = crypt(plaintext_ptr, salt_ptr); #endif if (tmp_ptr == NULL) { + pam_overwrite_array(keybuf); pam_overwrite_string(dec_c2_cryptbuf); free(dec_c2_cryptbuf); #ifdef HAVE_CRYPT_R @@ -156,6 +158,7 @@ char *bigcrypt(const char *key, const char *salt) } D(("key=|%s|, salt=|%s|\nbuf=|%s|\n", key, salt, dec_c2_cryptbuf)); + pam_overwrite_array(keybuf); #ifdef HAVE_CRYPT_R pam_overwrite_object(cdata); free(cdata); diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 3a223949..b915ce66 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -350,7 +350,7 @@ static int check_old_password(const char *forwho, const char *newpass) if (opwfile == NULL) return PAM_ABORT; - while (getline(&buf, &n, opwfile) != -1) { + for (; getline(&buf, &n, opwfile) != -1; pam_overwrite_n(buf, n)) { if (!strncmp(buf, forwho, len) && (buf[len] == ':' || buf[len] == ',')) { char *sptr; @@ -372,6 +372,7 @@ static int check_old_password(const char *forwho, const char *newpass) break; } } + pam_overwrite_n(buf, n); free(buf); fclose(opwfile); diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 2c95bba2..426d4028 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -729,7 +729,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, goto done; } - while (getline(&buf, &bufsize, opwfile) != -1) { + for (; getline(&buf, &bufsize, opwfile) != -1; pam_overwrite_n(buf, bufsize)) { if (!strncmp(buf, forwho, len) && strchr(":,\n", buf[len]) != NULL) { char *ep, *sptr = NULL; long value; @@ -779,6 +779,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, break; } } + pam_overwrite_n(buf, bufsize); free(buf); fclose(opwfile); -- cgit v1.2.3 From c25a858bb548b4eb881dabbf10aed4a08b11e973 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Mon, 15 Jan 2024 21:36:38 +0100 Subject: pam_unix: do not allow comma as a field separator The opasswd file shall not use comma as a separator. Enforce colon just like pam_pwhistory does as well. A comma can be part of a user name, although its usage is discouraged. If such a user exists, it could happen that stored passwords of another user are checked. Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/pam_unix_passwd.c | 13 ++++++------- modules/pam_unix/passverify.c | 4 ++-- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/pam_unix_passwd.c b/modules/pam_unix/pam_unix_passwd.c index 444d61ca..897de6ce 100644 --- a/modules/pam_unix/pam_unix_passwd.c +++ b/modules/pam_unix/pam_unix_passwd.c @@ -351,14 +351,13 @@ static int check_old_password(const char *forwho, const char *newpass) return PAM_ABORT; for (; getline(&buf, &n, opwfile) != -1; pam_overwrite_n(buf, n)) { - if (!strncmp(buf, forwho, len) && (buf[len] == ':' || - buf[len] == ',')) { + if (!strncmp(buf, forwho, len) && buf[len] == ':') { char *sptr; buf[strlen(buf) - 1] = '\0'; - /* s_luser = */ strtok_r(buf, ":,", &sptr); - /* s_uid = */ strtok_r(NULL, ":,", &sptr); - /* s_npas = */ strtok_r(NULL, ":,", &sptr); - s_pas = strtok_r(NULL, ":,", &sptr); + /* s_luser = */ strtok_r(buf, ":", &sptr); + /* s_uid = */ strtok_r(NULL, ":", &sptr); + /* s_npas = */ strtok_r(NULL, ":", &sptr); + s_pas = strtok_r(NULL, ",", &sptr); while (s_pas != NULL) { char *md5pass = Goodcrypt_md5(newpass, s_pas); if (md5pass == NULL || !strcmp(md5pass, s_pas)) { @@ -366,7 +365,7 @@ static int check_old_password(const char *forwho, const char *newpass) retval = PAM_AUTHTOK_ERR; break; } - s_pas = strtok_r(NULL, ":,", &sptr); + s_pas = strtok_r(NULL, ",", &sptr); _pam_delete(md5pass); } break; diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 426d4028..5c4f862e 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -730,7 +730,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, } for (; getline(&buf, &bufsize, opwfile) != -1; pam_overwrite_n(buf, bufsize)) { - if (!strncmp(buf, forwho, len) && strchr(":,\n", buf[len]) != NULL) { + if (!strncmp(buf, forwho, len) && strchr(":\n", buf[len]) != NULL) { char *ep, *sptr = NULL; long value; found = 1; @@ -752,7 +752,7 @@ save_old_password(pam_handle_t *pamh, const char *forwho, const char *oldpass, found = 0; continue; } - s_pas = strtok_r(NULL, ":", &sptr); + s_pas = strtok_r(NULL, "", &sptr); value = strtol(s_npas, &ep, 10); if (value < 0 || value >= INT_MAX || s_npas == ep || *ep != '\0') npas = 0; -- cgit v1.2.3 From 470b5bdd8fd29d6b35e3a80f9a57bdd4b2438200 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 19 Jan 2024 10:09:00 +0100 Subject: pam_unix: do not warn if password aging is disabled Later checks will print a warning if daysleft is 0. If password aging is disabled, leave daysleft at -1. Resolves: https://github.com/linux-pam/linux-pam/issues/743 Fixes: 9ebc14085a3b ("pam_unix: allow disabled password aging") Signed-off-by: Tobias Stoeckmann --- modules/pam_unix/passverify.c | 1 - 1 file changed, 1 deletion(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 5c4f862e..1bc98fa2 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -314,7 +314,6 @@ PAMH_ARG_DECL(int check_shadow_expiry, } if (spent->sp_lstchg < 0) { D(("password aging disabled")); - *daysleft = 0; return PAM_SUCCESS; } if (curdays < spent->sp_lstchg) { -- cgit v1.2.3 From 0e80c788850c4a699e4bfb3ab7b44e354b8fdfd7 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Tue, 16 Jan 2024 15:12:58 +0100 Subject: modules: zero out crypt_r(3) data before usage The manual page of crypt_r(3) recommends to zero the entire data object. --- modules/pam_pwhistory/opasswd.c | 4 +--- modules/pam_unix/bigcrypt.c | 3 +-- modules/pam_unix/passverify.c | 6 ++---- modules/pam_userdb/pam_userdb.c | 3 +-- 4 files changed, 5 insertions(+), 11 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c index b7711e03..a4bcbaae 100644 --- a/modules/pam_pwhistory/opasswd.c +++ b/modules/pam_pwhistory/opasswd.c @@ -127,9 +127,7 @@ compare_password(const char *newpass, const char *oldpass) char *outval; int retval; #ifdef HAVE_CRYPT_R - struct crypt_data output; - - output.initialized = 0; + struct crypt_data output = { 0 }; outval = crypt_r (newpass, oldpass, &output); #else diff --git a/modules/pam_unix/bigcrypt.c b/modules/pam_unix/bigcrypt.c index be7cdb93..1b32c3f2 100644 --- a/modules/pam_unix/bigcrypt.c +++ b/modules/pam_unix/bigcrypt.c @@ -67,12 +67,11 @@ char *bigcrypt(const char *key, const char *salt) return NULL; } #ifdef HAVE_CRYPT_R - cdata = malloc(sizeof(*cdata)); + cdata = calloc(1, sizeof(*cdata)); if(!cdata) { free(dec_c2_cryptbuf); return NULL; } - cdata->initialized = 0; #endif /* fill KEYBUF_SIZE with key */ diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 1bc98fa2..30045333 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -144,9 +144,8 @@ PAMH_ARG_DECL(int verify_pwd_hash, #endif #ifdef HAVE_CRYPT_R struct crypt_data *cdata; - cdata = malloc(sizeof(*cdata)); + cdata = calloc(1, sizeof(*cdata)); if (cdata != NULL) { - cdata->initialized = 0; pp = x_strdup(crypt_r(p, hash, cdata)); pam_overwrite_object(cdata); free(cdata); @@ -503,9 +502,8 @@ PAMH_ARG_DECL(char * create_password_hash, #endif /* CRYPT_GENSALT_IMPLEMENTS_AUTO_ENTROPY */ #ifdef HAVE_CRYPT_R sp = NULL; - cdata = malloc(sizeof(*cdata)); + cdata = calloc(1, sizeof(*cdata)); if (cdata != NULL) { - cdata->initialized = 0; sp = crypt_r(password, salt, cdata); } #else diff --git a/modules/pam_userdb/pam_userdb.c b/modules/pam_userdb/pam_userdb.c index 0b5e5965..7e1407f4 100644 --- a/modules/pam_userdb/pam_userdb.c +++ b/modules/pam_userdb/pam_userdb.c @@ -287,11 +287,10 @@ user_lookup (pam_handle_t *pamh, const char *database, const char *cryptmode, } else { #ifdef HAVE_CRYPT_R struct crypt_data *cdata = NULL; - cdata = malloc(sizeof(*cdata)); + cdata = calloc(1, sizeof(*cdata)); if (cdata == NULL) { pam_syslog(pamh, LOG_CRIT, "malloc failed: struct crypt_data"); } else { - cdata->initialized = 0; cryptpw = crypt_r(pass, pwhash, cdata); } #else -- cgit v1.2.3 From 8d0c575336ad301cd14e16ad2fdec6fe621764b8 Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Thu, 28 Mar 2024 21:58:35 +0000 Subject: pam_unix: allow empty passwords with non-empty hashes Before the change pam_unix has different behaviours for a user with empty password for these two `/etc/shadow` entries: nulloktest:$6$Yy4ty2jJ$bsVQWo8qlXC6UHq1/qTC3UR60ZJKmKApJ3Wj7DreAy8FxlVKtlDnplFQ7jMLVlDqordE7e4t49GvTb.aI59TP0:1:::::: nulloktest::1:::::: The entry with a hash was rejected and the entry without was accepted. The rejection happened because 9e74e90147c "pam_unix: avoid determining if user exists" introduced the following rejection check (slightly simplified): ... } else if (p[0] == '\0' && nullok) { if (hash[0] != '\0') { retval = PAM_AUTH_ERR; } We should not reject the user with a hash assuming it's non-empty. The change does that by pushing empty password check into `verify_pwd_hash()`. `NixOS` generates such hashed entries for empty passwords as if they were non-empty using the following perl code: sub hashPassword { my ($password) = @_; my $salt = ""; my @chars = ('.', '/', 0..9, 'A'..'Z', 'a'..'z'); $salt .= $chars[rand 64] for (1..8); return crypt($password, '$6$' . $salt . '$'); } Resolves: https://github.com/linux-pam/linux-pam/issues/758 Fixes: 9e74e90147c "pam_unix: avoid determining if user exists" Signed-off-by: Sergei Trofimovich --- modules/pam_unix/passverify.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 30045333..1c83f1aa 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -76,9 +76,13 @@ PAMH_ARG_DECL(int verify_pwd_hash, strip_hpux_aging(hash); hash_len = strlen(hash); - if (!hash_len) { + + if (p && p[0] == '\0' && !nullok) { + /* The passed password is empty */ + retval = PAM_AUTH_ERR; + } else if (!hash_len) { /* the stored password is NULL */ - if (nullok) { /* this means we've succeeded */ + if (p && p[0] == '\0' && nullok) { /* this means we've succeeded */ D(("user has empty password - access granted")); retval = PAM_SUCCESS; } else { @@ -1109,12 +1113,6 @@ helper_verify_password(const char *name, const char *p, int nullok) if (pwd == NULL || hash == NULL) { helper_log_err(LOG_NOTICE, "check pass; user unknown"); retval = PAM_USER_UNKNOWN; - } else if (p[0] == '\0' && nullok) { - if (hash[0] == '\0') { - retval = PAM_SUCCESS; - } else { - retval = PAM_AUTH_ERR; - } } else { retval = verify_pwd_hash(p, hash, nullok); } -- cgit v1.2.3 From 42064cdf2b5e41eee71aa76f64c5ef6b43c1ca73 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Sat, 20 Jan 2024 14:07:11 +0100 Subject: pam_unix: compare password hashes in constant time Compare the hashes in constant time as a defense-in-depth mechanism, since performance is not a priority. --- modules/pam_unix/passverify.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 1c83f1aa..624ba783 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -94,7 +94,7 @@ PAMH_ARG_DECL(int verify_pwd_hash, } else { if (pam_str_skip_prefix(hash, "$1$") != NULL) { pp = Goodcrypt_md5(p, hash); - if (pp && strcmp(pp, hash) != 0) { + if (pp && !pam_consttime_streq(pp, hash)) { _pam_delete(pp); pp = Brokencrypt_md5(p, hash); } @@ -163,7 +163,7 @@ PAMH_ARG_DECL(int verify_pwd_hash, /* the moment of truth -- do we agree with the password? */ D(("comparing state of pp[%s] and hash[%s]", pp ? pp : "(null)", hash)); - if (pp && strcmp(pp, hash) == 0) { + if (pp && pam_consttime_streq(pp, hash)) { retval = PAM_SUCCESS; } else { retval = PAM_AUTH_ERR; -- cgit v1.2.3 From aca37d3400e31ef01f3f79b64dd8660d872aaf8f Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Sun, 25 Aug 2024 08:00:00 +0000 Subject: build: rename SCONFIGDIR config.h macro to SCONFIG_DIR This way it is visibly different from the configure variable SCONFIGDIR, which is helpful, because their values are slightly different: the macro is quoted while the configure variable is not quoted, and this difference may cause problems with other build systems. --- configure.ac | 2 +- modules/pam_access/pam_access.c | 4 ++-- modules/pam_env/pam_env.c | 2 +- modules/pam_faillock/faillock_config.c | 2 +- modules/pam_group/pam_group.c | 2 +- modules/pam_limits/pam_limits.c | 2 +- modules/pam_namespace/pam_namespace.h | 8 ++++---- modules/pam_pwhistory/opasswd.c | 2 +- modules/pam_pwhistory/pwhistory_config.c | 2 +- modules/pam_sepermit/pam_sepermit.c | 2 +- modules/pam_time/pam_time.c | 2 +- modules/pam_unix/passverify.c | 2 +- modules/pam_unix/passverify.h | 2 +- 13 files changed, 17 insertions(+), 17 deletions(-) (limited to 'modules/pam_unix/passverify.c') diff --git a/configure.ac b/configure.ac index b05a61dc..d965b61d 100644 --- a/configure.ac +++ b/configure.ac @@ -299,7 +299,7 @@ AC_MSG_RESULT([Defining \$ISA to "$ISA"]) AC_ARG_ENABLE(sconfigdir, AS_HELP_STRING([--enable-sconfigdir=DIR],[path to module conf files @<:@default=$sysconfdir/security@:>@]), SCONFIGDIR=$enableval, SCONFIGDIR=$sysconfdir/security) -AC_DEFINE_UNQUOTED([SCONFIGDIR], ["$SCONFIGDIR"], +AC_DEFINE_UNQUOTED([SCONFIG_DIR], ["$SCONFIGDIR"], [Directory for PAM modules system configuration files]) AC_SUBST(SCONFIGDIR) diff --git a/modules/pam_access/pam_access.c b/modules/pam_access/pam_access.c index 0540176e..f54b4b33 100644 --- a/modules/pam_access/pam_access.c +++ b/modules/pam_access/pam_access.c @@ -56,8 +56,8 @@ #include "pam_cc_compat.h" #include "pam_inline.h" -#define PAM_ACCESS_CONFIG (SCONFIGDIR "/access.conf") -#define ACCESS_CONF_GLOB (SCONFIGDIR "/access.d/*.conf") +#define PAM_ACCESS_CONFIG (SCONFIG_DIR "/access.conf") +#define ACCESS_CONF_GLOB (SCONFIG_DIR "/access.d/*.conf") #ifdef VENDOR_SCONFIGDIR #define VENDOR_PAM_ACCESS_CONFIG (VENDOR_SCONFIGDIR "/access.conf") #define VENDOR_ACCESS_CONF_GLOB (VENDOR_SCONFIGDIR "/access.d/*.conf") diff --git a/modules/pam_env/pam_env.c b/modules/pam_env/pam_env.c index 1bb7b2c3..8b499e5f 100644 --- a/modules/pam_env/pam_env.c +++ b/modules/pam_env/pam_env.c @@ -52,7 +52,7 @@ typedef struct var { #define DEFAULT_USER_ENVFILE ".pam_environment" #define DEFAULT_USER_READ_ENVFILE 0 -#define DEFAULT_CONF_FILE (SCONFIGDIR "/pam_env.conf") +#define DEFAULT_CONF_FILE (SCONFIG_DIR "/pam_env.conf") #ifdef VENDOR_SCONFIGDIR #define VENDOR_DEFAULT_CONF_FILE (VENDOR_SCONFIGDIR "/pam_env.conf") #endif diff --git a/modules/pam_faillock/faillock_config.c b/modules/pam_faillock/faillock_config.c index 91c8001d..42e2b3ec 100644 --- a/modules/pam_faillock/faillock_config.c +++ b/modules/pam_faillock/faillock_config.c @@ -48,7 +48,7 @@ #include "faillock_config.h" #include "faillock.h" -#define FAILLOCK_DEFAULT_CONF SCONFIGDIR "/faillock.conf" +#define FAILLOCK_DEFAULT_CONF SCONFIG_DIR "/faillock.conf" #ifdef VENDOR_SCONFIGDIR #define VENDOR_FAILLOCK_DEFAULT_CONF VENDOR_SCONFIGDIR "/faillock.conf" #endif diff --git a/modules/pam_group/pam_group.c b/modules/pam_group/pam_group.c index 2483059a..82829847 100644 --- a/modules/pam_group/pam_group.c +++ b/modules/pam_group/pam_group.c @@ -24,7 +24,7 @@ #include #include -#define PAM_GROUP_CONF SCONFIGDIR "/group.conf" +#define PAM_GROUP_CONF SCONFIG_DIR "/group.conf" #ifdef VENDOR_SCONFIGDIR # define VENDOR_PAM_GROUP_CONF VENDOR_SCONFIGDIR "/group.conf" #endif diff --git a/modules/pam_limits/pam_limits.c b/modules/pam_limits/pam_limits.c index 1197e25c..80f581a5 100644 --- a/modules/pam_limits/pam_limits.c +++ b/modules/pam_limits/pam_limits.c @@ -126,7 +126,7 @@ struct pam_limit_s { /* Limits from globbed files. */ #define LIMITS_CONF_GLOB (LIMITS_FILE_DIR "/*.conf") -#define LIMITS_FILE (SCONFIGDIR "/limits.conf") +#define LIMITS_FILE (SCONFIG_DIR "/limits.conf") #ifdef VENDOR_SCONFIGDIR #define VENDOR_LIMITS_FILE (VENDOR_SCONFIGDIR "/limits.conf") diff --git a/modules/pam_namespace/pam_namespace.h b/modules/pam_namespace/pam_namespace.h index fd393d17..bff28485 100644 --- a/modules/pam_namespace/pam_namespace.h +++ b/modules/pam_namespace/pam_namespace.h @@ -90,10 +90,10 @@ /* * Module defines */ -#define PAM_NAMESPACE_CONFIG (SCONFIGDIR "/namespace.conf") -#define NAMESPACE_INIT_SCRIPT (SCONFIGDIR "/namespace.init") -#define NAMESPACE_D_DIR (SCONFIGDIR "/namespace.d/") -#define NAMESPACE_D_GLOB (SCONFIGDIR "/namespace.d/*.conf") +#define PAM_NAMESPACE_CONFIG (SCONFIG_DIR "/namespace.conf") +#define NAMESPACE_INIT_SCRIPT (SCONFIG_DIR "/namespace.init") +#define NAMESPACE_D_DIR (SCONFIG_DIR "/namespace.d/") +#define NAMESPACE_D_GLOB (SCONFIG_DIR "/namespace.d/*.conf") #ifdef VENDOR_SCONFIGDIR #define VENDOR_NAMESPACE_INIT_SCRIPT (VENDOR_SCONFIGDIR "/namespace.init") #define VENDOR_PAM_NAMESPACE_CONFIG (VENDOR_SCONFIGDIR "/namespace.conf") diff --git a/modules/pam_pwhistory/opasswd.c b/modules/pam_pwhistory/opasswd.c index 289b4461..d291761b 100644 --- a/modules/pam_pwhistory/opasswd.c +++ b/modules/pam_pwhistory/opasswd.c @@ -76,7 +76,7 @@ #define RANDOM_DEVICE "/dev/urandom" #endif -#define DEFAULT_OLD_PASSWORDS_FILE SCONFIGDIR "/opasswd" +#define DEFAULT_OLD_PASSWORDS_FILE SCONFIG_DIR "/opasswd" typedef struct { char *user; diff --git a/modules/pam_pwhistory/pwhistory_config.c b/modules/pam_pwhistory/pwhistory_config.c index 2f299b54..296a7110 100644 --- a/modules/pam_pwhistory/pwhistory_config.c +++ b/modules/pam_pwhistory/pwhistory_config.c @@ -46,7 +46,7 @@ #include "pam_inline.h" #include "pwhistory_config.h" -#define PWHISTORY_DEFAULT_CONF SCONFIGDIR "/pwhistory.conf" +#define PWHISTORY_DEFAULT_CONF SCONFIG_DIR "/pwhistory.conf" #ifdef VENDOR_SCONFIGDIR #define VENDOR_PWHISTORY_DEFAULT_CONF (VENDOR_SCONFIGDIR "/pwhistory.conf") diff --git a/modules/pam_sepermit/pam_sepermit.c b/modules/pam_sepermit/pam_sepermit.c index e8e1cf5b..34b6d5eb 100644 --- a/modules/pam_sepermit/pam_sepermit.c +++ b/modules/pam_sepermit/pam_sepermit.c @@ -63,7 +63,7 @@ #include "pam_inline.h" -#define SEPERMIT_CONF_FILE (SCONFIGDIR "/sepermit.conf") +#define SEPERMIT_CONF_FILE (SCONFIG_DIR "/sepermit.conf") #ifdef VENDOR_SCONFIGDIR # define SEPERMIT_VENDOR_CONF_FILE (VENDOR_SCONFIGDIR "/sepermit.conf"); #endif diff --git a/modules/pam_time/pam_time.c b/modules/pam_time/pam_time.c index 2b9387fb..65a7bd65 100644 --- a/modules/pam_time/pam_time.c +++ b/modules/pam_time/pam_time.c @@ -33,7 +33,7 @@ #include #endif -#define PAM_TIME_CONF (SCONFIGDIR "/time.conf") +#define PAM_TIME_CONF (SCONFIG_DIR "/time.conf") #ifdef VENDOR_SCONFIGDIR #define VENDOR_PAM_TIME_CONF (VENDOR_SCONFIGDIR "/time.conf") #endif diff --git a/modules/pam_unix/passverify.c b/modules/pam_unix/passverify.c index 624ba783..e8d0b91d 100644 --- a/modules/pam_unix/passverify.c +++ b/modules/pam_unix/passverify.c @@ -364,7 +364,7 @@ PAMH_ARG_DECL(int check_shadow_expiry, #define PW_TMPFILE "/etc/npasswd" #define SH_TMPFILE "/etc/nshadow" -#define OPW_TMPFILE SCONFIGDIR "/nopasswd" +#define OPW_TMPFILE SCONFIG_DIR "/nopasswd" /* * i64c - convert an integer to a radix 64 character diff --git a/modules/pam_unix/passverify.h b/modules/pam_unix/passverify.h index c4c8df5f..1636791c 100644 --- a/modules/pam_unix/passverify.h +++ b/modules/pam_unix/passverify.h @@ -9,7 +9,7 @@ #define PAM_UNIX_RUN_HELPER PAM_CRED_INSUFFICIENT -#define OLD_PASSWORDS_FILE SCONFIGDIR "/opasswd" +#define OLD_PASSWORDS_FILE SCONFIG_DIR "/opasswd" int is_pwd_shadowed(const struct passwd *pwd); -- cgit v1.2.3