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/unix_chkpwd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'modules/pam_unix/unix_chkpwd.c') 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 32112a7b6075e23d7acba37b9272be4a3926bd33 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Mon, 15 Jan 2024 16:20:52 +0100 Subject: pam_unix: refactor audit logging Split the audit logging code into a separate file, to be reused by unix_update(8). --- modules/pam_unix/Makefile.am | 6 +++--- modules/pam_unix/audit.c | 45 ++++++++++++++++++++++++++++++++++++++++++ modules/pam_unix/audit.h | 7 +++++++ modules/pam_unix/passverify.h | 1 + modules/pam_unix/unix_chkpwd.c | 38 +++++------------------------------ 5 files changed, 61 insertions(+), 36 deletions(-) create mode 100644 modules/pam_unix/audit.c create mode 100644 modules/pam_unix/audit.h (limited to 'modules/pam_unix/unix_chkpwd.c') diff --git a/modules/pam_unix/Makefile.am b/modules/pam_unix/Makefile.am index 4a774559..c510f87f 100644 --- a/modules/pam_unix/Makefile.am +++ b/modules/pam_unix/Makefile.am @@ -43,7 +43,7 @@ pam_unix_la_LIBADD = $(top_builddir)/libpam/libpam.la \ securelib_LTLIBRARIES = pam_unix.la -noinst_HEADERS = md5.h support.h yppasswd.h bigcrypt.h passverify.h +noinst_HEADERS = audit.h md5.h support.h yppasswd.h bigcrypt.h passverify.h sbin_PROGRAMS = unix_chkpwd if WITH_SELINUX @@ -63,14 +63,14 @@ bigcrypt_SOURCES = bigcrypt.c bigcrypt_main.c bigcrypt_CFLAGS = $(AM_CFLAGS) bigcrypt_LDADD = @LIBCRYPT@ -unix_chkpwd_SOURCES = unix_chkpwd.c md5_good.c md5_broken.c bigcrypt.c \ +unix_chkpwd_SOURCES = unix_chkpwd.c audit.c md5_good.c md5_broken.c bigcrypt.c \ passverify.c unix_chkpwd_CFLAGS = $(AM_CFLAGS) @EXE_CFLAGS@ -DHELPER_COMPILE=\"unix_chkpwd\" unix_chkpwd_LDFLAGS = @EXE_LDFLAGS@ unix_chkpwd_LDADD = @LIBCRYPT@ @LIBSELINUX@ @LIBAUDIT@ if WITH_SELINUX -unix_update_SOURCES = unix_update.c md5_good.c md5_broken.c bigcrypt.c \ +unix_update_SOURCES = unix_update.c audit.c md5_good.c md5_broken.c bigcrypt.c \ passverify.c unix_update_CFLAGS = $(AM_CFLAGS) @EXE_CFLAGS@ -DHELPER_COMPILE=\"unix_update\" unix_update_LDFLAGS = @EXE_LDFLAGS@ diff --git a/modules/pam_unix/audit.c b/modules/pam_unix/audit.c new file mode 100644 index 00000000..1547a652 --- /dev/null +++ b/modules/pam_unix/audit.c @@ -0,0 +1,45 @@ +#include "audit.h" + +#include "config.h" + +#ifdef HAVE_LIBAUDIT + +#include +#include + +#include + +#include + +#include "passverify.h" + +int audit_log(int type, const char *uname, int retval) +{ + int audit_fd, rc; + + audit_fd = audit_open(); + if (audit_fd < 0) { + /* You get these error codes only when the kernel doesn't have + * audit compiled in. */ + if (errno == EINVAL || errno == EPROTONOSUPPORT || + errno == EAFNOSUPPORT) + return PAM_SUCCESS; + + helper_log_err(LOG_CRIT, "audit_open() failed: %m"); + return PAM_AUTH_ERR; + } + + + + rc = audit_log_acct_message(audit_fd, type, NULL, "PAM:" HELPER_COMPILE, + uname, -1, NULL, NULL, NULL, retval == PAM_SUCCESS); + if (rc == -EPERM && geteuid() != 0) { + rc = 0; + } + + audit_close(audit_fd); + + return rc < 0 ? PAM_AUTH_ERR : PAM_SUCCESS; +} + +#endif /* HAVE_LIBAUDIT */ diff --git a/modules/pam_unix/audit.h b/modules/pam_unix/audit.h new file mode 100644 index 00000000..321232a1 --- /dev/null +++ b/modules/pam_unix/audit.h @@ -0,0 +1,7 @@ +#ifndef PAM_UNIX_AUDIT_H +#define PAM_UNIX_AUDIT_H + +int +audit_log(int type, const char *uname, int rc); + +#endif /* PAM_UNIX_AUDIT_H */ diff --git a/modules/pam_unix/passverify.h b/modules/pam_unix/passverify.h index 9276347c..c4c8df5f 100644 --- a/modules/pam_unix/passverify.h +++ b/modules/pam_unix/passverify.h @@ -4,6 +4,7 @@ #include #include +#include #include #define PAM_UNIX_RUN_HELPER PAM_CRED_INSUFFICIENT diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index 50570dbc..5f47133c 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -27,6 +27,7 @@ #include #ifdef HAVE_LIBAUDIT #include +#include "audit.h" #endif #include @@ -59,35 +60,6 @@ static int _check_expiry(const char *uname) return retval; } -#ifdef HAVE_LIBAUDIT -static int _audit_log(int type, const char *uname, int rc) -{ - int audit_fd; - - audit_fd = audit_open(); - if (audit_fd < 0) { - /* You get these error codes only when the kernel doesn't have - * audit compiled in. */ - if (errno == EINVAL || errno == EPROTONOSUPPORT || - errno == EAFNOSUPPORT) - return PAM_SUCCESS; - - helper_log_err(LOG_CRIT, "audit_open() failed: %m"); - return PAM_AUTH_ERR; - } - - rc = audit_log_acct_message(audit_fd, type, NULL, "PAM:unix_chkpwd", - uname, -1, NULL, NULL, NULL, rc == PAM_SUCCESS); - if (rc == -EPERM && geteuid() != 0) { - rc = 0; - } - - audit_close(audit_fd); - - return rc < 0 ? PAM_AUTH_ERR : PAM_SUCCESS; -} -#endif - int main(int argc, char *argv[]) { char pass[PAM_MAX_RESP_SIZE + 1]; @@ -117,7 +89,7 @@ int main(int argc, char *argv[]) ,"inappropriate use of Unix helper binary [UID=%d]" ,getuid()); #ifdef HAVE_LIBAUDIT - _audit_log(AUDIT_ANOM_EXEC, getuidname(getuid()), PAM_SYSTEM_ERR); + audit_log(AUDIT_ANOM_EXEC, getuidname(getuid()), PAM_SYSTEM_ERR); #endif fprintf(stderr ,"This binary is not designed for running in this way\n" @@ -157,7 +129,7 @@ int main(int argc, char *argv[]) nullok = 0; else { #ifdef HAVE_LIBAUDIT - _audit_log(AUDIT_ANOM_EXEC, getuidname(getuid()), PAM_SYSTEM_ERR); + audit_log(AUDIT_ANOM_EXEC, getuidname(getuid()), PAM_SYSTEM_ERR); #endif return PAM_SYSTEM_ERR; } @@ -185,7 +157,7 @@ int main(int argc, char *argv[]) /* no need to log blank pass test */ #ifdef HAVE_LIBAUDIT if (getuid() != 0) - _audit_log(AUDIT_USER_AUTH, user, PAM_AUTH_ERR); + audit_log(AUDIT_USER_AUTH, user, PAM_AUTH_ERR); #endif helper_log_err(LOG_NOTICE, "password check failed for user (%s)", user); } @@ -200,7 +172,7 @@ int main(int argc, char *argv[]) } else { if (getuid() != 0) { #ifdef HAVE_LIBAUDIT - return _audit_log(AUDIT_USER_AUTH, user, PAM_SUCCESS); + return audit_log(AUDIT_USER_AUTH, user, PAM_SUCCESS); #else return PAM_SUCCESS; #endif -- cgit v1.2.3 From 8e577fb4c55674260143a325c01f47d8dff712af Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Thu, 4 Jan 2024 18:23:58 +0100 Subject: pam_unix: support setgid version of unix_chkpwd(8) In case unix_chkpwd(8) is not a setuid but a setgid binary, reset to the real group as well. Also check the privileges are permanently lost, see: https://wiki.sei.cmu.edu/confluence/display/c/POS37-C.+Ensure+that+privilege+relinquishment+is+successful See also the current Debian patch: https://sources.debian.org/src/pam/1.5.2-9.1/debian/patches-applied/pam_unix_dont_trust_chkpwd_caller.patch/ --- modules/pam_unix/unix_chkpwd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/unix_chkpwd.c') diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index 5f47133c..43fcbd82 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -110,8 +110,13 @@ int main(int argc, char *argv[]) /* if the caller specifies the username, verify that user matches it */ if (user == NULL || strcmp(user, argv[1])) { - /* no match -> permanently change to the real user and proceed */ - if (setuid(getuid()) != 0) + uid_t ruid = getuid(); + gid_t rgid = getgid(); + + /* no match -> permanently change to the real user and group, + * check for no-return, and proceed */ + if (setgid(rgid) != 0 || setuid(ruid) != 0 || + (rgid != 0 && setgid(0) != -1) || (ruid != 0 && setuid(0) != -1)) return PAM_AUTH_ERR; } user = argv[1]; -- cgit v1.2.3 From b4a1c95102c7ba4e2e344fef0e2451aa6041c664 Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Tue, 16 Jan 2024 15:49:55 +0100 Subject: pam_unix: fix typos in comments --- modules/pam_unix/unix_chkpwd.c | 2 +- modules/pam_unix/unix_update.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'modules/pam_unix/unix_chkpwd.c') diff --git a/modules/pam_unix/unix_chkpwd.c b/modules/pam_unix/unix_chkpwd.c index 43fcbd82..820136d5 100644 --- a/modules/pam_unix/unix_chkpwd.c +++ b/modules/pam_unix/unix_chkpwd.c @@ -78,7 +78,7 @@ int main(int argc, char *argv[]) /* * we establish that this program is running with non-tty stdin. * this is to discourage casual use. It does *NOT* prevent an - * intruder from repeatadly running this program to determine the + * intruder from repeatedly running this program to determine the * password of the current user (brute force attack, but one for * which the attacker must already have gained access to the user's * account). diff --git a/modules/pam_unix/unix_update.c b/modules/pam_unix/unix_update.c index 95e99494..e17d6f87 100644 --- a/modules/pam_unix/unix_update.c +++ b/modules/pam_unix/unix_update.c @@ -149,7 +149,7 @@ int main(int argc, char *argv[]) /* * we establish that this program is running with non-tty stdin. * this is to discourage casual use. It does *NOT* prevent an - * intruder from repeatadly running this program to determine the + * intruder from repeatedly running this program to determine the * password of the current user (brute force attack, but one for * which the attacker must already have gained access to the user's * account). -- cgit v1.2.3