diff mbox

Do not bind WDS station to VLAN on reconnect

Message ID 20160301142810.2e256f11f41d400ad02655ea@ubnt.com
State Changes Requested
Headers show

Commit Message

Dmitry Ivanov March 1, 2016, 12:28 p.m. UTC
Currently, doing so leads to connectivity loss when WDS station
reconnects without sending deauthentication request first (for example,
on power cycle).

Signed-off-by: Dmitry Ivanov <dima@ubnt.com>
---
 src/ap/ieee802_11.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Jouni Malinen March 4, 2016, 7:13 p.m. UTC | #1
On Tue, Mar 01, 2016 at 02:28:10PM +0200, Dmitry Ivanov wrote:
> Currently, doing so leads to connectivity loss when WDS station
> reconnects without sending deauthentication request first (for example,
> on power cycle).

Shouldn't that be fixed instead? Or am I missing something here? It does
not look desirable to skip VLAN binding in any case.. Or are you saying
that the STA is already bound to a specific VLAN and skipping this
operation does not actually break anything for VLAN binding in practice?

I don't really want to apply this without more detailed description of
the issue and how this fixes it without causing security issues.
Dmitry Ivanov March 15, 2016, 3:15 p.m. UTC | #2
WDS station is bound to specific interface such as wlan0.sta1. Current
code rebinds it back to to wlan0 in case of reassociation. Because of
this, traffic is not flowing. My patch fixes this problem.

On Fri, Mar 4, 2016 at 9:13 PM, Jouni Malinen <j@w1.fi> wrote:
> On Tue, Mar 01, 2016 at 02:28:10PM +0200, Dmitry Ivanov wrote:
>> Currently, doing so leads to connectivity loss when WDS station
>> reconnects without sending deauthentication request first (for example,
>> on power cycle).
>
> Shouldn't that be fixed instead? Or am I missing something here? It does
> not look desirable to skip VLAN binding in any case.. Or are you saying
> that the STA is already bound to a specific VLAN and skipping this
> operation does not actually break anything for VLAN binding in practice?
>
> I don't really want to apply this without more detailed description of
> the issue and how this fixes it without causing security issues.
>
> --
> Jouni Malinen                                            PGP id EFC895FA
michael-dev March 16, 2016, 4:52 p.m. UTC | #3
Am 15.03.2016 um 16:15 schrieb Dmitrijs Ivanovs:
> WDS station is bound to specific interface such as wlan0.sta1. Current
> code rebinds it back to to wlan0 in case of reassociation. Because of
> this, traffic is not flowing. My patch fixes this problem.

If this code rebinds the station to wlan0, sta->vlan_id is not properly
set. As the station is currently bound to wlan0.sta1, sta->vlan_id was
once properly set and then got cleared (sometime after ap_sta_bind_vlan
was called). Clearing sta->vlan_id does not immediately populate into
the station being bound to wlan0 - this is put into effect by
ap_sta_bind_vlan.

So the fix should be about setting sta->vlan_id properly.

Currently sta->vlan_id is set by ap_sta_set_vlan during 802.11 auth
(e.g. for using macaddr_acl=2), during 802.1x (RADIUS Access-Accept) and
when authenticating using PMKSA cache. These code paths also clear
sta->vlan_id for example if RADIUS Access-Accept does not include a VLAN
information.

Regards,
M. Braun
diff mbox

Patch

diff --git a/src/ap/ieee802_11.c b/src/ap/ieee802_11.c
index b36e68d..eecb894 100644
--- a/src/ap/ieee802_11.c
+++ b/src/ap/ieee802_11.c
@@ -2758,9 +2758,7 @@  static void handle_assoc_cb(struct hostapd_data *hapd,
 					  sta->aid, 1);
 		if (!ret)
 			hostapd_set_wds_encryption(hapd, sta, ifname_wds);
-	}
-
-	if (sta->eapol_sm == NULL) {
+	} else if (sta->eapol_sm == NULL) {
 		/*
 		 * This STA does not use RADIUS server for EAP authentication,
 		 * so bind it to the selected VLAN interface now, since the