| Message ID | 20240428131344.334314-4-mail@david-bauer.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | wpa_supplicant: optimize OWE roaming behavior | expand |
On Sun, Apr 28, 2024 at 03:13:42PM +0200, David Bauer wrote: > When adding these networks hidden, they get re-added for the same BSSID > when scanning for the transition-SSID. Skip adding the OWE-SSIDs in case > the SSID was not explicitly scanned for. This breaks multiple hwsim test cases for OWE since the expected BSS entry is not available. While I think I understand what this is trying to do, it is not really acceptable to break test cases, so I cannot apply this as-is. At minimum, those test cases would need to be modified, but I'm not really sure this is actually correct behavior since BSSs with a hidden SSID have been added to the scan results in all existing cases for years. > + /* Don't add hidden OWE transition networks with RSN. They are explicitly scanned for. */ > + rsn = wpa_scan_get_ie(res, WLAN_EID_RSN); > + owe = wpa_scan_get_vendor_ie(res, OWE_IE_VENDOR_TYPE); > + if (owe && rsn && (ssid[1] == 0 || ssid[2] == 0)) > + return; And this should likely be within #ifdef CONFIG_OWE even if that were to compile successfully without such conditional check.
Hi Jouni, thanks for your review. On 8/1/24 17:13, Jouni Malinen wrote: > On Sun, Apr 28, 2024 at 03:13:42PM +0200, David Bauer wrote: >> When adding these networks hidden, they get re-added for the same BSSID >> when scanning for the transition-SSID. Skip adding the OWE-SSIDs in case >> the SSID was not explicitly scanned for. > > This breaks multiple hwsim test cases for OWE since the expected BSS > entry is not available. While I think I understand what this is trying > to do, it is not really acceptable to break test cases, so I cannot > apply this as-is. At minimum, those test cases would need to be > modified, but I'm not really sure this is actually correct behavior > since BSSs with a hidden SSID have been added to the scan results in all > existing cases for years. Understood. How about replacing hidden scan entries when we discover the SSID name upon trying to connect to the transition network? I hope it is understandable what i mean. Currently the scan-list is flooded with multiple entries for a single BSSID. As all roaming code just takes the first one upon iterating, this effectively breaks roaming at the moment. Best David > >> + /* Don't add hidden OWE transition networks with RSN. They are explicitly scanned for. */ >> + rsn = wpa_scan_get_ie(res, WLAN_EID_RSN); >> + owe = wpa_scan_get_vendor_ie(res, OWE_IE_VENDOR_TYPE); >> + if (owe && rsn && (ssid[1] == 0 || ssid[2] == 0)) >> + return; > > And this should likely be within #ifdef CONFIG_OWE even if that were to > compile successfully without such conditional check. >
diff --git a/wpa_supplicant/bss.c b/wpa_supplicant/bss.c index e528af280..b9bf06c45 100644 --- a/wpa_supplicant/bss.c +++ b/wpa_supplicant/bss.c @@ -909,7 +909,7 @@ void wpa_bss_update_scan_res(struct wpa_supplicant *wpa_s, struct wpa_scan_res *res, struct os_reltime *fetch_time) { - const u8 *ssid, *p2p, *mesh; + const u8 *ssid, *p2p, *mesh, *owe, *rsn; struct wpa_bss *bss; if (wpa_s->conf->ignore_old_scan_res) { @@ -940,6 +940,12 @@ void wpa_bss_update_scan_res(struct wpa_supplicant *wpa_s, return; } + /* Don't add hidden OWE transition networks with RSN. They are explicitly scanned for. */ + rsn = wpa_scan_get_ie(res, WLAN_EID_RSN); + owe = wpa_scan_get_vendor_ie(res, OWE_IE_VENDOR_TYPE); + if (owe && rsn && (ssid[1] == 0 || ssid[2] == 0)) + return; + p2p = wpa_scan_get_vendor_ie(res, P2P_IE_VENDOR_TYPE); #ifdef CONFIG_P2P if (p2p == NULL &&
When adding these networks hidden, they get re-added for the same BSSID when scanning for the transition-SSID. Skip adding the OWE-SSIDs in case the SSID was not explicitly scanned for. Signed-off-by: David Bauer <mail@david-bauer.net> --- wpa_supplicant/bss.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)