Skip to content

Commit

Permalink
Prefer TCP to UDP for password changes
Browse files Browse the repository at this point in the history
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
  • Loading branch information
frozencemetery authored and greghudson committed Oct 9, 2018
1 parent eb5d2c9 commit d7b3018
Showing 1 changed file with 39 additions and 65 deletions.
104 changes: 39 additions & 65 deletions src/lib/krb5/os/changepw.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit d7b3018

Please sign in to comment.