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 afe296be95bbb1e1fa8f5578b327185c7abd266d..3d703d42c7d28cd2c8ff108cba365a3a066546cd 100644 (file)
--- a/src/include/ChangeLog
+++ b/src/include/ChangeLog
@@ -1,3 +1,9 @@
+2004-02-26  Jeffrey Altman <jaltman@mit.edu>
+
+    * 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  <raeburn@mit.edu>
 
        * 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 019d67a457bce6a45685fbee527cd445af980394..da3959032189aaacbbb417af75b34b450b4c3b32 100644 (file)
--- 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 7ca42a356077569e77e679a9585d5ffed37df65e..e00e503bf5167b6ed1b43c89ac2719aedaa2e055 100644 (file)
--- 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 f9db28805c491bf89f3f3600c704bd527b986f21..3a2544d574c9f83946d9d4dbd8a4680a35996919 100644 (file)
--- 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 0abd32f570c779953231c5e9ea84ab5d08899a55..acef6b9dc482004b79e8114b73c4efed7af8f29e 100644 (file)
--- 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 0cb9fa052debadd8bd032cd295f6495d86e6ecf1..7d70782e9e84bee52da64bad17ae8d760f366226 100644 (file)
--- 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 38a88ee140206c024dbd809d6bd3b5ccda6a0c6f..3861c15022d9a803402f40e7053b54d5cbd8e447 100644 (file)
--- 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 fdd7c514ad5f4a260678b0e5e8cf99f4ecee84c3..50d91d1a2956698ff145e9bda510c3878638b8f4 100644 (file)
--- 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 244d18e123089524b049b86397cc07dfa3d5e6c1..3b6b24288bd31e6957f57650b1466a21a4f56f6e 100644 (file)
--- 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 2890ccfc23bd5338aafc8fc670ff59af60d3e11b..90fe4df27d4b3757c6018f5b1a0b2ca53bf66cfe 100644 (file)
--- 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 0ca8e93c33236bc6ff597729380ded3594906d58..eeb586162d1e152b8ca783af1068cf5892f6c9e8 100644 (file)
--- 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 66a5a520f668b7c034d10e5620b706bb6248f00f..c2fdb35d789b796c7d43249002c93bd5bfb3f411 100644 (file)
--- 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: