From 2b0b78316114bb6d5f4e9a9b1d52bb563a7b1bbe Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Mon, 13 Nov 2017 11:43:47 -0500 Subject: [PATCH 1/9] simplified copying vendor attrs (less buffers and mem copies) to PAM environment and allow optional attrs (i.e. those specified with a * instead of =) to be added to the environment --- pam_tacplus.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/pam_tacplus.c b/pam_tacplus.c index 324cd5d4..af9b42f5 100644 --- a/pam_tacplus.c +++ b/pam_tacplus.c @@ -654,33 +654,29 @@ int pam_sm_acct_mgmt(pam_handle_t * pamh, int flags, int argc, attr = arep.attr; while (attr != NULL) { - char attribute[attr->attr_len]; - char value[attr->attr_len]; - char *sep; - - sep = index(attr->attr, '='); - if (sep == NULL) - sep = index(attr->attr, '*'); - if (sep != NULL) { - bcopy(attr->attr, attribute, attr->attr_len - strlen(sep)); - attribute[attr->attr_len - strlen(sep)] = '\0'; - bcopy(sep, value, strlen(sep)); - value[strlen(sep)] = '\0'; + size_t len = strcspn(attr->attr, "=*"); + if (len < attr->attr_len) { + char avpair[attr->attr_len+1]; + memcpy(avpair, attr->attr, attr->attr_len+1); /* Also copy terminating NUL */ + if (ctrl & PAM_TAC_DEBUG) + syslog(LOG_DEBUG, "%s: returned attribute `%s' from server", + __FUNCTION__, avpair); + + avpair[len] = '='; // replace '*' by '=' size_t i; - for (i = 0; attribute[i] != '\0'; i++) { - attribute[i] = toupper(attribute[i]); - if (attribute[i] == '-') - attribute[i] = '_'; + for (i = 0; i < len; i++) { + avpair[i] = toupper(avpair[i]); + if (avpair[i] == '-') + avpair[i] = '_'; } if (ctrl & PAM_TAC_DEBUG) - syslog(LOG_DEBUG, "%s: returned attribute `%s%s' from server", - __FUNCTION__, attribute, value); + syslog(LOG_DEBUG, "%s: setting PAM environment `%s'", + __FUNCTION__, avpair); /* make returned attributes available for other PAM modules via PAM environment */ - if (pam_putenv(pamh, - strncat(attribute, value, strlen(value))) != PAM_SUCCESS) + if (pam_putenv(pamh, avpair) != PAM_SUCCESS) syslog(LOG_WARNING, "%s: unable to set PAM environment", __FUNCTION__); From 310326e1fd32d403650123182926b6b22e0fd1cf Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Mon, 13 Nov 2017 14:58:21 -0500 Subject: [PATCH 2/9] changed memcpy to bcopy --- pam_tacplus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pam_tacplus.c b/pam_tacplus.c index af9b42f5..198d096c 100644 --- a/pam_tacplus.c +++ b/pam_tacplus.c @@ -657,7 +657,7 @@ int pam_sm_acct_mgmt(pam_handle_t * pamh, int flags, int argc, size_t len = strcspn(attr->attr, "=*"); if (len < attr->attr_len) { char avpair[attr->attr_len+1]; - memcpy(avpair, attr->attr, attr->attr_len+1); /* Also copy terminating NUL */ + bcopy(attr->attr, avpair, attr->attr_len+1); /* Also copy terminating NUL */ if (ctrl & PAM_TAC_DEBUG) syslog(LOG_DEBUG, "%s: returned attribute `%s' from server", From 63f72f7dd47ae85b596a83c039a3c873422c7dc8 Mon Sep 17 00:00:00 2001 From: Philip Prindeville Date: Fri, 29 Dec 2017 10:51:05 -0700 Subject: [PATCH 3/9] Add SElinux tips --- INSTALL | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/INSTALL b/INSTALL index 20998407..c12f41a2 100644 --- a/INSTALL +++ b/INSTALL @@ -93,6 +93,10 @@ of `autoconf'. targets like `make install' and `make uninstall' work correctly. This target is generally not run by end users. + 9. If you're running SElinux in Enforcing mode, you need to set + your policy controls appropriately. For Fedora/RHEL/CentOS, this + is done with "setsebool -P nis_enabled 1" (as root). + Compilers and Options ===================== From ab17e62324c48e63be3f71ef6ca07633bbc50b95 Mon Sep 17 00:00:00 2001 From: Philip Prindeville Date: Thu, 28 Dec 2017 14:08:37 -0700 Subject: [PATCH 4/9] Fix compile-time warnings --- libtac/lib/author_r.c | 1 + tacc.c | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/libtac/lib/author_r.c b/libtac/lib/author_r.c index 148f7eaa..fa101e64 100644 --- a/libtac/lib/author_r.c +++ b/libtac/lib/author_r.c @@ -187,6 +187,7 @@ int tac_author_read(int fd, struct areply *re) { /* XXX support optional vs mandatory arguments */ case TAC_PLUS_AUTHOR_STATUS_PASS_REPL: tac_free_attrib(&re->attr); + /*FALLTHRU*/ case TAC_PLUS_AUTHOR_STATUS_PASS_ADD: { u_char *argp; diff --git a/tacc.c b/tacc.c index 80693238..cba02292 100644 --- a/tacc.c +++ b/tacc.c @@ -58,7 +58,6 @@ /* prototypes */ void sighandler(int sig); -void showusage(char *argv0); unsigned long getservername(char *serv); void showusage(char *progname); void showversion(char *progname); @@ -163,8 +162,12 @@ int main(int argc, char **argv) { break; case 'V': showversion(argv[0]); + /*NOTREACHED*/ + break; case 'h': showusage(argv[0]); + /*NOTREACHED*/ + break; case 'u': user = optarg; break; From ec797c84701ccafbfcccb836f87a1160cf86ef4c Mon Sep 17 00:00:00 2001 From: Philip Prindeville Date: Thu, 28 Dec 2017 14:10:24 -0700 Subject: [PATCH 5/9] linux/random.h doesn't actually declare getrandom() --- configure.ac | 4 ++-- libtac/lib/magic.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index e34c7694..b24f10d3 100644 --- a/configure.ac +++ b/configure.ac @@ -47,8 +47,8 @@ esac dnl -------------------------------------------------------------------- dnl Checks for header files. AC_HEADER_STDC -AC_CHECK_HEADERS([arpa/inet.h fcntl.h netdb.h netinet/in.h stdlib.h string.h strings.h sys/socket.h sys/time.h ]) -AC_CHECK_HEADERS([syslog.h unistd.h openssl/md5.h openssl/rand.h linux/random.h sys/random.h]) +AC_CHECK_HEADERS([arpa/inet.h fcntl.h netdb.h netinet/in.h stdlib.h string.h strings.h sys/socket.h sys/time.h]) +AC_CHECK_HEADERS([syslog.h unistd.h openssl/md5.h openssl/rand.h sys/random.h]) AC_CHECK_HEADER(security/pam_appl.h, [], [AC_MSG_ERROR([PAM libraries missing. Install with "yum install pam-devel" or "apt-get install libpam-dev".])] ) AM_CONDITIONAL(MY_MD5, [test "$ac_cv_header_openssl_md5_h" = "no" ]) AM_CONDITIONAL(TACC, [test "$ac_cv_lib_crypto_RAND_pseudo_bytes" = "yes"]) diff --git a/libtac/lib/magic.c b/libtac/lib/magic.c index a320df59..138c6d48 100644 --- a/libtac/lib/magic.c +++ b/libtac/lib/magic.c @@ -70,10 +70,10 @@ magic() #elif defined(HAVE_GETRANDOM) -# if defined(HAVE_LINUX_RANDOM_H) -# include -# elif defined(HAVE_SYS_RANDOM_H) +# if defined(HAVE_SYS_RANDOM_H) # include +# else +# error no header containing getrandom(2) declaration # endif /* From e12a75b717ac587bbc5814cbb7f2311be99998d8 Mon Sep 17 00:00:00 2001 From: Martin Belanger Date: Thu, 21 Dec 2017 14:59:52 -0500 Subject: [PATCH 6/9] RAND_pseudo_bytes() has been deprecated in OpenSSL 1.1.0. They tell us to use RAND_bytes() instead. Modified by Philip Prindeville --- configure.ac | 7 +++++-- libtac/lib/magic.c | 4 ++++ pam_tacplus.c | 4 ++++ tacc.c | 4 ++++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index b24f10d3..ac7c692f 100644 --- a/configure.ac +++ b/configure.ac @@ -36,7 +36,10 @@ dnl Checks for libraries. AC_CHECK_LIB(pam, pam_start) AC_CHECK_LIB(tac, tac_connect) AC_CHECK_LIB(crypto, MD5_Init) -AC_CHECK_LIB(crypto, RAND_pseudo_bytes) +AC_CHECK_LIB(crypto, RAND_pseudo_bytes, + [AC_DEFINE([HAVE_RAND_PSEUDO_BYTES], [1], [Define to 1 if you have the `RAND_pseudo_bytes' function.])]) +AC_CHECK_LIB(crypto, RAND_bytes, + [AC_DEFINE([HAVE_RAND_BYTES], [1], [Define to 1 if you have the `RAND_bytes' function.])]) AC_CHECK_LIB(util, logwtmp) case "$host" in @@ -51,7 +54,7 @@ AC_CHECK_HEADERS([arpa/inet.h fcntl.h netdb.h netinet/in.h stdlib.h string.h str AC_CHECK_HEADERS([syslog.h unistd.h openssl/md5.h openssl/rand.h sys/random.h]) AC_CHECK_HEADER(security/pam_appl.h, [], [AC_MSG_ERROR([PAM libraries missing. Install with "yum install pam-devel" or "apt-get install libpam-dev".])] ) AM_CONDITIONAL(MY_MD5, [test "$ac_cv_header_openssl_md5_h" = "no" ]) -AM_CONDITIONAL(TACC, [test "$ac_cv_lib_crypto_RAND_pseudo_bytes" = "yes"]) +AM_CONDITIONAL(TACC, [test "$ac_cv_lib_crypto_RAND_bytes" = "yes" || test "$ac_cv_lib_crypto_RAND_pseudo_bytes" = "yes"]) dnl -------------------------------------------------------------------- dnl Checks for typedefs, structures, and compiler characteristics. diff --git a/libtac/lib/magic.c b/libtac/lib/magic.c index 138c6d48..97aa0351 100644 --- a/libtac/lib/magic.c +++ b/libtac/lib/magic.c @@ -63,7 +63,11 @@ magic() { u_int32_t num; +#ifdef HAVE_RAND_BYTES + RAND_bytes((unsigned char *)&num, sizeof(num)); +#else RAND_pseudo_bytes((unsigned char *)&num, sizeof(num)); +#endif return num; } diff --git a/pam_tacplus.c b/pam_tacplus.c index 198d096c..19e2acae 100644 --- a/pam_tacplus.c +++ b/pam_tacplus.c @@ -711,7 +711,11 @@ PAM_EXTERN int pam_sm_open_session(pam_handle_t * pamh, int flags, int argc, const char **argv) { #if defined(HAVE_OPENSSL_RAND_H) && defined(HAVE_LIBCRYPTO) +# if defined(HAVE_RAND_BYTES) + RAND_bytes((unsigned char *) &task_id, sizeof(task_id)); +# else RAND_pseudo_bytes((unsigned char *) &task_id, sizeof(task_id)); +# endif #else task_id=(short int) magic(); #endif diff --git a/tacc.c b/tacc.c index cba02292..5d0585c4 100644 --- a/tacc.c +++ b/tacc.c @@ -319,7 +319,11 @@ int main(int argc, char **argv) { struct tac_attrib *attr = NULL; sprintf(buf, "%lu", time(0)); tac_add_attrib(&attr, "start_time", buf); +#ifdef HAVE_RAND_BYTES + RAND_bytes((unsigned char *) &task_id, sizeof(task_id)); +#else RAND_pseudo_bytes((unsigned char *) &task_id, sizeof(task_id)); +#endif sprintf(buf, "%hu", task_id); tac_add_attrib(&attr, "task_id", buf); tac_add_attrib(&attr, "service", service); From 22d1e39948304e5852325088cf04c74deba53fa4 Mon Sep 17 00:00:00 2001 From: Philip Prindeville Date: Sun, 31 Dec 2017 13:34:51 -0700 Subject: [PATCH 7/9] Fix 'unused' warnings when building with clang/llvm --- libtac/include/libtac.h | 12 ++++++++++++ support.c | 4 +--- tacc.c | 12 +++--------- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/libtac/include/libtac.h b/libtac/include/libtac.h index c872ff71..23cdc287 100644 --- a/libtac/include/libtac.h +++ b/libtac/include/libtac.h @@ -44,6 +44,18 @@ extern "C" { #endif #include "tacplus.h" +#if defined(__clang__) +#define __CLANG_PREREQ(maj, min) ((__clang_major__ > (maj)) || (__clang_major__ == (maj) && __clang_minor__ >= (min))) +#else +#define __CLANG_PREREQ(maj, min) (0) +#endif + +#if __GNUC_PREREQ(3, 2) || __CLANG_PREREQ(4, 0) +#define __Unused __attribute__ ((unused)) +#else +#define __Unused /* unused */ +#endif + #if defined(DEBUGTAC) && !defined(TACDEBUG) # ifdef __GNUC__ #define TACDEBUG(level, fmt, ...) syslog(level, fmt, ## __VA_ARGS__) diff --git a/support.c b/support.c index 2406b321..ad45580f 100644 --- a/support.c +++ b/support.c @@ -109,14 +109,12 @@ int converse(pam_handle_t * pamh, int nargs, const struct pam_message *message, } /* stolen from pam_stress */ -int tacacs_get_password (pam_handle_t * pamh, int flags, +int tacacs_get_password (pam_handle_t * pamh, int flags __Unused, int ctrl, char **password) { const void *pam_pass; char *pass = NULL; - flags = flags; /* unused */ - if (ctrl & PAM_TAC_DEBUG) syslog (LOG_DEBUG, "%s: called", __FUNCTION__); diff --git a/tacc.c b/tacc.c index 5d0585c4..2914c8a6 100644 --- a/tacc.c +++ b/tacc.c @@ -444,9 +444,7 @@ int main(int argc, char **argv) { exit(EXIT_OK); } -void sighandler(int sig) { - sig = sig; /* unused */ - +void sighandler(int sig __Unused) { TACDEBUG(LOG_DEBUG, "caught signal %d", sig); } @@ -572,19 +570,15 @@ unsigned long getservername(char *serv) { return (-1); } -void timeout_handler(int signum) { - signum = signum; /* unused */ - +void timeout_handler(int signum __Unused) { syslog(LOG_ERR, "timeout reading password from user %s", user); } #ifdef TACDEBUG_AT_RUNTIME -void logmsg(int level, const char *fmt, ...) +void logmsg(int level __Unused, const char *fmt, ...) { va_list ap; - level = level; /* unused */ - va_start(ap, fmt); vfprintf(stderr, fmt, ap); va_end(ap); From acf2469b5225dac794cb51fe8a9432a2a6433b55 Mon Sep 17 00:00:00 2001 From: Philip Prindeville Date: Mon, 1 Jan 2018 16:04:19 -0700 Subject: [PATCH 8/9] Check for overflowing tac_login --- tacc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tacc.c b/tacc.c index 2914c8a6..b857cb68 100644 --- a/tacc.c +++ b/tacc.c @@ -176,8 +176,7 @@ int main(int argc, char **argv) { break; case 'L': // tac_login is a global variable initialized in libtac - bzero(tac_login, sizeof(tac_login)); - strncpy(tac_login, optarg, sizeof(tac_login) - 1); + xstrcpy(tac_login, optarg, sizeof(tac_login)); break; case 'p': pass = optarg; From 3337b95461bbea20407b321829e9a139ef077946 Mon Sep 17 00:00:00 2001 From: Philip Prindeville Date: Mon, 1 Jan 2018 16:09:13 -0700 Subject: [PATCH 9/9] Favor pututxline() over logwtmp() --- configure.ac | 4 +++- tacc.c | 49 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index ac7c692f..ff1b44e4 100644 --- a/configure.ac +++ b/configure.ac @@ -40,7 +40,9 @@ AC_CHECK_LIB(crypto, RAND_pseudo_bytes, [AC_DEFINE([HAVE_RAND_PSEUDO_BYTES], [1], [Define to 1 if you have the `RAND_pseudo_bytes' function.])]) AC_CHECK_LIB(crypto, RAND_bytes, [AC_DEFINE([HAVE_RAND_BYTES], [1], [Define to 1 if you have the `RAND_bytes' function.])]) -AC_CHECK_LIB(util, logwtmp) +AC_CHECK_LIB(c, pututxline, + [AC_DEFINE([HAVE_PUTUTXLINE], [1], [Define to 1 if you have the `pututxline' function.])], + [AC_CHECK_LIB(util, logwtmp)]) case "$host" in sparc-* | sparc64-*) diff --git a/tacc.c b/tacc.c index b857cb68..f61e2d70 100644 --- a/tacc.c +++ b/tacc.c @@ -19,7 +19,6 @@ #include #include #include -#include #include #include #include @@ -34,6 +33,12 @@ #include "config.h" #endif +#if defined(HAVE_PUTUTXLINE) +#include +#elif defined(HAVE_LOGWTMP) +#include +#endif + #include "tacplus.h" #include "libtac.h" @@ -78,6 +83,13 @@ typedef unsigned char flag; flag quiet = 0; char *user = NULL; /* global, because of signal handler */ +#if defined(HAVE_PUTUTXLINE) +struct utmpx utmpx; +#endif + +/* take the length of a string constant without the NUL */ +#define C_STRLEN(str) (sizeof("" str) - 1) + /* command line options */ static struct option long_options[] = { @@ -353,10 +365,28 @@ int main(int argc, char **argv) { } /* log in local utmp */ -#ifdef HAVE_LOGWTMP - if (log_wtmp) + if (log_wtmp) { +#if defined(HAVE_PUTUTXLINE) + struct timeval tv; + + gettimeofday(&tv, NULL); + + memset(&utmpx, 0, sizeof(utmpx)); + utmpx.ut_type = USER_PROCESS; + utmpx.ut_pid = getpid(); + xstrcpy(utmpx.ut_line, tty, sizeof(utmpx.ut_line)); + strncpy(utmpx.ut_id, tty + C_STRLEN("tty"), sizeof(utmpx.ut_id)); + xstrcpy(utmpx.ut_host, "dialup", sizeof(utmpx.ut_host)); + utmpx.ut_tv.tv_sec = tv.tv_sec; + utmpx.ut_tv.tv_usec = tv.tv_usec; + xstrcpy(utmpx.ut_user, user, sizeof(utmpx.ut_user)); + /* ut_addr unused ... */ + setutxent(); + pututxline(&utmpx); +#elif defined(HAVE_LOGWTMP) logwtmp(tty, user, "dialup"); #endif + } if (command != NULL) { int ret; @@ -435,10 +465,19 @@ int main(int argc, char **argv) { } /* logout from utmp */ -#ifdef HAVE_LOGWTMP - if (log_wtmp) + if (log_wtmp) { +#if defined(HAVE_PUTUTXLINE) + utmpx.ut_type = DEAD_PROCESS; + memset(utmpx.ut_line, 0, sizeof(utmpx.ut_line)); + memset(utmpx.ut_user, 0, sizeof(utmpx.ut_user)); + memset(utmpx.ut_host, 0, sizeof(utmpx.ut_host)); + utmpx.ut_tv.tv_sec = utmpx.ut_tv.tv_usec = 0; + setutxent(); + pututxline(&utmpx); +#elif defined(HAVE_LOGWTMP) logwtmp(tty, "", ""); #endif + } exit(EXIT_OK); }