From: Sam Hartman Date: Sun, 19 May 1996 18:52:51 +0000 (+0000) Subject: Significant security fixes to ksu X-Git-Tag: krb5-1.0-beta6~67 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=f7ef18fa8444d74f7ad6e3c1ae6804ef00b24f79;p=krb5.git 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 e3b5d2dd3..bba29b6e0 100644 --- a/src/clients/ksu/ChangeLog +++ b/src/clients/ksu/ChangeLog @@ -1,3 +1,27 @@ +Sun May 19 13:41:21 1996 Sam Hartman + + * main.c: Force source ccache to stay open between transactions. + +Sun May 19 03:24:26 1996 Sam Hartman + + * 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 + + * 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 * 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 95cbe84c4..69975204e 100644 --- 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 c626f488f..6d66a3786 100644 --- 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 d1a0b4473..e0ee71d5d 100644 --- 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 dbcf1a283..de7d05500 100644 --- 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 fd8232fb2..2f6f9686d 100644 --- 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 e8003f3fd..b09fd6fe1 100644 --- 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))){