From 328eb9db6a2b03f0724e9e5c3fa724bc5e30aaa4 Mon Sep 17 00:00:00 2001 From: Greg Hudson Date: Mon, 25 Apr 2011 17:28:42 +0000 Subject: [PATCH] Refactor krb5int_rd_chpw_rep() and make it properly handle both framed and unframed KRB-ERROR messages. Eliminate krb5int_rd_setpw_rep() and krb5int_setpw_result_code_string() by making the chpw versions of those functions handle RFC 3244 replies. ticket: 6893 git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@24899 dc483132-0cff-0310-8789-dd5450dbe970 --- src/lib/krb5/krb/chpw.c | 466 +++++++++++------------------------ src/lib/krb5/krb/int-proto.h | 18 ++ src/lib/krb5/os/changepw.c | 30 +-- 3 files changed, 165 insertions(+), 349 deletions(-) diff --git a/src/lib/krb5/krb/chpw.c b/src/lib/krb5/krb/chpw.c index 3f359ba71..305808bd6 100644 --- a/src/lib/krb5/krb/chpw.c +++ b/src/lib/krb5/krb/chpw.c @@ -5,6 +5,7 @@ #include #include "k5-int.h" +#include "int-proto.h" #include "auth_con.h" @@ -73,166 +74,172 @@ cleanup: return(ret); } -krb5_error_code -krb5int_rd_chpw_rep(krb5_context context, krb5_auth_context auth_context, - krb5_data *packet, int *result_code, krb5_data *result_data) +/* Decode error_packet as a KRB-ERROR message and retrieve its e-data into + * *edata_out. */ +static krb5_error_code +get_error_edata(krb5_context context, const krb5_data *error_packet, + krb5_data **edata_out) { - char *ptr; - unsigned int plen, vno; - krb5_data ap_rep; - krb5_ap_rep_enc_part *ap_rep_enc; krb5_error_code ret; - krb5_data cipherresult; - krb5_data clearresult; krb5_error *krberror = NULL; - krb5_replay_data replay; - krb5_keyblock *tmp; - if (packet->length < 4) - /* either this, or the server is printing bad messages, - or the caller passed in garbage */ - return(KRB5KRB_AP_ERR_MODIFIED); + *edata_out = NULL; - ptr = packet->data; - - /* verify length */ + ret = krb5_rd_error(context, error_packet, &krberror); + if (ret) + return ret; - plen = (*ptr++ & 0xff); - plen = (plen<<8) | (*ptr++ & 0xff); - - if (plen != packet->length) { - /* - * MS KDCs *may* send back a KRB_ERROR. Although - * not 100% correct via RFC3244, it's something - * we can workaround here. - */ - if (krb5_is_krb_error(packet)) { - - if ((ret = krb5_rd_error(context, packet, &krberror))) - return(ret); - - if (krberror->e_data.data == NULL) - ret = ERROR_TABLE_BASE_krb5 + (krb5_error_code) krberror->error; - else - ret = KRB5KRB_AP_ERR_MODIFIED; - krb5_free_error(context, krberror); - return(ret); - } else { - return(KRB5KRB_AP_ERR_MODIFIED); - } + if (krberror->e_data.data == NULL) { + /* Return a krb5 error code based on the error number. */ + ret = ERROR_TABLE_BASE_krb5 + (krb5_error_code)krberror->error; + goto cleanup; } + ret = krb5_copy_data(context, &krberror->e_data, edata_out); - /* verify version number */ - - vno = (*ptr++ & 0xff); - vno = (vno<<8) | (*ptr++ & 0xff); +cleanup: + krb5_free_error(context, krberror); + return ret; +} - if (vno != 1) - return(KRB5KDC_ERR_BAD_PVNO); +/* Decode a reply to produce the clear-text output. */ +static krb5_error_code +get_clear_result(krb5_context context, krb5_auth_context auth_context, + const krb5_data *packet, krb5_data **clear_out, + krb5_boolean *is_error_out) +{ + krb5_error_code ret; + char *ptr, *end = packet->data + packet->length; + unsigned int plen, vno, aplen; + krb5_data ap_rep, cipher, error; + krb5_ap_rep_enc_part *ap_rep_enc; + krb5_replay_data replay; + krb5_key send_subkey = NULL; + krb5_data clear = empty_data(); - /* read, check ap-rep length */ + *clear_out = NULL; + *is_error_out = FALSE; - ap_rep.length = (*ptr++ & 0xff); - ap_rep.length = (ap_rep.length<<8) | (*ptr++ & 0xff); + /* Check for an unframed KRB-ERROR (expected for RFC 3244 requests; also + * received from MS AD for version 1 requests). */ + if (krb5_is_krb_error(packet)) { + *is_error_out = TRUE; + return get_error_edata(context, packet, clear_out); + } - if (ptr + ap_rep.length >= packet->data + packet->length) - return(KRB5KRB_AP_ERR_MODIFIED); + if (packet->length < 6) + return KRB5KRB_AP_ERR_MODIFIED; - if (ap_rep.length) { - /* verify ap_rep */ - ap_rep.data = ptr; - ptr += ap_rep.length; + /* Decode and verify the length. */ + ptr = packet->data; + plen = (*ptr++ & 0xff); + plen = (plen << 8) | (*ptr++ & 0xff); + if (plen != packet->length) + return KRB5KRB_AP_ERR_MODIFIED; - /* - * Save send_subkey to later smash recv_subkey. - */ - ret = krb5_auth_con_getsendsubkey(context, auth_context, &tmp); - if (ret) - return ret; + /* Decode and verify the version number. */ + vno = (*ptr++ & 0xff); + vno = (vno << 8) | (*ptr++ & 0xff); + if (vno != 1 && vno != 0xff80) + return KRB5KDC_ERR_BAD_PVNO; + + /* Decode and check the AP-REP length. */ + aplen = (*ptr++ & 0xff); + aplen = (aplen << 8) | (*ptr++ & 0xff); + if (aplen > end - ptr) + return KRB5KRB_AP_ERR_MODIFIED; + + /* A zero-length AP-REQ indicates a framed KRB-ERROR response. (Expected + * for protocol version 1; specified but unusual for RFC 3244 requests.) */ + if (aplen == 0) { + *is_error_out = TRUE; + error = make_data(ptr, end - ptr); + return get_error_edata(context, &error, clear_out); + } - ret = krb5_rd_rep(context, auth_context, &ap_rep, &ap_rep_enc); - if (ret) { - krb5_free_keyblock(context, tmp); - return(ret); - } + /* We have an AP-REP. Save send_subkey to later smash recv_subkey. */ + ret = krb5_auth_con_getsendsubkey_k(context, auth_context, &send_subkey); + if (ret) + return ret; - krb5_free_ap_rep_enc_part(context, ap_rep_enc); + /* Verify the AP-REP. */ + ap_rep = make_data(ptr, aplen); + ptr += ap_rep.length; + ret = krb5_rd_rep(context, auth_context, &ap_rep, &ap_rep_enc); + if (ret) + goto cleanup; + krb5_free_ap_rep_enc_part(context, ap_rep_enc); - /* extract and decrypt the result */ + /* Smash recv_subkey to be send_subkey, per spec. */ + ret = krb5_auth_con_setrecvsubkey_k(context, auth_context, send_subkey); + if (ret) + goto cleanup; - cipherresult.data = ptr; - cipherresult.length = (packet->data + packet->length) - ptr; + /* Extract and decrypt the result. */ + cipher = make_data(ptr, end - ptr); + ret = krb5_rd_priv(context, auth_context, &cipher, &clear, &replay); + if (ret) + goto cleanup; - /* - * Smash recv_subkey to be send_subkey, per spec. - */ - ret = krb5_auth_con_setrecvsubkey(context, auth_context, tmp); - krb5_free_keyblock(context, tmp); - if (ret) - return ret; + ret = krb5_copy_data(context, &clear, clear_out); + if (ret) + goto cleanup; + *is_error_out = FALSE; - ret = krb5_rd_priv(context, auth_context, &cipherresult, &clearresult, - &replay); +cleanup: + krb5_k_free_key(context, send_subkey); + krb5_free_data_contents(context, &clear); + return ret; +} - if (ret) - return(ret); - } else { - cipherresult.data = ptr; - cipherresult.length = (packet->data + packet->length) - ptr; +krb5_error_code +krb5int_rd_chpw_rep(krb5_context context, krb5_auth_context auth_context, + krb5_data *packet, int *result_code_out, + krb5_data *result_data_out) +{ + krb5_error_code ret; + krb5_data result_data, *clear = NULL; + krb5_boolean is_error; + char *ptr; + int result_code; - if ((ret = krb5_rd_error(context, &cipherresult, &krberror))) - return(ret); + *result_code_out = 0; + *result_data_out = empty_data(); - clearresult = krberror->e_data; - } + ret = get_clear_result(context, auth_context, packet, &clear, &is_error); + if (ret) + return ret; - if (clearresult.length < 2) { + if (clear->length < 2) { ret = KRB5KRB_AP_ERR_MODIFIED; goto cleanup; } - ptr = clearresult.data; - - *result_code = (*ptr++ & 0xff); - *result_code = (*result_code<<8) | (*ptr++ & 0xff); - - if ((*result_code < KRB5_KPASSWD_SUCCESS) || - (*result_code > KRB5_KPASSWD_INITIAL_FLAG_NEEDED)) { + /* Decode and check the result code. */ + ptr = clear->data; + result_code = (*ptr++ & 0xff); + result_code = (result_code << 8) | (*ptr++ & 0xff); + if (result_code < KRB5_KPASSWD_SUCCESS || + result_code > KRB5_KPASSWD_INITIAL_FLAG_NEEDED) { ret = KRB5KRB_AP_ERR_MODIFIED; goto cleanup; } - /* all success replies should be authenticated/encrypted */ - - if ((ap_rep.length == 0) && (*result_code == KRB5_KPASSWD_SUCCESS)) { + /* Successful replies must not come from errors. */ + if (is_error && result_code == KRB5_KPASSWD_SUCCESS) { ret = KRB5KRB_AP_ERR_MODIFIED; goto cleanup; } - result_data->length = (clearresult.data + clearresult.length) - ptr; - - if (result_data->length) { - result_data->data = (char *) malloc(result_data->length); - if (result_data->data == NULL) { - ret = ENOMEM; - goto cleanup; - } - memcpy(result_data->data, ptr, result_data->length); - } else { - result_data->data = NULL; - } - - ret = 0; + result_data = make_data(ptr, clear->data + clear->length - ptr); + ret = krb5int_copy_data_contents(context, &result_data, result_data_out); + if (ret) + goto cleanup; + *result_code_out = result_code; cleanup: - if (ap_rep.length) { - free(clearresult.data); - } else { - krb5_free_error(context, krberror); - } - - return(ret); + krb5_free_data(context, clear); + return ret; } krb5_error_code KRB5_CALLCONV @@ -252,12 +259,21 @@ krb5_chpw_result_code_string(krb5_context context, int result_code, case KRB5_KPASSWD_SOFTERROR: *code_string = "Password change rejected"; break; + case KRB5_KPASSWD_ACCESSDENIED: + *code_string = "Access denied"; + break; + case KRB5_KPASSWD_BAD_VERSION: + *code_string = "Wrong protocol version"; + break; + case KRB5_KPASSWD_INITIAL_FLAG_NEEDED: + *code_string = "Initial password required"; + break; default: *code_string = "Password change failed"; break; } - return(0); + return 0; } krb5_error_code @@ -333,209 +349,3 @@ cleanup: } return ret; } - -krb5_error_code -krb5int_rd_setpw_rep(krb5_context context, krb5_auth_context auth_context, - krb5_data *packet, - int *result_code, krb5_data *result_data) -{ - char *ptr; - unsigned int message_length, version_number; - krb5_data ap_rep; - krb5_ap_rep_enc_part *ap_rep_enc; - krb5_error_code ret; - krb5_data cipherresult; - krb5_data clearresult; - krb5_keyblock *tmpkey; - /* - ** validate the packet length - - */ - if (packet->length < 4) - return(KRB5KRB_AP_ERR_MODIFIED); - - ptr = packet->data; - - /* - ** see if it is an error - */ - if (krb5_is_krb_error(packet)) { - krb5_error *krberror; - if ((ret = krb5_rd_error(context, packet, &krberror))) - return(ret); - if (krberror->e_data.data == NULL) { - ret = ERROR_TABLE_BASE_krb5 + (krb5_error_code) krberror->error; - krb5_free_error(context, krberror); - return (ret); - } - clearresult = krberror->e_data; - krberror->e_data.data = NULL; /*So we can free it later*/ - krberror->e_data.length = 0; - krb5_free_error(context, krberror); - ap_rep.length = 0; - - } else { /* Not an error*/ - - /* - ** validate the message length - - ** length is big endian - */ - message_length = (((ptr[0] << 8)&0xff) | (ptr[1]&0xff)); - ptr += 2; - /* - ** make sure the message length and packet length agree - - */ - if (message_length != packet->length) - return(KRB5KRB_AP_ERR_MODIFIED); - /* - ** get the version number - - */ - version_number = (((ptr[0] << 8)&0xff) | (ptr[1]&0xff)); - ptr += 2; - /* - ** make sure we support the version returned - - */ - /* - ** set password version is 0xff80, change password version is 1 - */ - if (version_number != 1 && version_number != 0xff80) - return(KRB5KDC_ERR_BAD_PVNO); - /* - ** now fill in ap_rep with the reply - - */ - /* - ** get the reply length - - */ - ap_rep.length = (((ptr[0] << 8)&0xff) | (ptr[1]&0xff)); - ptr += 2; - /* - ** validate ap_rep length agrees with the packet length - - */ - if (ptr + ap_rep.length >= packet->data + packet->length) - return(KRB5KRB_AP_ERR_MODIFIED); - /* - ** if data was returned, set the ap_rep ptr - - */ - if (ap_rep.length) { - ap_rep.data = ptr; - ptr += ap_rep.length; - - /* - * Save send_subkey to later smash recv_subkey. - */ - ret = krb5_auth_con_getsendsubkey(context, auth_context, &tmpkey); - if (ret) - return ret; - - ret = krb5_rd_rep(context, auth_context, &ap_rep, &ap_rep_enc); - if (ret) { - krb5_free_keyblock(context, tmpkey); - return(ret); - } - - krb5_free_ap_rep_enc_part(context, ap_rep_enc); - /* - ** now decrypt the result - - */ - cipherresult.data = ptr; - cipherresult.length = (packet->data + packet->length) - ptr; - - /* - * Smash recv_subkey to be send_subkey, per spec. - */ - ret = krb5_auth_con_setrecvsubkey(context, auth_context, tmpkey); - krb5_free_keyblock(context, tmpkey); - if (ret) - return ret; - - ret = krb5_rd_priv(context, auth_context, &cipherresult, &clearresult, - NULL); - if (ret) - return(ret); - } /*We got an ap_rep*/ - else - return (KRB5KRB_AP_ERR_MODIFIED); - } /*Response instead of error*/ - - /* - ** validate the cleartext length - */ - if (clearresult.length < 2) { - ret = KRB5KRB_AP_ERR_MODIFIED; - goto cleanup; - } - /* - ** now decode the result - - */ - ptr = clearresult.data; - - *result_code = (((ptr[0] << 8)&0xff) | (ptr[1]&0xff)); - ptr += 2; - - /* - ** result code 5 is access denied - */ - if ((*result_code < KRB5_KPASSWD_SUCCESS) || (*result_code > 5)) { - ret = KRB5KRB_AP_ERR_MODIFIED; - goto cleanup; - } - /* - ** all success replies should be authenticated/encrypted - */ - if ((ap_rep.length == 0) && (*result_code == KRB5_KPASSWD_SUCCESS)) { - ret = KRB5KRB_AP_ERR_MODIFIED; - goto cleanup; - } - - if (result_data) { - result_data->length = (clearresult.data + clearresult.length) - ptr; - - if (result_data->length) { - result_data->data = (char *) malloc(result_data->length); - if (result_data->data) - memcpy(result_data->data, ptr, result_data->length); - } else - result_data->data = NULL; - } - ret = 0; - -cleanup: - krb5_free_data_contents(context, &clearresult); - return(ret); -} - -krb5_error_code -krb5int_setpw_result_code_string(krb5_context context, int result_code, - const char **code_string) -{ - switch (result_code) { - case KRB5_KPASSWD_MALFORMED: - *code_string = "Malformed request error"; - break; - case KRB5_KPASSWD_HARDERROR: - *code_string = "Server error"; - break; - case KRB5_KPASSWD_AUTHERROR: - *code_string = "Authentication error"; - break; - case KRB5_KPASSWD_SOFTERROR: - *code_string = "Password change rejected"; - break; - case 5: /* access denied */ - *code_string = "Access denied"; - break; - case 6: /* bad version */ - *code_string = "Wrong protocol version"; - break; - case 7: /* initial flag is needed */ - *code_string = "Initial password required"; - break; - case 0: - *code_string = "Success"; - break; - default: - *code_string = "Password change failed"; - break; - } - - return(0); -} diff --git a/src/lib/krb5/krb/int-proto.h b/src/lib/krb5/krb/int-proto.h index 5cd3fc895..4a3cea80d 100644 --- a/src/lib/krb5/krb/int-proto.h +++ b/src/lib/krb5/krb/int-proto.h @@ -160,4 +160,22 @@ krb5_error_code k5_privsafe_check_addrs(krb5_context context, krb5_auth_context ac, krb5_address *msg_s_addr, krb5_address *msg_r_addr); +krb5_error_code +krb5int_mk_chpw_req(krb5_context context, krb5_auth_context auth_context, + krb5_data *ap_req, char *passwd, krb5_data *packet); + +krb5_error_code +krb5int_rd_chpw_rep(krb5_context context, krb5_auth_context auth_context, + krb5_data *packet, int *result_code, + krb5_data *result_data); + +krb5_error_code KRB5_CALLCONV +krb5_chpw_result_code_string(krb5_context context, int result_code, + char **result_codestr); + +krb5_error_code +krb5int_mk_setpw_req(krb5_context context, krb5_auth_context auth_context, + krb5_data *ap_req, krb5_principal targetprinc, + char *passwd, krb5_data *packet); + #endif /* KRB5_INT_FUNC_PROTO__ */ diff --git a/src/lib/krb5/os/changepw.c b/src/lib/krb5/os/changepw.c index 5c383ecd4..4ad8f32eb 100644 --- a/src/lib/krb5/os/changepw.c +++ b/src/lib/krb5/os/changepw.c @@ -36,6 +36,7 @@ #include "os-proto.h" #include "cm.h" #include "../krb/auth_con.h" +#include "../krb/int-proto.h" #include #include @@ -300,18 +301,10 @@ change_set_password(krb5_context context, &remote_kaddr))) break; - if (set_password_for) - code = krb5int_rd_setpw_rep(callback_ctx.context, - callback_ctx.auth_context, - &chpw_rep, - &local_result_code, - result_string); - else - code = krb5int_rd_chpw_rep(callback_ctx.context, - callback_ctx.auth_context, - &chpw_rep, - &local_result_code, - result_string); + code = krb5int_rd_chpw_rep(callback_ctx.context, + callback_ctx.auth_context, + &chpw_rep, &local_result_code, + result_string); if (code) { if (code == KRB5KRB_ERR_RESPONSE_TOO_BIG && !use_tcp) { @@ -327,15 +320,10 @@ change_set_password(krb5_context context, *result_code = local_result_code; if (result_code_string) { - if (set_password_for) - code = krb5int_setpw_result_code_string(callback_ctx.context, - local_result_code, - (const char **)&code_string); - else - code = krb5_chpw_result_code_string(callback_ctx.context, - local_result_code, - &code_string); - if(code) + 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); -- 2.26.2