On Tue, Mar 08, 2016 at 01:25:41PM +0200, Ilan Peer wrote:
> When a station has associated to a HS2.0 network, request
> the driver to do the following, based on the BSS capabilities:
>
> 1. Enable gratuitous ARP filtering
> 2. Enable unsolicited Neighbor Advertisement filtering
> 3. Enable GTK filtering if DGAF disabled bit is zero
>
> HS2.0 requires the filtering to start only after an IP
> address is obtained. However, the configuration is done
> regardless of obtaining an IP address, so this might
> not be good enough.
That "and obtained an IP address" part looks a bit strange.. Why would
this filtering not be enabled before IP address assignment completes?
> diff --git a/wpa_supplicant/hs20_supplicant.c b/wpa_supplicant/hs20_supplicant.c
> +void hs20_configure_frame_filters(struct wpa_supplicant *wpa_s)
> + if (filter)
> + wpa_drv_configure_frame_filters(wpa_s, filter);
Why is this call skipped for filter == 0? Wouldn't it be fine and
potentially even preferred to clear the setting here if for any reason
the driver setting was not cleared previously?
> diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
> @@ -751,6 +751,9 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
> wpas_connect_work_done(wpa_s);
> /* Reinitialize normal_scan counter */
> wpa_s->normal_scans = 0;
> +#ifdef CONFIG_HS20
> + hs20_configure_frame_filters(wpa_s);
> +#endif /* CONFIG_HS20 */
While this might be closer to the "once .. obtained an IP address",
wouldn't it be more robust to set these filter before even starting the
association? There might be a race condition here (even if very unlikely
to be hit) of IP packets getting exchanged before this call is executed.
> @@ -816,8 +819,15 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
> if (state == WPA_COMPLETED)
> wpa_supplicant_start_bgscan(wpa_s);
> - else if (state < WPA_ASSOCIATED)
> + else if (state < WPA_ASSOCIATED) {
> wpa_supplicant_stop_bgscan(wpa_s);
> +
> +#ifdef CONFIG_HS20
> + /* clear possibly configured frame filters */
> + if (old_state == WPA_COMPLETED)
> + wpa_drv_configure_frame_filters(wpa_s, 0);
> +#endif /* CONFIG_HS20 */
> + }
What is this trying to do by requiring state < WPA_ASSOCIATED and
old_state == WPA_COMPLETED? What if the STA gets disconnected when going
through rekeying (state is WPA_4WAY_HANDSHAKE or WPA_GROUP_HANDSHAKE)?
I'm not sure wpa_suppliant_set_state() is really the best location for
either of these frame filter calls. In other words, these sound more
like items that should be done when starting an association and when
processing a disassociation. In addition, there should likely be code to
clear the configured values when deinitializing the interface.
--
Jouni Malinen PGP id EFC895FA
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
