[PATCHv2] DPP: prevent processing dpp action frames when stopped

[Michal Kazior]

On Sat, Mar 6, 2021 at 12:17 PM Jouni Malinen <j at w1.fi> wrote:
>
> On Wed, Mar 03, 2021 at 10:26:05AM +0000, Michal Kazior wrote:
> > diff --git a/src/ap/dpp_hostapd.c b/src/ap/dpp_hostapd.c
> > @@ -706,6 +706,7 @@ int hostapd_dpp_listen(struct hostapd_data *hapd, const char *cmd)
> >
> >  void hostapd_dpp_listen_stop(struct hostapd_data *hapd)
> >  {
> > +     hapd->dpp_allowed_roles = 0;
> >       hostapd_drv_dpp_listen(hapd, false);
> >       /* TODO: Stop listen operation on non-operating channel */
> >  }
>
> This breaks chirping mode since hostapd_dpp_chirp_next() calls
> hostapd_dpp_listen_stop() in the middle of the iteration. I don't think
> this approach in hostapd_dpp_listen_stop() changing dpp_allowed_roles is
> going to be acceptable.

Right, good point!


> I'm not sure I fully understood your use case, but please note that the
> DPP_STOP_LISTEN command is also calling hostapd_dpp_stop(). Would that
> function be more appropriate place for clearing dpp_allowed_roles?

Yes, hooking this up to hostapd_dpp_stop() should work for me.


> Or
> well, I'm not sure clearing dpp_allowed_roles is really the correct
> thing to do to address this issue, i.e., it might be safer to provide a
> different variable for tracking whether there should be any action
> taken based on received DPP Action frames.

As is hostapd allows itself to be configured in a multi-BSS case in a
way where you have race. An interface with an empty
dpp_configurator_params can get to respond to dpp auth or chirp first.
When that happens dpp auth eventually fails because it has no clue
what to do.

You could argue the system integrator calling hostapd cli should make
sure to configure dpp_configurator_params on BSSes interfaces all the
same.. DPP responder isn't all that much tied to a single BSS in the
first place anyway, right? It's giving out complete credentials
(ssid+whatever-akm-data) to whoever it ends up talking to. But then
why have dpp_configurator_params per BSS in the first place? And even
if there is a case, I fail to see how it could work reliably given the
race condition.

We could make dpp_configurator_params global, but this would break
prior expectations of the API. Given your previous remark on the
dpp_auth_init() you're probably not too keen on this approach, no?

I think I'll just try filling in dpp_configurator_params on all BSSes
hostapd is managing and see if this works for me.


Michał