diff mbox

WPS: Exclude missing UUID-E from PBC overlap detection

Message ID alpine.DEB.2.02.1505141501410.4508@mzhang-6430-lnx
State Changes Requested
Headers show

Commit Message

Min Zhang May 14, 2015, 10:02 p.m. UTC
Missing UUID-E shouldn't result in PBC overlap. An AP selected as
PBC registrar can Beacon w/o UUID-E attr as not required, but only
include unique UUID-E in its Probe Resp. The Andriod PBC scan seemly
just uses Beacon, so the NULL uuid check results in a false overlap.

Also commit 44cd430f877a558744def067ab38c4f9e4b283fc "Fixed PBC
overlap detection to handle case of missing UUID-E" tried but still
ended up PBC overlap on missing UUID-E.

The patch effectively exclude Beacon Frame from PBC overlap
detection. It is inline with WPS PBC overlap spec that only mentioned
Probe not Beacon. Another way is let PBC scan uses Probe Resp so it
can extract the actual UUID-E to compare.

Note the WPS spec also only suggested Selected Registrar as PBC
overlap detection. Andriod added UUID-E matching on top which I think
it is better since it allows unrelated PBC overlap.

Signed-off-by: Min Zhang <mzhang@roku.com>
---
  wpa_supplicant/wps_supplicant.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jouni Malinen May 27, 2015, 9:16 a.m. UTC | #1
On Thu, May 14, 2015 at 03:02:42PM -0700, Min Zhang wrote:
> Missing UUID-E shouldn't result in PBC overlap. An AP selected as
> PBC registrar can Beacon w/o UUID-E attr as not required, but only
> include unique UUID-E in its Probe Resp. The Andriod PBC scan seemly
> just uses Beacon, so the NULL uuid check results in a false overlap.

UUID-E is required to be in the Beacon frame if the AP has multiple
radios and is in active PBC mode.

> Also commit 44cd430f877a558744def067ab38c4f9e4b283fc "Fixed PBC
> overlap detection to handle case of missing UUID-E" tried but still
> ended up PBC overlap on missing UUID-E.

By design.. In case of two single-band APs in active PBC mode, neither
would include UUID-E in Beacon frames and that case would still be a PBC
session overlap.

Please note that commit 4a393fe9f8191109a715fb1d4916fbf852530125 ('WPS:
Do not indicate PBC overlap for the same BSS') addresses some corner
cases in this area.

> The patch effectively exclude Beacon Frame from PBC overlap
> detection. It is inline with WPS PBC overlap spec that only mentioned
> Probe not Beacon. Another way is let PBC scan uses Probe Resp so it
> can extract the actual UUID-E to compare.

This breaks PBC overlap detection for passive scanning as far as I can
tell.

> diff --git a/wpa_supplicant/wps_supplicant.c b/wpa_supplicant/wps_supplicant.c
> @@ -1697,7 +1697,7 @@ int wpas_wps_scan_pbc_overlap(struct wpa_supplicant *wpa_s,
> -		if (sel_uuid == NULL || uuid == NULL ||
> +		if (sel_uuid != NULL && uuid != NULL &&
>  		    os_memcmp(sel_uuid, uuid, UUID_LEN) != 0) {
>  			ret = 1; /* PBC overlap */

I don't think I can apply this since this would disable PBC overlap
detection for cases where passive scanning (on just seeing a Beacon
frame during Active scan) is used to find APs that operate only on a
single band.

If you think that there is a case where the current implementation
results in invalid PBC overlap detection, please provide detailed debug
logs showing such a case.
diff mbox

Patch

diff --git a/wpa_supplicant/wps_supplicant.c b/wpa_supplicant/wps_supplicant.c
index 40a5c69..a2162a4 100644
--- a/wpa_supplicant/wps_supplicant.c
+++ b/wpa_supplicant/wps_supplicant.c
@@ -1697,7 +1697,7 @@  int wpas_wps_scan_pbc_overlap(struct wpa_supplicant *wpa_s,
  		uuid = wps_get_uuid_e(ie);
  		wpa_hexdump(MSG_DEBUG, "WPS: UUID of the other BSS",
  			    uuid, UUID_LEN);
-		if (sel_uuid == NULL || uuid == NULL ||
+		if (sel_uuid != NULL && uuid != NULL &&
  		    os_memcmp(sel_uuid, uuid, UUID_LEN) != 0) {
  			ret = 1; /* PBC overlap */
  			wpa_msg(wpa_s, MSG_INFO, "WPS: PBC overlap detected: "