Significant security fixes to ksu

* Use source euid all throughout procedure of opening source ccache,
Richard's code had a race condition.

* Use target euid while looking up .k5login and constructing target ccache.

* Avoid chowns completely; they create race conditions.  Fchown could
have been used if we wanted to be really careful, but they aren't
necessary and we would have to violate abstractions.

* Clean up several conditions that would allow users to delete
arbitrary files of the user they were ksuing to without authorization.

git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@8049 dc483132-0cff-0310-8789-dd5450dbe970

diff --git a/src/clients/ksu/ChangeLog b/src/clients/ksu/ChangeLog
index e3b5d2dd33da56809ae447fca4f936b8a3ba8450..bba29b6e0b06387f2ba9e13c6b0b6f82cd752bcb 100644 (file)
--- a/src/clients/ksu/ChangeLog
+++ b/src/clients/ksu/ChangeLog
@@ -1,3 +1,27 @@
+Sun May 19 13:41:21 1996  Sam Hartman  <hartmans@mit.edu>
+
+       * main.c: Force source ccache to stay open between transactions.
+
+Sun May 19 03:24:26 1996  Sam Hartman  <hartmans@tertius.mit.edu>
+
+       * krb_auth_su.c: Use target uid while creating ccache
+
+       * ccache.c: Set uid to target before creating target cache.
+
+       * ksu.h: Add target_uid to copy_ccache and copy_ccache_restricted
+
+Sat May 18 16:39:15 1996  Sam Hartman  <hartmans@tertius.mit.edu>
+
+       * configure.in: Use libkrb5util to get krb5_seteuid
+
+       * heuristic.c (get_best_princ_for_target): Remove seteuid around stat call and insert call to krb5_seteuid before accessing .k5login or .k5users.
+
+       * main.c (main): Insert appropriate calls to krb5_seteuid so that
+        files are accessed as appropriate.  Also, remove code to copy
+        tickets obtained while running ksu overthe source cache; this is
+        not appropriate because it changes the ownership of the source
+        cache.
+
 Mon May 13 06:50:12 1996  Richard Basch  <basch@lehman.com>
 
        * authorization.c: users in the .k5login were not permitted to use

diff --git a/src/clients/ksu/ccache.c b/src/clients/ksu/ccache.c
index 95cbe84c4fcce14f6de7e95850d048ee69ce8979..69975204e111c60c8b698180f674a9d3d0000b3c 100644 (file)
--- a/src/clients/ksu/ccache.c
+++ b/src/clients/ksu/ccache.c
@@ -44,12 +44,13 @@ void show_credential();
 */
 
 krb5_error_code krb5_ccache_copy (context, cc_def, cc_other_tag, 
-                                 primary_principal, cc_out, stored)
+                                 primary_principal, cc_out, stored, target_uid)
     /* IN */
     krb5_context context;
     krb5_ccache cc_def;
     char *cc_other_tag;
     krb5_principal primary_principal;
+uid_t target_uid;
     /* OUT */
     krb5_ccache *cc_out;
     krb5_boolean *stored;
@@ -74,6 +75,7 @@ struct stat st_temp;
     cc_def_name = krb5_cc_get_name(context, cc_def);    
     cc_other_name = krb5_cc_get_name(context, *cc_other);    
 
+
     if ( ! stat(cc_def_name, &st_temp)){
        if((retval = krb5_get_nonexp_tkts(context,cc_def,&cc_def_creds_arr))){
                return retval;
@@ -83,7 +85,19 @@ struct stat st_temp;
     *stored = krb5_find_princ_in_cred_list(context, cc_def_creds_arr,
                                           primary_principal);
 
-
+#ifdef HAVE_LSTAT
+    if (!lstat( cc_other_name, &st_temp)) {
+#else /*HAVE_LSTAT*/
+    if (!stat( cc_other_name, &st_temp)) {
+#endif
+      return EINVAL;
+    }
+    
+      if (krb5_seteuid(0)||krb5_seteuid(target_uid)) {
+       return errno;
+      }
+      
+      
     if ((retval = krb5_cc_initialize(context, *cc_other, primary_principal))){
        return retval; 
     }
@@ -621,11 +635,12 @@ with k5 beta 3 release.
 ************************************************************************/
 
 krb5_error_code krb5_ccache_copy_restricted (context, cc_def, cc_other_tag, 
-                                            prst, cc_out, stored)
+                                            prst, cc_out, stored, target_uid)
     krb5_context context;
     krb5_ccache cc_def;
     char *cc_other_tag;
     krb5_principal prst;
+uid_t target_uid;
     /* OUT */
     krb5_ccache *cc_out;
     krb5_boolean *stored;
@@ -658,6 +673,19 @@ struct stat st_temp;
 
     }
 
+#ifdef HAVE_LSTAT
+    if (!lstat( cc_other_name, &st_temp)) {
+#else /*HAVE_LSTAT*/
+    if (!stat( cc_other_name, &st_temp)) {
+#endif
+      return EINVAL;
+    }
+    
+      if (krb5_seteuid(0)||krb5_seteuid(target_uid)) {
+       return errno;
+      }
+      
+    
     if ((retval = krb5_cc_initialize(context, *cc_other, prst))){
        return retval; 
     }

diff --git a/src/clients/ksu/configure.in b/src/clients/ksu/configure.in
index c626f488f72a33de446234bb696b324ae7f42326..6d66a3786272a48add010873150c3e3aac2c4276 100644 (file)
--- a/src/clients/ksu/configure.in
+++ b/src/clients/ksu/configure.in
@@ -4,8 +4,9 @@ AC_PROG_INSTALL
 AC_CHECK_LIB(ndbm,main)
 AC_CHECK_LIB(dbm,main)
 AC_HEADER_STDARG
-AC_CHECK_FUNCS(getusershell seteuid setreuid setresuid)
+AC_CHECK_FUNCS(getusershell lstat )
 AC_CHECK_HEADERS(unistd.h)
+USE_KRB5UTIL_LIBRARY
 KRB5_LIBRARIES
 V5_USE_SHARED_LIB
 V5_AC_OUTPUT_MAKEFILE

diff --git a/src/clients/ksu/heuristic.c b/src/clients/ksu/heuristic.c
index d1a0b4473de5fbc5c1e32b7793f8f094160861c6..e0ee71d5d63a10aece8773c19bc5f96e525b3599 100644 (file)
--- a/src/clients/ksu/heuristic.c
+++ b/src/clients/ksu/heuristic.c
@@ -555,52 +555,11 @@ krb5_error_code get_best_princ_for_target(context, source_uid, target_uid,
 
     cc_source_name = krb5_cc_get_name(context, cc_source);
 
-       /* Reset the euid while we open the source ccache */
-#if defined(_POSIX_SAVED_IDS) && defined(HAVE_SETEUID)
-    if (seteuid(source_uid)) {
-       com_err(prog_name, errno, "while setting the effective uid");
-       exit(1);
-    }
-#else
-# if defined(HAVE_SETRESUID)
-    if (setresuid(-1, source_uid, -1)) {
-       com_err(prog_name, errno, "while setting the effective uid");
-       exit(1);
-    }
-# else
-#  if defined(HAVE_SETREUID)
-    if (setreuid(0, source_uid)) {
-       com_err(prog_name, errno, "while setting the real/effective uid");
-       exit(1);
-    }
-#  endif /* HAVE_SETREUID */
-# endif /* HAVE_SETRESUID */
-#endif /* _POSIX_SAVED_IDS */
        
     if (! stat(cc_source_name, &st_temp))
        if (retval = krb5_cc_get_principal(context, cc_source, &cc_def_princ))
            return retval;
 
-#if defined(_POSIX_SAVED_IDS) && defined(HAVE_SETEUID)
-    if (seteuid(0)) {
-       com_err(prog_name, errno, "while setting the effective uid");
-       exit(1);
-    }
-#else
-# if defined(HAVE_SETRESUID)
-    if (setresuid(-1, 0, -1)) {
-       com_err(prog_name, errno, "while setting the effective uid");
-       exit(1);
-    }
-# else
-#  if defined(HAVE_SETREUID)
-    if (setreuid(source_uid, 0)) {
-       com_err(prog_name, errno, "while setting the real/effective uid");
-       exit(1);
-    }
-#  endif /* HAVE_SETREUID */
-# endif /* HAVE_SETRESUID */
-#endif /* _POSIX_SAVED_IDS */
        
     if (retval=krb5_parse_name(context, target_user, &target_client))
        return retval;
@@ -636,6 +595,11 @@ krb5_error_code get_best_princ_for_target(context, source_uid, target_uid,
        return 0;
     }
 
+    /* Become root, then target for looking at .k5login.*/
+    if (krb5_seteuid(0) || krb5_seteuid(target_uid) ) {
+      return errno;
+    }
+    
     /* if .k5users and .k5login do not exist */        
     if (stat(k5login_path, &tb) && stat(k5users_path, &tb) ){
        *client = target_client;

diff --git a/src/clients/ksu/krb_auth_su.c b/src/clients/ksu/krb_auth_su.c
index dbcf1a283254c10623bd0978fc1d4f971003d92f..de7d055007d4f7b18ac4c85308fc6cc5e6311224 100644 (file)
--- a/src/clients/ksu/krb_auth_su.c
+++ b/src/clients/ksu/krb_auth_su.c
@@ -52,12 +52,13 @@ krb5_preauthtype * preauth_ptr = NULL;
 
 
 krb5_boolean krb5_auth_check(context, client_pname, hostname, options,
-                            target_user, cc, path_passwd)
+                            target_user, cc, path_passwd, target_uid)
     krb5_context context;
     krb5_principal client_pname;
     char *hostname;
     opt_info *options;
     char *target_user;
+uid_t target_uid;
     krb5_ccache cc;
     int *path_passwd;
 {
@@ -143,6 +144,11 @@ krb5_boolean zero_password;
        if (! got_it){
 
 #ifdef GET_TGT_VIA_PASSWD
+         if (krb5_seteuid(0)||krb5_seteuid(target_uid)) {
+           com_err("ksu", errno, "while switching to target uid");
+           return FALSE;
+         }
+         
 
                fprintf(stderr,"WARNING: Your password may be exposed if you enter it here and are logged \n");
                fprintf(stderr,"         in remotely using an unsecure (non-encrypted) channel. \n");
@@ -150,9 +156,16 @@ krb5_boolean zero_password;
                /*get the ticket granting ticket, via passwd(promt for passwd)*/
                if (krb5_get_tkt_via_passwd (context, &cc, client, tgtq.server,
                                       options, & zero_password) == FALSE){ 
+krb5_seteuid(0);
+
                                return FALSE;
                }
                *path_passwd = 1;
+               if (krb5_seteuid(0)) {
+                 com_err("ksu", errno, "while reclaiming root uid");
+                 return FALSE;
+               }
+
 #else
        plain_dump_principal (client);
        fprintf(stderr,"does not have any appropriate tickets in the cache.\n");

diff --git a/src/clients/ksu/ksu.h b/src/clients/ksu/ksu.h
index fd8232fb204bcda9e5f0b779546fd2201618361d..2f6f9686d3ce1d890aa80c922c990efbea82c8ca 100644 (file)
--- a/src/clients/ksu/ksu.h
+++ b/src/clients/ksu/ksu.h
@@ -79,7 +79,7 @@ typedef struct opt_info{
 /* krb_auth_su.c */
 extern krb5_boolean krb5_auth_check
         PROTOTYPE((krb5_context, krb5_principal, char *, opt_info *,
-                  char *, krb5_ccache, int *));
+                  char *, krb5_ccache, int *, uid_t));
 
 extern krb5_boolean krb5_fast_auth
         PROTOTYPE((krb5_context, krb5_principal, krb5_principal, char *,
@@ -105,7 +105,7 @@ extern krb5_error_code get_best_principal
 /* ccache.c */
 extern krb5_error_code krb5_ccache_copy
        PROTOTYPE((krb5_context, krb5_ccache, char *, krb5_principal, 
-                  krb5_ccache *, krb5_boolean *));
+                  krb5_ccache *, krb5_boolean *, uid_t));
 
 extern krb5_error_code krb5_store_all_creds
        PROTOTYPE((krb5_context, krb5_ccache, krb5_creds **, krb5_creds **));
@@ -141,7 +141,7 @@ extern krb5_error_code krb5_store_some_creds
 
 extern krb5_error_code krb5_ccache_copy_restricted
        PROTOTYPE((krb5_context, krb5_ccache, char *, krb5_principal, 
-                  krb5_ccache *, krb5_boolean *));
+                  krb5_ccache *, krb5_boolean *, uid_t));
 
 extern krb5_error_code krb5_ccache_refresh
        PROTOTYPE((krb5_context, krb5_ccache));

diff --git a/src/clients/ksu/main.c b/src/clients/ksu/main.c
index e8003f3fdb88ec1d4aef2a39e6dc7c420d23b4dd..b09fd6fe198f35ce112b30f175284eb2c31b163f 100644 (file)
--- a/src/clients/ksu/main.c
+++ b/src/clients/ksu/main.c
@@ -67,6 +67,9 @@ void usage (){
 
 #define DEBUG
 
+/* These are file static so sweep_up can get to them*/
+static uid_t source_uid, target_uid;
+
 int
 main (argc, argv)
        int argc;
@@ -90,7 +93,6 @@ char * source_user;
 
 krb5_ccache cc_source = NULL;
 char * cc_source_tag = NULL; 
-uid_t source_uid, target_uid;
 uid_t source_gid, target_gid;
 char * cc_source_tag_tmp = NULL;
 char * cc_target_tag_tmp=NULL; 
@@ -375,6 +377,10 @@ char * dir_of_cc_source;
                if (cc_source_tag_tmp == (char *) 1) 
                        cc_source_tag_tmp = cc_source_tag;
        }
+       if (krb5_seteuid(source_uid)) {
+         com_err ( prog_name, errno, "while setting euid to source user");
+         exit(1);
+       }
        
        /* get a handle for the cache */      
        if ((retval = krb5_cc_resolve(ksu_context, cc_source_tag, &cc_source))){
@@ -382,6 +388,12 @@ char * dir_of_cc_source;
                exit(1);
        }
 
+       if (((retval = krb5_cc_set_flags(ksu_context,  cc_source, 0x0)) != 0)
+           && (retval != KRB5_FCC_NOFILE)) {
+         com_err(prog_name, retval, "while opening ccache");
+         exit(1);
+       }
+         
        if ((retval = get_best_princ_for_target(ksu_context, source_uid,
                        target_uid, source_user, target_user, cc_source, 
                        &options, cmd, localhostname, &client, &hp))){
@@ -389,6 +401,15 @@ char * dir_of_cc_source;
                exit(1);
        }
 
+       /* We may be running as either source or target, depending on
+          what happened; become source.*/
+       if ( geteuid() != source_uid) {
+         if (krb5_seteuid(0) || krb5_seteuid(source_uid) ) {
+           com_err(prog_name, errno, "while returning to source uid after finding best principal");
+           exit(1);
+         }
+       }
+       
        if (auth_debug){
                if (hp){        
                        fprintf(stderr,
@@ -426,12 +447,6 @@ char * dir_of_cc_source;
                                        "while initializing source cache");    
                                exit(1);
                        }
-                       if (chown(cc_source_tag_tmp, source_uid, source_gid)){  
-                               com_err(prog_name, errno, 
-                                       "while changing owner for %s",
-                                       cc_source_tag_tmp);   
-                               exit(1);
-                       }
                }
        }
 
@@ -439,13 +454,17 @@ char * dir_of_cc_source;
        if (cc_target_tag == NULL) {
 
                cc_target_tag = (char *)calloc(KRB5_SEC_BUFFSIZE ,sizeof(char));
+               /* make sure that the new ticket file does not already exist
+                  This is run as source_uid because it is reasonable to
+                  require the source user to have write to where the target
+                  cache will be created.*/
+               
                do {
                        sprintf(cc_target_tag, "%s%d.%d", KRB5_SECONDARY_CACHE,
                                target_uid, gen_sym());
                        cc_target_tag_tmp = strchr(cc_target_tag, ':') + 1;
 
                }while ( !stat ( cc_target_tag_tmp, &st_temp)); 
-               /* make sure that the new ticket file does not already exist */ 
        }
 
 
@@ -469,6 +488,11 @@ char * dir_of_cc_source;
           takes place, the target user becomes the owner of the cache.         
         */           
 
+       /* we continue to run as source uid until
+          the middle of the copy, when becomewe become the target user
+          The cache is owned by the target user.*/
+       
+       
        if (! use_source_cache){
                        
                /* if root ksu's to a regular user, then      
@@ -478,16 +502,16 @@ char * dir_of_cc_source;
                if ((source_uid == 0) && (target_uid != 0)) {
 
                        if ((retval = krb5_ccache_copy_restricted(ksu_context,  cc_source,
-                               cc_target_tag, client, &cc_target, &stored))){
+                               cc_target_tag, client, &cc_target, &stored, target_uid))){
                                com_err (prog_name, retval, 
                                     "while copying cache %s to %s",
                                     krb5_cc_get_name(ksu_context, cc_source),cc_target_tag);
                                exit(1);
                        }
 
-        } else{
+               } else{
                        if ((retval = krb5_ccache_copy(ksu_context, cc_source, cc_target_tag,
-                                            client,&cc_target, &stored))){
+                                            client,&cc_target, &stored, target_uid))){
                                com_err (prog_name, retval, 
                                        "while copying cache %s to %s",
                                        krb5_cc_get_name(ksu_context, cc_source),
@@ -508,6 +532,13 @@ char * dir_of_cc_source;
                                "while searching for client in source ccache");
                                exit(1);
                }
+
+       }
+               /* Become root for authentication*/
+
+       if (krb5_seteuid(0)) {
+       com_err(prog_name, errno, "while reclaiming root uid");
+       exit(1);
        }
 
        if ((source_uid == 0) || (target_uid == source_uid)){
@@ -553,9 +584,9 @@ char * dir_of_cc_source;
                char * client_name;
 
                        auth_val = krb5_auth_check(ksu_context, client, localhostname, &options,
-                               target_user,cc_target, &path_passwd); 
+                               target_user,cc_target, &path_passwd, target_uid); 
                
-               /* if kerbereros authentication failed then exit */     
+               /* if Kerberos authentication failed then exit */     
                if (auth_val ==FALSE){
                        fprintf(stderr, "Authentication failed.\n");
                         syslog(LOG_WARNING,
@@ -565,6 +596,12 @@ char * dir_of_cc_source;
                        exit(1);
                }
 
+#if 0
+               /* At best, this avoids a single kdc request
+                  It is hard to implement dealing with file permissions and
+                  is unnecessary.  It is important
+                  to properly handle races in chown if this code is ever re-enabled.
+                  */
                /* cache the tickets if possible in the source cache */ 
                if (!path_passwd && !use_source_cache){         
 
@@ -584,7 +621,8 @@ char * dir_of_cc_source;
                                exit(1);
                        }
                }
-                       
+#endif /*0*/
+
                if ((retval = krb5_unparse_name(ksu_context, client, &client_name))) {
                                 com_err (prog_name, retval, "When unparsing name");
                         sweep_up(ksu_context, use_source_cache, cc_target);
@@ -596,13 +634,26 @@ char * dir_of_cc_source;
                        prog_name,target_user,client_name,
                        source_user,ontty());
 
+               /* Run authorization as target.*/
+               if (krb5_seteuid(target_uid)) {
+                 com_err(prog_name, errno, "whiel switching to target for authorization check");
+                   sweep_up(ksu_context, use_source_cache, cc_target);
+                 exit(1);
+               }
+               
                if ((retval = krb5_authorization(ksu_context, client,target_user,
                         cmd, &authorization_val, &exec_cmd))){
                               com_err(prog_name,retval,"while checking authorization");
+krb5_seteuid(0); /*So we have some chance of sweeping up*/
                       sweep_up(ksu_context, use_source_cache, cc_target);
                       exit(1);
                }
 
+               if (krb5_seteuid(0)) {
+                 com_err(prog_name, errno, "while switching back from  target after authorization check");
+                   sweep_up(ksu_context, use_source_cache, cc_target);
+                 exit(1);
+               }
                if (authorization_val == TRUE){
 
                   if (cmd) {   
@@ -720,15 +771,6 @@ char * dir_of_cc_source;
 
        if (!use_source_cache){ 
 
-       /* set up ownership  on cache for target user */       
-
-               if (chown(cc_target_tag_tmp, target_uid, target_gid)){      
-                       com_err(prog_name, errno, "while changing owner for %s",
-                               cc_target_tag_tmp);   
-                       sweep_up(ksu_context, use_source_cache, cc_target);
-                       exit(1);
-               }
-
        }
        
        /* set permissions */
@@ -884,7 +926,10 @@ krb5_error_code retval;
 char * cc_name;
 struct stat  st_temp;
 
-       if (! use_source_cache){
+krb5_seteuid(0);
+krb5_seteuid(target_uid);
+
+if (! use_source_cache){
                cc_name = krb5_cc_get_name(context, cc);
                if ( ! stat(cc_name, &st_temp)){
                        if ((retval = krb5_cc_destroy(context, cc))){