From f153e41bb2c559c6ae37040afb13f8a9084a9dda Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Sat, 2 May 2015 16:03:12 +0300 Subject: [PATCH] EAP-EKE: Do not pass full request to eap_eke_build_fail() This function is only using the Identifier field from the EAP request header, so there is no need to pass it a pointer to the full message. This makes it a bit easier to analyze the area that gets access to unverified message payload. Signed-off-by: Jouni Malinen --- src/eap_peer/eap_eke.c | 81 ++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/src/eap_peer/eap_eke.c b/src/eap_peer/eap_eke.c index 9fec66c06..987af83f5 100644 --- a/src/eap_peer/eap_eke.c +++ b/src/eap_peer/eap_eke.c @@ -195,15 +195,14 @@ static int eap_eke_supp_mac(u8 mac) static struct wpabuf * eap_eke_build_fail(struct eap_eke_data *data, struct eap_method_ret *ret, - const struct wpabuf *reqData, - u32 failure_code) + u8 id, u32 failure_code) { struct wpabuf *resp; wpa_printf(MSG_DEBUG, "EAP-EKE: Sending EAP-EKE-Failure/Response - code=0x%x", failure_code); - resp = eap_eke_build_msg(data, eap_get_id(reqData), 4, EAP_EKE_FAILURE); + resp = eap_eke_build_msg(data, id, 4, EAP_EKE_FAILURE); if (resp) wpabuf_put_be32(resp, failure_code); @@ -230,9 +229,10 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, const u8 *pos, *end; const u8 *prop = NULL; u8 idtype; + u8 id = eap_get_id(reqData); if (data->state != IDENTITY) { - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -240,7 +240,7 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, if (payload_len < 2 + 4) { wpa_printf(MSG_DEBUG, "EAP-EKE: Too short ID/Request Data"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -253,7 +253,7 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, if (pos + num_prop * 4 > end) { wpa_printf(MSG_DEBUG, "EAP-EKE: Too short ID/Request Data (num_prop=%u)", num_prop); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -293,7 +293,7 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, if (prop == NULL) { wpa_printf(MSG_DEBUG, "EAP-EKE: No acceptable proposal found"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_NO_PROPOSAL_CHOSEN); } @@ -301,7 +301,7 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, if (pos == end) { wpa_printf(MSG_DEBUG, "EAP-EKE: Too short ID/Request Data to include IDType/Identity"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -312,7 +312,7 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, os_free(data->serverid); data->serverid = os_malloc(end - pos); if (data->serverid == NULL) { - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } os_memcpy(data->serverid, pos, end - pos); @@ -320,11 +320,11 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, wpa_printf(MSG_DEBUG, "EAP-EKE: Sending EAP-EKE-ID/Response"); - resp = eap_eke_build_msg(data, eap_get_id(reqData), + resp = eap_eke_build_msg(data, id, 2 + 4 + 1 + data->peerid_len, EAP_EKE_ID); if (resp == NULL) { - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -339,7 +339,7 @@ static struct wpabuf * eap_eke_process_id(struct eap_eke_data *data, data->msgs = wpabuf_alloc(wpabuf_len(reqData) + wpabuf_len(resp)); if (data->msgs == NULL) { wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpabuf_put_buf(data->msgs, reqData); @@ -366,10 +366,11 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, u8 pub[EAP_EKE_MAX_DH_LEN]; const u8 *password; size_t password_len; + u8 id = eap_get_id(reqData); if (data->state != COMMIT) { wpa_printf(MSG_DEBUG, "EAP-EKE: EAP-EKE-Commit/Request received in unexpected state (%d)", data->state); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -378,7 +379,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, password = eap_get_config_password(sm, &password_len); if (password == NULL) { wpa_printf(MSG_INFO, "EAP-EKE: No password configured!"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PASSWD_NOT_FOUND); } @@ -387,7 +388,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, if (pos + data->sess.dhcomp_len > end) { wpa_printf(MSG_DEBUG, "EAP-EKE: Too short EAP-EKE-Commit"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -405,7 +406,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, data->serverid, data->serverid_len, data->peerid, data->peerid_len, key) < 0) { wpa_printf(MSG_INFO, "EAP-EKE: Failed to derive key"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -416,7 +417,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, if (eap_eke_dh_init(data->sess.dhgroup, data->dh_priv, pub) < 0) { wpa_printf(MSG_INFO, "EAP-EKE: Failed to initialize DH"); os_memset(key, 0, sizeof(key)); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -424,7 +425,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, { wpa_printf(MSG_INFO, "EAP-EKE: Failed to derive shared secret"); os_memset(key, 0, sizeof(key)); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -433,18 +434,18 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, data->peerid, data->peerid_len) < 0) { wpa_printf(MSG_INFO, "EAP-EKE: Failed to derive Ke/Ki"); os_memset(key, 0, sizeof(key)); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpa_printf(MSG_DEBUG, "EAP-EKE: Sending EAP-EKE-Commit/Response"); - resp = eap_eke_build_msg(data, eap_get_id(reqData), + resp = eap_eke_build_msg(data, id, data->sess.dhcomp_len + data->sess.pnonce_len, EAP_EKE_COMMIT); if (resp == NULL) { os_memset(key, 0, sizeof(key)); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -453,7 +454,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, if (eap_eke_dhcomp(&data->sess, key, pub, rpos) < 0) { wpa_printf(MSG_INFO, "EAP-EKE: Failed to build DHComponent_P"); os_memset(key, 0, sizeof(key)); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } os_memset(key, 0, sizeof(key)); @@ -463,7 +464,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, if (random_get_bytes(data->nonce_p, data->sess.nonce_len)) { wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpa_hexdump_key(MSG_DEBUG, "EAP-EKE: Nonce_P", @@ -472,7 +473,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, if (eap_eke_prot(&data->sess, data->nonce_p, data->sess.nonce_len, wpabuf_put(resp, 0), &prot_len) < 0) { wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpa_hexdump(MSG_DEBUG, "EAP-EKE: PNonce_P", @@ -484,7 +485,7 @@ static struct wpabuf * eap_eke_process_commit(struct eap_sm *sm, if (wpabuf_resize(&data->msgs, wpabuf_len(reqData) + wpabuf_len(resp)) < 0) { wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpabuf_put_buf(data->msgs, reqData); @@ -509,11 +510,12 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, u8 auth_s[EAP_EKE_MAX_HASH_LEN]; size_t decrypt_len; u8 *auth; + u8 id = eap_get_id(reqData); if (data->state != CONFIRM) { wpa_printf(MSG_DEBUG, "EAP-EKE: EAP-EKE-Confirm/Request received in unexpected state (%d)", data->state); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -524,7 +526,7 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, if (pos + data->sess.pnonce_ps_len + data->sess.prf_len > end) { wpa_printf(MSG_DEBUG, "EAP-EKE: Too short EAP-EKE-Confirm"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PROTO_ERROR); } @@ -532,19 +534,19 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, if (eap_eke_decrypt_prot(&data->sess, pos, data->sess.pnonce_ps_len, nonces, &decrypt_len) < 0) { wpa_printf(MSG_INFO, "EAP-EKE: Failed to decrypt PNonce_PS"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_AUTHENTICATION_FAIL); } if (decrypt_len != (size_t) 2 * data->sess.nonce_len) { wpa_printf(MSG_INFO, "EAP-EKE: PNonce_PS protected data length does not match length of Nonce_P and Nonce_S"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_AUTHENTICATION_FAIL); } wpa_hexdump_key(MSG_DEBUG, "EAP-EKE: Received Nonce_P | Nonce_S", nonces, 2 * data->sess.nonce_len); if (os_memcmp(data->nonce_p, nonces, data->sess.nonce_len) != 0) { wpa_printf(MSG_INFO, "EAP-EKE: Received Nonce_P does not match transmitted Nonce_P"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_AUTHENTICATION_FAIL); } @@ -556,30 +558,30 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, if (eap_eke_derive_ka(&data->sess, data->serverid, data->serverid_len, data->peerid, data->peerid_len, data->nonce_p, data->nonce_s) < 0) { - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } if (eap_eke_auth(&data->sess, "EAP-EKE server", data->msgs, auth_s) < 0) { - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpa_hexdump(MSG_DEBUG, "EAP-EKE: Auth_S", auth_s, data->sess.prf_len); if (os_memcmp_const(auth_s, pos + data->sess.pnonce_ps_len, data->sess.prf_len) != 0) { wpa_printf(MSG_INFO, "EAP-EKE: Auth_S does not match"); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_AUTHENTICATION_FAIL); } wpa_printf(MSG_DEBUG, "EAP-EKE: Sending EAP-EKE-Confirm/Response"); - resp = eap_eke_build_msg(data, eap_get_id(reqData), + resp = eap_eke_build_msg(data, id, data->sess.pnonce_len + data->sess.prf_len, EAP_EKE_CONFIRM); if (resp == NULL) { - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -587,7 +589,7 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, if (eap_eke_prot(&data->sess, data->nonce_s, data->sess.nonce_len, wpabuf_put(resp, 0), &prot_len) < 0) { wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpabuf_put(resp, prot_len); @@ -595,7 +597,7 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, auth = wpabuf_put(resp, data->sess.prf_len); if (eap_eke_auth(&data->sess, "EAP-EKE peer", data->msgs, auth) < 0) { wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } wpa_hexdump(MSG_DEBUG, "EAP-EKE: Auth_P", auth, data->sess.prf_len); @@ -606,7 +608,7 @@ static struct wpabuf * eap_eke_process_confirm(struct eap_eke_data *data, data->msk, data->emsk) < 0) { wpa_printf(MSG_INFO, "EAP-EKE: Failed to derive MSK/EMSK"); wpabuf_free(resp); - return eap_eke_build_fail(data, ret, reqData, + return eap_eke_build_fail(data, ret, id, EAP_EKE_FAIL_PRIVATE_INTERNAL_ERROR); } @@ -638,7 +640,8 @@ static struct wpabuf * eap_eke_process_failure(struct eap_eke_data *data, wpa_printf(MSG_INFO, "EAP-EKE: Failure-Code 0x%x", code); } - return eap_eke_build_fail(data, ret, reqData, EAP_EKE_FAIL_NO_ERROR); + return eap_eke_build_fail(data, ret, eap_get_id(reqData), + EAP_EKE_FAIL_NO_ERROR); }