From e34fe90bed52f7845a1ba3924c436b5a00dc33a4 Mon Sep 17 00:00:00 2001 From: Zhanna Tsitkov Date: Thu, 30 Apr 2009 16:27:08 +0000 Subject: [PATCH] Uninitialized variable may cause unpredictable behavior in kdc_verify_preauth git-svn-id: svn://anonsvn.mit.edu/krb5/trunk@22294 dc483132-0cff-0310-8789-dd5450dbe970 --- .../encrypted_challenge_main.c | 477 +++++++++--------- 1 file changed, 239 insertions(+), 238 deletions(-) diff --git a/src/plugins/preauth/encrypted_challenge/encrypted_challenge_main.c b/src/plugins/preauth/encrypted_challenge/encrypted_challenge_main.c index 692449150..95d0f8a31 100644 --- a/src/plugins/preauth/encrypted_challenge/encrypted_challenge_main.c +++ b/src/plugins/preauth/encrypted_challenge/encrypted_challenge_main.c @@ -41,23 +41,23 @@ static int preauth_flags static krb5_error_code process_preauth (krb5_context context, - void *plugin_context, - void *request_context, - krb5_get_init_creds_opt *opt, - preauth_get_client_data_proc get_data_proc, - struct _krb5_preauth_client_rock *rock, - krb5_kdc_req *request, - krb5_data *encoded_request_body, - krb5_data *encoded_previous_request, - krb5_pa_data *padata, - krb5_prompter_fct prompter, - void *prompter_data, - preauth_get_as_key_proc gak_fct, - void *gak_data, - krb5_data *salt, - krb5_data *s2kparams, - krb5_keyblock *as_key, - krb5_pa_data ***out_padata) + void *plugin_context, + void *request_context, + krb5_get_init_creds_opt *opt, + preauth_get_client_data_proc get_data_proc, + struct _krb5_preauth_client_rock *rock, + krb5_kdc_req *request, + krb5_data *encoded_request_body, + krb5_data *encoded_previous_request, + krb5_pa_data *padata, + krb5_prompter_fct prompter, + void *prompter_data, + preauth_get_as_key_proc gak_fct, + void *gak_data, + krb5_data *salt, + krb5_data *s2kparams, + krb5_keyblock *as_key, + krb5_pa_data ***out_padata) { krb5_error_code retval = 0; krb5_enctype enctype = 0; @@ -66,150 +66,150 @@ static krb5_error_code process_preauth krb5int_access kaccess; if (krb5int_accessor(&kaccess, KRB5INT_ACCESS_VERSION) != 0) - return 0; + return 0; retval = fast_get_armor_key(context, get_data_proc, rock, &armor_key); if (retval || armor_key == NULL) - return 0; - retval = get_data_proc(context, rock, krb5plugin_preauth_client_get_etype, &etype_data); + return 0; + retval = get_data_proc(context, rock, krb5plugin_preauth_client_get_etype, &etype_data); if (retval == 0) { - enctype = *((krb5_enctype *)etype_data->data); - if (as_key->length == 0 ||as_key->enctype != enctype) - retval = gak_fct(context, request->client, - enctype, prompter, prompter_data, - salt, s2kparams, - as_key, gak_data); + enctype = *((krb5_enctype *)etype_data->data); + if (as_key->length == 0 ||as_key->enctype != enctype) + retval = gak_fct(context, request->client, + enctype, prompter, prompter_data, + salt, s2kparams, + as_key, gak_data); } if (padata->length) { - krb5_enc_data *enc = NULL; - krb5_data scratch; - scratch.length = padata->length; - scratch.data = (char *) padata->contents; - if (retval == 0) - retval = krb5_c_fx_cf2_simple(context,armor_key, "kdcchallengearmor", - as_key, "challengelongterm", &challenge_key); - if (retval == 0) - retval =kaccess.decode_enc_data(&scratch, &enc); - scratch.data = NULL; - if (retval == 0) { - scratch.data = malloc(enc->ciphertext.length); - scratch.length = enc->ciphertext.length; - if (scratch.data == NULL) - retval = ENOMEM; - } - if (retval == 0) - retval = krb5_c_decrypt(context, challenge_key, - KRB5_KEYUSAGE_ENC_CHALLENGE_KDC, NULL, - enc, &scratch); + krb5_enc_data *enc = NULL; + krb5_data scratch; + scratch.length = padata->length; + scratch.data = (char *) padata->contents; + if (retval == 0) + retval = krb5_c_fx_cf2_simple(context,armor_key, "kdcchallengearmor", + as_key, "challengelongterm", &challenge_key); + if (retval == 0) + retval =kaccess.decode_enc_data(&scratch, &enc); + scratch.data = NULL; + if (retval == 0) { + scratch.data = malloc(enc->ciphertext.length); + scratch.length = enc->ciphertext.length; + if (scratch.data == NULL) + retval = ENOMEM; + } + if (retval == 0) + retval = krb5_c_decrypt(context, challenge_key, + KRB5_KEYUSAGE_ENC_CHALLENGE_KDC, NULL, + enc, &scratch); /*Per draft 11 of the preauth framework, the client MAY but * is not required to actually check the timestamp from the KDC other than * to confirm it decrypts. This code does not perform that check. */ - if (scratch.data) - krb5_free_data_contents(context, &scratch); - if (retval == 0) - fast_set_kdc_verified(context, get_data_proc, rock); - if (enc) - kaccess.free_enc_data(context, enc); + if (scratch.data) + krb5_free_data_contents(context, &scratch); + if (retval == 0) + fast_set_kdc_verified(context, get_data_proc, rock); + if (enc) + kaccess.free_enc_data(context, enc); } else { /*No padata; we send*/ - krb5_enc_data enc; - krb5_pa_data *pa = NULL; - krb5_pa_data **pa_array = NULL; - krb5_data *encoded_ts = NULL; - krb5_pa_enc_ts ts; - enc.ciphertext.data = NULL; - if (retval == 0) - retval = krb5_us_timeofday(context, &ts.patimestamp, &ts.pausec); - if (retval == 0) - retval = kaccess.encode_enc_ts(&ts, &encoded_ts); - if (retval == 0) - retval = krb5_c_fx_cf2_simple(context, - armor_key, "clientchallengearmor", - as_key, "challengelongterm", - &challenge_key); - if (retval == 0) - retval = kaccess.encrypt_helper(context, challenge_key, - KRB5_KEYUSAGE_ENC_CHALLENGE_CLIENT, - encoded_ts, &enc); - if (encoded_ts) - krb5_free_data(context, encoded_ts); - encoded_ts = NULL; - if (retval == 0) { - retval = kaccess.encode_enc_data(&enc, &encoded_ts); - krb5_free_data_contents(context, &enc.ciphertext); - } - if (retval == 0) { - pa = calloc(1, sizeof(krb5_pa_data)); - if (pa == NULL) - retval = ENOMEM; - } - if (retval == 0) { - pa_array = calloc(2, sizeof(krb5_pa_data *)); - if (pa_array == NULL) - retval = ENOMEM; - } - if (retval == 0) { - pa->length = encoded_ts->length; - pa->contents = (unsigned char *) encoded_ts->data; - pa->pa_type = KRB5_PADATA_ENCRYPTED_CHALLENGE; - free(encoded_ts); - encoded_ts = NULL; - pa_array[0] = pa; - pa = NULL; - *out_padata = pa_array; - pa_array = NULL; - } - if (pa) - free(pa); - if (encoded_ts) - krb5_free_data(context, encoded_ts); - if (pa_array) - free(pa_array); + krb5_enc_data enc; + krb5_pa_data *pa = NULL; + krb5_pa_data **pa_array = NULL; + krb5_data *encoded_ts = NULL; + krb5_pa_enc_ts ts; + enc.ciphertext.data = NULL; + if (retval == 0) + retval = krb5_us_timeofday(context, &ts.patimestamp, &ts.pausec); + if (retval == 0) + retval = kaccess.encode_enc_ts(&ts, &encoded_ts); + if (retval == 0) + retval = krb5_c_fx_cf2_simple(context, + armor_key, "clientchallengearmor", + as_key, "challengelongterm", + &challenge_key); + if (retval == 0) + retval = kaccess.encrypt_helper(context, challenge_key, + KRB5_KEYUSAGE_ENC_CHALLENGE_CLIENT, + encoded_ts, &enc); + if (encoded_ts) + krb5_free_data(context, encoded_ts); + encoded_ts = NULL; + if (retval == 0) { + retval = kaccess.encode_enc_data(&enc, &encoded_ts); + krb5_free_data_contents(context, &enc.ciphertext); + } + if (retval == 0) { + pa = calloc(1, sizeof(krb5_pa_data)); + if (pa == NULL) + retval = ENOMEM; + } + if (retval == 0) { + pa_array = calloc(2, sizeof(krb5_pa_data *)); + if (pa_array == NULL) + retval = ENOMEM; + } + if (retval == 0) { + pa->length = encoded_ts->length; + pa->contents = (unsigned char *) encoded_ts->data; + pa->pa_type = KRB5_PADATA_ENCRYPTED_CHALLENGE; + free(encoded_ts); + encoded_ts = NULL; + pa_array[0] = pa; + pa = NULL; + *out_padata = pa_array; + pa_array = NULL; + } + if (pa) + free(pa); + if (encoded_ts) + krb5_free_data(context, encoded_ts); + if (pa_array) + free(pa_array); } if (challenge_key) - krb5_free_keyblock(context, challenge_key); + krb5_free_keyblock(context, challenge_key); if (armor_key) - krb5_free_keyblock(context, armor_key); + krb5_free_keyblock(context, armor_key); if (etype_data != NULL) - get_data_proc(context, rock, krb5plugin_preauth_client_free_etype, - &etype_data); + get_data_proc(context, rock, krb5plugin_preauth_client_free_etype, + &etype_data); return retval; } - - - + + + static krb5_error_code kdc_include_padata (krb5_context context, - krb5_kdc_req *request, - struct _krb5_db_entry_new *client, - struct _krb5_db_entry_new *server, - preauth_get_entry_data_proc get_entry_proc, - void *pa_module_context, - krb5_pa_data *data) + krb5_kdc_req *request, + struct _krb5_db_entry_new *client, + struct _krb5_db_entry_new *server, + preauth_get_entry_data_proc get_entry_proc, + void *pa_module_context, + krb5_pa_data *data) { krb5_error_code retval = 0; krb5_keyblock *armor_key = NULL; retval = fast_kdc_get_armor_key(context, get_entry_proc, request, client, &armor_key); if (retval) - return retval; + return retval; if (armor_key == 0) - return ENOENT; + return ENOENT; krb5_free_keyblock(context, armor_key); return 0; } static krb5_error_code kdc_verify_preauth (krb5_context context, - struct _krb5_db_entry_new *client, - krb5_data *req_pkt, - krb5_kdc_req *request, - krb5_enc_tkt_part *enc_tkt_reply, - krb5_pa_data *data, - preauth_get_entry_data_proc get_entry_proc, - void *pa_module_context, - void **pa_request_context, - krb5_data **e_data, - krb5_authdata ***authz_data) + struct _krb5_db_entry_new *client, + krb5_data *req_pkt, + krb5_kdc_req *request, + krb5_enc_tkt_part *enc_tkt_reply, + krb5_pa_data *data, + preauth_get_entry_data_proc get_entry_proc, + void *pa_module_context, + void **pa_request_context, + krb5_data **e_data, + krb5_authdata ***authz_data) { krb5_error_code retval = 0; krb5_timestamp now; @@ -221,108 +221,108 @@ static krb5_error_code kdc_verify_preauth krb5_keyblock *client_keys = NULL; krb5_data *client_data = NULL; krb5_keyblock *challenge_key = NULL; - int i; + int i = 0; plain.data = NULL; if (krb5int_accessor(&kaccess, KRB5INT_ACCESS_VERSION) != 0) - return 0; + return 0; retval = fast_kdc_get_armor_key(context, get_entry_proc, request, client, &armor_key); if (retval == 0 &&armor_key == NULL) { - retval = ENOENT; - krb5_set_error_message(context, ENOENT, "Encrypted Challenge used outside of FAST tunnel"); + retval = ENOENT; + krb5_set_error_message(context, ENOENT, "Encrypted Challenge used outside of FAST tunnel"); } scratch.data = (char *) data->contents; scratch.length = data->length; if (retval == 0) - retval = kaccess.decode_enc_data(&scratch, &enc); + retval = kaccess.decode_enc_data(&scratch, &enc); if (retval == 0) { - plain.data = malloc(enc->ciphertext.length); - plain.length = enc->ciphertext.length; - if (plain.data == NULL) - retval = ENOMEM; + plain.data = malloc(enc->ciphertext.length); + plain.length = enc->ciphertext.length; + if (plain.data == NULL) + retval = ENOMEM; } if (retval == 0) - retval = get_entry_proc(context, request, client, - krb5plugin_preauth_keys, &client_data); + retval = get_entry_proc(context, request, client, + krb5plugin_preauth_keys, &client_data); if (retval == 0) { - client_keys = (krb5_keyblock *) client_data->data; - for (i = 0; client_keys[i].enctype&& (retval == 0); i++ ) { - retval = krb5_c_fx_cf2_simple(context, - armor_key, "clientchallengearmor", - &client_keys[i], "challengelongterm", - &challenge_key); - if (retval == 0) - retval = krb5_c_decrypt(context, challenge_key, - KRB5_KEYUSAGE_ENC_CHALLENGE_CLIENT, - NULL, enc, &plain); - if (challenge_key) - krb5_free_keyblock(context, challenge_key); - challenge_key = NULL; - if (retval == 0) - break; - /*We failed to decrypt. Try next key*/ - retval = 0; - krb5_free_keyblock_contents(context, &client_keys[i]); - } - if (client_keys[i].enctype == 0) { - retval = KRB5KDC_ERR_PREAUTH_FAILED; - krb5_set_error_message(context, retval, "Incorrect password in encrypted challenge"); - } else { /*not run out of keys*/ - int j; - assert (retval == 0); - for (j = i+1; client_keys[j].enctype; j++) - krb5_free_keyblock_contents(context, &client_keys[j]); - } - + client_keys = (krb5_keyblock *) client_data->data; + for (i = 0; client_keys[i].enctype&& (retval == 0); i++ ) { + retval = krb5_c_fx_cf2_simple(context, + armor_key, "clientchallengearmor", + &client_keys[i], "challengelongterm", + &challenge_key); + if (retval == 0) + retval = krb5_c_decrypt(context, challenge_key, + KRB5_KEYUSAGE_ENC_CHALLENGE_CLIENT, + NULL, enc, &plain); + if (challenge_key) + krb5_free_keyblock(context, challenge_key); + challenge_key = NULL; + if (retval == 0) + break; + /*We failed to decrypt. Try next key*/ + retval = 0; + krb5_free_keyblock_contents(context, &client_keys[i]); + } + if (client_keys[i].enctype == 0) { + retval = KRB5KDC_ERR_PREAUTH_FAILED; + krb5_set_error_message(context, retval, "Incorrect password in encrypted challenge"); + } else { /*not run out of keys*/ + int j; + assert (retval == 0); + for (j = i+1; client_keys[j].enctype; j++) + krb5_free_keyblock_contents(context, &client_keys[j]); + } + } if (retval == 0) - retval = kaccess.decode_enc_ts(&plain, &ts); + retval = kaccess.decode_enc_ts(&plain, &ts); if (retval == 0) - retval = krb5_timeofday(context, &now); + retval = krb5_timeofday(context, &now); if (retval == 0) { - if (labs(now-ts->patimestamp) < context->clockskew) { - enc_tkt_reply->flags |= TKT_FLG_PRE_AUTH; -/*If this fails, we won't generate a reply to the client. That may - * cause the client to fail, but at this point the KDC has considered - this a success, so the return value is ignored. */ - fast_kdc_replace_reply_key(context, get_entry_proc, request); - krb5_c_fx_cf2_simple(context, armor_key, "kdcchallengearmor", - &client_keys[i], "challengelongterm", - (krb5_keyblock **) pa_request_context); - } else { /*skew*/ - retval = KRB5KRB_AP_ERR_SKEW; - } + if (labs(now-ts->patimestamp) < context->clockskew) { + enc_tkt_reply->flags |= TKT_FLG_PRE_AUTH; + /*If this fails, we won't generate a reply to the client. That may + * cause the client to fail, but at this point the KDC has considered + * this a success, so the return value is ignored. */ + fast_kdc_replace_reply_key(context, get_entry_proc, request); + krb5_c_fx_cf2_simple(context, armor_key, "kdcchallengearmor", + &client_keys[i], "challengelongterm", + (krb5_keyblock **) pa_request_context); + } else { /*skew*/ + retval = KRB5KRB_AP_ERR_SKEW; + } } if (client_keys) { - if (client_keys[i].enctype) - krb5_free_keyblock_contents(context, &client_keys[i]); - krb5_free_data(context, client_data); + if (client_keys[i].enctype) + krb5_free_keyblock_contents(context, &client_keys[i]); + krb5_free_data(context, client_data); } if (armor_key) - krb5_free_keyblock(context, armor_key); + krb5_free_keyblock(context, armor_key); if (plain.data) - free(plain.data); + free(plain.data); if (enc) - kaccess.free_enc_data(context, enc); + kaccess.free_enc_data(context, enc); if (ts) - kaccess.free_enc_ts(context, ts); + kaccess.free_enc_ts(context, ts); return retval; } static krb5_error_code kdc_return_preauth (krb5_context context, - krb5_pa_data * padata, - struct _krb5_db_entry_new *client, - krb5_data *req_pkt, - krb5_kdc_req *request, - krb5_kdc_rep *reply, - struct _krb5_key_data *client_keys, - krb5_keyblock *encrypting_key, - krb5_pa_data **send_pa, - preauth_get_entry_data_proc get_entry_proc, - void *pa_module_context, - void **pa_request_context) + krb5_pa_data * padata, + struct _krb5_db_entry_new *client, + krb5_data *req_pkt, + krb5_kdc_req *request, + krb5_kdc_rep *reply, + struct _krb5_key_data *client_keys, + krb5_keyblock *encrypting_key, + krb5_pa_data **send_pa, + preauth_get_entry_data_proc get_entry_proc, + void *pa_module_context, + void **pa_request_context) { krb5_error_code retval = 0; krb5_keyblock *challenge_key = *pa_request_context; @@ -334,41 +334,41 @@ static krb5_error_code kdc_return_preauth krb5int_access kaccess; if (krb5int_accessor(&kaccess, KRB5INT_ACCESS_VERSION) != 0) - return 0; + return 0; if (challenge_key == NULL) - return 0; + return 0; * pa_request_context = NULL; /*this function will free the - * challenge key*/ + * challenge key*/ retval = krb5_us_timeofday(context, &ts.patimestamp, &ts.pausec); if (retval == 0) - retval = kaccess.encode_enc_ts(&ts, &plain); + retval = kaccess.encode_enc_ts(&ts, &plain); if (retval == 0) - retval = kaccess.encrypt_helper(context, challenge_key, - KRB5_KEYUSAGE_ENC_CHALLENGE_KDC, - plain, &enc); + retval = kaccess.encrypt_helper(context, challenge_key, + KRB5_KEYUSAGE_ENC_CHALLENGE_KDC, + plain, &enc); if (retval == 0) - retval = kaccess.encode_enc_data(&enc, &encoded); + retval = kaccess.encode_enc_data(&enc, &encoded); if (retval == 0) { - pa = calloc(1, sizeof(krb5_pa_data)); - if (pa == NULL) - retval = ENOMEM; + pa = calloc(1, sizeof(krb5_pa_data)); + if (pa == NULL) + retval = ENOMEM; } if (retval == 0) { - pa->pa_type = KRB5_PADATA_ENCRYPTED_CHALLENGE; - pa->contents = (unsigned char *) encoded->data; - pa->length = encoded->length; - encoded->data = NULL; - *send_pa = pa; - pa = NULL; + pa->pa_type = KRB5_PADATA_ENCRYPTED_CHALLENGE; + pa->contents = (unsigned char *) encoded->data; + pa->length = encoded->length; + encoded->data = NULL; + *send_pa = pa; + pa = NULL; } if (challenge_key) - krb5_free_keyblock(context, challenge_key); + krb5_free_keyblock(context, challenge_key); if (encoded) - krb5_free_data(context, encoded); + krb5_free_data(context, encoded); if (plain) - krb5_free_data(context, plain); + krb5_free_data(context, plain); if (enc.ciphertext.data) - krb5_free_data_contents(context, &enc.ciphertext); + krb5_free_data_contents(context, &enc.ciphertext); return retval; } @@ -384,25 +384,26 @@ krb5_preauthtype supported_pa_types[] = { struct krb5plugin_preauth_server_ftable_v1 preauthentication_server_1 = { "Encrypted challenge", &supported_pa_types[0], -NULL, -NULL, -kdc_preauth_flags, + NULL, + NULL, + kdc_preauth_flags, kdc_include_padata, kdc_verify_preauth, kdc_return_preauth, -NULL + NULL }; struct krb5plugin_preauth_client_ftable_v1 preauthentication_client_1 = { - "Encrypted Challenge", /* name */ - &supported_pa_types[0], /* pa_type_list */ - NULL, /* enctype_list */ - NULL, /* plugin init function */ - NULL, /* plugin fini function */ - preauth_flags, /* get flags function */ - NULL, /* request init function */ - NULL, /* request fini function */ - process_preauth, /* process function */ - NULL, /* try_again function */ -NULL /* get init creds opt function */ + "Encrypted Challenge", /* name */ + &supported_pa_types[0], /* pa_type_list */ + NULL, /* enctype_list */ + NULL, /* plugin init function */ + NULL, /* plugin fini function */ + preauth_flags, /* get flags function */ + NULL, /* request init function */ + NULL, /* request fini function */ + process_preauth, /* process function */ + NULL, /* try_again function */ + NULL /* get init creds opt function */ }; + -- 2.26.2