Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: binary MIME-Version: 1.0 X-Mailer: MIME-tools 5.420 (Entity 5.420) X-RT-Original-Encoding: iso-8859-1 Content-Length: 10936 From krb5-bugs-incoming-bounces@PCH.MIT.EDU Mon Jan 7 14:11:07 2008 Received: from pch.mit.edu (PCH.MIT.EDU [18.7.21.90]) by krbdev.mit.edu (8.12.9) with ESMTP id m07JB7HW027326; Mon, 7 Jan 2008 14:11:07 -0500 (EST) Received: from pch.mit.edu (pch.mit.edu [127.0.0.1]) by pch.mit.edu (8.13.6/8.12.8) with ESMTP id m07JB23A012036; Mon, 7 Jan 2008 14:11:02 -0500 Received: from fort-point-station.mit.edu (FORT-POINT-STATION.MIT.EDU [18.7.7.76]) by pch.mit.edu (8.13.6/8.12.8) with ESMTP id m07IXtJV016594 for ; Mon, 7 Jan 2008 13:33:55 -0500 Received: from mit.edu (M24-004-BARRACUDA-3.MIT.EDU [18.7.7.114]) by fort-point-station.mit.edu (8.13.6/8.9.2) with ESMTP id m07IXjTp021228 for ; Mon, 7 Jan 2008 13:33:46 -0500 (EST) Received: from mx1.redhat.com (mx1.redhat.com [66.187.233.31]) by mit.edu (Spam Firewall) with ESMTP id B5A09D2610E for ; Mon, 7 Jan 2008 13:33:12 -0500 (EST) Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id m07IXCfr014062 for ; Mon, 7 Jan 2008 13:33:12 -0500 Received: from blade.boston.redhat.com (blade.boston.redhat.com [172.16.80.50]) by int-mx1.corp.redhat.com (8.13.1/8.13.1) with ESMTP id m07IXBV2004763 for ; Mon, 7 Jan 2008 13:33:12 -0500 Received: from blade.boston.redhat.com (localhost.localdomain [127.0.0.1]) by blade.boston.redhat.com (8.14.2/8.14.2) with ESMTP id m07IXBPv029980 for ; Mon, 7 Jan 2008 13:33:11 -0500 Received: (from nalin@localhost) by blade.boston.redhat.com (8.14.2/8.14.2/Submit) id m07IXBAm029979; Mon, 7 Jan 2008 13:33:11 -0500 Date: Mon, 7 Jan 2008 13:33:11 -0500 Message-Id: <200801071833.m07IXBAm029979@blade.boston.redhat.com> To: krb5-bugs@mit.edu Subject: kpasswd routines don't switch to tcp From: nalin@redhat.com X-send-pr-version: 3.99 X-Scanned-By: MIMEDefang 2.42 X-Scanned-By: MIMEDefang 2.58 on 172.16.52.254 X-Spam-Score: 0.55 X-Spam-Flag: NO X-Mailman-Approved-At: Mon, 07 Jan 2008 14:11:02 -0500 X-BeenThere: krb5-bugs-incoming@mailman.mit.edu X-Mailman-Version: 2.1.6 Precedence: list Reply-To: nalin@redhat.com Sender: krb5-bugs-incoming-bounces@PCH.MIT.EDU Errors-To: krb5-bugs-incoming-bounces@PCH.MIT.EDU >Submitter-Id: net >Originator: >Organization: >Confidential: no >Synopsis: kpasswd routines don't switch to tcp >Severity: non-critical >Priority: low >Category: krb5-libs >Class: sw-bug >Release: 1.6.3 >Environment: System: Linux blade.boston.redhat.com 2.6.23-6.fc8 #1 SMP Thu Oct 11 13:36:39 EDT 2007 x86_64 x86_64 x86_64 GNU/Linux Architecture: x86_64 >Description: The current logic in src/lib/krb5/os/changepw.c makes reference to switching to TCP when an error is encountered, but doesn't seem to do so in practice. >How-To-Repeat: Saw this with a Windows-based KDC for which the UDP kpasswd port had accidentally been firewalled off, but early FreeIPA kpasswd servers which only served TCP clients experienced the same problem. >Fix: Either notice the errors and retry with TCP in a second pass in krb5_change_set_password(), or add TCP server addresses in with UDP addresses in the same pass. Both patches are below. The second looks much larger than it really should because it removes the loop which encloses the calls to krb5int_sendto() and the processing of server responses. Force a second iteration for KDC_UNREACH, REALM_CANT_RESOLVE, or RESPONSE_TOO_BIG errors. Index: src/lib/krb5/os/changepw.c =================================================================== --- src/lib/krb5/os/changepw.c (revision 20199) +++ src/lib/krb5/os/changepw.c (working copy) @@ -251,11 +251,22 @@ NULL, NULL ))) { - - /* - * Here we may want to switch to TCP on some errors. - * right? - */ + /* if we're not using a stream socket, and it's an error which + * might reasonably be specific to a datagram "connection", try + * again with a stream socket */ + if (!useTcp) { + switch (code) { + case KRB5_KDC_UNREACH: + case KRB5_REALM_CANT_RESOLVE: + case KRB5KRB_ERR_RESPONSE_TOO_BIG: + /* should we do this for more result codes than these? */ + krb5int_free_addrlist (&al); + useTcp = 1; + continue; + default: + break; + } + } break; } Attempt to resolve both connectionless and connected kpasswd servers, returning an error if we can't find _any_ servers. Concatenate the two server lists and just make one call to krb5int_sendto() to send the request. Index: src/lib/krb5/os/changepw.c =================================================================== --- src/lib/krb5/os/changepw.c (revision 20199) +++ src/lib/krb5/os/changepw.c (working copy) @@ -199,14 +199,14 @@ krb5_address remote_kaddr; krb5_boolean useTcp = 0; GETSOCKNAME_ARG3_TYPE addrlen; - krb5_error_code code = 0; + krb5_error_code code = 0, code2 = 0; char *code_string; - int local_result_code; + int local_result_code, i; struct sendto_callback_context callback_ctx; struct sendto_callback_info callback_info; struct sockaddr_storage remote_addr; - struct addrlist al = ADDRLIST_INIT; + struct addrlist al = ADDRLIST_INIT, al2 = ADDRLIST_INIT; memset( &callback_ctx, 0, sizeof(struct sendto_callback_context)); callback_ctx.context = context; @@ -225,109 +225,104 @@ &callback_ctx.ap_req))) goto cleanup; - do { - if ((code = krb5_locate_kpasswd(callback_ctx.context, - krb5_princ_realm(callback_ctx.context, - creds->server), - &al, useTcp))) - break; - + code = krb5_locate_kpasswd(callback_ctx.context, + krb5_princ_realm(callback_ctx.context, + creds->server), + &al, useTcp); + code2 = krb5_locate_kpasswd(callback_ctx.context, + krb5_princ_realm(callback_ctx.context, + creds->server), + &al2, !useTcp); + if ((al.naddrs + al2.naddrs) == 0) { + if (!code) + code = code2 ? code2 : KRB5_REALM_CANT_RESOLVE; + goto cleanup; + } + + if (al2.naddrs > 0) { + if (krb5int_grow_addrlist(&al, al2.naddrs)) + goto cleanup; + for (i = 0; i < al2.naddrs; i++) + al.addrs[al.naddrs++] = al2.addrs[i]; + al2.naddrs = 0; + } + - addrlen = sizeof(remote_addr); - - callback_info.context = (void*) &callback_ctx; - callback_info.pfn_callback = kpasswd_sendto_msg_callback; - callback_info.pfn_cleanup = kpasswd_sendto_msg_cleanup; - - if ((code = krb5int_sendto(callback_ctx.context, - NULL, - &al, - &callback_info, - &chpw_rep, - NULL, - NULL, - ss2sa(&remote_addr), - &addrlen, - NULL, - NULL, - NULL - ))) { - - /* - * Here we may want to switch to TCP on some errors. - * right? - */ - break; - } - + addrlen = sizeof(remote_addr); + + callback_info.context = (void*) &callback_ctx; + callback_info.pfn_callback = kpasswd_sendto_msg_callback; + callback_info.pfn_cleanup = kpasswd_sendto_msg_cleanup; + + if ((code = krb5int_sendto(callback_ctx.context, + NULL, + &al, + &callback_info, + &chpw_rep, + NULL, + NULL, + ss2sa(&remote_addr), + &addrlen, + NULL, + NULL, + NULL + ))) + goto cleanup; + - remote_kaddr.addrtype = ADDRTYPE_INET; - remote_kaddr.length = sizeof(ss2sin(&remote_addr)->sin_addr); - remote_kaddr.contents = (krb5_octet *) &ss2sin(&remote_addr)->sin_addr; - - if ((code = krb5_auth_con_setaddrs(callback_ctx.context, - callback_ctx.auth_context, - NULL, - &remote_kaddr))) - break; - + remote_kaddr.addrtype = ADDRTYPE_INET; + remote_kaddr.length = sizeof(ss2sin(&remote_addr)->sin_addr); + remote_kaddr.contents = (krb5_octet *) &ss2sin(&remote_addr)->sin_addr; + + if ((code = krb5_auth_con_setaddrs(callback_ctx.context, + callback_ctx.auth_context, + NULL, + &remote_kaddr))) + goto cleanup; + - if (set_password_for) - code = krb5int_rd_setpw_rep(callback_ctx.context, - callback_ctx.auth_context, - &chpw_rep, - &local_result_code, - result_string); - else - code = krb5int_rd_chpw_rep(callback_ctx.context, - callback_ctx.auth_context, - &chpw_rep, - &local_result_code, - result_string); - - if (code) { - if (code == KRB5KRB_ERR_RESPONSE_TOO_BIG && !useTcp ) { - krb5int_free_addrlist (&al); - useTcp = 1; - continue; - } - - break; - } - - if (result_code) - *result_code = local_result_code; - + if (set_password_for) + code = krb5int_rd_setpw_rep(callback_ctx.context, + callback_ctx.auth_context, + &chpw_rep, + &local_result_code, + result_string); + else + code = krb5int_rd_chpw_rep(callback_ctx.context, + callback_ctx.auth_context, + &chpw_rep, + &local_result_code, + result_string); + + if (code) + goto cleanup; + + if (result_code) + *result_code = local_result_code; + - if (result_code_string) { - if (set_password_for) - code = krb5int_setpw_result_code_string(callback_ctx.context, - local_result_code, - (const char **)&code_string); - else - code = krb5_chpw_result_code_string(callback_ctx.context, - local_result_code, - &code_string); - if(code) - goto cleanup; - - result_code_string->length = strlen(code_string); - result_code_string->data = malloc(result_code_string->length); - if (result_code_string->data == NULL) { - code = ENOMEM; - goto cleanup; - } - strncpy(result_code_string->data, code_string, result_code_string->length); - } - - if (code == KRB5KRB_ERR_RESPONSE_TOO_BIG && !useTcp ) { - krb5int_free_addrlist (&al); - useTcp = 1; - } else { - break; - } - } while (TRUE); + if (result_code_string) { + if (set_password_for) + code = krb5int_setpw_result_code_string(callback_ctx.context, + local_result_code, + (const char **) &code_string); + else + code = krb5_chpw_result_code_string(callback_ctx.context, + local_result_code, + &code_string); + if (code) + goto cleanup; + + result_code_string->length = strlen(code_string); + result_code_string->data = malloc(result_code_string->length); + if (result_code_string->data == NULL) { + code = ENOMEM; + goto cleanup; + } + strncpy(result_code_string->data, code_string, result_code_string->length); + } cleanup: if (callback_ctx.auth_context != NULL) krb5_auth_con_free(callback_ctx.context, callback_ctx.auth_context); + krb5int_free_addrlist (&al2); krb5int_free_addrlist (&al); krb5_free_data_contents(callback_ctx.context, &callback_ctx.ap_req);