[PATCH v2] Reject auths during explicit roam requests

[Matthew Wang]

Thanks Ben, the patch is a good idea, but doesn't exactly address my
issue. The D-Bus/wpa_cli roam command sets the reassociate bit, and
this short circuits the roam logic in wpa_supplicant_need_to_roam (it
just returns true at the top of the function instead of going into
need_to_roam_within_ess).

On Mon, Mar 1, 2021 at 3:58 PM Ben Greear <greearb at candelatech.com> wrote:
>
> On 3/1/21 3:34 PM, Matthew Wang wrote:
> > The roam D-Bus and wpa_cli commands flip the reassociate bit before
> > calling wpa_supplicant_connect. wpa_supplicant connect eventually aborts
> > ongoing scans, which causes scan results to be reported. Since the
> > reassociate bit is set, this will trigger a connection attempt based on
> > the aborted scan's scan results and cancel the initial connetion
> > request. This often causes wpa_supplicant to reassociate to the same AP
> > it is currently associated to.
> >
> > Add a roam_in_progress flag to indicate that we're currently attempting
> > to roam via D-Bus so that we don't initate another connection attempt.
>
> I had similar issues, in case it helps, here is my patch:
>
> https://github.com/greearb/hostap-ct/commit/50b1c17d0a782dc4d354ac7366500763611ea5af
>
> I first relaxed thresholds by default, and then allowed configuring them.  When something
> beyond hostapd is controlling roaming, I set the thresholds so that it would never automatically
> roam based on scan results.
>
> Lord git only knows how much of this was built on previous patches in my tree...
>
> Thanks,
> Ben
>
>
> >
> > Signed-off-by: Matthew Wang <matthewmwang at chromium.org>
> > Series-to: hostap at lists.infradead.org
> > Series-cc: j at w1.fi, matthewmwang at chromium.org
> > Change-Id: Ibeb6cceddabd10cab6938411f54ed63f4d4428c1
> > ---
> >   wpa_supplicant/ctrl_iface.c             | 9 +++++++++
> >   wpa_supplicant/dbus/dbus_new_handlers.c | 9 +++++++++
> >   wpa_supplicant/sme.c                    | 7 +++++++
> >   wpa_supplicant/wpa_supplicant_i.h       | 1 +
> >   4 files changed, 26 insertions(+)
> >
> > diff --git a/wpa_supplicant/ctrl_iface.c b/wpa_supplicant/ctrl_iface.c
> > index 0d4c2bc8f0b..048fe72d472 100644
> > --- a/wpa_supplicant/ctrl_iface.c
> > +++ b/wpa_supplicant/ctrl_iface.c
> > @@ -5686,6 +5686,15 @@ static int wpa_supplicant_ctrl_iface_roam(struct wpa_supplicant *wpa_s,
> >       wpa_s->reassociate = 1;
> >       wpa_supplicant_connect(wpa_s, bss, ssid);
> >
> > +     /*
> > +      * Indicate that an explicitly requested roam is in progress so scan
> > +      * results that come in before the 'sme-connect' radio work gets
> > +      * executed do not override the original connection attempt.
> > +      */
> > +     if (radio_work_pending(wpa_s, "sme-connect")) {
> > +             wpa_s->roam_in_progress = true;
> > +     }
> > +
> >       return 0;
> >   #endif /* CONFIG_NO_SCAN_PROCESSING */
> >   }
> > diff --git a/wpa_supplicant/dbus/dbus_new_handlers.c b/wpa_supplicant/dbus/dbus_new_handlers.c
> > index 7d20f2123a8..6803558accd 100644
> > --- a/wpa_supplicant/dbus/dbus_new_handlers.c
> > +++ b/wpa_supplicant/dbus/dbus_new_handlers.c
> > @@ -2005,6 +2005,15 @@ DBusMessage * wpas_dbus_handler_roam(DBusMessage *message,
> >       wpa_s->reassociate = 1;
> >       wpa_supplicant_connect(wpa_s, bss, ssid);
> >
> > +     /*
> > +      * Indicate that an explicitly requested roam is in progress so scan
> > +      * results that come in before the 'sme-connect' radio work gets
> > +      * executed do not override the original connection attempt.
> > +      */
> > +     if (radio_work_pending(wpa_s, "sme-connect")) {
> > +             wpa_s->roam_in_progress = true;
> > +     }
> > +
> >       return NULL;
> >   #endif /* CONFIG_NO_SCAN_PROCESSING */
> >   }
> > diff --git a/wpa_supplicant/sme.c b/wpa_supplicant/sme.c
> > index 522c8297f7e..dde80863a98 100644
> > --- a/wpa_supplicant/sme.c
> > +++ b/wpa_supplicant/sme.c
> > @@ -941,6 +941,8 @@ static void sme_auth_start_cb(struct wpa_radio_work *work, int deinit)
> >       struct wpa_connect_work *cwork = work->ctx;
> >       struct wpa_supplicant *wpa_s = work->wpa_s;
> >
> > +     wpa_s->roam_in_progress = false;
> > +
> >       if (deinit) {
> >               if (work->started)
> >                       wpa_s->connect_work = NULL;
> > @@ -981,6 +983,11 @@ void sme_authenticate(struct wpa_supplicant *wpa_s,
> >               return;
> >       }
> >
> > +     if (wpa_s->roam_in_progress) {
> > +             wpa_dbg(wpa_s, MSG_DEBUG,
> > +                     "SME: Reject sme_authenticate() in favor of explicit roam request");
> > +             return;
> > +     }
> >       if (radio_work_pending(wpa_s, "sme-connect")) {
> >               /*
> >                * The previous sme-connect work might no longer be valid due to
> > diff --git a/wpa_supplicant/wpa_supplicant_i.h b/wpa_supplicant/wpa_supplicant_i.h
> > index 7d14b627d89..b6c339ab711 100644
> > --- a/wpa_supplicant/wpa_supplicant_i.h
> > +++ b/wpa_supplicant/wpa_supplicant_i.h
> > @@ -621,6 +621,7 @@ struct wpa_supplicant {
> >       u8 pending_bssid[ETH_ALEN]; /* If wpa_state == WPA_ASSOCIATING, this
> >                                    * field contains the target BSSID. */
> >       int reassociate; /* reassociation requested */
> > +     bool roam_in_progress; /* roam in progress */
> >       unsigned int reassoc_same_bss:1; /* reassociating to the same BSS */
> >       unsigned int reassoc_same_ess:1; /* reassociating to the same ESS */
> >       int disconnected; /* all connections disabled; i.e., do no reassociate
> >
>
>
> --
> Ben Greear <greearb at candelatech.com>
> Candela Technologies Inc  http://www.candelatech.com