diff mbox series

[3/3] DPP: prevent processing dpp action frames when stopped

Message ID 20210208153207.108755-3-kazikcz@gmail.com
State Changes Requested
Headers show
Series [1/3] wpa_supplicant: expose DPP config object AKM | expand

Commit Message

Michał Kazior Feb. 8, 2021, 3:32 p.m. UTC 
From: Michal Kazior <michal@plume.com>

DPP configurator params can be configured per
interface. DPP listening can also be started and
stopped per interface.

However DPP rx processing was always doomed to
process incomming frames even on interfaces which
were not explicitly started to do DPP listen. This
happened because not only the initial value but
also because the dpp_allowed_roles could never be
reset to 0.

This would result in random failures in
configuring Enrollees when running multiple AP
interfaces if some of these APs happened to not
have DPP configurator params set.

Signed-off-by: Michal Kazior <michal@plume.com>
---
 src/ap/dpp_hostapd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jouni Malinen Feb. 9, 2021, 6:53 p.m. UTC | #1 
On Mon, Feb 08, 2021 at 03:32:07PM +0000, Michal Kazior wrote:
> DPP configurator params can be configured per
> interface. DPP listening can also be started and
> stopped per interface.
> 
> However DPP rx processing was always doomed to
> process incomming frames even on interfaces which
> were not explicitly started to do DPP listen. This
> happened because not only the initial value but
> also because the dpp_allowed_roles could never be
> reset to 0.

This is not really the way the DPP_LISTEN was designed for hostapd
originally. The expectation was the AP side does not need an explicit
DPP_LISTEN command to receive DPP frames on the operating channel, i.e.,
DPP_LISTEN would only be needed to do an off-channel listen operation.
Only later extensions to being able to configure parameters and HW RX
filters brought in cases where the DPP_LISTEN command might be needed.

> This would result in random failures in
> configuring Enrollees when running multiple AP
> interfaces if some of these APs happened to not
> have DPP configurator params set.

Can you please provide more details on where you are seeing issues from
this? The Responder should ignore the incoming DPP messages if they are
not targeting it (have a match in the bootstrapping key).

> diff --git a/src/ap/dpp_hostapd.c b/src/ap/dpp_hostapd.c

>  void hostapd_dpp_listen_stop(struct hostapd_data *hapd)
>  {
> +	hapd->dpp_allowed_roles = 0;

This might be a reasonable thing to do on an explicit DPP_LISTEN_STOP
operation.

>  int hostapd_dpp_init(struct hostapd_data *hapd)
>  {
> -	hapd->dpp_allowed_roles = DPP_CAPAB_CONFIGURATOR | DPP_CAPAB_ENROLLEE;
> +	hapd->dpp_allowed_roles = 0;

But this is the part that I'm a bit worried about since it changes the
past expectation of not requiring an explicit DPP_LISTEN command in some
cases.
Michal Kazior Feb. 10, 2021, 9:43 a.m. UTC | #2 
On Tue, Feb 9, 2021 at 7:53 PM Jouni Malinen <j@w1.fi> wrote:
>
> On Mon, Feb 08, 2021 at 03:32:07PM +0000, Michal Kazior wrote:
> > DPP configurator params can be configured per
> > interface. DPP listening can also be started and
> > stopped per interface.
> >
> > However DPP rx processing was always doomed to
> > process incomming frames even on interfaces which
> > were not explicitly started to do DPP listen. This
> > happened because not only the initial value but
> > also because the dpp_allowed_roles could never be
> > reset to 0.
>
> This is not really the way the DPP_LISTEN was designed for hostapd
> originally. The expectation was the AP side does not need an explicit
> DPP_LISTEN command to receive DPP frames on the operating channel, i.e.,
> DPP_LISTEN would only be needed to do an off-channel listen operation.
> Only later extensions to being able to configure parameters and HW RX
> filters brought in cases where the DPP_LISTEN command might be needed.
>
> > This would result in random failures in
> > configuring Enrollees when running multiple AP
> > interfaces if some of these APs happened to not
> > have DPP configurator params set.
>
> Can you please provide more details on where you are seeing issues from
> this? The Responder should ignore the incoming DPP messages if they are
> not targeting it (have a match in the bootstrapping key).

When running global hostapd instances BIs are shared across all
interfaces. If some of them don't have dpp_configurator_params set
then it's up to the driver's frame event ordering which will determine
whether dpp configuration will be handed out properly or not. If the
problem is hit the configurator responds with status=5 to Enrollee.

For example I have wlan0+wlan02. Only wlan02 has the
dpp_configurator_params set. wlan0 does not. If wlan0 kicks off first
then status=5 is reported to Enrollee for ConfReq:

> $ awk '/Enrollee/ || /DPP-/ || /sa=/' /tmp/hostapd.log
> 1612949875.849099: PTKSA: Deinit. n_ptksa=0
> 1612949878.849732: nl80211: RX frame da=ff:ff:ff:ff:ff:ff sa=02:00:00:00:01:00 bssid=ff:ff:ff:ff:ff:ff freq=2437 ssi_signal=-50 fc=0xd0 seq_ctrl=0x0 stype=13 (WLAN_FC_STYPE_ACTION) len=222
> 1612949878.849734: wlan0: DPP-RX src=02:00:00:00:01:00 freq=2437 type=0
> 1612949878.849745: wlan0: DPP-TX dst=02:00:00:00:01:00 freq=2437 type=1
> 1612949878.849749: nl80211: RX frame da=ff:ff:ff:ff:ff:ff sa=02:00:00:00:01:00 bssid=ff:ff:ff:ff:ff:ff freq=2437 ssi_signal=-50 fc=0xd0 seq_ctrl=0x0 stype=13 (WLAN_FC_STYPE_ACTION) len=222
> 1612949878.849750: wlan02: DPP-RX src=02:00:00:00:01:00 freq=2437 type=0
> 1612949878.849769: wlan02: DPP-TX dst=02:00:00:00:01:00 freq=2437 type=1
> 1612949878.849774: wlan0: DPP-TX-STATUS dst=02:00:00:00:01:00 result=SUCCESS
> 1612949878.849776: nl80211: RX frame da=02:00:00:00:00:00 sa=02:00:00:00:01:00 bssid=ff:ff:ff:ff:ff:ff freq=2437 ssi_signal=-50 fc=0xd0 seq_ctrl=0x10 stype=13 (WLAN_FC_STYPE_ACTION) len=129
> 1612949878.849778: wlan0: DPP-RX src=02:00:00:00:01:00 freq=2437 type=2
> 1612949878.849785: wlan0: DPP-AUTH-SUCCESS init=0
> 1612949878.849789: nl80211: RX frame da=02:00:00:00:00:00 sa=02:00:00:00:01:00 bssid=ff:ff:ff:ff:ff:ff freq=2437 ssi_signal=-50 fc=0xd0 seq_ctrl=0x20 stype=13 (WLAN_FC_STYPE_ACTION) len=227
> 1612949878.849791: wlan0: DPP-CONF-REQ-RX src=02:00:00:00:01:00
> 1612949878.849792: DPP: Enrollee Nonce - hexdump(len=16): 0a 30 74 29 5e c2 69 a0 c5 01 6c ff 49 aa ce 06
> 1612949878.849793: DPP: Enrollee name = 'Test'
> 1612949878.849800: wlan0: DPP-BAND-SUPPORT 81,82,83,84,115,116,117,118,119,120,121,122,123,124,125,126,127,128,129,130
> 1612949878.849800: DPP: No configuration available for Enrollee(sta) - reject configuration request
> 1612949878.849804: wlan0: DPP-CONF-SENT
> 1612949878.849819: wlan02: DPP-TX-STATUS dst=02:00:00:00:01:00 result=SUCCESS
> 1612949879.850821: wlan02: DPP-FAIL No Auth Confirm received

This is on hwsim, but I'm seeing it on qca closed driver too.


> > diff --git a/src/ap/dpp_hostapd.c b/src/ap/dpp_hostapd.c
>
> >  void hostapd_dpp_listen_stop(struct hostapd_data *hapd)
> >  {
> > +     hapd->dpp_allowed_roles = 0;
>
> This might be a reasonable thing to do on an explicit DPP_LISTEN_STOP
> operation.
>
> >  int hostapd_dpp_init(struct hostapd_data *hapd)
> >  {
> > -     hapd->dpp_allowed_roles = DPP_CAPAB_CONFIGURATOR | DPP_CAPAB_ENROLLEE;
> > +     hapd->dpp_allowed_roles = 0;
>
> But this is the part that I'm a bit worried about since it changes the
> past expectation of not requiring an explicit DPP_LISTEN command in some
> cases.

I have no good reason to keep this except it kind of makes sense from
a consistency point of view. But I do understand you don't want to
break past expectations. We can drop this part.


Michał
diff mbox series

Patch

diff --git a/src/ap/dpp_hostapd.c b/src/ap/dpp_hostapd.c
index e106df513..a787666cf 100644
--- a/src/ap/dpp_hostapd.c
+++ b/src/ap/dpp_hostapd.c
@@ -704,6 +704,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 */
 }
@@ -2226,7 +2227,7 @@  static int hostapd_dpp_add_controllers(struct hostapd_data *hapd)
 
 int hostapd_dpp_init(struct hostapd_data *hapd)
 {
-	hapd->dpp_allowed_roles = DPP_CAPAB_CONFIGURATOR | DPP_CAPAB_ENROLLEE;
+	hapd->dpp_allowed_roles = 0;
 	hapd->dpp_init_done = 1;
 	return hostapd_dpp_add_controllers(hapd);
 }