From 620862049e72a9c8d95612728f8d3f5ff10cfcc4 Mon Sep 17 00:00:00 2001 From: Nicolas Williams Date: Thu, 12 Oct 2017 12:24:05 -0500 Subject: [PATCH] Use roken_get_*() instead of getpwuuid() Using non-reentrant getpwuid() (or getpwnam(), or getspnam()) can be dangerous. We had a report of a login application / PAM that calls those, and Heimdal, by calling them too, clobbered the cached struct passwd used by the login app / PAM. --- appl/afsutil/pagsh.c | 12 ++-- appl/otp/otp.c | 8 +-- configure.ac | 1 + kadmin/NTMakefile | 8 +-- lib/hx509/softp11.c | 34 ++------- lib/krb5/config_file.c | 8 +-- lib/krb5/get_default_principal.c | 116 ++++++------------------------- 7 files changed, 42 insertions(+), 145 deletions(-) diff --git a/appl/afsutil/pagsh.c b/appl/afsutil/pagsh.c index 5c5d09185..377ac6174 100644 --- a/appl/afsutil/pagsh.c +++ b/appl/afsutil/pagsh.c @@ -99,6 +99,7 @@ main(int argc, char **argv) { int f; char tf[1024]; + char shellbuf[MAX_PATH]; char *p; char *path; @@ -166,13 +167,10 @@ main(int argc, char **argv) (unsigned long)((argc + 10)*sizeof(char *))); if(*argv == NULL) { - path = getenv("SHELL"); - if(path == NULL){ - struct passwd *pw = k_getpwuid(geteuid()); - if (pw == NULL) - errx(1, "no such user: %d", (int)geteuid()); - path = strdup(pw->pw_shell); - } + if (roken_get_shell(shellbuf, sizeof(shellbuf)) != NULL) + path = strdup(shellbuf); + else + path = strdup("/bin/sh"); } else { path = strdup(*argv++); } diff --git a/appl/otp/otp.c b/appl/otp/otp.c index 516669f1d..8699ba866 100644 --- a/appl/otp/otp.c +++ b/appl/otp/otp.c @@ -292,6 +292,7 @@ main (int argc, char **argv) int uid = getuid(); OtpAlgorithm *alg = otp_find_alg (OTP_ALG_DEFAULT); int optidx = 0; + char userbuf[128]; setprogname (argv[0]); if(getarg(args, num_args, argc, argv, &optidx)) @@ -332,12 +333,9 @@ main (int argc, char **argv) return list_otps (argc, argv, user); if (user == NULL) { - struct passwd *pwd; - - pwd = k_getpwuid(uid); - if (pwd == NULL) + user = roken_get_username(userbuf, sizeof(userbuf)); + if (user == NULL) err (1, "You don't exist"); - user = pwd->pw_name; } /* diff --git a/configure.ac b/configure.ac index 2f1bd686d..76c59671b 100644 --- a/configure.ac +++ b/configure.ac @@ -524,6 +524,7 @@ KRB_CAPABILITIES rk_DLADDR AC_CHECK_GETPWNAM_R_POSIX +AC_CHECK_GETPWUID_R_POSIX dnl detect doors on solaris if test "$enable_pthread_support" != no; then diff --git a/kadmin/NTMakefile b/kadmin/NTMakefile index f50e118be..69680d0dc 100644 --- a/kadmin/NTMakefile +++ b/kadmin/NTMakefile @@ -84,7 +84,7 @@ $(OBJ)\kadmin-commands.c $(OBJ)\kadmin-commands.h: kadmin-commands.in cd $(SRCDIR) $(SBINDIR)\kadmin.exe: $(KADMIN_OBJS) $(KADMIN_LIBS) - $(EXECONLINK) + $(EXECONLINK) Secur32.lib Shell32.lib $(EXEPREP) KADMIND_OBJS= \ @@ -100,7 +100,7 @@ KADMIND_LIBS=\ $(COMMON_LIBS) $(LIBEXECDIR)\kadmind.exe: $(KADMIND_OBJS) $(KADMIND_LIBS) - $(EXECONLINK) + $(EXECONLINK) Secur32.lib Shell32.lib $(EXEPREP) all:: $(INCFILES) $(SBIN_PROGRAMS) $(LIBEXEC_PROGRAMS) @@ -115,13 +115,13 @@ clean:: NOINST_PROGRAMS=$(OBJ)\add_random_users.exe $(OBJ)\add_random_users.exe: $(OBJ)\add_random_users.obj $(LIBKADM5SRV) $(LIBKADM5CLNT) $(COMMON_LIBS) - $(EXECONLINK) + $(EXECONLINK) Secur32.lib Shell32.lib $(EXEPREP_NODIST) TEST_BINARIES=$(OBJ)\test_util.exe $(OBJ)\test_util.exe: $(OBJ)\test_util.obj $(OBJ)\util.obj $(KADMIN_LIBS) - $(EXECONLINK) + $(EXECONLINK) Secur32.lib Shell32.lib $(EXEPREP_NODIST) test-binaries: $(TEST_BINARIES) diff --git a/lib/hx509/softp11.c b/lib/hx509/softp11.c index b7a989b12..5c86afd30 100644 --- a/lib/hx509/softp11.c +++ b/lib/hx509/softp11.c @@ -819,45 +819,25 @@ static char * get_config_file_for_user(void) { char *fn; - -#ifndef _WIN32 - char *home; int ret; fn = secure_getenv("SOFTPKCS11RC"); if (fn) fn = strdup(fn); - home = secure_getenv("HOME"); - if (fn == NULL && home == NULL) { - struct passwd *pw = getpwuid(getuid()); - if(pw != NULL) - home = pw->pw_dir; - } if (fn == NULL) { + char homebuf[MAX_PATH]; + const char *home = roken_get_appdatadir(homebuf, sizeof(homebuf)); + if (home) { ret = asprintf(&fn, "%s/.soft-token.rc", home); if (ret == -1) fn = NULL; - } else + } else { +#ifndef WIN32 fn = strdup("/etc/soft-token.rc"); +#endif + } } -#else /* Windows */ - - char appdatafolder[MAX_PATH]; - - fn = getenv("SOFTPKCS11RC"); - - /* Retrieve the roaming AppData folder for the current user. The - current user is the user account represented by the current - thread token. */ - - if (fn == NULL && - SUCCEEDED(SHGetFolderPath(NULL, CSIDL_APPDATA, NULL, SHGFP_TYPE_CURRENT, appdatafolder))) { - - asprintf(&fn, "%s\\.soft-token.rc", appdatafolder); - } - -#endif /* _WIN32 */ return fn; } diff --git a/lib/krb5/config_file.c b/lib/krb5/config_file.c index 716413439..6d66acf71 100644 --- a/lib/krb5/config_file.c +++ b/lib/krb5/config_file.c @@ -569,6 +569,7 @@ krb5_config_parse_file_multi (krb5_context context, if (ISTILDE(fname[0]) && ISPATHSEP(fname[1])) { #ifndef KRB5_USE_PATH_TOKENS const char *home = NULL; + char homebuf[MAX_PATH]; if (!_krb5_homedir_access(context)) { context->config_include_depth--; @@ -577,12 +578,7 @@ krb5_config_parse_file_multi (krb5_context context, return EPERM; } - home = secure_getenv("HOME"); - if (home == NULL) { - struct passwd *pw = getpwuid(getuid()); - if(pw != NULL) - home = pw->pw_dir; - } + home = roken_get_appdatadir(homebuf, sizeof(homebuf)); if (home) { int aret; diff --git a/lib/krb5/get_default_principal.c b/lib/krb5/get_default_principal.c index e102e5a1f..55c698557 100644 --- a/lib/krb5/get_default_principal.c +++ b/lib/krb5/get_default_principal.c @@ -37,113 +37,37 @@ * Try to find out what's a reasonable default principal. */ -static const char* -get_env_user(void) -{ - const char *user = getenv("USER"); - if(user == NULL) - user = getenv("LOGNAME"); - if(user == NULL) - user = getenv("USERNAME"); - return user; -} - -#ifndef _WIN32 - -/* - * Will only use operating-system dependant operation to get the - * default principal, for use of functions that in ccache layer to - * avoid recursive calls. - */ - KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL _krb5_get_default_principal_local (krb5_context context, krb5_principal *princ) { - krb5_error_code ret; const char *user; - uid_t uid; + const char *second_component = NULL; + char userbuf[128]; *princ = NULL; - uid = getuid(); - if(uid == 0) { - user = getlogin(); - if(user == NULL) - user = get_env_user(); - if(user != NULL && strcmp(user, "root") != 0) - ret = krb5_make_principal(context, princ, NULL, user, "root", NULL); - else - ret = krb5_make_principal(context, princ, NULL, "root", NULL); - } else { - struct passwd *pw = getpwuid(uid); - if(pw != NULL) - user = pw->pw_name; - else { - user = get_env_user(); - if(user == NULL) - user = getlogin(); - } - if(user == NULL) { - krb5_set_error_message(context, ENOTTY, - N_("unable to figure out current " - "principal", "")); - return ENOTTY; /* XXX */ - } - ret = krb5_make_principal(context, princ, NULL, user, NULL); + /* + * NOTE: We depend on roken_get_username() preferentially using + * getlogin_r() first when !issuid() && getuid() == 0, otherwise we + * won't figure out to output /root@DEFAULT_REALM. + */ + user = roken_get_username(userbuf, sizeof(userbuf)); + if (user == NULL) { + krb5_set_error_message(context, ENOTTY, + N_("unable to figure out current principal", + "")); + return ENOTTY; /* XXX */ } - return ret; + +#ifndef WIN32 + if (!issuid() && getuid() == 0 && strcmp(user, "root") != 0) + second_component = "root"; /* We'll use /root */ +#endif; + return krb5_make_principal(context, princ, NULL, user, + second_component, NULL); } -#else /* _WIN32 */ - -#define SECURITY_WIN32 -#include - -KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL -_krb5_get_default_principal_local(krb5_context context, - krb5_principal *princ) -{ - /* See if we can get the principal first. We only expect this to - work if logged into a domain. */ - { - char username[1024]; - ULONG sz = sizeof(username); - - if (GetUserNameEx(NameUserPrincipal, username, &sz)) { - return krb5_parse_name_flags(context, username, - KRB5_PRINCIPAL_PARSE_ENTERPRISE, - princ); - } - } - - /* Just get the Windows username. This should pretty much always - work. */ - { - char username[1024]; - DWORD dsz = sizeof(username); - - if (GetUserName(username, &dsz)) { - return krb5_make_principal(context, princ, NULL, username, NULL); - } - } - - /* Failing that, we look at the environment */ - { - const char * username = get_env_user(); - - if (username == NULL) { - krb5_set_error_string(context, - "unable to figure out current principal"); - return ENOTTY; /* Really? */ - } - - return krb5_make_principal(context, princ, NULL, username, NULL); - } -} - -#endif - KRB5_LIB_FUNCTION krb5_error_code KRB5_LIB_CALL krb5_get_default_principal (krb5_context context, krb5_principal *princ)