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

10 files changed, 83 insertions, 24 deletions

diff --git a/src/lib/krb4/ChangeLog b/src/lib/krb4/ChangeLog
index 7ca42a3..e00e503 100644
--- a/src/lib/krb4/ChangeLog
+++ b/src/lib/krb4/ChangeLog
@@ -1,3 +1,9 @@
+2004-02-26  Jeffrey Altman <jaltman@mit.edu>
+
+    * 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  <hartmans@avalanche-breakdown.mit.edu>
 
 	* 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 f9db288..3a2544d 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 0abd32f..acef6b9 100644
--- a/src/lib/krb5/krb/ChangeLog
+++ b/src/lib/krb5/krb/ChangeLog
@@ -1,3 +1,13 @@
+2004-02-26  Jeffrey Altman <jaltman@mit.edu>
+  
+    * 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  <raeburn@mit.edu>
 
 	* 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 0cb9fa0..7d70782 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 38a88ee..3861c15 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 fdd7c51..50d91d1 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 244d18e..3b6b242 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 2890ccf..90fe4df 100644
--- a/src/lib/krb5/os/ChangeLog
+++ b/src/lib/krb5/os/ChangeLog
@@ -1,3 +1,14 @@
+2004-02-26  Jeffrey Altman <jaltman@mit.edu>
+
+    * 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  <raeburn@mit.edu>
 
 	* 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 0ca8e93..eeb5861 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 66a5a52..c2fdb35 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: