diff --git a/api-auth.c b/api-auth.c index 55df461..f6af390 100644 --- a/api-auth.c +++ b/api-auth.c @@ -494,6 +494,37 @@ pam_sm_setcred(pam_handle_t *pamh, int flags, int argc, const char **argv) if (reinit) { const char *name, *k5name; + /* + * Solaris su calls pam_setcred as root with PAM_REINITIALIZE_CREDS, + * preserving the user-supplied environment. An xlock program may + * also do this if it's setuid root and doesn't drop credentials + * before calling pam_setcred. + * + * There isn't any safe way of reinitializing the exiting ticket cache + * for the user if we're setuid without calling setreuid(). Calling + * setreuid() is possible, but if the calling application is threaded, + * it will change credentials for the whole application, with possibly + * bizarre and unintended (and insecure) results. Trying to verify + * ownership of the existing ticket cache before using it fails under + * various race conditions (for example, having one of the elements of + * the path be a symlink and changing the target of that symlink + * between our check and the call to krb5_cc_resolve. Without calling + * setreuid(), we run the risk of replacing a file owned by another + * user with a credential cache. + * + * We could fail with an error in the setuid case, which would be + * maximally safe, but it would prevent use of the module for + * authentication with programs such as Solaris su. Failure to + * reinitialize the cache is normally not a serious problem, just a + * missing feature. We therefore log an error and exit with + * PAM_SUCCESS for the setuid case. + */ + if (pamk5_compat_issetugid()) { + pamk5_error(args, "credential reinitialization in a setuid" + " context ignored"); + pamret = PAM_SUCCESS; + goto done; + } name = pamk5_get_krb5ccname(args, "KRB5CCNAME"); if (name == NULL) name = krb5_cc_default_name(ctx->context); diff --git a/compat.c b/compat.c index e6ad6b0..1bf981d 100644 --- a/compat.c +++ b/compat.c @@ -24,6 +24,7 @@ # include #endif #include +#include #if !defined(HAVE_KRB5_GET_ERROR_MESSAGE) && !defined(HAVE_KRB5_GET_ERR_TEXT) # if !defined(HAVE_KRB5_GET_ERROR_STRING) @@ -146,6 +147,39 @@ pamk5_compat_free_error(krb5_context ctx, const char *msg) /* + * AIX's NAS Kerberos implementation mysteriously provides the struct and the + * krb5_verify_init_creds function but not this function. + */ +#ifndef HAVE_KRB5_VERIFY_INIT_CREDS_OPT_INIT +void +krb5_verify_init_creds_opt_init(krb5_verify_init_creds_opt *opt) +{ + opt->flags = 0; + opt->ap_req_nofail = 0; +} +#endif + + +/* + * MIT provides a krb5_init_secure_context that ignores all the environment + * variables that may otherwise influence context creation. We call that + * function if we detect that we're setuid. Heimdal doesn't have this + * function, but instead automatically ignores the environment variables if it + * detects we're setuid. This means that we should be able to fall back + * safely to krb5_init_context if krb5_init_secure_context isn't available. + */ +krb5_error_code +pamk5_compat_secure_context(krb5_context *ctx) +{ +#ifdef HAVE_KRB5_INIT_SECURE_CONTEXT + return krb5_init_secure_context(ctx); +#else + return krb5_init_context(ctx); +#endif +} + + +/* * Linux PAM provides a thread-safe version of getpwnam that we want to use if * available. If it's not, fall back on getpwnam. (Ideally, we should check * for getpwnam_r and use it, but I haven't written that routine.) @@ -162,14 +196,19 @@ pamk5_compat_getpwnam(struct pam_args *args UNUSED, const char *user) /* - * AIX's NAS Kerberos implementation mysteriously provides the struct and the - * krb5_verify_init_creds function but not this function. + * Call the Solaris issetugid function if available. If not, check whether + * the real and effective UIDs and GIDs match. */ -#ifndef HAVE_KRB5_VERIFY_INIT_CREDS_OPT_INIT -void -krb5_verify_init_creds_opt_init(krb5_verify_init_creds_opt *opt) +int +pamk5_compat_issetugid(void) { - opt->flags = 0; - opt->ap_req_nofail = 0; -} +#ifdef HAVE_ISSETUGID + return issetugid(); +#else + if (getuid() != geteuid()) + return 1; + if (getgid() != getegid()) + return 1; + return 0; #endif +} diff --git a/configure.ac b/configure.ac index 6835a2d..2d04f58 100644 --- a/configure.ac +++ b/configure.ac @@ -21,6 +22,10 @@ AC_PROG_MAKE_SET AC_CANONICAL_HOST AC_AIX +dnl Check for the Solaris issetugid function, which is nicer than comparing +dnl real and effective UIDs and GIDs. +AC_CHECK_FUNCS([issetugid]) + dnl Probe for the functionality of the PAM libraries and their include file dnl naming. Mac OS X puts them in pam/* instead of security/*. AC_SEARCH_LIBS([pam_set_data], [pam]) @@ -46,6 +51,7 @@ AC_CHECK_FUNCS([krb5_appdefault_string \ krb5_get_init_creds_opt_set_change_password_prompt \ krb5_get_init_creds_opt_set_default_flags \ krb5_get_init_creds_opt_set_pa \ + krb5_init_secure_context \ krb5_verify_init_creds_opt_init]) AC_CHECK_FUNCS([krb5_get_init_creds_opt_set_pkinit], [RRA_FUNC_KRB5_GET_INIT_CREDS_OPT_SET_PKINIT_ARGS]) diff --git a/context.c b/context.c index 9a70aa7..8475d81 100644 --- a/context.c +++ b/context.c @@ -66,7 +66,10 @@ pamk5_context_new(struct pam_args *args) goto done; } ctx->name = strdup(name); - retval = krb5_init_context(&ctx->context); + if (pamk5_compat_issetugid()) + retval = pamk5_compat_secure_context(&ctx->context); + else + retval = krb5_init_context(&ctx->context); if (retval != 0) { pamk5_error_krb5(args, "krb5_init_context", retval); retval = PAM_SERVICE_ERR; diff --git a/internal.h b/internal.h index 48c5b74..7356e8a 100644 --- a/internal.h +++ b/internal.h @@ -247,6 +247,12 @@ krb5_error_code pamk5_compat_set_realm(struct pam_args *, const char *) __attribute__((__visibility__("hidden"))); void pamk5_compat_free_realm(struct pam_args *) __attribute__((__visibility__("hidden"))); +krb5_error_code pamk5_compat_secure_context(krb5_context *) + __attribute__((__visibility__("hidden"))); + +/* Calls issetugid if available, otherwise checks effective IDs. */ +int pamk5_compat_issetugid(void) + __attribute__((__visibility__("hidden"))); /* Calls pam_modutil_getpwnam if available, otherwise getpwnam. */ struct passwd *pamk5_compat_getpwnam(struct pam_args *, const char *) diff --git a/options.c b/options.c index b03ee0a..e8f9da5 100644 --- a/options.c +++ b/options.c @@ -276,7 +276,10 @@ pamk5_args_parse(pam_handle_t *pamh, int flags, int argc, const char **argv) * proceed; we'll die soon enough later and this way we'll die after we * know whether to debug things. */ - retval = krb5_init_context(&c); + if (pamk5_compat_issetugid()) + retval = pamk5_compat_secure_context(&c); + else + retval = krb5_init_context(&c); if (retval != 0) c = NULL; if (c != NULL) {