From 3448ff697b826539b9ca5770eff8a181d3d9d694 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 2fc232f1db..365f8c25bf 100644 --- a/components/wpa_supplicant/src/common/sae.c +++ b/components/wpa_supplicant/src/common/sae.c @@ -2018,8 +2018,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 */ @@ -2079,6 +2082,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 2e25cef6b3fdc812f3dbe3e41ea34963064f93f2 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 84dc7782b74ff800fbaf4f5d81f49f1f0dcaecef 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 365f8c25bf..a533c92346 100644 --- a/components/wpa_supplicant/src/common/sae.c +++ b/components/wpa_supplicant/src/common/sae.c @@ -2032,6 +2032,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 7d943fb0324d4ac62b434209d7c7d5d3d27508c6 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 58575b1807..0164eb1607 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -236,6 +236,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; @@ -272,6 +319,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 4e9f740a0e9f0582eed504d7ff7bec927bc97b68 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 0164eb1607..433ec38acb 100644 --- a/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c +++ b/components/wpa_supplicant/esp_supplicant/src/esp_wpa3.c @@ -257,7 +257,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) { @@ -265,7 +265,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;