From a04d277251b2f63c57b507bb00ab8df406e30938 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 e8dbc7f067..e3b4a732a4 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -402,6 +402,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) @@ -495,6 +546,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 fbbd0e29e9603f642daffe04c228e351d16781cc 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 e3b4a732a4..9fad4fd34d 100644 --- a/components/wpa_supplicant/src/ap/ieee802_11.c +++ b/components/wpa_supplicant/src/ap/ieee802_11.c @@ -425,7 +425,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) @@ -435,7 +435,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 46f3eedcefdc6d0e288d24e3cc4982a0839f98c0 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 9a6389978cda2891a303e95b892ef969844a6af5 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 9747793081..20d623147b 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -242,6 +242,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; @@ -278,6 +325,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 ce2e5455e8bb8a8488cb9b94866a953a6f909f9d 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 20d623147b..c0d8c719bf 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -263,7 +263,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) { @@ -271,7 +271,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;