From dbe7c4a78d447495d7277e2cdc2e45a9a9206c6d Mon Sep 17 00:00:00 2001 From: Shyamal Khachane Date: Fri, 21 Jul 2023 14:43:05 +0530 Subject: [PATCH] fix(esp_wifi): Backport some wifi fixes 1. Fix memory leak caused by assoc IE and retry timer 2. Discard commit frame received at confirmed state in SAE 3. Ignore immediate assoc req received from the station while we are processing the older one. Fix regression caused by 4cb4faa9 --- components/esp_common/src/esp_err_to_name.c | 3 +++ components/esp_wifi/include/esp_wifi.h | 1 + components/esp_wifi/lib | 2 +- .../esp_supplicant/src/esp_common.c | 10 +++++++- .../esp_supplicant/src/esp_common_i.h | 1 + .../esp_supplicant/src/esp_hostap.c | 1 + .../esp_supplicant/src/esp_wpa3.c | 25 +++++++++++++------ .../esp_supplicant/src/esp_wpa_main.c | 25 ++++++++++++++++--- components/wpa_supplicant/src/ap/ieee802_11.c | 3 +-- components/wpa_supplicant/src/ap/sta_info.c | 1 - components/wpa_supplicant/src/ap/wpa_auth.c | 9 ++++--- 11 files changed, 61 insertions(+), 20 deletions(-) diff --git a/components/esp_common/src/esp_err_to_name.c b/components/esp_common/src/esp_err_to_name.c index 44da5814524..8e4b94c901b 100644 --- a/components/esp_common/src/esp_err_to_name.c +++ b/components/esp_common/src/esp_err_to_name.c @@ -407,6 +407,9 @@ static const esp_err_msg_t esp_err_msg_table[] = { # endif # ifdef ESP_ERR_WIFI_TWT_SETUP_REJECT ERR_TBL_IT(ESP_ERR_WIFI_TWT_SETUP_REJECT), /* 12314 0x301a The twt setup request was rejected by the AP */ +# endif +# ifdef ESP_ERR_WIFI_DISCARD + ERR_TBL_IT(ESP_ERR_WIFI_DISCARD), /* 12315 0x301b Discard frame */ # endif // components/wpa_supplicant/esp_supplicant/include/esp_wps.h # ifdef ESP_ERR_WIFI_REGISTRAR diff --git a/components/esp_wifi/include/esp_wifi.h b/components/esp_wifi/include/esp_wifi.h index ae01e63375b..e159ca859de 100644 --- a/components/esp_wifi/include/esp_wifi.h +++ b/components/esp_wifi/include/esp_wifi.h @@ -87,6 +87,7 @@ extern "C" { #define ESP_ERR_WIFI_TWT_SETUP_TIMEOUT (ESP_ERR_WIFI_BASE + 24) /*!< Timeout of receiving twt setup response frame, timeout times can be set during twt setup */ #define ESP_ERR_WIFI_TWT_SETUP_TXFAIL (ESP_ERR_WIFI_BASE + 25) /*!< TWT setup frame tx failed */ #define ESP_ERR_WIFI_TWT_SETUP_REJECT (ESP_ERR_WIFI_BASE + 26) /*!< The twt setup request was rejected by the AP */ +#define ESP_ERR_WIFI_DISCARD (ESP_ERR_WIFI_BASE + 27) /*!< Discard frame */ /** * @brief WiFi stack configuration parameters passed to esp_wifi_init call. diff --git a/components/esp_wifi/lib b/components/esp_wifi/lib index e3e15870cb5..115268a9779 160000 --- a/components/esp_wifi/lib +++ b/components/esp_wifi/lib @@ -1 +1 @@ -Subproject commit e3e15870cb5d199a59867f66dd236e0d9ff5a127 +Subproject commit 115268a9779e7ff2d859cf524eff70269ddced61 diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_common.c b/components/wpa_supplicant/esp_supplicant/src/esp_common.c index 12adbad686b..19176028fd8 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_common.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_common.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2020-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2020-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -245,6 +245,14 @@ static int handle_assoc_frame(u8 *frame, size_t len, #endif /* CONFIG_IEEE80211R */ #endif /* defined(CONFIG_IEEE80211KV) || defined(CONFIG_IEEE80211R) */ +void esp_supplicant_unset_all_appie(void) +{ + uint8_t appie; + for (appie = WIFI_APPIE_PROBEREQ; appie < WIFI_APPIE_RAM_MAX; appie++) { + esp_wifi_unset_appie_internal(appie); + } +} + static int ieee80211_handle_rx_frm(u8 type, u8 *frame, size_t len, u8 *sender, u32 rssi, u8 channel, u64 current_tsf) { diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_common_i.h b/components/wpa_supplicant/esp_supplicant/src/esp_common_i.h index c738dba6714..d7d64f36432 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_common_i.h +++ b/components/wpa_supplicant/esp_supplicant/src/esp_common_i.h @@ -42,6 +42,7 @@ bool mbo_bss_profile_match(u8 *bssid); #endif int esp_supplicant_common_init(struct wpa_funcs *wpa_cb); void esp_supplicant_common_deinit(void); +void esp_supplicant_unset_all_appie(void); void esp_set_scan_ie(void); void esp_set_assoc_ie(uint8_t *bssid, const u8 *ies, size_t ies_len, bool add_mdie); void supplicant_sta_conn_handler(uint8_t* bssid); diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c index 481be139e7e..3aae31d5fd0 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_hostap.c @@ -233,6 +233,7 @@ bool hostap_deinit(void *data) return true; } esp_wifi_unset_appie_internal(WIFI_APPIE_WPA); + esp_wifi_unset_appie_internal(WIFI_APPIE_ASSOC_RESP); #ifdef CONFIG_SAE wpa3_hostap_auth_deinit(); diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c index 7fcafba72ee..e4c9a4e03ac 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -239,9 +239,8 @@ static int wpa3_parse_sae_commit(u8 *buf, u32 len, u16 status) int ret; if (g_sae_data.state != SAE_COMMITTED) { - wpa_printf(MSG_ERROR, "wpa3: failed to parse SAE commit in state(%d)!", - g_sae_data.state); - return ESP_FAIL; + wpa_printf(MSG_DEBUG, "wpa3: Discarding commit frame received in state %d", g_sae_data.state); + return ESP_ERR_WIFI_DISCARD; } if (status == WLAN_STATUS_ANTI_CLOGGING_TOKEN_REQ) { @@ -264,7 +263,10 @@ static int wpa3_parse_sae_commit(u8 *buf, u32 len, u16 status) ret = sae_parse_commit(&g_sae_data, buf, len, NULL, 0, g_allowed_groups, (status == WLAN_STATUS_SAE_HASH_TO_ELEMENT || status == WLAN_STATUS_SAE_PK)); - if (ret) { + if (ret == SAE_SILENTLY_DISCARD) { + wpa_printf(MSG_DEBUG, "wpa3: Discarding commit frame due to reflection attack"); + return ESP_ERR_WIFI_DISCARD; + } else if (ret) { wpa_printf(MSG_ERROR, "wpa3: could not parse commit(%d)", ret); return ret; } @@ -410,6 +412,11 @@ static void wpa3_process_rx_commit(wpa3_hostap_auth_event_t *evt) goto free; } } + + if (!sta->lock) { + sta->lock = os_semphr_create(1, 1); + } + if (sta->lock && os_semphr_take(sta->lock, 0)) { sta->sae_commit_processing = true; ret = handle_auth_sae(hapd, sta, frm->msg, frm->len, frm->bssid, frm->auth_transaction, frm->status); @@ -423,9 +430,10 @@ static void wpa3_process_rx_commit(wpa3_hostap_auth_event_t *evt) uint16_t aid = 0; if (ret != WLAN_STATUS_SUCCESS && ret != WLAN_STATUS_ANTI_CLOGGING_TOKEN_REQ) { - if (esp_wifi_ap_get_sta_aid(frm->bssid, &aid) == ESP_OK && aid == 0) { + esp_wifi_ap_get_sta_aid(frm->bssid, &aid); + if (aid == 0) { esp_wifi_ap_deauth_internal(frm->bssid, ret); - } + } } } @@ -463,8 +471,9 @@ static void wpa3_process_rx_confirm(wpa3_hostap_auth_event_t *evt) } os_semphr_give(sta->lock); if (ret != WLAN_STATUS_SUCCESS) { - uint16_t aid = -1; - if (esp_wifi_ap_get_sta_aid(frm->bssid, &aid) == ESP_OK && aid == 0) { + uint16_t aid = 0; + esp_wifi_ap_get_sta_aid(frm->bssid, &aid); + if (aid == 0) { esp_wifi_ap_deauth_internal(frm->bssid, ret); } } diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa_main.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa_main.c index 36b2b44b931..6cb4ec0932d 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa_main.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa_main.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2019-2022 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2019-2023 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -304,18 +304,31 @@ static bool hostap_sta_join(void **sta, u8 *bssid, u8 *wpa_ie, u8 wpa_ie_len,u8 } if (*sta && !esp_wifi_ap_is_sta_sae_reauth_node(bssid)) { - ap_free_sta(hapd, *sta); + ap_free_sta(hapd, *sta); } sta_info = ap_sta_add(hapd, bssid); if (!sta_info) { wpa_printf(MSG_ERROR, "failed to add station " MACSTR, MAC2STR(bssid)); - goto fail; + return false; } + +#ifdef CONFIG_SAE + if (sta_info->lock && os_semphr_take(sta_info->lock, 0) != TRUE) { + wpa_printf(MSG_INFO, "Ignore assoc request as softap is busy with sae calculation for station "MACSTR, MAC2STR(bssid)); + return false; + } +#endif /* CONFIG_SAE */ + #ifdef CONFIG_WPS_REGISTRAR if (check_n_add_wps_sta(hapd, sta_info, wpa_ie, wpa_ie_len, pmf_enable, subtype) == 0) { if (sta_info->eapol_sm) { *sta = sta_info; +#ifdef CONFIG_SAE + if (sta_info->lock) { + os_semphr_give(sta_info->lock); + } +#endif /* CONFIG_SAE */ return true; } } else { @@ -324,6 +337,11 @@ static bool hostap_sta_join(void **sta, u8 *bssid, u8 *wpa_ie, u8 wpa_ie_len,u8 #endif if (wpa_ap_join(sta_info, bssid, wpa_ie, wpa_ie_len, rsnxe, rsnxe_len, pmf_enable, subtype)) { *sta = sta_info; +#ifdef CONFIG_SAE + if (sta_info->lock) { + os_semphr_give(sta_info->lock); + } +#endif /* CONFIG_SAE */ return true; } @@ -398,6 +416,7 @@ int esp_supplicant_init(void) int esp_supplicant_deinit(void) { esp_supplicant_common_deinit(); + esp_supplicant_unset_all_appie(); eloop_destroy(); wpa_cb = NULL; return esp_wifi_unregister_wpa_cb_internal(); diff --git a/components/wpa_supplicant/src/ap/ieee802_11.c b/components/wpa_supplicant/src/ap/ieee802_11.c index 2b2b2c09a25..e8dbc7f0671 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -559,7 +559,7 @@ int handle_auth_sae(struct hostapd_data *hapd, struct sta_info *sta, } if (sae_check_confirm(sta->sae, buf, len) < 0) { - resp = WLAN_STATUS_UNSPECIFIED_FAILURE; + resp = WLAN_STATUS_CHALLENGE_FAIL; goto reply; } sta->sae->rc = peer_send_confirm; @@ -569,7 +569,6 @@ int handle_auth_sae(struct hostapd_data *hapd, struct sta_info *sta, } else { wpa_printf(MSG_ERROR, "unexpected SAE authentication transaction %u (status=%u )", auth_transaction, status); if (status != WLAN_STATUS_SUCCESS) { - resp = -1; goto remove_sta; } resp = WLAN_STATUS_UNKNOWN_AUTH_TRANSACTION; diff --git a/components/wpa_supplicant/src/ap/sta_info.c b/components/wpa_supplicant/src/ap/sta_info.c index 66e856ebeed..515179d69a9 100644 --- a/components/wpa_supplicant/src/ap/sta_info.c +++ b/components/wpa_supplicant/src/ap/sta_info.c @@ -175,7 +175,6 @@ struct sta_info * ap_sta_add(struct hostapd_data *hapd, const u8 *addr) #ifdef CONFIG_SAE sta->sae_commit_processing = false; sta->remove_pending = false; - sta->lock = os_semphr_create(1, 1); #endif /* CONFIG_SAE */ return sta; diff --git a/components/wpa_supplicant/src/ap/wpa_auth.c b/components/wpa_supplicant/src/ap/wpa_auth.c index ce76023a56f..6f086e18dd1 100644 --- a/components/wpa_supplicant/src/ap/wpa_auth.c +++ b/components/wpa_supplicant/src/ap/wpa_auth.c @@ -488,6 +488,7 @@ static void wpa_free_sta_sm(struct wpa_state_machine *sm) wpa_printf( MSG_DEBUG, "wpa_free_sta_sm: free eapol=%p\n", sm->last_rx_eapol_key); os_free(sm->last_rx_eapol_key); os_free(sm->wpa_ie); + os_free(sm->rsnxe); os_free(sm); } @@ -2635,7 +2636,7 @@ bool wpa_ap_remove(u8* bssid) } else { sta->remove_pending = true; } - return true; + return true; } #endif /* CONFIG_SAE */ @@ -2644,10 +2645,10 @@ bool wpa_ap_remove(u8* bssid) if (wps_get_status() == WPS_STATUS_PENDING) { u8 *addr = os_malloc(ETH_ALEN); - if (!addr) { + if (!addr) { return false; - } - os_memcpy(addr, sta->addr, ETH_ALEN); + } + os_memcpy(addr, sta->addr, ETH_ALEN); eloop_register_timeout(0, 10000, ap_free_sta_timeout, hapd, addr); } else #endif