[PATCH] wpa_supplicant: Wait for eapol 4/4 tx-status before setting key.
Ben Greear
greearb at candelatech.com
Thu Jul 6 14:42:14 PDT 2017
On 06/13/2017 11:29 AM, greearb at candelatech.com wrote:
> From: Wojciech Dubowik <Wojciech.Dubowik at neratec.com>
>
> Supplicant is using generic L2 send function for EAPOL
> messages which doesn't give back status whether frame has been
> acked or not. It can lead to wrong wpa states when EAPOL 4/4
> is lost i.e. client is in connected state but keys aren't
> established on AP side.
> Fix that by using nl80211_send_eapol_data as for AP side
> and check in conneced state that 4/4 EAPOL has been acked.
>
> As a combined improvement, do not actually set the keys until
> we receive notification that the 4/4 message was sent. This fixes
> races in ath10k CT firmware, and may eventually let other firmware
> remove hacks that were needed to work around this key-setting
> race.
Any comments on this? We have been testing it for a while, and it
seems to work well.
Thanks,
Ben
>
> Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik at neratec.com>
> Signed-off-by: Ben Greear <greearb at candelatech.com>
> ---
> src/drivers/driver.h | 12 +++
> src/drivers/driver_nl80211.c | 11 +++
> src/rsn_supp/wpa.c | 205 +++++++++++++++++++++++++++++++------------
> src/rsn_supp/wpa.h | 12 +++
> src/rsn_supp/wpa_i.h | 5 ++
> wpa_supplicant/driver_i.h | 10 +++
> wpa_supplicant/events.c | 22 +++--
> wpa_supplicant/wpas_glue.c | 9 ++
> 8 files changed, 223 insertions(+), 63 deletions(-)
>
> diff --git a/src/drivers/driver.h b/src/drivers/driver.h
> index 85d5117..d1327f0 100644
> --- a/src/drivers/driver.h
> +++ b/src/drivers/driver.h
> @@ -2701,6 +2701,18 @@ struct wpa_driver_ops {
> const u8 *own_addr, u32 flags);
>
> /**
> + * send_eapol - Send an EAPOL packet (STA only)
> + * @priv: private driver interface data
> + * @addr: Destination MAC address
> + * @data: EAPOL packet starting with IEEE 802.1X header
> + * @data_len: Length of the EAPOL packet in octets
> + *
> + * Returns: 0 on success, -1 on failure
> + */
> + int (*send_eapol)(void *priv, const u8 *addr, const u8 *data,
> + size_t data_len);
> +
> + /**
> * sta_deauth - Deauthenticate a station (AP only)
> * @priv: Private driver interface data
> * @own_addr: Source address and BSSID for the Deauthentication frame
> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
> index 0d5a04d..d7ee936 100644
> --- a/src/drivers/driver_nl80211.c
> +++ b/src/drivers/driver_nl80211.c
> @@ -4901,6 +4901,16 @@ static int wpa_driver_nl80211_hapd_send_eapol(
> return res;
> }
>
> +static int wpa_driver_nl80211_send_eapol(
> + void *priv, const u8 *addr, const u8 *data,
> + size_t data_len)
> +{
> + struct i802_bss *bss = priv;
> +
> + return nl80211_send_eapol_data(bss, addr, data, data_len);
> +}
> +
> +
>
> static int wpa_driver_nl80211_sta_set_flags(void *priv, const u8 *addr,
> unsigned int total_flags,
> @@ -10543,6 +10553,7 @@ const struct wpa_driver_ops wpa_driver_nl80211_ops = {
> .sta_add = wpa_driver_nl80211_sta_add,
> .sta_remove = driver_nl80211_sta_remove,
> .hapd_send_eapol = wpa_driver_nl80211_hapd_send_eapol,
> + .send_eapol = wpa_driver_nl80211_send_eapol,
> .sta_set_flags = wpa_driver_nl80211_sta_set_flags,
> .hapd_init = i802_init,
> .hapd_deinit = i802_deinit,
> diff --git a/src/rsn_supp/wpa.c b/src/rsn_supp/wpa.c
> index 8a1d164..be6ab74 100644
> --- a/src/rsn_supp/wpa.c
> +++ b/src/rsn_supp/wpa.c
> @@ -696,6 +696,8 @@ static void wpa_supplicant_process_1_of_4(struct wpa_sm *sm,
> }
> #endif /* CONFIG_P2P */
>
> + sm->waiting_for_4_of_4_sent = 0; /* not yet */
> +
> if (wpa_supplicant_send_2_of_4(sm, sm->bssid, key, ver, sm->snonce,
> kde, kde_len, ptk) < 0)
> goto failed;
> @@ -1344,15 +1346,14 @@ int wpa_supplicant_send_4_of_4(struct wpa_sm *sm, const unsigned char *dst,
> }
>
>
> -static void wpa_supplicant_process_3_of_4(struct wpa_sm *sm,
> - const struct wpa_eapol_key *key,
> - u16 ver, const u8 *key_data,
> - size_t key_data_len)
> +static void wpa_supplicant_process_3_of_4_send(struct wpa_sm *sm,
> + const struct wpa_eapol_key *key,
> + u16 ver, const u8 *key_data,
> + size_t key_data_len)
> {
> u16 key_info, keylen;
> struct wpa_eapol_ie_parse ie;
>
> - wpa_sm_set_state(sm, WPA_4WAY_HANDSHAKE);
> wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG, "WPA: RX message 3 of 4-Way "
> "Handshake from " MACSTR " (ver=%d)", MAC2STR(sm->bssid), ver);
>
> @@ -1412,64 +1413,16 @@ static void wpa_supplicant_process_3_of_4(struct wpa_sm *sm,
> }
> #endif /* CONFIG_P2P */
>
> + sm->waiting_for_4_of_4_sent = 1;
> if (wpa_supplicant_send_4_of_4(sm, sm->bssid, key, ver, key_info,
> &sm->ptk) < 0) {
> + sm->waiting_for_4_of_4_sent = 0;
> goto failed;
> }
> -
> - /* SNonce was successfully used in msg 3/4, so mark it to be renewed
> - * for the next 4-Way Handshake. If msg 3 is received again, the old
> - * SNonce will still be used to avoid changing PTK. */
> - sm->renew_snonce = 1;
> -
> - if (key_info & WPA_KEY_INFO_INSTALL) {
> - if (wpa_supplicant_install_ptk(sm, key))
> - goto failed;
> - }
> -
> - if (key_info & WPA_KEY_INFO_SECURE) {
> - wpa_sm_mlme_setprotection(
> - sm, sm->bssid, MLME_SETPROTECTION_PROTECT_TYPE_RX,
> - MLME_SETPROTECTION_KEY_TYPE_PAIRWISE);
> - eapol_sm_notify_portValid(sm->eapol, TRUE);
> - }
> - wpa_sm_set_state(sm, WPA_GROUP_HANDSHAKE);
> -
> - if (sm->group_cipher == WPA_CIPHER_GTK_NOT_USED) {
> - wpa_supplicant_key_neg_complete(sm, sm->bssid,
> - key_info & WPA_KEY_INFO_SECURE);
> - } else if (ie.gtk &&
> - wpa_supplicant_pairwise_gtk(sm, key,
> - ie.gtk, ie.gtk_len, key_info) < 0) {
> - wpa_msg(sm->ctx->msg_ctx, MSG_INFO,
> - "RSN: Failed to configure GTK");
> - goto failed;
> - }
> -
> - if (ieee80211w_set_keys(sm, &ie) < 0) {
> - wpa_msg(sm->ctx->msg_ctx, MSG_INFO,
> - "RSN: Failed to configure IGTK");
> - goto failed;
> - }
> -
> - if (ie.gtk)
> - wpa_sm_set_rekey_offload(sm);
> -
> - if (sm->proto == WPA_PROTO_RSN && wpa_key_mgmt_suite_b(sm->key_mgmt)) {
> - struct rsn_pmksa_cache_entry *sa;
> -
> - sa = pmksa_cache_add(sm->pmksa, sm->pmk, sm->pmk_len, NULL,
> - sm->ptk.kck, sm->ptk.kck_len,
> - sm->bssid, sm->own_addr,
> - sm->network_ctx, sm->key_mgmt, NULL);
> - if (!sm->cur_pmksa)
> - sm->cur_pmksa = sa;
> - }
> -
> - sm->msg_3_of_4_ok = 1;
> return;
>
> failed:
> + wpa_sm_set_state(sm, WPA_4WAY_HANDSHAKE);
> wpa_sm_deauthenticate(sm, WLAN_REASON_UNSPECIFIED);
> }
>
> @@ -1979,6 +1932,131 @@ static int wpa_supp_aead_decrypt(struct wpa_sm *sm, u8 *buf, size_t buf_len,
> }
> #endif /* CONFIG_FILS */
>
> +static void wpa_supplicant_process_4_of_4_sent(struct wpa_sm *sm)
> +{
> + size_t key_data_len;
> + struct wpa_eapol_key *key;
> + u16 key_info, ver;
> + struct wpa_eapol_ie_parse ie;
> + u8 *mic, *key_data;
> + size_t mic_len;
> + u8 *buf = sm->last_3_of_4_buf;
> +
> + mic_len = wpa_mic_len(sm->key_mgmt);
> +
> + key = (struct wpa_eapol_key *) (buf + sizeof(struct ieee802_1x_hdr));
> + mic = (u8 *) (key + 1);
> + key_data = mic + mic_len + 2;
> +
> + wpa_sm_set_state(sm, WPA_4WAY_HANDSHAKE);
> + wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG, "WPA: Transmitted 4 of 4-Way "
> + "Handshake from " MACSTR, MAC2STR(sm->bssid));
> +
> + key_info = WPA_GET_BE16(key->key_info);
> + ver = key_info & WPA_KEY_INFO_TYPE_MASK;
> +
> + sm->waiting_for_4_of_4_sent = 0;
> +
> + key_data_len = WPA_GET_BE16(mic + mic_len);
> +
> + /* Decrypt the key so we can parse the IEs, etc */
> +#ifdef CONFIG_FILS
> + if (!mic_len && (key_info & WPA_KEY_INFO_ENCR_KEY_DATA)) {
> + if (wpa_supp_aead_decrypt(sm, buf, sm->last_3_of_4_len, &key_data_len))
> + goto failed;
> + }
> +#endif /* CONFIG_FILS */
> +
> + if ((sm->proto == WPA_PROTO_RSN || sm->proto == WPA_PROTO_OSEN) &&
> + (key_info & WPA_KEY_INFO_ENCR_KEY_DATA) && mic_len) {
> + if (wpa_supplicant_decrypt_key_data(sm, key, mic_len,
> + ver, key_data,
> + &key_data_len))
> + goto failed;
> + }
> +
> + wpa_hexdump(MSG_DEBUG, "WPA: IE KeyData", key_data, key_data_len);
> + if (wpa_supplicant_parse_ies(key_data, key_data_len, &ie) < 0)
> + goto failed;
> +
> + /* SNonce was successfully used in msg 3/4, so mark it to be renewed
> + * for the next 4-Way Handshake. If msg 3 is received again, the old
> + * SNonce will still be used to avoid changing PTK. */
> + sm->renew_snonce = 1;
> +
> + if (key_info & WPA_KEY_INFO_INSTALL) {
> + if (wpa_supplicant_install_ptk(sm, key))
> + goto failed;
> + }
> +
> + if (key_info & WPA_KEY_INFO_SECURE) {
> + wpa_sm_mlme_setprotection(
> + sm, sm->bssid, MLME_SETPROTECTION_PROTECT_TYPE_RX,
> + MLME_SETPROTECTION_KEY_TYPE_PAIRWISE);
> + eapol_sm_notify_portValid(sm->eapol, TRUE);
> + }
> + wpa_sm_set_state(sm, WPA_GROUP_HANDSHAKE);
> +
> + if (sm->group_cipher == WPA_CIPHER_GTK_NOT_USED) {
> + wpa_supplicant_key_neg_complete(sm, sm->bssid,
> + key_info & WPA_KEY_INFO_SECURE);
> + } else if (ie.gtk &&
> + wpa_supplicant_pairwise_gtk(sm, key,
> + ie.gtk, ie.gtk_len, key_info) < 0) {
> + wpa_msg(sm->ctx->msg_ctx, MSG_INFO,
> + "RSN: Failed to configure GTK");
> + goto failed;
> + }
> +
> + if (ieee80211w_set_keys(sm, &ie) < 0) {
> + wpa_msg(sm->ctx->msg_ctx, MSG_INFO,
> + "RSN: Failed to configure IGTK");
> + goto failed;
> + }
> +
> + if (ie.gtk)
> + wpa_sm_set_rekey_offload(sm);
> +
> + if (sm->proto == WPA_PROTO_RSN && wpa_key_mgmt_suite_b(sm->key_mgmt)) {
> + struct rsn_pmksa_cache_entry *sa;
> +
> + sa = pmksa_cache_add(sm->pmksa, sm->pmk, sm->pmk_len, NULL,
> + sm->ptk.kck, sm->ptk.kck_len,
> + sm->bssid, sm->own_addr,
> + sm->network_ctx, sm->key_mgmt, NULL);
> + if (!sm->cur_pmksa)
> + sm->cur_pmksa = sa;
> + }
> +
> + sm->msg_3_of_4_ok = 1;
> + return;
> +
> +failed:
> + wpa_sm_deauthenticate(sm, WLAN_REASON_UNSPECIFIED);
> +}
> +
> +void wpa_sm_eapol_tx_status_available(struct wpa_sm *sm, int is_available)
> +{
> + sm->eapol_tx_status_available = is_available;
> +}
> +
> +/* De-auth if return is < 0 */
> +int wpa_sm_eapol_tx_status(struct wpa_sm *sm, const u8 *dst,
> + const u8 *buf, size_t len, int ack)
> +{
> + wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG,
> + "EAPOL_TX_STATUS: ACK(%d) waiting 4/4-tx-status: %d", ack, sm->waiting_for_4_of_4_sent);
> + if (ack && sm->waiting_for_4_of_4_sent) {
> + wpa_supplicant_process_4_of_4_sent(sm);
> + }
> + else if (!ack && sm->waiting_for_4_of_4_sent) {
> + wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG,
> + "EAPOL 4/4 Not acked, disconnecting");
> + return -1;
> + }
> + return 0;
> +}
> +
> #ifdef CONFIG_TESTING_OPTIONS
> /* Mostly same as below, but this should not change any state. Returns the
> * message type so we can make decisions before feeding this into the state
> @@ -2391,8 +2469,19 @@ int wpa_sm_rx_eapol(struct wpa_sm *sm, const u8 *src_addr,
> } else if (key_info & (WPA_KEY_INFO_MIC |
> WPA_KEY_INFO_ENCR_KEY_DATA)) {
> /* 3/4 4-Way Handshake */
> - wpa_supplicant_process_3_of_4(sm, key, ver, key_data,
> - key_data_len);
> + /* Save buffer for doing the second half of the 4/4 processing
> + * once we get 4/4 ack status
> + */
> + int my_len = sizeof(sm->last_3_of_4_buf);
> + if (len < my_len)
> + my_len = len;
> + memcpy(sm->last_3_of_4_buf, buf, my_len);
> + sm->last_3_of_4_len = my_len;
> +
> + wpa_supplicant_process_3_of_4_send(sm, key, ver, key_data,
> + key_data_len);
> + if (!sm->eapol_tx_status_available)
> + wpa_supplicant_process_4_of_4_sent(sm);
> } else {
> /* 1/4 4-Way Handshake */
> wpa_supplicant_process_1_of_4(sm, src_addr, key,
> diff --git a/src/rsn_supp/wpa.h b/src/rsn_supp/wpa.h
> index 7e9873c..3eb9d72 100644
> --- a/src/rsn_supp/wpa.h
> +++ b/src/rsn_supp/wpa.h
> @@ -213,6 +213,9 @@ void wpa_sm_set_rx_replay_ctr(struct wpa_sm *sm, const u8 *rx_replay_counter);
> void wpa_sm_set_ptk_kck_kek(struct wpa_sm *sm,
> const u8 *ptk_kck, size_t ptk_kck_len,
> const u8 *ptk_kek, size_t ptk_kek_len);
> +int wpa_sm_eapol_tx_status(struct wpa_sm *sm, const u8 *dst,
> + const u8 *buf, size_t len, int ack);
> +void wpa_sm_eapol_tx_status_available(struct wpa_sm *sm, int is_available);
>
> #else /* CONFIG_NO_WPA */
>
> @@ -395,6 +398,15 @@ static inline void wpa_sm_set_ptk_kck_kek(struct wpa_sm *sm, const u8 *ptk_kck,
> {
> }
>
> +static int wpa_sm_eapol_tx_status(struct wpa_sm *sm, const u8 *dst,
> + const u8 *buf, size_t len, int ack)
> +{
> +}
> +
> +static void wpa_sm_eapol_tx_status_available(struct wpa_sm *sm, int is_available)
> +{
> +}
> +
> #endif /* CONFIG_NO_WPA */
>
> #ifdef CONFIG_PEERKEY
> diff --git a/src/rsn_supp/wpa_i.h b/src/rsn_supp/wpa_i.h
> index 75af31e..0d305a3 100644
> --- a/src/rsn_supp/wpa_i.h
> +++ b/src/rsn_supp/wpa_i.h
> @@ -170,6 +170,11 @@ struct wpa_sm {
> #ifdef CONFIG_OWE
> struct crypto_ecdh *owe_ecdh;
> #endif /* CONFIG_OWE */
> +
> + u8 waiting_for_4_of_4_sent; /* boolean */
> + u8 eapol_tx_status_available;
> + u16 last_3_of_4_len;
> + u8 last_3_of_4_buf[1500];
> };
>
>
> diff --git a/wpa_supplicant/driver_i.h b/wpa_supplicant/driver_i.h
> index 0ef39b7..c5cd342 100644
> --- a/wpa_supplicant/driver_i.h
> +++ b/wpa_supplicant/driver_i.h
> @@ -356,6 +356,16 @@ static inline int wpa_drv_hapd_send_eapol(struct wpa_supplicant *wpa_s,
> return -1;
> }
>
> +static inline int wpa_drv_send_eapol(struct wpa_supplicant *wpa_s,
> + const u8 *addr, const u8 *data,
> + size_t data_len)
> +{
> + if (wpa_s->driver->hapd_send_eapol)
> + return wpa_s->driver->send_eapol(wpa_s->drv_priv, addr,
> + data, data_len);
> + return -1;
> +}
> +
> static inline int wpa_drv_sta_set_flags(struct wpa_supplicant *wpa_s,
> const u8 *addr, int total_flags,
> int flags_or, int flags_and)
> diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
> index 1d3c1a5..32cd1aa 100644
> --- a/wpa_supplicant/events.c
> +++ b/wpa_supplicant/events.c
> @@ -4046,13 +4046,25 @@ void wpa_supplicant_event(void *ctx, enum wpa_event_type event,
> }
> #endif /* CONFIG_AP */
> break;
> -#ifdef CONFIG_AP
> case EVENT_EAPOL_TX_STATUS:
> - ap_eapol_tx_status(wpa_s, data->eapol_tx_status.dst,
> - data->eapol_tx_status.data,
> - data->eapol_tx_status.data_len,
> - data->eapol_tx_status.ack);
> + if (wpa_s->ap_iface) {
> + ap_eapol_tx_status(wpa_s, data->eapol_tx_status.dst,
> + data->eapol_tx_status.data,
> + data->eapol_tx_status.data_len,
> + data->eapol_tx_status.ack);
> + }
> + else {
> + if (wpa_sm_eapol_tx_status(wpa_s->wpa, data->eapol_tx_status.dst,
> + data->eapol_tx_status.data,
> + data->eapol_tx_status.data_len,
> + data->eapol_tx_status.ack) < 0) {
> + wpa_s->own_disconnect_req = 1;
> + wpa_supplicant_deauthenticate(
> + wpa_s, WLAN_REASON_4WAY_HANDSHAKE_TIMEOUT);
> + }
> + }
> break;
> +#ifdef CONFIG_AP
> case EVENT_DRIVER_CLIENT_POLL_OK:
> ap_client_poll_ok(wpa_s, data->client_poll.addr);
> break;
> diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
> index ad20dc9..c656eec 100644
> --- a/wpa_supplicant/wpas_glue.c
> +++ b/wpa_supplicant/wpas_glue.c
> @@ -97,6 +97,7 @@ static u8 * wpa_alloc_eapol(const struct wpa_supplicant *wpa_s, u8 type,
> static int wpa_ether_send(struct wpa_supplicant *wpa_s, const u8 *dest,
> u16 proto, const u8 *buf, size_t len)
> {
> + int ret;
> #ifdef CONFIG_TESTING_OPTIONS
> if (wpa_s->ext_eapol_frame_io && proto == ETH_P_EAPOL) {
> size_t hex_len = 2 * len + 1;
> @@ -111,6 +112,14 @@ static int wpa_ether_send(struct wpa_supplicant *wpa_s, const u8 *dest,
> return 0;
> }
> #endif /* CONFIG_TESTING_OPTIONS */
> + ret = wpa_drv_send_eapol(wpa_s, dest, buf, len);
> +
> + wpa_sm_eapol_tx_status_available(wpa_s->wpa, ret >= 0);
> +
> + if (ret >= 0)
> + return ret;
> +
> + wpa_dbg(wpa_s, MSG_DEBUG, "wpa_drv_send_eapol rv (%d)", ret);
>
> if (wpa_s->l2) {
> return l2_packet_send(wpa_s->l2, dest, proto, buf, len);
>
--
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc http://www.candelatech.com
More information about the Hostap
mailing list