From: Robbie Harwood <rharwood@redhat.com>
Date: Mon, 8 Oct 2018 20:02:12 +0000 (-0400)
Subject: Prefer TCP to UDP for password changes
X-Git-Tag: krb5-1.17-beta1~28
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d7b3018d338fc9c989c3fa17505870f23c3759a8;p=thirdparty%2Fkrb5.git

Prefer TCP to UDP for password changes

When password changes are performed over UDP, spotty networks may
cause the client to retransmit.  This leads to replay errors if the
kpasswd server receives both requests, which hide the actual request
status and make it appear that the password has not been changed, when
it may in fact have been.  Use TCP instead with UDP fallback to avoid
this issue.

ticket: 7905
---

diff --git a/src/lib/krb5/os/changepw.c b/src/lib/krb5/os/changepw.c
index e4db57084d..9f968da7f2 100644
--- a/src/lib/krb5/os/changepw.c
+++ b/src/lib/krb5/os/changepw.c
@@ -59,13 +59,12 @@ struct sendto_callback_context {
 
 static krb5_error_code
 locate_kpasswd(krb5_context context, const krb5_data *realm,
-               struct serverlist *serverlist, krb5_boolean no_udp)
+               struct serverlist *serverlist)
 {
     krb5_error_code code;
 
     code = k5_locate_server(context, realm, serverlist, locate_service_kpasswd,
-                            no_udp);
-
+                            FALSE);
     if (code == KRB5_REALM_CANT_RESOLVE || code == KRB5_REALM_UNKNOWN) {
         code = k5_locate_server(context, realm, serverlist,
                                 locate_service_kadmin, TRUE);
@@ -76,7 +75,7 @@ locate_kpasswd(krb5_context context, const krb5_data *realm,
             for (i = 0; i < serverlist->nservers; i++) {
                 struct server_entry *s = &serverlist->servers[i];
 
-                if (!no_udp && s->transport == TCP)
+                if (s->transport == TCP)
                     s->transport = TCP_OR_UDP;
                 if (s->hostname != NULL)
                     s->port = DEFAULT_KPASSWD_PORT;
@@ -214,7 +213,6 @@ change_set_password(krb5_context context,
                     krb5_data *result_string)
 {
     krb5_data                   chpw_rep;
-    krb5_boolean                no_udp = FALSE;
     GETSOCKNAME_ARG3_TYPE       addrlen;
     krb5_error_code             code = 0;
     char                        *code_string;
@@ -246,73 +244,49 @@ change_set_password(krb5_context context,
     callback_ctx.remote_seq_num = callback_ctx.auth_context->remote_seq_number;
     callback_ctx.local_seq_num = callback_ctx.auth_context->local_seq_number;
 
-    do {
-        k5_transport_strategy strategy = no_udp ? NO_UDP : UDP_FIRST;
+    code = locate_kpasswd(callback_ctx.context, &creds->server->realm, &sl);
+    if (code)
+        goto cleanup;
 
-        code = locate_kpasswd(callback_ctx.context, &creds->server->realm, &sl,
-                              no_udp);
-        if (code)
-            break;
-
-        addrlen = sizeof(remote_addr);
-
-        callback_info.data = &callback_ctx;
-        callback_info.pfn_callback = kpasswd_sendto_msg_callback;
-        callback_info.pfn_cleanup = kpasswd_sendto_msg_cleanup;
-        krb5_free_data_contents(callback_ctx.context, &chpw_rep);
-
-        code = k5_sendto(callback_ctx.context, NULL, &creds->server->realm,
-                         &sl, strategy, &callback_info, &chpw_rep,
-                         ss2sa(&remote_addr), &addrlen, NULL, NULL, NULL);
-        if (code) {
-            /*
-             * Here we may want to switch to TCP on some errors.
-             * right?
-             */
-            break;
-        }
+    addrlen = sizeof(remote_addr);
 
-        code = krb5int_rd_chpw_rep(callback_ctx.context,
-                                   callback_ctx.auth_context,
-                                   &chpw_rep, &local_result_code,
-                                   result_string);
+    callback_info.data = &callback_ctx;
+    callback_info.pfn_callback = kpasswd_sendto_msg_callback;
+    callback_info.pfn_cleanup = kpasswd_sendto_msg_cleanup;
+    krb5_free_data_contents(callback_ctx.context, &chpw_rep);
 
-        if (code) {
-            if (code == KRB5KRB_ERR_RESPONSE_TOO_BIG && !no_udp) {
-                k5_free_serverlist(&sl);
-                no_udp = 1;
-                continue;
-            }
+    code = k5_sendto(callback_ctx.context, NULL, &creds->server->realm,
+                     &sl, UDP_LAST, &callback_info, &chpw_rep,
+                     ss2sa(&remote_addr), &addrlen, NULL, NULL, NULL);
+    if (code)
+        goto cleanup;
 
-            break;
-        }
+    code = krb5int_rd_chpw_rep(callback_ctx.context,
+                               callback_ctx.auth_context,
+                               &chpw_rep, &local_result_code,
+                               result_string);
 
-        if (result_code)
-            *result_code = local_result_code;
-
-        if (result_code_string) {
-            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)
+        goto cleanup;
+
+    if (result_code)
+        *result_code = local_result_code;
 
-        if (code == KRB5KRB_ERR_RESPONSE_TOO_BIG && !no_udp) {
-            k5_free_serverlist(&sl);
-            no_udp = 1;
-        } else {
-            break;
+    if (result_code_string) {
+        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;
         }
-    } while (TRUE);
+        strncpy(result_code_string->data, code_string, result_code_string->length);
+    }
 
 cleanup:
     if (callback_ctx.auth_context != NULL)