On א', 2016-03-20 at 22:05 +0200, Jouni Malinen wrote:
> 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?
>
That's coming from the HS20 rel. 2 specification, section 6.5 that states
"Once the mobile device has associated to an HS2.0 network and obtained an IP
address, the mobile device ..."
> > 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?
>
This is a bug. Will fix.
> > 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.
>
The motivation was to adhere to the "IP assignment" requirement. I do not think
that this would be a real issue as the port is not yet authorized, so lower driver
should not pass any IP packets anyway.
> > @@ -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.
>
I'll move the clearing of the flags as you suggested, and also clear these settings
on interface de-init. Do you still want the setting of the flags to be earlier although
it does not adhere to the "IP assignment" requirement?
Thanks,
Ilan.
_______________________________________________
Hostap mailing list
Hostap@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/hostap
