Regression caused by commit df88129

Regression caused by commit df88129

[Larry Finger]

Jouni and Johannes,

A recent pull of the wireless-testing tree caused my wireless to break. The 
wireless devices can authenticate and associate as follows:

wlan0: authenticate with c0:3f:0e:be:2b:44
wlan0: capabilities/regulatory prevented using AP HT/VHT configuration, downgraded
wlan0: send auth to c0:3f:0e:be:2b:44 (try 1/3)
wlan0: authenticated
wlan0: associate with c0:3f:0e:be:2b:44 (try 1/3)
wlan0: RX AssocResp from c0:3f:0e:be:2b:44 (capab=0x411 status=0 aid=3)
wlan0: associated
IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
b43-phy0 debug: Using hardware based encryption for keyidx: 0, mac: 
c0:3f:0e:be:2b:44
b43-phy0 debug: Using hardware based encryption for keyidx: 1, mac: 
ff:ff:ff:ff:ff:ff
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

These dropped frame messages persist, and prevent the connection from being 
completed. The code in question is in routine ieee80211_subif_start_xmit() of 
file net/mac80211/tx.c as follows:

==============================================================
         if (unlikely(!ieee80211_vif_is_mesh(&sdata->vif) &&
                      !is_multicast_ether_addr(hdr.addr1) && !authorized &&
                      (cpu_to_be16(ethertype) != sdata->control_port_protocol ||
                       !ether_addr_equal(sdata->vif.addr, skb->data + ETH_ALEN)))) {
#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
                 net_info_ratelimited("%s: dropped frame to %pM (unauthorized 
port)\n",
                                     dev->name, hdr.addr1);
#endif

                 I802_DEBUG_INC(local->tx_handlers_drop_unauth_port);

                 goto fail_rcu;
         }
==============================================================

When the if statement is rewritten as 'if (A || B)', it turns out that A is 
true, and B is false.

The regression was bisected to commit df881293c6ba9a12868491a717b25cb14ec1fa4a.

==============================================================
commit df881293c6ba9a12868491a717b25cb14ec1fa4a
Author: Jouni Malinen <jouni@qca.qualcomm.com>
Date:   Thu Feb 14 21:10:54 2013 +0200

     cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station

     The information of the peer's capabilities is required for the driver
     to perform TDLS Peer UAPSD operations. This information of the peer is
     passed by the supplicant using NL80211_CMD_SET_STATION command. This
     commit enhances the function nl80211_set_station to pass this
     information of the peer to the driver in case this command is used
     with the TDLS peer STA.

     In addition, make the HT/VHT capability configuration handled more
     consistently for other STA cases (reject both instead of just HT).

     Signed-off-by: Jouni Malinen <jouni@qca.qualcomm.com>
     Signed-off-by: Johannes Berg <johannes.berg@intel.com>
==============================================================

If you need any more debugging output for this problem, please let me know.

Thanks,

Larry

Re: Regression caused by commit df88129

[Malinen, Jouni]

On 2/17/13 8:31 AM, "Larry Finger" <Larry.Finger@lwfinger.net> wrote:

>A recent pull of the wireless-testing tree caused my wireless to break.
>The
>wireless devices can authenticate and associate as follows:

>wlan0: RX AssocResp from c0:3f:0e:be:2b:44 (capab=0x411 status=0 aid=3)
>wlan0: associated
>IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
>wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

Which wpa_supplicant version are you using here? I'm assuming this is with
WPA2.

It sounds like the AP STA entry does not get authorized properly. I did see
some issues when working on the patch, but fixed those. The version that
was
sent out should have worked fine since I did manage to complete TDLS setup
with it (i.e., send and receive frames through the AP to another STA).

I will be flying for the rest of Sunday, so may not be able to do much
about this before Monday.

- Jouni

Re: Regression caused by commit df88129

[Malinen, Jouni]

On 2/17/13 8:31 AM, "Larry Finger" <Larry.Finger@lwfinger.net> wrote:

>IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
>wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

I do see one of these (which, I'd assume, is actually expected if you
have something on the system that runs to transmit before 4-way
handshake is completed).

>wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
>wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

But this does not show up in my tests with mac80211_hwsim and the
current wireless-testing.git and hostap.git snapshots.

>The regression was bisected to commit
>df881293c6ba9a12868491a717b25cb14ec1fa4a.

Did you confirm that the latest snapshot works with this reverted?

>If you need any more debugging output for this problem, please let me
>know.

Since I was unable to reproduce, some more debug output could be
useful. Please at least verify that the STA entry for the AP is not
marked authorized. The next step would be to check whether sta_set_flags
in wpa_supplicant driver_nl80211.c is failing. Unfortunately, it looks
like the current implementation does not make this very clear in the
debug log, so some additional prints may be needed there.

Please also describe the security configuration (WPA2-Personal/CCMP?)
that you are using.

- Jouni

Re: Regression caused by commit df88129

[Larry Finger]

On 02/17/2013 08:41 AM, Malinen, Jouni wrote:
>
>
> On 2/17/13 8:31 AM, "Larry Finger" <Larry.Finger@lwfinger.net> wrote:
>
>> IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
>> wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
>
> I do see one of these (which, I'd assume, is actually expected if you
> have something on the system that runs to transmit before 4-way
> handshake is completed).

I also have seen one of these for some time.

>> wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
>> wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
>
> But this does not show up in my tests with mac80211_hwsim and the
> current wireless-testing.git and hostap.git snapshots.
>
>> The regression was bisected to commit
>> df881293c6ba9a12868491a717b25cb14ec1fa4a.
>
> Did you confirm that the latest snapshot works with this reverted?

With the commit reverted, the latest kernel-wireless version works.

>> If you need any more debugging output for this problem, please let me
>> know.
>
> Since I was unable to reproduce, some more debug output could be
> useful. Please at least verify that the STA entry for the AP is not
> marked authorized. The next step would be to check whether sta_set_flags
> in wpa_supplicant driver_nl80211.c is failing. Unfortunately, it looks
> like the current implementation does not make this very clear in the
> debug log, so some additional prints may be needed there.

Just to be clear, b43 is used as a station, not as AP. The card I am using has a 
G PHY. I added printk statements for every setting of 'authorized' in 
net/mac80211/tx.c. It is set false at line 1984, thus 'multicast' is false. 
Furthermore, sta is not NULL, but WLAN_STA_AUTHORIZED is not set in the sta 
flags. Note: sta->_flags is 0x60023.

Although sdata->vif.type is set to 2 (NL80211_IFTYPE_STATION), variable 
'authorized' was not touched at line 1908, thus "sdata->wdev.wiphy->flags & 
WIPHY_FLAG_SUPPORTS_TDLS" is false. The content of sdata->wdev.wiphy->flags is 
0x1401f8.

> Please also describe the security configuration (WPA2-Personal/CCMP?)
> that you are using.

I'm running openSUSE 12.3 RC1, which has version 1.1 of wpa_supplicant. The 
router is a Netgear WNDR3300 running standard firmware and set for WPA2-PSK(AES) 
encryption.

Checking with newer versions of the supplicant is being a problem. I built both 
2.0 from the tarball and 2.1-devel from the git repo, but nether will run with 
NetworkManager. I will see if I can resolve that problem later.

Thanks,

Larry

Re: Regression caused by commit df88129

[Johannes Berg]

On Sun, 2013-02-17 at 00:31 -0600, Larry Finger wrote:
> Jouni and Johannes,
> 
> A recent pull of the wireless-testing tree caused my wireless to break. The 
> wireless devices can authenticate and associate as follows:
> 
> wlan0: authenticate with c0:3f:0e:be:2b:44
> wlan0: capabilities/regulatory prevented using AP HT/VHT configuration, downgraded
> wlan0: send auth to c0:3f:0e:be:2b:44 (try 1/3)
> wlan0: authenticated
> wlan0: associate with c0:3f:0e:be:2b:44 (try 1/3)
> wlan0: RX AssocResp from c0:3f:0e:be:2b:44 (capab=0x411 status=0 aid=3)
> wlan0: associated
> IPv6: ADDRCONF(NETDEV_CHANGE): wlan0: link becomes ready
> wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
> b43-phy0 debug: Using hardware based encryption for keyidx: 0, mac: 
> c0:3f:0e:be:2b:44
> b43-phy0 debug: Using hardware based encryption for keyidx: 1, mac: 
> ff:ff:ff:ff:ff:ff
> wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)
> wlan0: dropped frame to c0:3f:0e:be:2b:44 (unauthorized port)

Yeah, it's obvious that this would happen for drivers not supporting
TDLS -- will send a patch.

johannes

[PATCH] cfg80211: fix station change if TDLS isn't supported

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Larry noticed (and bisected) that commit df881293c6ba9a12868491a717b25
"cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station"
broke secure connections. This is is the case only for drivers that
don't support TDLS, where any kind of change, even just the change of
authorized flag that is required for normal operation, was rejected
now. To fix this, remove the checks. I have some patches that will add
proper verification for all the different cases later.

Cc: Jouni Malinen <j@w1.fi>
Bisected-by: Larry Finger <Larry.Finger@lwfinger.net>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/wireless/nl80211.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 580ffea..d86af75 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -3423,14 +3423,6 @@ static int nl80211_set_station_tdls(struct genl_info *info,
 	struct nlattr *nla;
 	int err;
 
-	/* Can only set if TDLS ... */
-	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS))
-		return -EOPNOTSUPP;
-
-	/* ... with external setup is supported */
-	if (!(rdev->wiphy.flags & WIPHY_FLAG_TDLS_EXTERNAL_SETUP))
-		return -EOPNOTSUPP;
-
 	/* Dummy STA entry gets updated once the peer capabilities are known */
 	if (info->attrs[NL80211_ATTR_HT_CAPABILITY])
 		params->ht_capa =
-- 
1.8.0

Re: [PATCH] cfg80211: fix station change if TDLS isn't supported

[Larry Finger]

On 02/18/2013 08:02 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Larry noticed (and bisected) that commit df881293c6ba9a12868491a717b25
> "cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station"
> broke secure connections. This is is the case only for drivers that
> don't support TDLS, where any kind of change, even just the change of
> authorized flag that is required for normal operation, was rejected
> now. To fix this, remove the checks. I have some patches that will add
> proper verification for all the different cases later.
>
> Cc: Jouni Malinen <j@w1.fi>
> Bisected-by: Larry Finger <Larry.Finger@lwfinger.net>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Johannes,

Thanks. This patch does fix the problem. The only thing I saw was an unused 
variable:

net/wireless/nl80211.c: In function ‘nl80211_set_station_tdls’:
net/wireless/nl80211.c:3421:37: warning: unused variable ‘rdev’ [-Wunused-variable]

Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

Larry

> ---
>   net/wireless/nl80211.c | 8 --------
>   1 file changed, 8 deletions(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 580ffea..d86af75 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -3423,14 +3423,6 @@ static int nl80211_set_station_tdls(struct genl_info *info,
>   	struct nlattr *nla;
>   	int err;
>
> -	/* Can only set if TDLS ... */
> -	if (!(rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_TDLS))
> -		return -EOPNOTSUPP;
> -
> -	/* ... with external setup is supported */
> -	if (!(rdev->wiphy.flags & WIPHY_FLAG_TDLS_EXTERNAL_SETUP))
> -		return -EOPNOTSUPP;
> -
>   	/* Dummy STA entry gets updated once the peer capabilities are known */
>   	if (info->attrs[NL80211_ATTR_HT_CAPABILITY])
>   		params->ht_capa =
>

Re: [PATCH] cfg80211: fix station change if TDLS isn't supported

[Johannes Berg]

On Mon, 2013-02-18 at 11:24 -0600, Larry Finger wrote:
> On 02/18/2013 08:02 AM, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > Larry noticed (and bisected) that commit df881293c6ba9a12868491a717b25
> > "cfg80211: Pass TDLS peer's QoS/HT/VHT information during set_station"
> > broke secure connections. This is is the case only for drivers that
> > don't support TDLS, where any kind of change, even just the change of
> > authorized flag that is required for normal operation, was rejected
> > now. To fix this, remove the checks. I have some patches that will add
> > proper verification for all the different cases later.
> >
> > Cc: Jouni Malinen <j@w1.fi>
> > Bisected-by: Larry Finger <Larry.Finger@lwfinger.net>
> > Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> 
> Johannes,
> 
> Thanks. This patch does fix the problem. The only thing I saw was an unused 
> variable:
> 
> net/wireless/nl80211.c: In function ‘nl80211_set_station_tdls’:
> net/wireless/nl80211.c:3421:37: warning: unused variable ‘rdev’ [-Wunused-variable]
> 
> Tested-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks, I'll fix that variable and apply the patch.

johannes