| Message ID | 20191220192128.85524-4-alexander@wetzel-home.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | wpa_supplicant: Don't incorrectly clear ie scan data | expand |
On Fri, Dec 20, 2019 at 08:21:28PM +0100, Alexander Wetzel wrote: > wpa_supplicant_set_suites() can be called without providing a scan > result. Don't flush ie elements in this case. Could you please provide more details and a debug log showing a case where this ends up flushing IE information incorrectly? Does this actually fix a connection in some cases? > diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > @@ -1378,7 +1378,7 @@ int wpa_supplicant_set_suites(struct wpa_supplicant *wpa_s, > wpa_sm_set_param(wpa_s->wpa, WPA_PARAM_RSN_ENABLED, > !!(ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN))); > > - if (bss || !wpa_s->ap_ies_from_associnfo) { > + if (bss && !wpa_s->ap_ies_from_associnfo) { > if (wpa_sm_set_ap_wpa_ie(wpa_s->wpa, bss_wpa, > bss_wpa ? 2 + bss_wpa[1] : 0) || This does not flush IEs if bss != NULL (that uses the BSS table entry from scan results). I'm not sure I understand why the IEs should not be cleared if there is neither a BSS entry nor IEs from association information.
>> wpa_supplicant_set_suites() can be called without providing a scan >> result. Don't flush ie elements in this case. > Could you please provide more details and a debug log showing a case > where this ends up flushing IE information incorrectly? Does this > actually fix a connection in some cases? This is not really "fixing" a connection in the current code. It just gets rid of a avoidable scan in some cases. The next version for Extended Key ID support patches depends on it or it breaks some owe connections when enabled and it looks like some connections are currently only working due to another decision or maybe a bug. (The IE mismatch detection issue outlined below.) Let's start with the reasoning why I think it should be changed: wpa_supplicant_set_suites() can be called with bss set to NULL: * @bss: Scan results for the selected BSS, or %NULL if not available Now when bss is NULL bss_wpa, bss_rsn, bss_rsnx and bss_osen are unconditionally NULL, too. Therefore ap_ies_from_associnfo = 0 is only the trigger to delete any ie data stored in the state machine. But any data stored there should still be "valid" and in fact may be needed later again. I'm pretty sure the intent for ap_ies_from_associnfo is to make sure we only store ie data from a beacon in the state machine, to have them later available in wpa_supplicant_validate_ie() to be verified against the IE from eapol#3. wpa_supplicant_validate_ie() is by the way also assuming that the variables are set and for some reason is not triggering a RSN mismatch when ap_wpa_ie or ap_rsn_ie the state machine is NULL. (It's done for ap_rsnxe.) We may want to fix that, too... I'll send you an alternate patch for this one including that, too. But that may trigger disconnects with some broken APs and is a bit more risky. With the code as it is today we just have some suboptimal cases we can see in the following tests: owe_transition_mode owe_transition_mode_connect_cmd owe_transition_mode_multi_bss owe_transition_mode_open_multiple_scans Looking at owe_transition_mode.log0 from a test run we see the following order of events: (I generously removed lines and just kept enough to see the picture. The full logfile is available here: https://www.awhome.eu/index.php/s/jDQfpZmDGwsLPXA and named cf28cfc12-orig-owe_transition_mode.log0) line log message 321 wlan0: Request association with 02:00:00:00:03:00 337 wlan0: WPA: Selected cipher suites: group 16 pairwise 16 key_mgmt 4194304 proto 2 339 wlan0: WPA: clearing AP WPA IE --- here we set the variable 340 WPA: set AP RSN IE - hexdump(len=22): 30 14 01 00 00 0f ac 04 01 00 00 0f ac 04 01 00 00 0f ac 12 cc 00 341 wlan0: WPA: clearing AP RSNXE 361 wlan0: SME: Trying to authenticate with 02:00:00:00:03:00 (SSID='owe-random' freq=2412 MHz) 367 wlan0: State: INACTIVE -> AUTHENTICATING 435 wlan0: State: AUTHENTICATING -> ASSOCIATING 497 wlan0: State: ASSOCIATING -> ASSOCIATED 508 wlan0: Associated to a new BSS: BSSID=02:00:00:00:03:00 511 wlan0: Network configuration found for the current AP 512 wlan0: WPA: Using WPA IE from AssocReq to set cipher suites 513 wlan0: WPA: Selected cipher suites: group 16 pairwise 16 key_mgmt 4194304 proto 2 --- here are the variables cleared without any good reason --- 515 wlan0: WPA: clearing AP WPA IE 516 wlan0: WPA: clearing AP RSN IE 517 wlan0: WPA: clearing AP RSNXE 532 wlan0: Associated with 02:00:00:00:03:00 583 wlan0: State: ASSOCIATED -> 4WAY_HANDSHAKE 592 wlan0: WPA: RX message 1 of 4-Way Handshake from 02:00:00:00:03:00 (ver=0) 606 wlan0: WPA: Sending EAPOL-Key 2/4 630 wlan0: State: 4WAY_HANDSHAKE -> 4WAY_HANDSHAKE 631 wlan0: WPA: RX message 3 of 4-Way Handshake from 02:00:00:00:03:00 (ver=0) --- here we need the deleted data again and trigger a scan the set the variables again --- 636 wlan0: WPA: No WPA/RSN IE for this AP known. Trying to get from scan results 637 nl80211: Received scan results (3 BSSes) 638 nl80211: Scan results indicate BSS status with 02:00:00:00:03:00 as associated --- but we don't get them and as a result bypass the (also broken?) IE mismatch check 647 wlan0: WPA: Could not find AP from the scan results 652 wlan0: WPA: Sending EAPOL-Key 4/4 >> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c >> @@ -1378,7 +1378,7 @@ int wpa_supplicant_set_suites(struct wpa_supplicant *wpa_s, >> wpa_sm_set_param(wpa_s->wpa, WPA_PARAM_RSN_ENABLED, >> !!(ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN))); >> >> - if (bss || !wpa_s->ap_ies_from_associnfo) { >> + if (bss && !wpa_s->ap_ies_from_associnfo) { >> if (wpa_sm_set_ap_wpa_ie(wpa_s->wpa, bss_wpa, >> bss_wpa ? 2 + bss_wpa[1] : 0) || > > This does not flush IEs if bss != NULL (that uses the BSS table entry > from scan results). I'm not sure I understand why the IEs should not be > cleared if there is neither a BSS entry nor IEs from association > information. I stopped digging into that after discovering that the bss is an optional argument. But as explained above we must not update the state machine variables with IEs from association requests. So I assume that was the intent of the code and we just use the wrong logical operation for that. Which then deletes the variables. It could be that the correct fix is to make the bss mandatory or at least never call wpa_supplicant_set_suites() when one of the variables are set... But that's not my understanding of the code. Alexander
On Sun, Dec 22, 2019 at 02:30:42PM +0100, Alexander Wetzel wrote: > This is not really "fixing" a connection in the current code. It just gets rid > of a avoidable scan in some cases. The next version for Extended Key ID > support patches depends on it or it breaks some owe connections when enabled > and it looks like some connections are currently only working due to another > decision or maybe a bug. (The IE mismatch detection issue outlined below.) That is not really a new scan, but a new instance of fetching the already existing scan results from the driver. Anyway, you are correct in this not being the expected code path to take here. It was just the proposed change that made me not understand what exactly is going on here.. I went through the debug log line by line to figure out what exactly happened. > Let's start with the reasoning why I think it should be changed: > wpa_supplicant_set_suites() can be called with bss set to NULL: > * @bss: Scan results for the selected BSS, or %NULL if not available wpa_supplicant_set_suites() should not be called at all in this case which is the main issue here.. That function is used when the driver instead of wpa_supplicant decides to roam to another BSS. That is not the case with OWE transition mode. > I'm pretty sure the intent for ap_ies_from_associnfo is to make sure we only > store ie data from a beacon in the state machine, to have them later available > in wpa_supplicant_validate_ie() to be verified against the IE from eapol#3. > wpa_supplicant_validate_ie() is by the way also assuming that the variables > are set and for some reason is not triggering a RSN mismatch when ap_wpa_ie or > ap_rsn_ie the state machine is NULL. (It's done for ap_rsnxe.) We may want to > fix that, too... I'll send you an alternate patch for this one including that, > too. But that may trigger disconnects with some broken APs and is a bit more > risky. The case of Beacon frame no including an RSNE sounds quite unlikely to be able to get to a state where RSN 4-way handshake is used since the RSNE from Beacon (or Probe Response) frame is the part that's used to negotiated use of RSN in the first place.. > With the code as it is today we just have some suboptimal cases we can see in > the following tests: > owe_transition_mode > owe_transition_mode_connect_cmd > owe_transition_mode_multi_bss > owe_transition_mode_open_multiple_scans > > Looking at owe_transition_mode.log0 from a test run we see the following order > of events: (I generously removed lines and just kept enough to see the picture. > The full logfile is available here: https://www.awhome.eu/index.php/s/jDQfpZmDGwsLPXA > and named cf28cfc12-orig-owe_transition_mode.log0) That location does not seem to be available anymore, but anyway, I think I understood most of it with one unclear, but not really directly related, item. > 321 wlan0: Request association with 02:00:00:00:03:00 > 337 wlan0: WPA: Selected cipher suites: group 16 pairwise 16 key_mgmt 4194304 proto 2 > 339 wlan0: WPA: clearing AP WPA IE > --- here we set the variable > 340 WPA: set AP RSN IE - hexdump(len=22): 30 14 01 00 00 0f ac 04 01 00 00 0f ac 04 01 00 00 0f ac 12 cc 00 > 341 wlan0: WPA: clearing AP RSNXE This is expected since wpa_supplicant has the updated BSS for the OWE BSS available at this point. > 508 wlan0: Associated to a new BSS: BSSID=02:00:00:00:03:00 > 511 wlan0: Network configuration found for the current AP That code path should not be entered at all.. The key debug entry that shows the incorrect behavior here is this one: Driver-initiated BSS selection changed the SSID to %s > 512 wlan0: WPA: Using WPA IE from AssocReq to set cipher suites > 513 wlan0: WPA: Selected cipher suites: group 16 pairwise 16 key_mgmt 4194304 proto 2 > --- here are the variables cleared without any good reason --- > 515 wlan0: WPA: clearing AP WPA IE > 516 wlan0: WPA: clearing AP RSN IE > 517 wlan0: WPA: clearing AP RSNXE Yes, none of this was supposed to happen since wpa_supplicant_set_suites() was not supposed to be called. > 631 wlan0: WPA: RX message 3 of 4-Way Handshake from 02:00:00:00:03:00 (ver=0) > --- here we need the deleted data again and trigger a scan the set the variables again --- > 636 wlan0: WPA: No WPA/RSN IE for this AP known. Trying to get from scan results This does not trigger a new scan; this is only fetching the current scan results from cfg80211. > 637 nl80211: Received scan results (3 BSSes) > 638 nl80211: Scan results indicate BSS status with 02:00:00:00:03:00 as associated > --- but we don't get them and as a result bypass the (also broken?) IE mismatch check > 647 wlan0: WPA: Could not find AP from the scan results That looks strange and is not something I see in my tests. That scan result has to be there in both wpa_supplicant and cfg80211 for the association to be started.. In practice, you should have seen this before association: nl80211: Trigger single channel scan to refresh cfg80211 BSS entry wlan0: nl80211: scan request nl80211: Scan SSID owe-random ... wlan0: nl80211: New scan results available nl80211: Scan results for missing cfg80211 BSS entry And now the entry for 02:00:00:00:03:00 with the correct (previously hidden) SSID is in place in cfg80211 (and it was already there with the correct SSID in wpa_supplicant BSS table). > >> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c > >> @@ -1378,7 +1378,7 @@ int wpa_supplicant_set_suites(struct wpa_supplicant *wpa_s, > >> wpa_sm_set_param(wpa_s->wpa, WPA_PARAM_RSN_ENABLED, > >> !!(ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN))); > >> > >> - if (bss || !wpa_s->ap_ies_from_associnfo) { > >> + if (bss && !wpa_s->ap_ies_from_associnfo) { > >> if (wpa_sm_set_ap_wpa_ie(wpa_s->wpa, bss_wpa, > >> bss_wpa ? 2 + bss_wpa[1] : 0) || So the reason I did not understand this (and well, still don't think this is correct) is that this function should never have been called in the first place. > I stopped digging into that after discovering that the bss is an optional > argument. But as explained above we must not update the state machine variables > with IEs from association requests. So I assume that was the intent of the code > and we just use the wrong logical operation for that. Which then deletes the > variables. > It could be that the correct fix is to make the bss mandatory or at least never > call wpa_supplicant_set_suites() when one of the variables are set... > But that's not my understanding of the code. Not calling wpa_supplicant_set_suites() is indeed the appropriate fix for this issue with OWE transition mode. [PATCH] OWE: Avoid incorrect profile update in transition mode The "unexpected" change of SSID between the current network profile (which uses the SSID from the open BSS in OWE transition mode) and the association with the OWE BSS (which uses a random, hidden SSID) resulted in wpa_supplicant incorrectly determining that this was a driver-initiated BSS selection ("Driver-initiated BSS selection changed the SSID to <the random SSID from OWE BSS>" in debug log). This ended up with updating security parameters based on the network profile inwpa_supplicant_set_suites() instead of using the already discovered information from scan results. In particular, this cleared the RSN supplicant state machine information of AP RSNE and resulted in having to fetch the scan results for the current BSS when processing EAPOL-Key msg 3/4. Fix this by recognizing the special case for OWE transition mode where the SSID for the associated AP does not actually match the SSID in the network profile. Signed-off-by: Jouni Malinen <j@w1.fi> --- wpa_supplicant/bss.h | 1 + wpa_supplicant/events.c | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/wpa_supplicant/bss.h b/wpa_supplicant/bss.h index 3ce8cd3f429a..0716761749f6 100644 --- a/wpa_supplicant/bss.h +++ b/wpa_supplicant/bss.h @@ -18,6 +18,7 @@ struct wpa_scan_res; #define WPA_BSS_AUTHENTICATED BIT(4) #define WPA_BSS_ASSOCIATED BIT(5) #define WPA_BSS_ANQP_FETCH_TRIED BIT(6) +#define WPA_BSS_OWE_TRANSITION BIT(7) struct wpa_bss_anqp_elem { struct dl_list list; diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c index 37ae306bbb9e..1149aa90bacd 100644 --- a/wpa_supplicant/events.c +++ b/wpa_supplicant/events.c @@ -188,6 +188,16 @@ static int wpa_supplicant_select_config(struct wpa_supplicant *wpa_s) drv_ssid_len) == 0) return 0; /* current profile still in use */ +#ifdef CONFIG_OWE + if ((wpa_s->current_ssid->key_mgmt & WPA_KEY_MGMT_OWE) && + wpa_s->current_bss && + (wpa_s->current_bss->flags & WPA_BSS_OWE_TRANSITION) && + drv_ssid_len == wpa_s->current_bss->ssid_len && + os_memcmp(drv_ssid, wpa_s->current_bss->ssid, + drv_ssid_len) == 0) + return 0; /* current profile still in use */ +#endif /* CONFIG_OWE */ + wpa_msg(wpa_s, MSG_DEBUG, "Driver-initiated BSS selection changed the SSID to %s", wpa_ssid_txt(drv_ssid, drv_ssid_len)); @@ -1025,6 +1035,7 @@ static void owe_trans_ssid(struct wpa_supplicant *wpa_s, struct wpa_bss *bss, wpa_ssid_txt(pos, ssid_len)); os_memcpy(bss->ssid, pos, ssid_len); bss->ssid_len = ssid_len; + bss->flags |= WPA_BSS_OWE_TRANSITION; #endif /* CONFIG_OWE */ }
>> I'm pretty sure the intent for ap_ies_from_associnfo is to make sure we only >> store ie data from a beacon in the state machine, to have them later available >> in wpa_supplicant_validate_ie() to be verified against the IE from eapol#3. >> wpa_supplicant_validate_ie() is by the way also assuming that the variables >> are set and for some reason is not triggering a RSN mismatch when ap_wpa_ie or >> ap_rsn_ie the state machine is NULL. (It's done for ap_rsnxe.) We may want to >> fix that, too... I'll send you an alternate patch for this one including that, >> too. But that may trigger disconnects with some broken APs and is a bit more >> risky. > > The case of Beacon frame no including an RSNE sounds quite unlikely to > be able to get to a state where RSN 4-way handshake is used since the > RSNE from Beacon (or Probe Response) frame is the part that's used to > negotiated use of RSN in the first place.. > I was more thinking about an attack: Utilize the fact that wpa_supplicant can be tricked into clearing the variables and clear the way to a downgrade attack. (I don't see a real attack vector, just a way to bypass some of the RSN checks for OWE.) >> With the code as it is today we just have some suboptimal cases we can see in >> the following tests: >> owe_transition_mode >> owe_transition_mode_connect_cmd >> owe_transition_mode_multi_bss >> owe_transition_mode_open_multiple_scans >> >> Looking at owe_transition_mode.log0 from a test run we see the following order >> of events: (I generously removed lines and just kept enough to see the picture. >> The full logfile is available here: https://www.awhome.eu/index.php/s/jDQfpZmDGwsLPXA >> and named cf28cfc12-orig-owe_transition_mode.log0) > > That location does not seem to be available anymore, but anyway, I think > I understood most of it with one unclear, but not really directly > related, item. > Yea, sorry. The share expired after one month. I shared it again: https://www.awhome.eu/index.php/s/fataWRskGzoqmFN >> 321 wlan0: Request association with 02:00:00:00:03:00 >> 337 wlan0: WPA: Selected cipher suites: group 16 pairwise 16 key_mgmt 4194304 proto 2 >> 339 wlan0: WPA: clearing AP WPA IE >> --- here we set the variable >> 340 WPA: set AP RSN IE - hexdump(len=22): 30 14 01 00 00 0f ac 04 01 00 00 0f ac 04 01 00 00 0f ac 12 cc 00 >> 341 wlan0: WPA: clearing AP RSNXE > > This is expected since wpa_supplicant has the updated BSS for the OWE > BSS available at this point. > >> 508 wlan0: Associated to a new BSS: BSSID=02:00:00:00:03:00 >> 511 wlan0: Network configuration found for the current AP > > That code path should not be entered at all.. The key debug entry that > shows the incorrect behavior here is this one: > Driver-initiated BSS selection changed the SSID to %s > >> 512 wlan0: WPA: Using WPA IE from AssocReq to set cipher suites >> 513 wlan0: WPA: Selected cipher suites: group 16 pairwise 16 key_mgmt 4194304 proto 2 >> --- here are the variables cleared without any good reason --- >> 515 wlan0: WPA: clearing AP WPA IE >> 516 wlan0: WPA: clearing AP RSN IE >> 517 wlan0: WPA: clearing AP RSNXE > > Yes, none of this was supposed to happen since > wpa_supplicant_set_suites() was not supposed to be called. > >> 631 wlan0: WPA: RX message 3 of 4-Way Handshake from 02:00:00:00:03:00 (ver=0) >> --- here we need the deleted data again and trigger a scan the set the variables again --- >> 636 wlan0: WPA: No WPA/RSN IE for this AP known. Trying to get from scan results > > This does not trigger a new scan; this is only fetching the current scan > results from cfg80211. > >> 637 nl80211: Received scan results (3 BSSes) >> 638 nl80211: Scan results indicate BSS status with 02:00:00:00:03:00 as associated >> --- but we don't get them and as a result bypass the (also broken?) IE mismatch check >> 647 wlan0: WPA: Could not find AP from the scan results > > That looks strange and is not something I see in my tests. That scan > result has to be there in both wpa_supplicant and cfg80211 for the Strange that it looks different in your test runs... I used the git version cf28cfc12 to create the log and still have the full run on the disc. But I just tried it with the near-current version bdb2eaf87 and my normal kernel 5.5.7-gentoo kernel and it still was there for me. No patches applied at all to hostapd or mac80211/nl80211, only patched iwlwifi which is not even loaded during the test runs. That said it's of course gone when I try ecb5219d8 where the RSN variables are no longer cleared. > association to be started.. In practice, you should have seen this > before association: > nl80211: Trigger single channel scan to refresh cfg80211 BSS entry > wlan0: nl80211: scan request > nl80211: Scan SSID owe-random > ... > wlan0: nl80211: New scan results available > nl80211: Scan results for missing cfg80211 BSS entry > yes, I see that in the old log. > And now the entry for 02:00:00:00:03:00 with the correct (previously > hidden) SSID is in place in cfg80211 (and it was already there with the > correct SSID in wpa_supplicant BSS table). > >>>> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c >>>> @@ -1378,7 +1378,7 @@ int wpa_supplicant_set_suites(struct wpa_supplicant *wpa_s, >>>> wpa_sm_set_param(wpa_s->wpa, WPA_PARAM_RSN_ENABLED, >>>> !!(ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN))); >>>> >>>> - if (bss || !wpa_s->ap_ies_from_associnfo) { >>>> + if (bss && !wpa_s->ap_ies_from_associnfo) { >>>> if (wpa_sm_set_ap_wpa_ie(wpa_s->wpa, bss_wpa, >>>> bss_wpa ? 2 + bss_wpa[1] : 0) || > > So the reason I did not understand this (and well, still don't think > this is correct) is that this function should never have been called in > the first place. > >> I stopped digging into that after discovering that the bss is an optional >> argument. But as explained above we must not update the state machine variables >> with IEs from association requests. So I assume that was the intent of the code >> and we just use the wrong logical operation for that. Which then deletes the >> variables. >> It could be that the correct fix is to make the bss mandatory or at least never >> call wpa_supplicant_set_suites() when one of the variables are set... >> But that's not my understanding of the code. > > Not calling wpa_supplicant_set_suites() is indeed the appropriate fix > for this issue with OWE transition mode. I did not dig deeper into that and you know the code much better than me so I assume the issue is now fixed. But what function has then ap_ies_from_associnfo? I assumed that the intend was to make sure only beacon RSN can be stored, so we have the beacon data handy when needed. As it is I could not find any obvious protection to not update the variables with data taken from non-beacon frames, risking that when we try to compare e.g. an eapol RSN against a beacon we are using non-beacon data. Alexander
On Sat, Mar 07, 2020 at 08:52:50PM +0100, Alexander Wetzel wrote: > I was more thinking about an attack: Utilize the fact that wpa_supplicant > can be tricked into clearing the variables and clear the way to a downgrade > attack. (I don't see a real attack vector, just a way to bypass some of the > RSN checks for OWE.) Clearing the AP's RSNE/RSNXE during association event handling should not really bypass such checks since the same information is available from scan results that can be fetched again when going through 4-way handshake. The expectation here is that even if the driver were to be doing BSS and network selection, it have to make the scan results available for the current BSS after having completed association. This OWE transition mode case was just a special corner case for that which happened to have the same issue with changing SSID in two places. > Yea, sorry. The share expired after one month. I shared it again: > https://www.awhome.eu/index.php/s/fataWRskGzoqmFN Thanks. It turns out that there was indeed two separate issues with the exact same reason behind them, i.e., both two possible paths for fetching the AP's RSNE/RSNXE information from Beacon/Probe Response frames failed when the SSID changed in the way it does in OWE transition mode (but not in any other case). > > > 637 nl80211: Received scan results (3 BSSes) > > > 638 nl80211: Scan results indicate BSS status with 02:00:00:00:03:00 as associated > > > --- but we don't get them and as a result bypass the (also broken?) IE mismatch check > > > 647 wlan0: WPA: Could not find AP from the scan results > > > > That looks strange and is not something I see in my tests. That scan > > result has to be there in both wpa_supplicant and cfg80211 for the > > Strange that it looks different in your test runs... Actually, I do see it now that I compared this against your full log. Those "missing" lines between those log entries above made me not notice this in my own logs. > I did not dig deeper into that and you know the code much better than me so > I assume the issue is now fixed. For the cfg80211-based drivers that use wpa_supplicant for selecting the network, yes, but for theoretical other cases maybe not (not that I'm aware of any such theoretical other case with support for OWE transition mode existing today).. > But what function has then ap_ies_from_associnfo? I assumed that the intend > was to make sure only beacon RSN can be stored, so we have the beacon data > handy when needed. It is used by other driver interfaces, e.g., NDIS on Windows, where the association event carries Beacon/Probe Response frame IEs for the AP. That functionality is not there in nl80211 (at least in the current version). > As it is I could not find any obvious protection to not update the variables > with data taken from non-beacon frames, risking that when we try to compare > e.g. an eapol RSN against a beacon we are using non-beacon data. I'm not sure what this "non-beacon frames" is referring to. The AP IEs are set based on scan results, i.e., Beacon and Probe Response frames, in all these different cases. And as far as the other issue with the scan results not being available when processing msg 3/4 is concerned, this takes care of that: [PATCH] OWE: Allow BSS entry with different SSID to be used in transition mode Similarly to the wpa_supplicant_select_config() case, wpa_get_beacon_ie() needs to handle the special case for OWE transition mode where the SSID in the network profile does not match the SSID of the OWE BSS (that has a hidden, random SSID). Accept such a BSS in case the current scan results needs to be fetched for verifying EAPOL-Key msg 3/4 IEs. Signed-off-by: Jouni Malinen <j@w1.fi> --- wpa_supplicant/wpas_glue.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/wpa_supplicant/wpas_glue.c b/wpa_supplicant/wpas_glue.c index 7162f8f1fe7c..39b05b2b902a 100644 --- a/wpa_supplicant/wpas_glue.c +++ b/wpa_supplicant/wpas_glue.c @@ -398,6 +398,13 @@ static int wpa_get_beacon_ie(struct wpa_supplicant *wpa_s) curr = bss; break; } +#ifdef CONFIG_OWE + if (ssid && (ssid->key_mgmt & WPA_KEY_MGMT_OWE) && + (bss->flags & WPA_BSS_OWE_TRANSITION)) { + curr = bss; + break; + } +#endif /* CONFIG_OWE */ } if (curr) {
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 0fee3c951..7c2d55880 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -1378,7 +1378,7 @@ int wpa_supplicant_set_suites(struct wpa_supplicant *wpa_s, wpa_sm_set_param(wpa_s->wpa, WPA_PARAM_RSN_ENABLED, !!(ssid->proto & (WPA_PROTO_RSN | WPA_PROTO_OSEN))); - if (bss || !wpa_s->ap_ies_from_associnfo) { + if (bss && !wpa_s->ap_ies_from_associnfo) { if (wpa_sm_set_ap_wpa_ie(wpa_s->wpa, bss_wpa, bss_wpa ? 2 + bss_wpa[1] : 0) || wpa_sm_set_ap_rsn_ie(wpa_s->wpa, bss_rsn,
wpa_supplicant_set_suites() can be called without providing a scan result. Don't flush ie elements in this case. Signed-off-by: Alexander Wetzel <alexander@wetzel-home.de> --- wpa_supplicant/wpa_supplicant.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)