Message ID | alpine.DEB.2.02.1505141501410.4508@mzhang-6430-lnx |
---|---|
State | Changes Requested |
Headers | show |
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 --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: "
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(-)