From: Tom Yu Date: Tue, 4 Sep 2007 18:53:09 +0000 (+0000) Subject: fix CVE-2007-4000 modify_policy vulnerability X-Git-Tag: krb5-1.7-alpha1~893 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=f92298e603212c0d1812e8f2955e9e02b4005ca7;p=krb5.git fix CVE-2007-4000 modify_policy vulnerability In kadm5_modify_policy_internal, check for nonexistence of policy before doing anything with it, to avoid memory corruption. ticket: new target_version: 1.6.3 tags: pullup git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@19914 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/lib/kadm5/srv/svr_policy.c b/src/lib/kadm5/srv/svr_policy.c index d57d2f158..512876b79 100644 --- a/src/lib/kadm5/srv/svr_policy.c +++ b/src/lib/kadm5/srv/svr_policy.c @@ -211,8 +211,9 @@ kadm5_modify_policy_internal(void *server_handle, if((mask & KADM5_POLICY)) return KADM5_BAD_MASK; - ret = krb5_db_get_policy(handle->context, entry->policy, &p, &cnt); - if( ret && (cnt==0) ) + if ((ret = krb5_db_get_policy(handle->context, entry->policy, &p, &cnt))) + return ret; + if (cnt != 1) return KADM5_UNK_POLICY; if ((mask & KADM5_PW_MAX_LIFE)) diff --git a/src/lib/krb5/krb/gc_frm_kdc.c b/src/lib/krb5/krb/gc_frm_kdc.c index 506538ca4..e8fd07207 100644 --- a/src/lib/krb5/krb/gc_frm_kdc.c +++ b/src/lib/krb5/krb/gc_frm_kdc.c @@ -92,6 +92,7 @@ struct tr_state { krb5_creds *cur_cc_tgt; krb5_creds *nxt_cc_tgt; unsigned int ntgts; + krb5_creds *offpath_tgt; }; /* @@ -168,7 +169,11 @@ static krb5_error_code init_rtree(struct tr_state *, static krb5_error_code do_traversal(krb5_context ctx, krb5_ccache, krb5_principal client, krb5_principal server, krb5_creds *out_cc_tgt, krb5_creds **out_tgt, - krb5_creds ***out_kdc_tgts); + krb5_creds ***out_kdc_tgts, int *tgtptr_isoffpath); +static krb5_error_code chase_offpath(struct tr_state *, krb5_principal, + krb5_principal); +static krb5_error_code offpath_loopchk(struct tr_state *ts, + krb5_creds *tgt, krb5_creds *reftgts[], int rcount); static krb5_error_code krb5_get_cred_from_kdc_opt(krb5_context, krb5_ccache, krb5_creds *, krb5_creds **, krb5_creds ***, int); @@ -434,6 +439,7 @@ find_nxt_kdc(struct tr_state *ts) krb5_principal *kdcptr; TR_DBG(ts, "find_nxt_kdc"); + assert(ts->ntgts > 0); assert(ts->nxt_tgt == ts->kdc_tgts[ts->ntgts-1]); if (krb5_princ_size(ts->ctx, ts->nxt_tgt->server) != 2) return KRB5_KDCREP_MODIFIED; @@ -448,21 +454,37 @@ find_nxt_kdc(struct tr_state *ts) break; } } - if (*kdcptr == NULL) { + if (*kdcptr != NULL) { + ts->nxt_kdc = kdcptr; + TR_DBG_RET(ts, "find_nxt_kdc", 0); + return 0; + } + + r2 = krb5_princ_component(ts->ctx, ts->kdc_list[0], 1); + if (r1 != NULL && r2 != NULL && data_eq(*r1, *r2)) { + TR_DBG_RET(ts, "find_nxt_kdc: looped back to local", + KRB5_KDCREP_MODIFIED); + return KRB5_KDCREP_MODIFIED; + } + + /* + * Realm is not in our list; we probably got an unexpected realm + * referral. + */ + ts->offpath_tgt = ts->nxt_tgt; + if (ts->cur_kdc == ts->kdc_list) { /* - * Not found; we probably got an unexpected realm referral. - * Don't touch NXT_KDC, thus allowing next_closest_tgt() to - * continue looping backwards. + * Local KDC referred us off path; trust it for caching + * purposes. */ - if (ts->ntgts > 0) { - /* Punt NXT_TGT from KDC_TGTS if bogus. */ - krb5_free_creds(ts->ctx, ts->kdc_tgts[--ts->ntgts]); - ts->kdc_tgts[ts->ntgts] = NULL; - } - TR_DBG_RET(ts, "find_nxt_kdc", KRB5_KDCREP_MODIFIED); - return KRB5_KDCREP_MODIFIED; + return 0; } - ts->nxt_kdc = kdcptr; + /* + * Unlink the off-path TGT from KDC_TGTS but don't free it, + * because we should return it. + */ + ts->kdc_tgts[--ts->ntgts] = NULL; + ts->nxt_tgt = ts->cur_tgt; TR_DBG_RET(ts, "find_nxt_kdc", 0); return 0; } @@ -577,10 +599,8 @@ next_closest_tgt(struct tr_state *ts, krb5_principal client) break; } /* - * Because try_kdc() validates referral TGTs, it can return an - * error indicating a bogus referral. The loop continues when - * it gets a bogus referral, which is arguably the right - * thing. (Previous implementation unconditionally failed.) + * In case of errors in try_kdc() or find_nxt_kdc(), continue + * looping through the KDC list. */ } /* @@ -689,7 +709,8 @@ do_traversal(krb5_context ctx, krb5_principal server, krb5_creds *out_cc_tgt, krb5_creds **out_tgt, - krb5_creds ***out_kdc_tgts) + krb5_creds ***out_kdc_tgts, + int *tgtptr_isoffpath) { krb5_error_code retval; struct tr_state state, *ts; @@ -717,13 +738,23 @@ do_traversal(krb5_context ctx, retval = next_closest_tgt(ts, client); if (retval) goto cleanup; + + if (ts->offpath_tgt != NULL) { + retval = chase_offpath(ts, client, server); + if (retval) + goto cleanup; + break; + } assert(ts->cur_kdc != ts->nxt_kdc); } if (NXT_TGT_IS_CACHED(ts)) { + assert(ts->offpath_tgt = NULL); *out_cc_tgt = *ts->cur_cc_tgt; *out_tgt = out_cc_tgt; MARK_CUR_CC_TGT_CLEAN(ts); + } else if (ts->offpath_tgt != NULL){ + *out_tgt = ts->offpath_tgt; } else { /* CUR_TGT is somewhere in KDC_TGTS; no need to copy. */ *out_tgt = ts->nxt_tgt; @@ -739,9 +770,121 @@ cleanup: free(ts->kdc_tgts); } else *out_kdc_tgts = ts->kdc_tgts; + *tgtptr_isoffpath = (ts->offpath_tgt != NULL); + return retval; +} + +/* + * chase_offpath() + * + * Chase off-path TGT referrals. + * + * If we are traversing a trusted path (either hierarchically derived + * or explicit capath) and get a TGT pointing to a realm off this + * path, query the realm referenced by that off-path TGT. Repeat + * until we get to the destination realm or encounter an error. + */ +static krb5_error_code +chase_offpath(struct tr_state *ts, + krb5_principal client, krb5_principal server) +{ + krb5_error_code retval; + krb5_creds mcred; + krb5_creds *cur_tgt, *nxt_tgt, *reftgts[KRB5_REFERRAL_MAXHOPS]; + krb5_data *rsrc, *rdst, *r1; + int rcount, i; + + rdst = krb5_princ_realm(ts->ctx, server); + cur_tgt = ts->offpath_tgt; + ts->offpath_tgt = NULL; + + for (rcount = 0; rcount < KRB5_REFERRAL_MAXHOPS; rcount++) { + nxt_tgt = NULL; + memset(&mcred, 0, sizeof(mcred)); + rsrc = krb5_princ_component(ts->ctx, cur_tgt->server, 1); + retval = krb5_tgtname(ts->ctx, rdst, rsrc, &mcred.server); + if (retval) + goto cleanup; + mcred.client = client; + retval = krb5_get_cred_via_tkt(ts->ctx, cur_tgt, + FLAGS2OPTS(cur_tgt->ticket_flags), + cur_tgt->addresses, &mcred, &nxt_tgt); + mcred.client = NULL; + krb5_free_principal(ts->ctx, mcred.server); + mcred.server = NULL; + if (retval) + goto cleanup; + if (!IS_TGS_PRINC(ts->ctx, nxt_tgt->server)) { + krb5_free_creds(ts->ctx, nxt_tgt); + retval = KRB5_KDCREP_MODIFIED; + goto cleanup; + } + r1 = krb5_princ_component(ts->ctx, nxt_tgt->server, 1); + if (data_eq(*rdst, *r1)) { + retval = 0; + goto cleanup; + } + /* Don't free CUR_TGT if it's in list of TGTs to cache. */ + if (cur_tgt != ts->nxt_tgt) { + krb5_free_creds(ts->ctx, cur_tgt); + } + retval = offpath_loopchk(ts, nxt_tgt, reftgts, rcount); + if (retval) + goto cleanup; + reftgts[rcount] = nxt_tgt; + cur_tgt = nxt_tgt; + nxt_tgt = NULL; + } + /* Max hop count exceeded. */ + retval = KRB5_KDCREP_MODIFIED; + +cleanup: + if (mcred.server != NULL) { + krb5_free_principal(ts->ctx, mcred.server); + } + /* Don't free CUR_TGT if it's in list of TGTs to cache. */ + if (cur_tgt != ts->nxt_tgt && rcount && cur_tgt != reftgts[rcount-1]) { + krb5_free_creds(ts->ctx, cur_tgt); + } + if (nxt_tgt != NULL) { + if (retval) + krb5_free_creds(ts->ctx, nxt_tgt); + else + ts->offpath_tgt = nxt_tgt; + } + for (i = 0; i < rcount; i++) { + krb5_free_creds(ts->ctx, reftgts[i]); + } return retval; } +/* + * offpath_loopchk() + * + * Check for loop back to previously-visited realms, both off-path and + * on-path. + */ +static krb5_error_code +offpath_loopchk(struct tr_state *ts, + krb5_creds *tgt, krb5_creds *reftgts[], int rcount) +{ + krb5_data *r1, *r2; + int i; + + r1 = krb5_princ_component(ts->ctx, tgt->server, 1); + for (i = 0; i < rcount; i++) { + r2 = krb5_princ_component(ts->ctx, reftgts[i]->server, 1); + if (data_eq(*r1, *r2)) + return KRB5_KDCREP_MODIFIED; + } + for (i = 0; i < ts->ntgts; i++) { + r2 = krb5_princ_component(ts->ctx, ts->kdc_tgts[i]->server, 1); + if (data_eq(*r1, *r2)) + return KRB5_KDCREP_MODIFIED; + } + return 0; +} + /* * krb5_get_cred_from_kdc_opt() * krb5_get_cred_from_kdc() @@ -786,6 +929,8 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache, krb5_error_code retval, subretval; krb5_principal client, server, supplied_server, out_supplied_server; krb5_creds tgtq, cc_tgt, *tgtptr, *referral_tgts[KRB5_REFERRAL_MAXHOPS]; + krb5_creds *otgtptr = NULL; + int tgtptr_isoffpath = 0; krb5_boolean old_use_conf_ktypes; char **hrealms; int referral_count, i; @@ -847,8 +992,10 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache, } else if (!HARD_CC_ERR(retval)) { DPRINTF(("gc_from_kdc: starting do_traversal to find " "initial TGT for referral\n")); + tgtptr_isoffpath = 0; + otgtptr = NULL; retval = do_traversal(context, ccache, client, server, - &cc_tgt, &tgtptr, tgts); + &cc_tgt, &tgtptr, tgts, &tgtptr_isoffpath); } if (retval) { DPRINTF(("gc_from_kdc: failed to find initial TGT for referral\n")); @@ -863,6 +1010,11 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache, * path, otherwise fall back to old-style assumptions. */ + /* + * Save TGTPTR because we rewrite it in the referral loop, and + * we might need to explicitly free it later. + */ + otgtptr = tgtptr; for (referral_count = 0; referral_count < KRB5_REFERRAL_MAXHOPS; referral_count++) { @@ -1061,6 +1213,11 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache, /* Free tgtptr data if reused from above. */ if (tgtptr == &cc_tgt) krb5_free_cred_contents(context, tgtptr); + tgtptr = NULL; + /* Free saved TGT in OTGTPTR if it was off-path. */ + if (tgtptr_isoffpath) + krb5_free_creds(context, otgtptr); + otgtptr = NULL; /* Free TGTS if previously filled by do_traversal() */ if (*tgts != NULL) { for (i = 0; (*tgts)[i] != NULL; i++) { @@ -1075,11 +1232,13 @@ krb5_get_cred_from_kdc_opt(krb5_context context, krb5_ccache ccache, if (!retval) { tgtptr = &cc_tgt; } else if (!HARD_CC_ERR(retval)) { + tgtptr_isoffpath = 0; retval = do_traversal(context, ccache, client, server, - &cc_tgt, &tgtptr, tgts); + &cc_tgt, &tgtptr, tgts, &tgtptr_isoffpath); } if (retval) goto cleanup; + otgtptr = tgtptr; /* * Finally have TGT for target realm! Try using it to get creds. @@ -1102,6 +1261,8 @@ cleanup: krb5_free_cred_contents(context, &tgtq); if (tgtptr == &cc_tgt) krb5_free_cred_contents(context, tgtptr); + if (tgtptr_isoffpath) + krb5_free_creds(context, otgtptr); context->use_conf_ktypes = old_use_conf_ktypes; /* Drop the original principal back into in_cred so that it's cached in the expected format. */