From 6b3bf4d0e7c1354bdb6c4d22c54247c5a67e942d 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 15a6b4a603..38a71d190c 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 37b918230c..bc703444cd 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 5f7a3b6d4822f6bd4bb5f2859f556d6e635a8a02 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 38a71d190c..f47a9b86cc 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 90317ded69f81c653969f148b962410d21aa1a16 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 bc703444cd..363aa0c4fe 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 c6fee11bfcbf7a376cdce1b90493652f979ab177 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 0365062780..7287ba5ff3 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 b18849638ef3420cd0b133260dfe056f749390ab 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 7287ba5ff3..b7c844aef5 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;