From: Ken Raeburn Date: Tue, 18 Mar 2008 20:07:14 +0000 (+0000) Subject: Fix MITKRB5-SA-2008-001 on trunk. Patch differs from the released one X-Git-Tag: krb5-1.7-alpha1~706 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=5b0bc6bcaeb275fa850815b56cb81086caef0c57;p=krb5.git Fix MITKRB5-SA-2008-001 on trunk. Patch differs from the released one for 1.6 because of code divergence. ticket: 5919 git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@20280 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/kdc/dispatch.c b/src/kdc/dispatch.c index 1d7c3bed7..ac0eb485d 100644 --- a/src/kdc/dispatch.c +++ b/src/kdc/dispatch.c @@ -107,7 +107,7 @@ dispatch(krb5_data *pkt, const krb5_fulladdr *from, krb5_data **response) retval = KRB5KRB_AP_ERR_MSG_TYPE; #ifndef NOCACHE /* put the response into the lookaside buffer */ - if (!retval) + if (!retval && *response != NULL) kdc_insert_lookaside(pkt, *response); #endif diff --git a/src/kdc/kerberos_v4.c b/src/kdc/kerberos_v4.c index 189cf7396..1ac951360 100644 --- a/src/kdc/kerberos_v4.c +++ b/src/kdc/kerberos_v4.c @@ -145,10 +145,8 @@ static void hang(void); #include "com_err.h" #include "extern.h" /* to pick up master_princ */ -static krb5_data *response; - -static void kerberos_v4 (struct sockaddr_in *, KTEXT); -static void kerb_err_reply (struct sockaddr_in *, KTEXT, long, char *); +static krb5_data *kerberos_v4 (struct sockaddr_in *, KTEXT); +static krb5_data *kerb_err_reply (struct sockaddr_in *, KTEXT, long, char *); static int set_tgtkey (char *, krb5_kvno, krb5_boolean); /* Attributes converted from V5 to V4 - internal representation */ @@ -264,12 +262,12 @@ process_v4(const krb5_data *pkt, const krb5_fulladdr *client_fulladdr, (void) klog(L_KRB_PERR, "V4 request too long."); return KRB5KRB_ERR_FIELD_TOOLONG; } + memset( &v4_pkt, 0, sizeof(v4_pkt)); v4_pkt.length = pkt->length; v4_pkt.mbz = 0; memcpy( v4_pkt.dat, pkt->data, pkt->length); - kerberos_v4( &client_sockaddr, &v4_pkt); - *resp = response; + *resp = kerberos_v4( &client_sockaddr, &v4_pkt); return(retval); } @@ -304,18 +302,20 @@ static char * v4_klog( int type, const char *format, ...) } static -int set_response(const char *msg, int len) +krb5_data *make_response(const char *msg, int len) { + krb5_data *response; + if ( !(response = (krb5_data *) malloc( sizeof *response))) { - return ENOMEM; + return 0; } if ( !(response->data = (char *) malloc( len))) { krb5_free_data(kdc_context, response); - return ENOMEM; + return 0; } response->length = len; memcpy( response->data, msg, len); - return( 0); + return response; } static void hang(void) @@ -591,7 +591,7 @@ static void str_length_check(char *str, int max_size) *cp = 0; } -void +static krb5_data * kerberos_v4(struct sockaddr_in *client, KTEXT pkt) { static KTEXT_ST rpkt_st; @@ -604,7 +604,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) KTEXT auth = &auth_st; AUTH_DAT ad_st; AUTH_DAT *ad = &ad_st; - + krb5_data *response = 0; static struct in_addr client_host; static int msg_byte_order; @@ -642,8 +642,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) inet_ntoa(client_host)); /* send an error reply */ req_name_ptr = req_inst_ptr = req_realm_ptr = ""; - kerb_err_reply(client, pkt, KERB_ERR_PKT_VER, lt); - return; + return kerb_err_reply(client, pkt, KERB_ERR_PKT_VER, lt); } /* check packet version */ @@ -653,8 +652,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) KRB_PROT_VERSION, req_version); /* send an error reply */ req_name_ptr = req_inst_ptr = req_realm_ptr = ""; - kerb_err_reply(client, pkt, KERB_ERR_PKT_VER, lt); - return; + return kerb_err_reply(client, pkt, KERB_ERR_PKT_VER, lt); } msg_byte_order = req_msg_type & 1; @@ -712,10 +710,10 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) if ((i = check_princ(req_name_ptr, req_inst_ptr, 0, &a_name_data, &k5key, 0, &ck5life))) { - kerb_err_reply(client, pkt, i, "check_princ failed"); + response = kerb_err_reply(client, pkt, i, "check_princ failed"); a_name_data.key_low = a_name_data.key_high = 0; krb5_free_keyblock_contents(kdc_context, &k5key); - return; + return response; } /* don't use k5key for client */ krb5_free_keyblock_contents(kdc_context, &k5key); @@ -727,11 +725,11 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) /* this does all the checking */ if ((i = check_princ(service, instance, lifetime, &s_name_data, &k5key, 1, &sk5life))) { - kerb_err_reply(client, pkt, i, "check_princ failed"); + response = kerb_err_reply(client, pkt, i, "check_princ failed"); a_name_data.key_high = a_name_data.key_low = 0; s_name_data.key_high = s_name_data.key_low = 0; krb5_free_keyblock_contents(kdc_context, &k5key); - return; + return response; } /* Bound requested lifetime with service and user */ v4req_end = krb_life_to_time(kerb_time.tv_sec, req_life); @@ -802,7 +800,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) rpkt = create_auth_reply(req_name_ptr, req_inst_ptr, req_realm_ptr, req_time_ws, 0, a_name_data.exp_date, a_name_data.key_version, ciph); - set_response((char *) rpkt->dat, rpkt->length); + response = make_response((char *) rpkt->dat, rpkt->length); memset(&a_name_data, 0, sizeof(a_name_data)); memset(&s_name_data, 0, sizeof(s_name_data)); break; @@ -828,9 +826,8 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) lt = klog(L_KRB_PERR, "APPL request with realm length too long from %s", inet_ntoa(client_host)); - kerb_err_reply(client, pkt, RD_AP_INCON, - "realm length too long"); - return; + return kerb_err_reply(client, pkt, RD_AP_INCON, + "realm length too long"); } auth->length += (int) *(pkt->dat + auth->length) + @@ -839,9 +836,8 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) lt = klog(L_KRB_PERR, "APPL request with funky tkt or req_id length from %s", inet_ntoa(client_host)); - kerb_err_reply(client, pkt, RD_AP_INCON, - "funky tkt or req_id length"); - return; + return kerb_err_reply(client, pkt, RD_AP_INCON, + "funky tkt or req_id length"); } memcpy(auth->dat, pkt->dat, auth->length); @@ -850,32 +846,29 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) tktrlm[REALM_SZ-1] = '\0'; kvno = (krb5_kvno)auth->dat[2]; if ((!allow_v4_crossrealm)&&strcmp(tktrlm, local_realm) != 0) { - lt = klog(L_ERR_UNK, - "Cross realm ticket from %s denied by policy,", tktrlm); - kerb_err_reply(client, pkt, - KERB_ERR_PRINCIPAL_UNKNOWN, lt); - return; + lt = klog(L_ERR_UNK, + "Cross realm ticket from %s denied by policy,", tktrlm); + return kerb_err_reply(client, pkt, + KERB_ERR_PRINCIPAL_UNKNOWN, lt); } if (set_tgtkey(tktrlm, kvno, 0)) { - lt = klog(L_ERR_UNK, + lt = klog(L_ERR_UNK, "FAILED set_tgtkey realm %s, kvno %d. Host: %s ", tktrlm, kvno, inet_ntoa(client_host)); /* no better error code */ - kerb_err_reply(client, pkt, - KERB_ERR_PRINCIPAL_UNKNOWN, lt); - return; + return kerb_err_reply(client, pkt, + KERB_ERR_PRINCIPAL_UNKNOWN, lt); } kerno = krb_rd_req(auth, "krbtgt", tktrlm, client_host.s_addr, - ad, 0); + ad, 0); if (kerno) { if (set_tgtkey(tktrlm, kvno, 1)) { lt = klog(L_ERR_UNK, "FAILED 3des set_tgtkey realm %s, kvno %d. Host: %s ", tktrlm, kvno, inet_ntoa(client_host)); /* no better error code */ - kerb_err_reply(client, pkt, - KERB_ERR_PRINCIPAL_UNKNOWN, lt); - return; + return kerb_err_reply(client, pkt, + KERB_ERR_PRINCIPAL_UNKNOWN, lt); } kerno = krb_rd_req(auth, "krbtgt", tktrlm, client_host.s_addr, ad, 0); @@ -885,8 +878,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) klog(L_ERR_UNK, "FAILED krb_rd_req from %s: %s", inet_ntoa(client_host), krb_get_err_text(kerno)); req_name_ptr = req_inst_ptr = req_realm_ptr = ""; - kerb_err_reply(client, pkt, kerno, "krb_rd_req failed"); - return; + return kerb_err_reply(client, pkt, kerno, "krb_rd_req failed"); } ptr = (char *) pkt->dat + auth->length; @@ -908,22 +900,21 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) req_realm_ptr = ad->prealm; if (strcmp(ad->prealm, tktrlm)) { - kerb_err_reply(client, pkt, KERB_ERR_PRINCIPAL_UNKNOWN, - "Can't hop realms"); - return; + return kerb_err_reply(client, pkt, KERB_ERR_PRINCIPAL_UNKNOWN, + "Can't hop realms"); } if (!strcmp(service, "changepw")) { - kerb_err_reply(client, pkt, KERB_ERR_PRINCIPAL_UNKNOWN, - "Can't authorize password changed based on TGT"); - return; + return kerb_err_reply(client, pkt, KERB_ERR_PRINCIPAL_UNKNOWN, + "Can't authorize password changed based on TGT"); } kerno = check_princ(service, instance, req_life, &s_name_data, &k5key, 1, &sk5life); if (kerno) { - kerb_err_reply(client, pkt, kerno, "check_princ failed"); + response = kerb_err_reply(client, pkt, kerno, + "check_princ failed"); s_name_data.key_high = s_name_data.key_low = 0; krb5_free_keyblock_contents(kdc_context, &k5key); - return; + return response; } /* Bound requested lifetime with service and user */ v4endtime = krb_life_to_time((KRB4_32)ad->time_sec, ad->life); @@ -979,7 +970,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) rpkt = create_auth_reply(ad->pname, ad->pinst, ad->prealm, time_ws, 0, 0, 0, ciph); - set_response((char *) rpkt->dat, rpkt->length); + response = make_response((char *) rpkt->dat, rpkt->length); memset(&s_name_data, 0, sizeof(s_name_data)); break; } @@ -1004,6 +995,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) break; } } + return response; } @@ -1013,7 +1005,7 @@ kerberos_v4(struct sockaddr_in *client, KTEXT pkt) * client. */ -void +static krb5_data * kerb_err_reply(struct sockaddr_in *client, KTEXT pkt, long int err, char *string) { static KTEXT_ST e_pkt_st; @@ -1024,8 +1016,7 @@ kerb_err_reply(struct sockaddr_in *client, KTEXT pkt, long int err, char *string strncat(e_msg, string, sizeof(e_msg) - 1 - 19); cr_err_reply(e_pkt, req_name_ptr, req_inst_ptr, req_realm_ptr, req_time_ws, err, e_msg); - set_response((char *) e_pkt->dat, e_pkt->length); - + return make_response((char *) e_pkt->dat, e_pkt->length); } static int diff --git a/src/kdc/network.c b/src/kdc/network.c index 266c6aa83..e9dcf8610 100644 --- a/src/kdc/network.c +++ b/src/kdc/network.c @@ -1069,6 +1069,8 @@ static void process_packet(struct connection *conn, const char *prog, com_err(prog, retval, "while dispatching (udp)"); return; } + if (response == NULL) + return; cc = send_to_from(port_fd, response->data, (socklen_t) response->length, 0, (struct sockaddr *)&saddr, saddr_len, (struct sockaddr *)&daddr, daddr_len);