From: Jouni Malinen <quic_jouni@quicinc.com>
Date: Wed, 30 Oct 2024 10:30:35 +0000 (+0200)
Subject: SAE: More robust password identifier checks for AP mode
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28ebad01e8d35827672ff562d4a23947dff23229;p=thirdparty%2Fhostap.git

SAE: More robust password identifier checks for AP mode

Do not update the more persistent sae->tmp->pw_id value based on each
received SAE commit message before having successfully processed the
commit. In particular, this includes checking for a matcing password
identifier in cases where the AP has enabled one or more SAE passwords
with identifiers.

A per-received message sae->tmp->parsed_pw_id is used during parsing and
processing of each individual message and sae->tmp->pw_id is set only
after having successfully processed a commit message. This avoids
getting sae->tmp->pw_id being bound to an unknown value.

An earlier commit addressed some of the sequences that could have this
issue, but it missed some cases. This newer more robust version covers
what the earlier commit did, so that part can be removed with the new
design.

Fixes: 761041b18ab2 ("SAE: Free password identifier if SAE commit is rejected due to it")
Signed-off-by: Jouni Malinen <quic_jouni@quicinc.com>
---

diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
index 98609c013..8115b68cc 100644
--- a/src/ap/ieee802_11.c
+++ b/src/ap/ieee802_11.c
@@ -621,7 +621,8 @@ static struct wpabuf * auth_build_sae_commit(struct hostapd_data *hapd,
 #endif /* CONFIG_IEEE80211BE */
 
 	if (sta->sae->tmp) {
-		rx_id = sta->sae->tmp->pw_id;
+		rx_id = sta->sae->tmp->parsed_pw_id ?
+			sta->sae->tmp->parsed_pw_id : sta->sae->tmp->pw_id;
 		use_pt = sta->sae->h2e;
 #ifdef CONFIG_SAE_PK
 		os_memcpy(sta->sae->tmp->own_addr, own_addr, ETH_ALEN);
@@ -713,7 +714,8 @@ static int auth_sae_send_commit(struct hostapd_data *hapd,
 	u16 status;
 
 	data = auth_build_sae_commit(hapd, sta, update, status_code);
-	if (!data && sta->sae->tmp && sta->sae->tmp->pw_id)
+	if (!data && sta->sae->tmp &&
+	    (sta->sae->tmp->pw_id || sta->sae->tmp->parsed_pw_id))
 		return WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER;
 	if (data == NULL)
 		return WLAN_STATUS_UNSPECIFIED_FAILURE;
@@ -1002,7 +1004,9 @@ static int sae_sm_step(struct hostapd_data *hapd, struct sta_info *sta,
 	switch (sta->sae->state) {
 	case SAE_NOTHING:
 		if (auth_transaction == 1) {
-			if (sta->sae->tmp) {
+			struct sae_temporary_data *tmp = sta->sae->tmp;
+
+			if (tmp) {
 				sta->sae->h2e =
 					(status_code ==
 					 WLAN_STATUS_SAE_HASH_TO_ELEMENT ||
@@ -1012,8 +1016,21 @@ static int sae_sm_step(struct hostapd_data *hapd, struct sta_info *sta,
 			}
 			ret = auth_sae_send_commit(hapd, sta,
 						   !allow_reuse, status_code);
+			if (ret == WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER)
+				wpa_msg(hapd->msg_ctx, MSG_INFO,
+					WPA_EVENT_SAE_UNKNOWN_PASSWORD_IDENTIFIER
+					MACSTR, MAC2STR(sta->addr));
 			if (ret)
 				return ret;
+
+			if (tmp && tmp->parsed_pw_id && !tmp->pw_id) {
+				tmp->pw_id = tmp->parsed_pw_id;
+				tmp->parsed_pw_id = NULL;
+				wpa_printf(MSG_DEBUG,
+					   "SAE: Known Password Identifier bound to this STA: '%s'",
+					   tmp->pw_id);
+			}
+
 			sae_set_state(sta, SAE_COMMITTED, "Sent Commit");
 
 			if (sae_process_commit(sta->sae) < 0)
@@ -1637,14 +1654,6 @@ reply:
 				data ? wpabuf_head(data) : (u8 *) "",
 				data ? wpabuf_len(data) : 0, "auth-sae");
 		sae_sme_send_external_auth_status(hapd, sta, resp);
-		if (sta->sae && sta->sae->tmp && sta->sae->tmp->pw_id &&
-		    resp == WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER &&
-		    auth_transaction == 1) {
-			wpa_printf(MSG_DEBUG,
-				   "SAE: Clear stored password identifier since this SAE commit was not accepted");
-			os_free(sta->sae->tmp->pw_id);
-			sta->sae->tmp->pw_id = NULL;
-		}
 	}
 
 remove_sta:
diff --git a/src/common/sae.c b/src/common/sae.c
index c93d3a6d2..6b06473e1 100644
--- a/src/common/sae.c
+++ b/src/common/sae.c
@@ -114,6 +114,7 @@ void sae_clear_temp_data(struct sae_data *sae)
 	wpabuf_free(tmp->own_rejected_groups);
 	wpabuf_free(tmp->peer_rejected_groups);
 	os_free(tmp->pw_id);
+	os_free(tmp->parsed_pw_id);
 	bin_clear_free(tmp, sizeof(*tmp));
 	sae->tmp = NULL;
 }
@@ -2061,8 +2062,8 @@ static int sae_parse_password_identifier(struct sae_data *sae,
 				   sae->tmp->pw_id);
 			return WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER;
 		}
-		os_free(sae->tmp->pw_id);
-		sae->tmp->pw_id = NULL;
+		os_free(sae->tmp->parsed_pw_id);
+		sae->tmp->parsed_pw_id = NULL;
 		return WLAN_STATUS_SUCCESS; /* No Password Identifier */
 	}
 
@@ -2089,14 +2090,14 @@ static int sae_parse_password_identifier(struct sae_data *sae,
 		return WLAN_STATUS_UNKNOWN_PASSWORD_IDENTIFIER;
 	}
 
-	os_free(sae->tmp->pw_id);
-	sae->tmp->pw_id = os_malloc(len + 1);
-	if (!sae->tmp->pw_id)
+	os_free(sae->tmp->parsed_pw_id);
+	sae->tmp->parsed_pw_id = os_malloc(len + 1);
+	if (!sae->tmp->parsed_pw_id)
 		return WLAN_STATUS_UNSPECIFIED_FAILURE;
-	os_memcpy(sae->tmp->pw_id, epos, len);
-	sae->tmp->pw_id[len] = '\0';
+	os_memcpy(sae->tmp->parsed_pw_id, epos, len);
+	sae->tmp->parsed_pw_id[len] = '\0';
 	wpa_hexdump_ascii(MSG_DEBUG, "SAE: Received Password Identifier",
-			  sae->tmp->pw_id, len);
+			  sae->tmp->parsed_pw_id, len);
 	*pos = epos + len;
 	return WLAN_STATUS_SUCCESS;
 }
diff --git a/src/common/sae.h b/src/common/sae.h
index 3a83c3be5..8f74353be 100644
--- a/src/common/sae.h
+++ b/src/common/sae.h
@@ -59,6 +59,7 @@ struct sae_temporary_data {
 	struct crypto_bignum *order_buf;
 	struct wpabuf *anti_clogging_token;
 	char *pw_id;
+	char *parsed_pw_id;
 	int vlan_id;
 	u8 bssid[ETH_ALEN];
 	struct wpabuf *own_rejected_groups;