[patch] pam_krb5-bug257075.patch

pam_krb5-bug257075.patch (text/plain), 8.93 KB, created by Robert Buchholz (RETIRED) on 2009-01-31 10:46:35 UTC

diff --git a/NEWS b/NEWS
index 925d27b..1b35249 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,29 @@
 
 pam-krb5 3.13 (unreleased)
 
+    SECURITY: When built against MIT Kerberos, if pam_krb5 is called in a
+    setuid context (effective UID or GID doesn't match the real UID or
+    GID), use krb5_init_secure_context instead of krb5_init_context.  This
+    ignores environment variable settings for the local Kerberos
+    configuration and keytab.  Previous versions could allow a local
+    attacker to point a setuid program that used PAM authentication at a
+    different Kerberos configuration under the attacker's control,
+    possibly resulting in privilege escalation.  Heimdal handles this
+    logic within the Kerberos libraries and therefore was not affected.
+
+    SECURITY: Disable pam_setcred(PAM_REINITIALIZE_CREDS) for setuid
+    applications.  If pam_krb5 detects this call in a setuid context, it
+    now logs an error and returns success without doing anything.  Solaris
+    su calls pam_setcred with that option rather than PAM_ESTABLISH_CREDS
+    after authentication and without wiping the environment, leading
+    previous versions of pam_krb5 to trust the KRB5CCNAME environment
+    variable for the ticket cache location.  This permitted an attacker to
+    use previous versions of pam_krb5 to overwrite arbitrary files with
+    Kerberos credential caches that were left owned by the attacker.
+    Setuid screen lock programs may also be affected.  Discovered by Derek
+    Chan and reported by Steven Luo.  Thanks to Sam Hartman and Jeffrey
+    Hutzelman for additional analysis.
+
     If a prefix of /usr is requested at configure time, install the PAM
     module into /lib/security or /lib64/security on Linux, matching the
     standard Linux-PAM module location.  Use lib64 instead of lib on
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 <security/pam_modutil.h>
 #endif
 #include <stdlib.h>
+#include <unistd.h>
 
 #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) {