From b6f91e403a284ede6d9f335f44d428e413e80b14 Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Fri, 9 Aug 2024 13:46:05 +0530 Subject: [PATCH 1/5] SAE: Check that peer's rejected groups are not enabled in AP Signed-off-by: Jouni Malinen --- components/wpa_supplicant/src/ap/ieee802_11.c | 56 +++++++++++++++++++ components/wpa_supplicant/src/common/sae.c | 8 ++- 2 files changed, 63 insertions(+), 1 deletion(-) diff --git a/components/wpa_supplicant/src/ap/ieee802_11.c b/components/wpa_supplicant/src/ap/ieee802_11.c index 15a6b4a6030e..38a71d190ce4 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -403,6 +403,57 @@ static int sae_status_success(struct hostapd_data *hapd, u16 status_code) } +static int sae_is_group_enabled(struct hostapd_data *hapd, int group) +{ + int *groups = NULL; + int default_groups[] = { 19, 0 }; + int i; + + if (!groups) { + groups = default_groups; + } + + for (i = 0; groups[i] > 0; i++) { + if (groups[i] == group) + return 1; + } + + return 0; +} + + +static int check_sae_rejected_groups(struct hostapd_data *hapd, + struct sae_data *sae) +{ + const struct wpabuf *groups; + size_t i, count; + const u8 *pos; + + if (!sae->tmp) + return 0; + groups = sae->tmp->peer_rejected_groups; + if (!groups) + return 0; + + pos = wpabuf_head(groups); + count = wpabuf_len(groups); + for (i = 0; i < count; i++) { + int enabled; + u16 group; + + group = WPA_GET_LE16(pos); + pos += 2; + enabled = sae_is_group_enabled(hapd, group); + wpa_printf(MSG_DEBUG, "SAE: Rejected group %u is %s", + group, enabled ? "enabled" : "disabled"); + if (enabled) + return 1; + } + + return 0; +} + + int handle_auth_sae(struct hostapd_data *hapd, struct sta_info *sta, u8 *buf, size_t len, u8 *bssid, u16 auth_transaction, u16 status) @@ -496,6 +547,11 @@ int handle_auth_sae(struct hostapd_data *hapd, struct sta_info *sta, goto remove_sta; } + if (check_sae_rejected_groups(hapd, sta->sae)) { + resp = WLAN_STATUS_UNSPECIFIED_FAILURE; + goto reply; + } + if (resp != WLAN_STATUS_SUCCESS) { goto reply; } diff --git a/components/wpa_supplicant/src/common/sae.c b/components/wpa_supplicant/src/common/sae.c index 37b918230cc4..bc703444cd1d 100644 --- a/components/wpa_supplicant/src/common/sae.c +++ b/components/wpa_supplicant/src/common/sae.c @@ -2049,8 +2049,11 @@ static int sae_parse_rejected_groups(struct sae_data *sae, wpa_hexdump(MSG_DEBUG, "SAE: Possible elements at the end of the frame", *pos, end - *pos); - if (!sae_is_rejected_groups_elem(*pos, end)) + if (!sae_is_rejected_groups_elem(*pos, end)) { + wpabuf_free(sae->tmp->peer_rejected_groups); + sae->tmp->peer_rejected_groups = NULL; return WLAN_STATUS_SUCCESS; + } epos = *pos; epos++; /* skip IE type */ @@ -2139,6 +2142,9 @@ u16 sae_parse_commit(struct sae_data *sae, const u8 *data, size_t len, res = sae_parse_rejected_groups(sae, &pos, end); if (res != WLAN_STATUS_SUCCESS) return res; + } else { + wpabuf_free(sae->tmp->peer_rejected_groups); + sae->tmp->peer_rejected_groups = NULL; } /* Optional Anti-Clogging Token Container element */ From ba76df4f6955489f02dfeb7a53e551b6e6c11fa7 Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Mon, 9 Sep 2024 18:49:11 +0530 Subject: [PATCH 2/5] SAE: Check for invalid Rejected Groups element length explicitly Instead of practically ignoring an odd octet at the end of the element, check for such invalid case explicitly. This is needed to avoid a potential group downgrade attack. Signed-off-by: Jouni Malinen --- components/wpa_supplicant/src/ap/ieee802_11.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/components/wpa_supplicant/src/ap/ieee802_11.c b/components/wpa_supplicant/src/ap/ieee802_11.c index 38a71d190ce4..f47a9b86cc50 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -426,7 +426,7 @@ static int check_sae_rejected_groups(struct hostapd_data *hapd, struct sae_data *sae) { const struct wpabuf *groups; - size_t i, count; + size_t i, count, len; const u8 *pos; if (!sae->tmp) @@ -436,7 +436,15 @@ static int check_sae_rejected_groups(struct hostapd_data *hapd, return 0; pos = wpabuf_head(groups); - count = wpabuf_len(groups); + len = wpabuf_len(groups); + if (len & 1) { + wpa_printf(MSG_DEBUG, + "SAE: Invalid length of the Rejected Groups element payload: %zu", + len); + return 1; + } + + count = len / 2; for (i = 0; i < count; i++) { int enabled; u16 group; From 05874b938e4ec1677944f46e84cb86b7b1e117dc Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Mon, 9 Sep 2024 18:51:10 +0530 Subject: [PATCH 3/5] SAE: Reject invalid Rejected Groups element in the parser There is no need to depend on all uses (i.e., both hostapd and wpa_supplicant) to verify that the length of the Rejected Groups field in the Rejected Groups element is valid (i.e., a multiple of two octets) since the common parser can reject the message when detecting this. Signed-off-by: Jouni Malinen --- components/wpa_supplicant/src/common/sae.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/components/wpa_supplicant/src/common/sae.c b/components/wpa_supplicant/src/common/sae.c index bc703444cd1d..363aa0c4fe9c 100644 --- a/components/wpa_supplicant/src/common/sae.c +++ b/components/wpa_supplicant/src/common/sae.c @@ -2063,6 +2063,12 @@ static int sae_parse_rejected_groups(struct sae_data *sae, epos++; /* skip ext ID */ len--; + if (len & 1) { + wpa_printf(MSG_DEBUG, + "SAE: Invalid length of the Rejected Groups element payload: %u", + len); + return WLAN_STATUS_UNSPECIFIED_FAILURE; + } wpabuf_free(sae->tmp->peer_rejected_groups); sae->tmp->peer_rejected_groups = wpabuf_alloc(len); if (!sae->tmp->peer_rejected_groups) From 7fc4d894a9f5af1e50911685dc83ab7c8ec1d88f Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Tue, 10 Sep 2024 10:22:08 +0530 Subject: [PATCH 4/5] SAE: Check that peer's rejected groups are not enabled Signed-off-by: Jouni Malinen --- .../esp_supplicant/src/esp_wpa3.c | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c index 27b0f05b1667..ed026551b0a3 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -244,6 +244,53 @@ static u8 *wpa3_build_sae_msg(u8 *bssid, u32 sae_msg_type, size_t *sae_msg_len) return buf; } +static int wpa3_sae_is_group_enabled(int group) +{ + int *groups = NULL; + int default_groups[] = { 19, 0 }; + int i; + + if (!groups) { + groups = default_groups; + } + + for (i = 0; groups[i] > 0; i++) { + if (groups[i] == group) { + return 1; + } + } + + return 0; +} + +static int wpa3_check_sae_rejected_groups(const struct wpabuf *groups) +{ + size_t i, count; + const u8 *pos; + + if (!groups) { + return 0; + } + + pos = wpabuf_head(groups); + count = wpabuf_len(groups) / 2; + for (i = 0; i < count; i++) { + int enabled; + u16 group; + + group = WPA_GET_LE16(pos); + pos += 2; + enabled = wpa3_sae_is_group_enabled(group); + wpa_printf(MSG_DEBUG, "SAE: Rejected group %u is %s", + group, enabled ? "enabled" : "disabled"); + if (enabled) { + return 1; + } + } + + return 0; +} + static int wpa3_parse_sae_commit(u8 *buf, u32 len, u16 status) { int ret; @@ -281,6 +328,9 @@ static int wpa3_parse_sae_commit(u8 *buf, u32 len, u16 status) wpa_printf(MSG_ERROR, "wpa3: could not parse commit(%d)", ret); return ret; } + if (g_sae_data.tmp && wpa3_check_sae_rejected_groups(g_sae_data.tmp->peer_rejected_groups)) { + return -1; + } ret = sae_process_commit(&g_sae_data); if (ret) { From f9aa42b13786e4800bc410b3b2a78848bd743f11 Mon Sep 17 00:00:00 2001 From: Jouni Malinen Date: Tue, 10 Sep 2024 10:25:11 +0530 Subject: [PATCH 5/5] SAE: Check for invalid Rejected Groups element length explicitly on STA Instead of practically ignoring an odd octet at the end of the element, check for such invalid case explicitly. This is needed to avoid a potential group downgrade attack. Fixes: 444d76f74f65 ("SAE: Check that peer's rejected groups are not enabled") Signed-off-by: Jouni Malinen --- .../wpa_supplicant/esp_supplicant/src/esp_wpa3.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c index ed026551b0a3..76fa0f5f32ed 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -265,7 +265,7 @@ static int wpa3_sae_is_group_enabled(int group) static int wpa3_check_sae_rejected_groups(const struct wpabuf *groups) { - size_t i, count; + size_t i, count, len; const u8 *pos; if (!groups) { @@ -273,7 +273,14 @@ static int wpa3_check_sae_rejected_groups(const struct wpabuf *groups) } pos = wpabuf_head(groups); - count = wpabuf_len(groups) / 2; + len = wpabuf_len(groups); + if (len & 1) { + wpa_printf(MSG_DEBUG, + "SAE: Invalid length of the Rejected Groups element payload: %zu", + len); + return 1; + } + count = len / 2; for (i = 0; i < count; i++) { int enabled; u16 group;