diff --git a/api-auth.c b/api-auth.c index 4a8a75a..c0f1b3d 100644 --- a/api-auth.c +++ b/api-auth.c @@ -542,6 +542,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 (getuid() != geteuid() || getgid() != getegid()) { + pamk5_error(args, "credential reinitialization in a setuid" + " context ignored"); + pamret = PAM_SUCCESS; + goto done; + } name = get_krb5ccname(args, "KRB5CCNAME"); if (name == NULL) name = krb5_cc_default_name(ctx->context); diff --git a/context.c b/context.c index d4a6ed3..59fe29a 100644 --- a/context.c +++ b/context.c @@ -22,6 +22,7 @@ #endif #include #include +#include #include "internal.h" @@ -31,6 +32,11 @@ # define PAM_INCOMPLETE PAM_SERVICE_ERR #endif +/* Heimdal doesn't need krb5_init_secure_context. */ +#if HAVE_KRB5_HEIMDAL +# define krb5_init_secure_context(c) krb5_init_context(c) +#endif + /* * Create a new context and populate it with the user from PAM and a new * Kerberos context. Set the default realm if one was configured. @@ -63,7 +69,10 @@ pamk5_context_new(struct pam_args *args) goto done; } ctx->name = strdup(name); - retval = krb5_init_context(&ctx->context); + if (getuid() != geteuid() || getgid() != getegid()) + retval = krb5_init_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/options.c b/options.c index 7ecbf0f..5ed3add 100644 --- a/options.c +++ b/options.c @@ -16,9 +16,15 @@ #include #include #include +#include #include "internal.h" +/* Heimdal doesn't need krb5_init_secure_context. */ +#if HAVE_KRB5_HEIMDAL +# define krb5_init_secure_context(c) krb5_init_context(c) +#endif + /* * Not all platforms have this, so just implement it ourselves. Copy a * certain number of characters of a string into a newly allocated @@ -275,7 +281,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 (getuid() != geteuid() || getgid() != getegid()) + retval = krb5_init_secure_context(&c); + else + retval = krb5_init_context(&c); if (retval != 0) c = NULL; if (c != NULL) {