From: Jeffrey Altman Date: Fri, 27 Feb 2004 05:24:39 +0000 (+0000) Subject: As discussed on the krbdev mailing list, krb5_get_init_creds_password() X-Git-Tag: krb5-1.4-beta1~575 X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=9cd5e4909839ba4871484372e8f89cba95c14a45;p=krb5.git As discussed on the krbdev mailing list, krb5_get_init_creds_password() suffered from a behavior in which it would unintentionally query a master KDC twice if in fact the KDC queried when krb5int_sendto() was called with use_master = 0 was in fact the master. This resulted in more than an additional protocol operation. There were two negative side effects. First, in the case of an incorrect password there would be two counts against the max retry attempts. Second, in the case of hardware pre-auth and an expired password, the user would be asked to enter their expired password twice before being told it was expired. This has been fixed by changing the use_master parameter into an in/out parameter and modifying krb5int_sendto() to indicate which KDC it received the response from. This allows the use_master parameter to be set to indicate whether or not the response came from a master KDC regardless of whether a master KDC was requested. ticket: new target_version: next tags: pullup git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@16137 dc483132-0cff-0310-8789-dd5450dbe970 --- diff --git a/src/include/ChangeLog b/src/include/ChangeLog index afe296be9..3d703d42c 100644 --- a/src/include/ChangeLog +++ b/src/include/ChangeLog @@ -1,3 +1,9 @@ +2004-02-26 Jeffrey Altman + + * k5-int.h: change prototype declarations necessary to support + the use of krb5_get_init_creds_password's use_master as an + in/out parameter + 2004-02-26 Ken Raeburn * win-mac.h (GETSOCKNAME_ARG2_TYPE, GETSOCKNAME_ARG3_TYPE): Set diff --git a/src/include/k5-int.h b/src/include/k5-int.h index 019d67a45..da3959032 100644 --- a/src/include/k5-int.h +++ b/src/include/k5-int.h @@ -504,10 +504,10 @@ struct addrlist; krb5_error_code krb5_lock_file (krb5_context, int, int); krb5_error_code krb5_unlock_file (krb5_context, int); krb5_error_code krb5_sendto_kdc (krb5_context, const krb5_data *, - const krb5_data *, krb5_data *, int, int); + const krb5_data *, krb5_data *, int *, int); krb5_error_code krb5int_sendto (krb5_context, const krb5_data *, const struct addrlist *, krb5_data *, - struct sockaddr *, socklen_t *); + struct sockaddr *, socklen_t *, int *); krb5_error_code krb5_get_krbhst (krb5_context, const krb5_data *, char *** ); krb5_error_code krb5_free_krbhst (krb5_context, char * const * ); krb5_error_code krb5_create_secure_file (krb5_context, const char * pathname); @@ -946,7 +946,7 @@ krb5_get_init_creds krb5_get_init_creds_opt *gic_options, krb5_gic_get_as_key_fct gak, void *gak_data, - int master, + int *master, krb5_kdc_rep **as_reply); void krb5int_populate_gic_opt ( @@ -1703,7 +1703,7 @@ typedef struct _krb5int_access { int, int, int, int); krb5_error_code (*sendto_udp) (krb5_context, const krb5_data *msg, const struct addrlist *, krb5_data *reply, - struct sockaddr *, socklen_t *); + struct sockaddr *, socklen_t *, int *); krb5_error_code (*add_host_to_list)(struct addrlist *lp, const char *hostname, int port, int secport, diff --git a/src/lib/krb4/ChangeLog b/src/lib/krb4/ChangeLog index 7ca42a356..e00e503bf 100644 --- a/src/lib/krb4/ChangeLog +++ b/src/lib/krb4/ChangeLog @@ -1,3 +1,9 @@ +2004-02-26 Jeffrey Altman + + * send_to_kdc.c: modify call to internals.sendto_udp to support + the new declaration which contains an additional output parameter + which will not be used. + 2004-02-24 Sam Hartman * rd_svc_key.c (krb54_get_service_keyblock): Remove ENCTYPE_LOCAL_DES3_HMAC_SHA1 diff --git a/src/lib/krb4/send_to_kdc.c b/src/lib/krb4/send_to_kdc.c index f9db28805..3a2544d57 100644 --- a/src/lib/krb4/send_to_kdc.c +++ b/src/lib/krb4/send_to_kdc.c @@ -181,7 +181,7 @@ krb4int_send_to_kdc_addr( message.length = pkt->length; message.data = (char *)pkt->dat; /* XXX yuck */ retval = internals.sendto_udp(NULL, &message, &al, &reply, addr, - addrlen); + addrlen, NULL); DEB(("sendto_udp returns %d\n", retval)); free_al: internals.free_addrlist(&al); diff --git a/src/lib/krb5/krb/ChangeLog b/src/lib/krb5/krb/ChangeLog index 0abd32f57..acef6b9dc 100644 --- a/src/lib/krb5/krb/ChangeLog +++ b/src/lib/krb5/krb/ChangeLog @@ -1,3 +1,13 @@ +2004-02-26 Jeffrey Altman + + * get_in_tkt.c, gic_keytab.c, gic_pwd.c, send_tgs.c: + Implement changes to support the use of + krb5_get_init_creds_password's use_master as an in/out + parameter. This allows us to prevent a duplicate request + being sent to the KDC in the situation that the password + used is incorrect. This behavior results a negative user + experience and had to be corrected. + 2004-02-13 Ken Raeburn * sendauth.c: Don't specify defaults for diff --git a/src/lib/krb5/krb/get_in_tkt.c b/src/lib/krb5/krb/get_in_tkt.c index 0cb9fa052..7d70782e9 100644 --- a/src/lib/krb5/krb/get_in_tkt.c +++ b/src/lib/krb5/krb/get_in_tkt.c @@ -90,7 +90,7 @@ send_as_request(krb5_context context, krb5_timestamp *time_now, krb5_error ** ret_err_reply, krb5_kdc_rep ** ret_as_reply, - int use_master) + int *use_master) { krb5_kdc_rep *as_reply = 0; krb5_error_code retval; @@ -444,6 +444,7 @@ krb5_get_in_tkt(krb5_context context, krb5_pa_data ** preauth_to_use = 0; int loopcount = 0; krb5_int32 do_more = 0; + int use_master = 0; if (! krb5_realm_compare(context, creds->client, creds->server)) return KRB5_IN_TKT_REALM_MISMATCH; @@ -535,7 +536,7 @@ krb5_get_in_tkt(krb5_context context, err_reply = 0; as_reply = 0; if ((retval = send_as_request(context, &request, &time_now, &err_reply, - &as_reply, 0))) + &as_reply, &use_master))) goto cleanup; if (err_reply) { @@ -738,7 +739,7 @@ krb5_get_init_creds(krb5_context context, krb5_get_init_creds_opt *options, krb5_gic_get_as_key_fct gak_fct, void *gak_data, - int use_master, + int *use_master, krb5_kdc_rep **as_reply) { krb5_error_code ret; diff --git a/src/lib/krb5/krb/gic_keytab.c b/src/lib/krb5/krb/gic_keytab.c index 38a88ee14..3861c1502 100644 --- a/src/lib/krb5/krb/gic_keytab.c +++ b/src/lib/krb5/krb/gic_keytab.c @@ -96,7 +96,7 @@ krb5_get_init_creds_keytab(krb5_context context, krb5_creds *creds, krb5_princip ret = krb5_get_init_creds(context, creds, client, NULL, NULL, start_time, in_tkt_service, options, krb5_get_as_key_keytab, (void *) keytab, - use_master,NULL); + &use_master,NULL); /* check for success */ @@ -117,7 +117,7 @@ krb5_get_init_creds_keytab(krb5_context context, krb5_creds *creds, krb5_princip ret2 = krb5_get_init_creds(context, creds, client, NULL, NULL, start_time, in_tkt_service, options, krb5_get_as_key_keytab, (void *) keytab, - use_master, NULL); + &use_master, NULL); if (ret2 == 0) { ret = 0; diff --git a/src/lib/krb5/krb/gic_pwd.c b/src/lib/krb5/krb/gic_pwd.c index fdd7c514a..50d91d1a2 100644 --- a/src/lib/krb5/krb/gic_pwd.c +++ b/src/lib/krb5/krb/gic_pwd.c @@ -124,7 +124,7 @@ krb5_get_init_creds_password(krb5_context context, krb5_creds *creds, krb5_princ ret = krb5_get_init_creds(context, creds, client, prompter, data, start_time, in_tkt_service, options, krb5_get_as_key_password, (void *) &pw0, - use_master, &as_reply); + &use_master, &as_reply); /* check for success */ @@ -149,7 +149,7 @@ krb5_get_init_creds_password(krb5_context context, krb5_creds *creds, krb5_princ ret2 = krb5_get_init_creds(context, creds, client, prompter, data, start_time, in_tkt_service, options, krb5_get_as_key_password, (void *) &pw0, - use_master, &as_reply); + &use_master, &as_reply); if (ret2 == 0) { ret = 0; @@ -195,7 +195,7 @@ krb5_get_init_creds_password(krb5_context context, krb5_creds *creds, krb5_princ prompter, data, start_time, "kadmin/changepw", &chpw_opts, krb5_get_as_key_password, (void *) &pw0, - use_master, NULL))) + &use_master, NULL))) goto cleanup; prompt[0].prompt = "Enter new password"; @@ -283,7 +283,7 @@ krb5_get_init_creds_password(krb5_context context, krb5_creds *creds, krb5_princ ret = krb5_get_init_creds(context, creds, client, prompter, data, start_time, in_tkt_service, options, krb5_get_as_key_password, (void *) &pw0, - use_master, &as_reply); + &use_master, &as_reply); cleanup: krb5int_set_prompt_types(context, 0); diff --git a/src/lib/krb5/krb/send_tgs.c b/src/lib/krb5/krb/send_tgs.c index 244d18e12..3b6b24288 100644 --- a/src/lib/krb5/krb/send_tgs.c +++ b/src/lib/krb5/krb/send_tgs.c @@ -138,7 +138,7 @@ krb5_send_tgs(krb5_context context, krb5_flags kdcoptions, krb5_timestamp time_now; krb5_pa_data **combined_padata; krb5_pa_data ap_req_padata; - int tcp_only = 0; + int tcp_only = 0, use_master; /* * in_creds MUST be a valid credential NOT just a partially filled in @@ -261,9 +261,10 @@ krb5_send_tgs(krb5_context context, krb5_flags kdcoptions, /* now send request & get response from KDC */ send_again: + use_master = 0; retval = krb5_sendto_kdc(context, scratch, krb5_princ_realm(context, sname), - &rep->response, 0, tcp_only); + &rep->response, &use_master, tcp_only); if (retval == 0) { if (krb5_is_krb_error(&rep->response)) { if (!tcp_only) { diff --git a/src/lib/krb5/os/ChangeLog b/src/lib/krb5/os/ChangeLog index 2890ccfc2..90fe4df27 100644 --- a/src/lib/krb5/os/ChangeLog +++ b/src/lib/krb5/os/ChangeLog @@ -1,3 +1,14 @@ +2004-02-26 Jeffrey Altman + + * sendto_kdc.c, send524.c: + The use_master parameter of sendto_kdc is now an in/out + parameter used to report to the caller whether or not + the responding KDC was in fact the master. This is + necessary to allow callers to prevent making an unnecessary + additional call to query the master if the original + query did not explicitly state that the master should be + queried. + 2004-02-25 Ken Raeburn * sendto_kdc.c (start_connection): Close socket if connect() call diff --git a/src/lib/krb5/os/send524.c b/src/lib/krb5/os/send524.c index 0ca8e93c3..eeb586162 100644 --- a/src/lib/krb5/os/send524.c +++ b/src/lib/krb5/os/send524.c @@ -102,7 +102,7 @@ krb5int_524_sendto_kdc (context, message, realm, reply, addr, addrlen) if (al.naddrs == 0) return KRB5_REALM_UNKNOWN; - retval = krb5int_sendto (context, message, &al, reply, addr, addrlen); + retval = krb5int_sendto (context, message, &al, reply, addr, addrlen, NULL); krb5int_free_addrlist (&al); return retval; #else diff --git a/src/lib/krb5/os/sendto_kdc.c b/src/lib/krb5/os/sendto_kdc.c index 66a5a520f..c2fdb35d7 100644 --- a/src/lib/krb5/os/sendto_kdc.c +++ b/src/lib/krb5/os/sendto_kdc.c @@ -262,11 +262,11 @@ merge_addrlists (struct addrlist *dest, struct addrlist *src) krb5_error_code krb5_sendto_kdc (krb5_context context, const krb5_data *message, const krb5_data *realm, krb5_data *reply, - int use_master, int tcp_only) + int *use_master, int tcp_only) { krb5_error_code retval; struct addrlist addrs; - int socktype1 = 0, socktype2 = 0; + int socktype1 = 0, socktype2 = 0, addr_used; /* * find KDC location(s) for realm @@ -282,7 +282,7 @@ krb5_sendto_kdc (krb5_context context, const krb5_data *message, */ dprint("krb5_sendto_kdc(%d@%p, \"%D\", use_master=%d, tcp_only=%d)\n", - message->length, message->data, realm, use_master, tcp_only); + message->length, message->data, realm, *use_master, tcp_only); if (!tcp_only && context->udp_pref_limit < 0) { int tmp; @@ -301,7 +301,7 @@ krb5_sendto_kdc (krb5_context context, const krb5_data *message, context->udp_pref_limit = tmp; } - retval = (use_master ? KRB5_KDC_UNREACH : KRB5_REALM_UNKNOWN); + retval = (*use_master ? KRB5_KDC_UNREACH : KRB5_REALM_UNKNOWN); if (tcp_only) socktype1 = SOCK_STREAM, socktype2 = 0; @@ -310,7 +310,7 @@ krb5_sendto_kdc (krb5_context context, const krb5_data *message, else socktype1 = SOCK_STREAM, socktype2 = SOCK_DGRAM; - retval = krb5_locate_kdc(context, realm, &addrs, use_master, socktype1, 0); + retval = krb5_locate_kdc(context, realm, &addrs, *use_master, socktype1, 0); if (socktype2) { struct addrlist addrs2; @@ -323,10 +323,37 @@ krb5_sendto_kdc (krb5_context context, const krb5_data *message, } if (addrs.naddrs > 0) { - retval = krb5int_sendto (context, message, &addrs, reply, 0, 0); - krb5int_free_addrlist (&addrs); - if (retval == 0) - return 0; + retval = krb5int_sendto (context, message, &addrs, reply, 0, 0, + &addr_used); + if (retval == 0) { + /* + * Set use_master to 1 if we ended up talking to a master when + * we didn't explicitly request to + */ + if (*use_master == 0) { + struct addrlist addrs3; + retval = krb5_locate_kdc(context, realm, &addrs3, 1, + addrs.addrs[addr_used]->ai_socktype, + addrs.addrs[addr_used]->ai_family); + if (retval == 0) { + int i; + for (i = 0; i < addrs3.naddrs; i++) { + if (addrs.addrs[addr_used]->ai_addrlen == + addrs3.addrs[i]->ai_addrlen && + memcmp(addrs.addrs[addr_used]->ai_addr, + addrs3.addrs[i]->ai_addr, + addrs.addrs[addr_used]->ai_addrlen) == 0) { + *use_master = 1; + break; + } + } + krb5int_free_addrlist (&addrs3); + } + } + krb5int_free_addrlist (&addrs); + return 0; + } + krb5int_free_addrlist (&addrs); } return retval; } @@ -963,8 +990,9 @@ service_fds (struct select_state *selstate, krb5_error_code krb5int_sendto (krb5_context context, const krb5_data *message, - const struct addrlist *addrs, krb5_data *reply, - struct sockaddr *localaddr, socklen_t *localaddrlen) + const struct addrlist *addrs, krb5_data *reply, + struct sockaddr *localaddr, socklen_t *localaddrlen, + int *addr_used) { int i, pass; int delay_this_pass = 2; @@ -1071,6 +1099,8 @@ krb5int_sendto (krb5_context context, const krb5_data *message, (int) reply->length, reply->data); retval = 0; conns[winning_conn].x.in.buf = 0; + if (addr_used) + *addr_used = winning_conn; if (localaddr != 0 && localaddrlen != 0 && *localaddrlen > 0) (void) getsockname(conns[winning_conn].fd, localaddr, localaddrlen); egress: