diff mbox series

wpa_supplicant: Use RSNE parsing for OSEN

Message ID 20191220192128.85524-3-alexander@wetzel-home.de
State Changes Requested
Headers show
Series wpa_supplicant: Use RSNE parsing for OSEN | expand

Commit Message

Alexander Wetzel Dec. 20, 2019, 7:21 p.m. UTC 
Use the existing RSNE parsing and checks also for OSEN.

Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de>
---
 src/rsn_supp/wpa.c         | 32 ++++++++++----
 wpa_supplicant/events.c    | 86 ++++++++++++++++++++++----------------
 wpa_supplicant/wpas_glue.c | 13 +++++-
 3 files changed, 87 insertions(+), 44 deletions(-)

Comments

Jouni Malinen Dec. 21, 2019, 3:19 p.m. UTC | #1 
On Fri, Dec 20, 2019 at 08:21:27PM +0100, Alexander Wetzel wrote:
> Use the existing RSNE parsing and checks also for OSEN.

Could you please provide more detailed commit message for this change,
i.e., why should this be done, does this fix something, how has this
been tested to confirm there are no regressions, etc.? I'm not at all
convinced that all OSEN AP implementations behave in a manner that is
consistent between RSN and OSEN and as such, some of these changes look
like high risk items for introducing interoperability issues. Some of
the changes might be safe to do for sharing same routines, but in
general, I'm not sure there is enough benefit from all these changes to
justify the increased risk for interoperability.
Alexander Wetzel Dec. 22, 2019, 1:26 p.m. UTC | #2 
>> Use the existing RSNE parsing and checks also for OSEN.
>
> Could you please provide more detailed commit message for this change,
> i.e., why should this be done, does this fix something, how has this
> been tested to confirm there are no regressions, etc.? I'm not at all
> convinced that all OSEN AP implementations behave in a manner that is
> consistent between RSN and OSEN and as such, some of these changes look
> like high risk items for introducing interoperability issues. Some of
> the changes might be safe to do for sharing same routines, but in
> general, I'm not sure there is enough benefit from all these changes to
> justify the increased risk for interoperability.
>

This only has been tested with the hostapd osen tests.
While it's kind of a cleanup it could well cause regressions...

The patch is the "fallout" from my attempts to get OSEN working with Extended
Key ID. Without this patch the OSEN element is not parsed, so any data in it
is inaccessible. (We never call wpa_sm_set_ap_rsn_ie() or a replacement for
OSEN.)
Now I finally figured out that the OSEN element can only be set by the AP,
depriving it of any way to detect if a STA is compatible. (I still may be
mistaken about that but I was sure enough to stop looking into it...) 
This of course killed the reason why I was trying to fix the OSEN parsing in
the first place:-)

The current code is kind of sitting between the chairs: Some functions have
useless code considering OSEN but some central pieces are missing.
Therefore I just wanted to share how far I got to fix that. I'm totally fine
to not apply it due to risk of regressions. (I may think about preparing a
patch to rip out the useless code in that case instead.)

Alexander
diff mbox series

Patch

diff --git a/src/rsn_supp/wpa.c b/src/rsn_supp/wpa.c
index 504feaf2a..99cdab298 100644
--- a/src/rsn_supp/wpa.c
+++ b/src/rsn_supp/wpa.c
@@ -1316,6 +1316,9 @@  static int wpa_supplicant_validate_ie(struct wpa_sm *sm,
 				      const unsigned char *src_addr,
 				      struct wpa_eapol_ie_parse *ie)
 {
+	const u8 *rsn_ie;
+	size_t rsn_ie_len;
+
 	if (sm->ap_wpa_ie == NULL && sm->ap_rsn_ie == NULL) {
 		wpa_dbg(sm->ctx->msg_ctx, MSG_DEBUG,
 			"WPA: No WPA/RSN IE for this AP known. "
@@ -1329,39 +1332,54 @@  static int wpa_supplicant_validate_ie(struct wpa_sm *sm,
 				"WPA: Found the current AP from "
 				"updated scan results");
 		}
+	} else {
+		wpa_msg(sm->ctx->msg_ctx, MSG_DEBUG,
+			"WPA: Using known WPA/RSN IE for this AP.");
+	}
+
+	if (ie->rsn_ie && ie->osen) {
+		wpa_msg(sm->ctx->msg_ctx, MSG_ERROR,
+			"WPA: Error - RSN and OSEN IE used together");
+		return -1;
+	} else if (ie->osen) {
+		rsn_ie = ie->osen;
+		rsn_ie_len = ie->osen_len;
+	} else {
+		rsn_ie = ie->rsn_ie;
+		rsn_ie_len = ie->rsn_ie_len;
 	}
 
-	if (ie->wpa_ie == NULL && ie->rsn_ie == NULL &&
+	if (ie->wpa_ie == NULL && rsn_ie == NULL &&
 	    (sm->ap_wpa_ie || sm->ap_rsn_ie)) {
 		wpa_report_ie_mismatch(sm, "IE in 3/4 msg does not match "
 				       "with IE in Beacon/ProbeResp (no IE?)",
 				       src_addr, ie->wpa_ie, ie->wpa_ie_len,
-				       ie->rsn_ie, ie->rsn_ie_len);
+				       rsn_ie, rsn_ie_len);
 		return -1;
 	}
 
 	if ((ie->wpa_ie && sm->ap_wpa_ie &&
 	     (ie->wpa_ie_len != sm->ap_wpa_ie_len ||
 	      os_memcmp(ie->wpa_ie, sm->ap_wpa_ie, ie->wpa_ie_len) != 0)) ||
-	    (ie->rsn_ie && sm->ap_rsn_ie &&
+	    (rsn_ie && sm->ap_rsn_ie &&
 	     wpa_compare_rsn_ie(wpa_key_mgmt_ft(sm->key_mgmt),
 				sm->ap_rsn_ie, sm->ap_rsn_ie_len,
-				ie->rsn_ie, ie->rsn_ie_len))) {
+				rsn_ie, rsn_ie_len))) {
 		wpa_report_ie_mismatch(sm, "IE in 3/4 msg does not match "
 				       "with IE in Beacon/ProbeResp",
 				       src_addr, ie->wpa_ie, ie->wpa_ie_len,
-				       ie->rsn_ie, ie->rsn_ie_len);
+				       rsn_ie, rsn_ie_len);
 		return -1;
 	}
 
 	if (sm->proto == WPA_PROTO_WPA &&
-	    ie->rsn_ie && sm->ap_rsn_ie == NULL && sm->rsn_enabled) {
+	    rsn_ie && sm->ap_rsn_ie == NULL && sm->rsn_enabled) {
 		wpa_report_ie_mismatch(sm, "Possible downgrade attack "
 				       "detected - RSN was enabled and RSN IE "
 				       "was in msg 3/4, but not in "
 				       "Beacon/ProbeResp",
 				       src_addr, ie->wpa_ie, ie->wpa_ie_len,
-				       ie->rsn_ie, ie->rsn_ie_len);
+				       rsn_ie, rsn_ie_len);
 		return -1;
 	}
 
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 9d808c55d..6fe4cdd2a 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -538,7 +538,7 @@  static int wpa_supplicant_ssid_bss_match(struct wpa_supplicant *wpa_s,
 {
 	struct wpa_ie_data ie;
 	int proto_match = 0;
-	const u8 *rsn_ie, *wpa_ie;
+	const u8 *rsn_ie, *wpa_ie, *osen;
 	int ret;
 	int wep_ok;
 
@@ -553,9 +553,21 @@  static int wpa_supplicant_ssid_bss_match(struct wpa_supplicant *wpa_s,
 		 (ssid->key_mgmt & WPA_KEY_MGMT_IEEE8021X_NO_WPA));
 
 	rsn_ie = wpa_bss_get_ie(bss, WLAN_EID_RSN);
-	while ((ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN)) && rsn_ie) {
+	osen = wpa_bss_get_vendor_ie(bss, OSEN_IE_VENDOR_TYPE);
+
+	while ((ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN)) &&
+	       (rsn_ie || osen)) {
 		proto_match++;
 
+		if (osen && (ssid->proto & WPA_PROTO_RSN || rsn_ie)) {
+			if (debug_print)
+				wpa_dbg(wpa_s, MSG_DEBUG,
+					"   skip - invalid RSN/OSEN mix");
+			break;
+		} else if (osen) {
+			rsn_ie = osen;
+		}
+
 		if (wpa_parse_wpa_ie(rsn_ie, 2 + rsn_ie[1], &ie)) {
 			if (debug_print)
 				wpa_dbg(wpa_s, MSG_DEBUG,
@@ -576,14 +588,6 @@  static int wpa_supplicant_ssid_bss_match(struct wpa_supplicant *wpa_s,
 			return 1;
 		}
 
-		if (!(ie.proto & ssid->proto) &&
-		    !(ssid->proto & WPA_PROTO_OSEN)) {
-			if (debug_print)
-				wpa_dbg(wpa_s, MSG_DEBUG,
-					"   skip RSN IE - proto mismatch");
-			break;
-		}
-
 		if (!(ie.pairwise_cipher & ssid->pairwise_cipher)) {
 			if (debug_print)
 				wpa_dbg(wpa_s, MSG_DEBUG,
@@ -729,21 +733,14 @@  static int wpa_supplicant_ssid_bss_match(struct wpa_supplicant *wpa_s,
 	}
 #endif /* CONFIG_OWE */
 
-	if ((ssid->proto & (WPA_PROTO_WPA | WPA_PROTO_RSN)) &&
+	if ((ssid->proto & (WPA_PROTO_WPA | WPA_PROTO_RSN | WPA_PROTO_OSEN)) &&
 	    wpa_key_mgmt_wpa(ssid->key_mgmt) && proto_match == 0) {
 		if (debug_print)
 			wpa_dbg(wpa_s, MSG_DEBUG,
-				"   skip - no WPA/RSN proto match");
+				"   skip - no WPA/RSN/OSEN proto match");
 		return 0;
 	}
 
-	if ((ssid->key_mgmt & WPA_KEY_MGMT_OSEN) &&
-	    wpa_bss_get_vendor_ie(bss, OSEN_IE_VENDOR_TYPE)) {
-		if (debug_print)
-			wpa_dbg(wpa_s, MSG_DEBUG, "   allow in OSEN");
-		return 1;
-	}
-
 	if (!wpa_key_mgmt_wpa(ssid->key_mgmt)) {
 		if (debug_print)
 			wpa_dbg(wpa_s, MSG_DEBUG, "   allow in non-WPA/WPA2");
@@ -2418,7 +2415,7 @@  static int wpas_fst_update_mbie(struct wpa_supplicant *wpa_s,
 static int wpa_supplicant_event_associnfo(struct wpa_supplicant *wpa_s,
 					  union wpa_event_data *data)
 {
-	int l, len, found = 0, found_x = 0, wpa_found, rsn_found;
+	int l, len, found = 0, found_x = 0, wpa_found, rsn_found, osen_found;
 	const u8 *p;
 #if defined(CONFIG_IEEE80211R) || defined(CONFIG_OWE)
 	u8 bssid[ETH_ALEN];
@@ -2491,10 +2488,9 @@  static int wpa_supplicant_event_associnfo(struct wpa_supplicant *wpa_s,
 			break;
 		}
 		if (!found &&
-		    ((p[0] == WLAN_EID_VENDOR_SPECIFIC && p[1] >= 6 &&
-		      (os_memcmp(&p[2], "\x00\x50\xF2\x01\x01\x00", 6) == 0)) ||
-		     (p[0] == WLAN_EID_VENDOR_SPECIFIC && p[1] >= 4 &&
-		      (os_memcmp(&p[2], "\x50\x6F\x9A\x12", 4) == 0)) ||
+		    ((p[0] == WLAN_EID_VENDOR_SPECIFIC && p[1] >= 4 &&
+		      (WPA_GET_BE32(&p[2]) == WPA_IE_VENDOR_TYPE ||
+		       WPA_GET_BE32(&p[2]) == OSEN_IE_VENDOR_TYPE)) ||
 		     (p[0] == WLAN_EID_RSN && p[1] >= 2))) {
 			if (wpa_sm_set_assoc_wpa_ie(wpa_s->wpa, p, len))
 				break;
@@ -2653,7 +2649,7 @@  no_pfs:
 
 	/* Go through the IEs and make a copy of the WPA/RSN IEs, if present.
 	 */
-	wpa_found = rsn_found = 0;
+	wpa_found = rsn_found = osen_found = 0;
 	while (p && l >= 2) {
 		len = p[1] + 2;
 		if (len > l) {
@@ -2661,11 +2657,16 @@  no_pfs:
 				    p, l);
 			break;
 		}
-		if (!wpa_found &&
-		    p[0] == WLAN_EID_VENDOR_SPECIFIC && p[1] >= 6 &&
-		    os_memcmp(&p[2], "\x00\x50\xF2\x01\x01\x00", 6) == 0) {
-			wpa_found = 1;
-			wpa_sm_set_ap_wpa_ie(wpa_s->wpa, p, len);
+		if (p[0] == WLAN_EID_VENDOR_SPECIFIC && p[1] >= 4) {
+			if (!wpa_found &&
+			    WPA_GET_BE32(&p[2]) == WPA_IE_VENDOR_TYPE) {
+				wpa_found = 1;
+				wpa_sm_set_ap_wpa_ie(wpa_s->wpa, p, len);
+			} else if (!osen_found &&
+				   WPA_GET_BE32(&p[2]) == OSEN_IE_VENDOR_TYPE) {
+				osen_found = 1;
+				wpa_sm_set_ap_rsn_ie(wpa_s->wpa, p, len);
+			}
 		}
 
 		if (!rsn_found &&
@@ -2673,6 +2674,11 @@  no_pfs:
 			rsn_found = 1;
 			wpa_sm_set_ap_rsn_ie(wpa_s->wpa, p, len);
 		}
+		if (!osen_found &&
+		    p[0] == WLAN_EID_RSN && p[1] >= 2) {
+			osen_found = 1;
+			wpa_sm_set_ap_rsn_ie(wpa_s->wpa, p, len);
+		}
 
 		if (p[0] == WLAN_EID_RSNX && p[1] >= 1)
 			wpa_sm_set_ap_rsnxe(wpa_s->wpa, p, len);
@@ -2681,13 +2687,21 @@  no_pfs:
 		p += len;
 	}
 
-	if (!wpa_found && data->assoc_info.beacon_ies)
-		wpa_sm_set_ap_wpa_ie(wpa_s->wpa, NULL, 0);
-	if (!rsn_found && data->assoc_info.beacon_ies) {
-		wpa_sm_set_ap_rsn_ie(wpa_s->wpa, NULL, 0);
-		wpa_sm_set_ap_rsnxe(wpa_s->wpa, NULL, 0);
+	if (rsn_found && osen_found) {
+		wpa_msg(wpa_s, MSG_ERROR,
+			"Invalid combination: RSN and OSEN IE found");
+		return -1;
+	}
+
+	if (data->assoc_info.beacon_ies) {
+		if (!wpa_found)
+			wpa_sm_set_ap_wpa_ie(wpa_s->wpa, NULL, 0);
+		if (!rsn_found && !osen_found)
+			wpa_sm_set_ap_rsn_ie(wpa_s->wpa, NULL, 0);
+		if (!rsn_found)
+			wpa_sm_set_ap_rsnxe(wpa_s->wpa, NULL, 0);
 	}
-	if (wpa_found || rsn_found)
+	if (wpa_found || rsn_found || osen_found)
 		wpa_s->ap_ies_from_associnfo = 1;
 
 	if (wpa_s->assoc_freq && data->assoc_info.freq &&
diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c
index 503163d5d..cbaff4ff3 100644
--- a/wpa_supplicant/wpas_glue.c
+++ b/wpa_supplicant/wpas_glue.c
@@ -377,7 +377,7 @@  static int wpa_get_beacon_ie(struct wpa_supplicant *wpa_s)
 	int ret = 0;
 	struct wpa_bss *curr = NULL, *bss;
 	struct wpa_ssid *ssid = wpa_s->current_ssid;
-	const u8 *ie;
+	const u8 *ie, *osen;
 
 	dl_list_for_each(bss, &wpa_s->bss, struct wpa_bss, list) {
 		if (os_memcmp(bss->bssid, wpa_s->bssid, ETH_ALEN) != 0)
@@ -396,7 +396,18 @@  static int wpa_get_beacon_ie(struct wpa_supplicant *wpa_s)
 		if (wpa_sm_set_ap_wpa_ie(wpa_s->wpa, ie, ie ? 2 + ie[1] : 0))
 			ret = -1;
 
+		osen = wpa_bss_get_vendor_ie(curr, OSEN_IE_VENDOR_TYPE);
 		ie = wpa_bss_get_ie(curr, WLAN_EID_RSN);
+
+		/* RSN and OSEN Elements are mutually exclusive but
+		 * have the same content and similar use cases.
+		 * Therefore just store the OSEN Element in ap_rsn_ie
+		 */
+		if (osen && ie)
+			ret = -1;
+		else if (osen)
+			ie = osen;
+
 		if (wpa_sm_set_ap_rsn_ie(wpa_s->wpa, ie, ie ? 2 + ie[1] : 0))
 			ret = -1;