patch to fix some kpasswd bugs in krb5-1.2.1

patch to fix some kpasswd bugs in krb5-1.2.1

Post by David Wrag » Fri, 06 Oct 2000 04:00:00



Hi,

In MIT krb5-1.2.1 (and probably in krb5-1.1.x too, but I haven't
checked), there are a few bugs in kpasswd due to
krb5_change_password().

- If you have more than one kpasswd_server set in krb5.conf,
krb5_locate_kpasswd() would segfault. (It is broken in cases where
krb5_locate_srv_conf returns more than one address).

- If you have no kpasswd_server and krb5_locate_kpasswd() falls back
to kadmin_server entries, it might still segfault, depending on the
contents of /etc/services. (Same cause as above)

- If you have multiple kpasswd_server entries, krb5_change_password()
can return ECONNREFUSED or EHOSTUNREACH for one server without
bothering to try servers in later entries. (The return code from
recvfrom() isn't checked for those conditios).

- With multiple kpasswd_server entries, even with the last problem
fixed, krb5_change_password() doesn't work if the first entry fails
(I've forgotten the error message).  (The auth_context hold a sequence
numbers that is incremented on each krb5_mk_chpw_req().  So for all
but the first attempt, this seqence number doesn't have its initial
value and the request fails).

The patch below fixes these problems, so that with multiple
kpasswd_server entries in krb5.conf, kpasswd works consistently as
long as one of the servers listed is up and running kadmind.

I checked krb5-current a week or two ago to see if these problems had
been fixed there, but changepw.c had no significant changes, just some
formatting improvements.  Those formatting changes would need to be
backed out for this patch to apply cleanly to krb5-current.  Do I need
to send this patch to krb...@mit.edu, or is it likely to be picked up
from here?

Regards,
David Wragg

diff -ruaN krb5-1.2.1/src/lib/krb5/os/changepw.c krb5-1.2.1.mod/src/lib/krb5/os/changepw.c
--- krb5-1.2.1/src/lib/krb5/os/changepw.c       Fri Jun 30 03:28:13 2000
+++ krb5-1.2.1.mod/src/lib/krb5/os/changepw.c   Sat Sep 30 23:25:14 2000
@@ -75,7 +75,7 @@
             /* success with admin_server but now we need to change the port */
             /* number to use DEFAULT_KPASSWD_PORT.                          */
             for ( i=0;i<*naddrs;i++ ) {
-                struct sockaddr_in *sin = (struct sockaddr_in *) addr_pp[i];
+                struct sockaddr_in *sin = (struct sockaddr_in *) *addr_pp + i;
                 sin->sin_port = htons(DEFAULT_KPASSWD_PORT);
             }
         }
@@ -96,7 +96,7 @@
                     /* success with admin_server but now we need to change the port */
                     /* number to use DEFAULT_KPASSWD_PORT.                          */
                     for ( i=0;i<*naddrs;i++ ) {
-                        struct sockaddr_in *sin = (struct sockaddr_in *) addr_pp[i];
+                        struct sockaddr_in *sin = (struct sockaddr_in *) *addr_pp + i;
                         sin->sin_port = htons(DEFAULT_KPASSWD_PORT);
                     }
                 }
@@ -123,31 +123,21 @@
     krb5_address local_kaddr, remote_kaddr;
     char *code_string;
     krb5_error_code code = 0;
-    int i, addrlen;
-    struct sockaddr *addr_p, local_addr, remote_addr, tmp_addr;
+    int i, local_result_code;
+    struct sockaddr *addr_p;
     int naddr_p;
-    int cc, local_result_code, tmp_len;
-    SOCKET s1 = INVALID_SOCKET, s2 = INVALID_SOCKET;
-
+    SOCKET s1 = INVALID_SOCKET;

     /* Initialize values so that cleanup call can safely check for NULL */
-    auth_context = NULL;
     addr_p = NULL;
+    auth_context = NULL;
     memset(&chpw_req, 0, sizeof(krb5_data));
     memset(&chpw_rep, 0, sizeof(krb5_data));
     memset(&ap_req, 0, sizeof(krb5_data));

-    /* initialize auth_context so that we know we have to free it */
-    if ((code = krb5_auth_con_init(context, &auth_context)))
-         goto cleanup;
-    
-    if (code = krb5_mk_req_extended(context, &auth_context, AP_OPTS_USE_SUBKEY,
-                                   NULL, creds, &ap_req))
-         goto cleanup;
-
-    if (code = krb5_locate_kpasswd(context,
+    if ((code = krb5_locate_kpasswd(context,
                                     krb5_princ_realm(context, creds->client),
-                                    &addr_p, &naddr_p))
+                                    &addr_p, &naddr_p)))
         goto cleanup;

     /* this is really obscure.  s1 is used for all communications.  it
@@ -172,40 +162,47 @@
            goto cleanup;
       }

-    if ((s2 = socket(AF_INET, SOCK_DGRAM, 0)) == INVALID_SOCKET)
-      {
-           code = SOCKET_ERRNO;
-           goto cleanup;
-      }
-
-    for (i=0; i<naddr_p; i++)
+    for (i=0; ; i++)
       {
+               int addrlen;
+               struct sockaddr local_addr, remote_addr, tmp_addr;
+               int cc, tmp_len;
+               SOCKET s2;
                fd_set fdset;
                struct timeval timeout;

-               if (connect(s2, &addr_p[i], sizeof(addr_p[i])) == SOCKET_ERROR)
+               code = 0;
+
+               if (i == naddr_p)
+                 {
+                   /* No more addresses to try */
+                   code = ECONNREFUSED;
+                   goto cleanup;
+                 }
+
+               if ((s2 = socket(AF_INET, SOCK_DGRAM, 0)) == INVALID_SOCKET)
                  {
-                   if ((SOCKET_ERRNO == ECONNREFUSED) || (SOCKET_ERRNO == EHOSTUNREACH))
-                         continue; /* try the next addr */
-                  
                    code = SOCKET_ERRNO;
                    goto cleanup;
                  }
+
+               if (connect(s2, &addr_p[i], sizeof(addr_p[i])) == SOCKET_ERROR)
+                 {
+                   code = SOCKET_ERRNO;
+                   goto tried;
+                 }

-        addrlen = sizeof(local_addr);
+               addrlen = sizeof(local_addr);

                if (getsockname(s2, &local_addr, &addrlen) < 0)
                  {
-                   if ((SOCKET_ERRNO == ECONNREFUSED) || (SOCKET_ERRNO == EHOSTUNREACH))
-                         continue; /* try the next addr */
-                  
                    code = SOCKET_ERRNO;
-                       goto cleanup;
+                   goto tried;
                  }

                /* some brain-dead OS's don't return useful information from
                 * the getsockname call.  Namely, windows and solaris.  */
-
+              
                if (((struct sockaddr_in *)&local_addr)->sin_addr.s_addr != 0)
                  {
                    local_kaddr.addrtype = ADDRTYPE_INET;
@@ -230,11 +227,8 @@
                addrlen = sizeof(remote_addr);
                if (getpeername(s2, &remote_addr, &addrlen) < 0)
                  {
-                   if ((SOCKET_ERRNO == ECONNREFUSED) || (SOCKET_ERRNO == EHOSTUNREACH))
-                         continue; /* try the next addr */
-                  
                    code = SOCKET_ERRNO;
-                       goto cleanup;
+                   goto tried;
                  }

                remote_kaddr.addrtype = ADDRTYPE_INET;
@@ -255,28 +249,34 @@
                  specified.  when rd_priv is called, *only* a remote address
                  is specified.  Are we having fun yet?  */

-               if (code = krb5_auth_con_setaddrs(context, auth_context, &local_kaddr, NULL))
-                 {
-                   code = SOCKET_ERRNO;
-                       goto cleanup;
-                 }
-
-               if (code = krb5_mk_chpw_req(context, auth_context, &ap_req, newpw, &chpw_req))
-                 {
-                   code = SOCKET_ERRNO;
-                       goto cleanup;
-                 }
+               /* We cannot reuse the auth_context, because of the
+                  sequence number it holds. */
+               if ((code = krb5_auth_con_init(context, &auth_context)))
+                 goto tried;
+    
+               if ((code = krb5_mk_req_extended(context, &auth_context,
+                                                AP_OPTS_USE_SUBKEY,
+                                                NULL, creds, &ap_req)))
+                 goto tried;
+
+               if ((code = krb5_auth_con_setaddrs(context, auth_context,
+                                                  &local_kaddr, NULL)))
+                 goto tried;
+
+               if ((code = krb5_mk_chpw_req(context, auth_context,
+                                            &ap_req, newpw, &chpw_req)))
+                 goto tried;

                if ((cc = sendto(s1, chpw_req.data, chpw_req.length, 0,
                                 (struct sockaddr *) &addr_p[i],
                                 sizeof(addr_p[i]))) != chpw_req.length)
                  {
-                   if ((cc < 0) && ((SOCKET_ERRNO == ECONNREFUSED) ||
-                                    (SOCKET_ERRNO == EHOSTUNREACH)))
-                         continue; /* try the next addr */
-                  
-                   code = (cc < 0) ? SOCKET_ERRNO : ECONNABORTED;
-                       goto cleanup;
+                   if (cc < 0)
+                     code = SOCKET_ERRNO;
+                   else
+                     code = ECONNABORTED;
+
+                   goto tried;
                  }

                chpw_rep.length = 1500;
@@ -290,10 +290,10 @@
                switch (select (s1 + 1, &fdset, 0, 0, &timeout)) {
                case -1:
                    code = SOCKET_ERRNO;
-                   goto cleanup;
+                   goto tried;
                case 0:
                    code = ETIMEDOUT;
-                   goto cleanup;
+                   goto tried;
                default:
                    /* fall through */
                    ;
@@ -308,64 +308,79 @@
                   SunOS 4.1.4 or Irix 5.3.  Thus we must actually accept the
                   value and discard it. */
                tmp_len = sizeof(tmp_addr);
-               if ((cc = recvfrom(s1, chpw_rep.data, chpw_rep.length, 0, &tmp_addr, &tmp_len)) < 0)
+               if ((cc = recvfrom(s1, chpw_rep.data, chpw_rep.length,
+                                  0, &tmp_addr, &tmp_len)) < 0)
                  {
                    code = SOCKET_ERRNO;
-                   goto cleanup;
                  }

-               closesocket(s1);
-               s1 = INVALID_SOCKET;
-               closesocket(s2);
-               s2 = INVALID_SOCKET;
-
                chpw_rep.length = cc;

-               if (code = krb5_auth_con_setaddrs(context, auth_context, NULL, &remote_kaddr))
-                 goto cleanup;
-
-               if(code = krb5_rd_chpw_rep(context, auth_context, &chpw_rep,
-                                       &local_result_code, result_string))
-                 goto cleanup;
-
-               if (result_code)
-                 *result_code = local_result_code;
+      tried:
+               closesocket(s2);

-               if (result_code_string)
-                 {
-                   if (code = krb5_chpw_result_code_string(context, local_result_code,
-                                                           &code_string))
-                         goto cleanup;
-
-                   result_code_string->length = strlen(code_string);
-                   if ((result_code_string->data =
-                           (char *) malloc(result_code_string->length)) == NULL)
-                         return(ENOMEM);
-                   strncpy(result_code_string->data, code_string, result_code_string->length);
-                 }
+               /* Were we successful in getting a response? */
+               if (!code)
+                 break;

-               code = 0;
-               goto cleanup;
+               /* Was the error fatal? */
+               if (code != ECONNREFUSED && code != EHOSTUNREACH)
+                 goto cleanup;
+              
+               /* Tidy up and try the next one. */
+               if (auth_context != NULL) {
+                 krb5_auth_con_free(context, auth_context);
+                 auth_context = NULL;
+               }
+              
+               krb5_free_data_contents(context, &ap_req);
+               krb5_free_data_contents(context, &chpw_req);
+               krb5_free_data_contents(context, &chpw_rep);
       }

-    code = SOCKET_ERRNO;
+    /* Now process the response */
+    if ((code = krb5_auth_con_setaddrs(context, auth_context,
+                                      NULL, &remote_kaddr)))
+      goto cleanup;
+    
+    if ((code = krb5_rd_chpw_rep(context, auth_context, &chpw_rep,
+                                &local_result_code, result_string)))
+      goto cleanup;
+    
+    if (result_code)
+      *result_code = local_result_code;

-cleanup:
+    if (result_code_string)
+      {
+       if ((code = krb5_chpw_result_code_string(context, local_result_code,
+                                                &code_string)))
+         goto cleanup;
+      
+       result_code_string->length = strlen(code_string);
+       result_code_string->data = (char *) 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(auth_context != NULL)
       krb5_auth_con_free(context, auth_context);

+    krb5_free_data_contents(context, &chpw_req);
+    krb5_free_data_contents(context, &chpw_rep);
+    krb5_free_data_contents(context, &ap_req);
+
     if(addr_p != NULL)
       krb5_xfree(addr_p);

     if(s1 != INVALID_SOCKET)
       closesocket(s1);
-    
-    if(s2 != INVALID_SOCKET)
-      closesocket(s2);
-      
-    krb5_free_data_contents(context, &chpw_req);
-    krb5_free_data_contents(context, &chpw_rep);
-       krb5_free_data_contents(context, &ap_req);
-    
+
     return(code);
 }

 
 
 

1. krb5-1.2.1 is released

-----BEGIN PGP SIGNED MESSAGE-----

The MIT Kerberos Team announces the availibility of MIT Kerberos 5
Release 1.2.1.  This is primarily a bugfix release.  Changes include:

* A bug in the gssapi library that prevented kadmin clients from
  working has been fixed.  For some reason this was not caught during
  beta testing.

* login.c now correctly sets the default ccache name.

* A memory leak in conv_princ.c has been fixed.

RETRIEVING KERBEROS 5 RELEASE 1.2.1
===================================
You may retrieve the Kerberos 5 Release 1.2.1 source from the
following URL:

        http://web.mit.edu/network/kerberos-form.html

Further information about Kerberos 5 may be found at the following
URL:

        http://web.mit.edu/kerberos/www/index.html

=========================
Tom Yu
MIT Information Systems
Kerberos Development Team

-----BEGIN PGP SIGNATURE-----
Version: 2.6.2

iQCVAwUBOVwbuKbDgE/zdoE9AQEu0wP5AfM4iMsG87xErB8yB2syZgS6bJC+RT0m
LTXDQDRBoY2uqqhIF8bhBBBgRoE9H1WzxX8wJhuVVgUP4OKoA7o78zPryv/8LoIV
LZKlVTXPerw2TTUL3lFyOt+822pBaoMeZPNmqX7k+dIDXCHoUrcTC6lh2+gt5jys
zOTzTel80Eo=
=TIMf
-----END PGP SIGNATURE-----

2. CD Drive not available

3. Bogus utmpx files on Solaris 8 with MIT's krb5-1.2.1

4. Problem with PHSS_5498/PHSS_5499?

5. How to build Krb5-1.2.1

6. Very slow Samba Server

7. Bogus utmpx files on Solaris 8 with MIT's krb5-1.2.1

8. tools for finding memory leaks?

9. kadmin problems again with krb5-1.2.1

10. Estdio 2.1: bugs in scanf %[] handling, plus fix

11. New cygwin patches for krb5-1.3-alpha2 and krb5-current (snapshot)

12. BUG & patch: lib/krb5/os/lock_file.c:krb5_lock_file()

13. PATCH: krb5-1.2.4 Set security on file cache in NT/2000