[PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Benoit Papillault]

Fix for the following issue : a STA connected to a WPA2 AP was showing
frames from others STA in tcpdump on wlan0 (promiscuous mode). In fact,
those frames are not decrypted and appears as 802.3 junk. This patch
just drops any protected data frames that have not been decrypted.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/rx.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index c9755f3..22ae6ee 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1397,6 +1397,14 @@ ieee80211_drop_unencrypted(struct ieee80211_rx_data *rx, __le16 fc)
 		     ieee80211_is_data(fc) &&
 		     (rx->key || rx->sdata->drop_unencrypted)))
 		return -EACCES;
+	/*
+	 * Drop encrypted frames that have not been decrypted. This
+	 * happens for frames that are sent by an AP to another STA
+	 */
+	if (ieee80211_has_protected(fc) &&
+	    !(status->flag & RX_FLAG_DECRYPTED)) {
+		return -EACCES;
+	}
 	if (rx->sta && test_sta_flags(rx->sta, WLAN_STA_MFP)) {
 		if (unlikely(ieee80211_is_unicast_robust_mgmt_frame(rx->skb) &&
 			     rx->key))
-- 
1.5.6.5

[PATCH 2/2] mac80211: Add HT IE to IBSS beacons and probe responses.

[Benoit Papillault]

The same code is also used for probe request generation

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/ibss.c |   86 ++++++++++++++++++++++++++++++++++++++++++++++++---
 net/mac80211/util.c |   23 +++++++------
 2 files changed, 93 insertions(+), 16 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index f3e9424..423befa 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -96,6 +96,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	sdata->drop_unencrypted = capability & WLAN_CAPABILITY_PRIVACY ? 1 : 0;
 
 	local->oper_channel = chan;
+	/* FIXME : we can have HT channels here */
 	local->oper_channel_type = NL80211_CHAN_NO_HT;
 	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
 
@@ -111,7 +112,10 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 		*pos++ = basic | (u8) (rate / 5);
 	}
 
-	/* Build IBSS probe response */
+	/* 
+	 * Build IBSS probe response template (also used for beacon template
+	 * in ieee80211_beacon_get_tim())
+	 */
 	mgmt = (void *) skb_put(skb, 24 + sizeof(mgmt->u.beacon));
 	memset(mgmt, 0, 24 + sizeof(mgmt->u.beacon));
 	mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
@@ -158,6 +162,64 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 		memcpy(pos, &supp_rates[8], rates);
 	}
 
+	if (sband->ht_cap.ht_supported) {
+		u16 cap = sband->ht_cap.cap;
+		struct ieee80211_ht_cap ht_cap;
+		struct ieee80211_ht_info ht_info;
+
+		if (ieee80211_disable_40mhz_24ghz &&
+		    sband->band == IEEE80211_BAND_2GHZ) {
+			cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+			cap &= ~IEEE80211_HT_CAP_SGI_40;
+		}
+
+		ht_cap.cap_info = cpu_to_le16(cap);
+		ht_cap.ampdu_params_info = sband->ht_cap.ampdu_factor |
+			(sband->ht_cap.ampdu_density <<
+			 IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
+		ht_cap.mcs = sband->ht_cap.mcs;
+		ht_cap.extended_ht_cap_info = cpu_to_le16(0);
+		ht_cap.tx_BF_cap_info = cpu_to_le32(0);
+		ht_cap.antenna_selection_info = 0;
+
+		/* HT Capabilities element */
+		pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_cap));
+		*pos++ = WLAN_EID_HT_CAPABILITY;
+		*pos++ = sizeof(struct ieee80211_ht_cap);
+		memcpy(pos, &ht_cap, sizeof(struct ieee80211_ht_cap));
+
+		ht_info.control_chan = 
+			ieee80211_frequency_to_channel(chan->center_freq);
+		ht_info.ht_param = 0;
+		/* FIXME : local->oper_channel_type is set to a fixed value */
+		switch (local->oper_channel_type) {
+		case NL80211_CHAN_NO_HT:
+		case NL80211_CHAN_HT20:
+			ht_info.ht_param |= IEEE80211_HT_PARAM_CHA_SEC_NONE;
+			break;
+		case NL80211_CHAN_HT40MINUS:
+			ht_info.ht_param |= IEEE80211_HT_PARAM_CHA_SEC_BELOW;
+			break;
+		case NL80211_CHAN_HT40PLUS:
+			ht_info.ht_param |= IEEE80211_HT_PARAM_CHA_SEC_ABOVE;
+			break;
+		}
+		if (sband->ht_cap.cap & IEEE80211_HT_CAP_SUP_WIDTH_20_40)
+			ht_info.ht_param |= IEEE80211_HT_PARAM_CHAN_WIDTH_ANY;
+		ht_info.operation_mode = cpu_to_le16(0);
+		ht_info.stbc_param = cpu_to_le16(0);
+		/* It seems that Basic MCS set and Supported MCS set
+		   are identical for the first 10 bytes */
+		memset(&ht_info.basic_set, 0, 16);
+		memcpy(&ht_info.basic_set, &ht_cap.mcs, 10);
+
+		/* HT Information element */
+		pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_info));
+		*pos++ = WLAN_EID_HT_INFORMATION;
+		*pos++ = sizeof(struct ieee80211_ht_info);
+		memcpy(pos, &ht_info, sizeof(struct ieee80211_ht_info));
+	}
+	  
 	if (ifibss->ie_len)
 		memcpy(skb_put(skb, ifibss->ie_len),
 		       ifibss->ie, ifibss->ie_len);
@@ -903,11 +965,25 @@ int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
 			sdata->u.ibss.ie_len = params->ie_len;
 	}
 
+	/*
+	 * Allocate IBSS probe response template (see
+	 * __ieee80211_sta_join_ibss for the needed size). According to IEEE
+	 * 802.11-2007 10.4.4.2, there is only 20 possibles values. We
+	 * support up IEEE80211_MAX_SUPP_RATES (currently 32) : so 8 for
+	 * Supported Rates and IEEE80211_MAX_SUPP_RATES-8 for Extended
+	 * Supported Rates
+	 */
+
 	skb = dev_alloc_skb(sdata->local->hw.extra_tx_headroom +
-			    36 /* bitrates */ +
-			    34 /* SSID */ +
-			    3  /* DS params */ +
-			    4  /* IBSS params */ +
+			    sizeof(struct ieee80211_hdr_3addr) + 
+			    12 /* struct ieee80211_mgmt.u.beacon */ +
+			    2 + IEEE80211_MAX_SSID_LEN /* max SSID */ +
+			    2 + 8 /* max Supported Rates */ +
+			    3 /* max DS params */ +
+			    4 /* IBSS params */ +
+			    2 + (IEEE80211_MAX_SUPP_RATES-8) /* max Ext Rates */ +
+			    2 + sizeof(struct ieee80211_ht_cap) +
+			    2 + sizeof(struct ieee80211_ht_info) +
 			    params->ie_len);
 	if (!skb)
 		return -ENOMEM;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index c453226..591cc71 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -958,7 +958,7 @@ int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 
 	if (sband->ht_cap.ht_supported) {
 		u16 cap = sband->ht_cap.cap;
-		__le16 tmp;
+		struct ieee80211_ht_cap ht_cap;
 
 		if (ieee80211_disable_40mhz_24ghz &&
 		    sband->band == IEEE80211_BAND_2GHZ) {
@@ -966,18 +966,19 @@ int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
 			cap &= ~IEEE80211_HT_CAP_SGI_40;
 		}
 
+		ht_cap.cap_info = cpu_to_le16(cap);
+		ht_cap.ampdu_params_info = sband->ht_cap.ampdu_factor |
+			(sband->ht_cap.ampdu_density <<
+			 IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
+		ht_cap.mcs = sband->ht_cap.mcs;
+		ht_cap.extended_ht_cap_info = cpu_to_le16(0);
+		ht_cap.tx_BF_cap_info = cpu_to_le32(0);
+		ht_cap.antenna_selection_info = 0;
+
 		*pos++ = WLAN_EID_HT_CAPABILITY;
 		*pos++ = sizeof(struct ieee80211_ht_cap);
-		memset(pos, 0, sizeof(struct ieee80211_ht_cap));
-		tmp = cpu_to_le16(cap);
-		memcpy(pos, &tmp, sizeof(u16));
-		pos += sizeof(u16);
-		*pos++ = sband->ht_cap.ampdu_factor |
-			 (sband->ht_cap.ampdu_density <<
-				IEEE80211_HT_AMPDU_PARM_DENSITY_SHIFT);
-		memcpy(pos, &sband->ht_cap.mcs, sizeof(sband->ht_cap.mcs));
-		pos += sizeof(sband->ht_cap.mcs);
-		pos += 2 + 4 + 1; /* ext info, BF cap, antsel */
+		memcpy(pos, &ht_cap, sizeof(struct ieee80211_ht_cap));
+		pos += sizeof(struct ieee80211_ht_cap);
 	}
 
 	/*
-- 
1.5.6.5

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Gábor Stefanik]

On Mon, Feb 15, 2010 at 12:37 AM, Benoit Papillault
<benoit.papillault@free.fr> wrote:
> Fix for the following issue : a STA connected to a WPA2 AP was showing
> frames from others STA in tcpdump on wlan0 (promiscuous mode). In fact,
> those frames are not decrypted and appears as 802.3 junk. This patch
> just drops any protected data frames that have not been decrypted.
>
> Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
> ---
>  net/mac80211/rx.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index c9755f3..22ae6ee 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1397,6 +1397,14 @@ ieee80211_drop_unencrypted(struct ieee80211_rx_data *rx, __le16 fc)
>                     ieee80211_is_data(fc) &&
>                     (rx->key || rx->sdata->drop_unencrypted)))
>                return -EACCES;
> +       /*
> +        * Drop encrypted frames that have not been decrypted. This
> +        * happens for frames that are sent by an AP to another STA
> +        */
> +       if (ieee80211_has_protected(fc) &&
> +           !(status->flag & RX_FLAG_DECRYPTED)) {
> +               return -EACCES;
> +       }
>        if (rx->sta && test_sta_flags(rx->sta, WLAN_STA_MFP)) {
>                if (unlikely(ieee80211_is_unicast_robust_mgmt_frame(rx->skb) &&
>                             rx->key))
> --
> 1.5.6.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

I'm not familiar with this part of the code; but have you tested if
this doesn't break monitor-while-operating mode (i.e. doesn't remove
other-STA frames from monitor interfaces)?

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Benoit PAPILLAULT]

Gábor Stefanik a écrit :
> On Mon, Feb 15, 2010 at 12:37 AM, Benoit Papillault
> <benoit.papillault@free.fr> wrote:
>   
>> Fix for the following issue : a STA connected to a WPA2 AP was showing
>> frames from others STA in tcpdump on wlan0 (promiscuous mode). In fact,
>> those frames are not decrypted and appears as 802.3 junk. This patch
>> just drops any protected data frames that have not been decrypted.
>>
>> Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
>> ---
>>  net/mac80211/rx.c |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
>> index c9755f3..22ae6ee 100644
>> --- a/net/mac80211/rx.c
>> +++ b/net/mac80211/rx.c
>> @@ -1397,6 +1397,14 @@ ieee80211_drop_unencrypted(struct ieee80211_rx_data *rx, __le16 fc)
>>                     ieee80211_is_data(fc) &&
>>                     (rx->key || rx->sdata->drop_unencrypted)))
>>                return -EACCES;
>> +       /*
>> +        * Drop encrypted frames that have not been decrypted. This
>> +        * happens for frames that are sent by an AP to another STA
>> +        */
>> +       if (ieee80211_has_protected(fc) &&
>> +           !(status->flag & RX_FLAG_DECRYPTED)) {
>> +               return -EACCES;
>> +       }
>>        if (rx->sta && test_sta_flags(rx->sta, WLAN_STA_MFP)) {
>>                if (unlikely(ieee80211_is_unicast_robust_mgmt_frame(rx->skb) &&
>>                             rx->key))
>> --
>> 1.5.6.5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>     
>
> I'm not familiar with this part of the code; but have you tested if
> this doesn't break monitor-while-operating mode (i.e. doesn't remove
> other-STA frames from monitor interfaces)?
>
>   
Yes, it has been tested in this case. In fact, this patch changes RX 
path only in ieee80211_rx_h_data / ieee80211_rx_h_action and 
ieee80211_rx_h_mgmt. In all 3 cases, it returns RX_DROP_MONITOR.

Regards,
Benoit

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On Mon, 2010-02-15 at 08:45 +0100, Benoit PAPILLAULT wrote:

> > I'm not familiar with this part of the code; but have you tested if
> > this doesn't break monitor-while-operating mode (i.e. doesn't remove
> > other-STA frames from monitor interfaces)?
> >
> >   
> Yes, it has been tested in this case. In fact, this patch changes RX 
> path only in ieee80211_rx_h_data / ieee80211_rx_h_action and 
> ieee80211_rx_h_mgmt. In all 3 cases, it returns RX_DROP_MONITOR.

???

        if (ieee80211_drop_unencrypted(rx, mgmt->frame_control))
                return RX_DROP_UNUSABLE;

However. GUYS!!! Read the code! Real monitor mode bypasses _ALL_ the RX
handlers, frames just short-circuit up right after they are received
from the driver.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/2] mac80211: Add HT IE to IBSS beacons and probe responses.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

On Mon, 2010-02-15 at 00:37 +0100, Benoit Papillault wrote:
> The same code is also used for probe request generation

Still NACK. You really need to make this depend on channel
configuration, which needs new nl80211 infrastructure.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1221 bytes --]

On Mon, 2010-02-15 at 00:37 +0100, Benoit Papillault wrote:
> Fix for the following issue : a STA connected to a WPA2 AP was showing
> frames from others STA in tcpdump on wlan0 (promiscuous mode). In
> fact,
> those frames are not decrypted and appears as 802.3 junk. This patch
> just drops any protected data frames that have not been decrypted.
> 
> Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
> ---
>  net/mac80211/rx.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index c9755f3..22ae6ee 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1397,6 +1397,14 @@ ieee80211_drop_unencrypted(struct
> ieee80211_rx_data *rx, __le16 fc)
>  		     ieee80211_is_data(fc) &&
>  		     (rx->key || rx->sdata->drop_unencrypted)))
>  		return -EACCES;
> +	/*
> +	 * Drop encrypted frames that have not been decrypted. This
> +	 * happens for frames that are sent by an AP to another STA
> +	 */
> +	if (ieee80211_has_protected(fc) &&
> +	    !(status->flag & RX_FLAG_DECRYPTED)) {
> +		return -EACCES;
> +	}

Comment #1. The && part of the if is useless. Please find out why.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/2] mac80211: Add HT IE to IBSS beacons and probe responses.

[Benoit PAPILLAULT]

Johannes Berg a écrit :
> On Mon, 2010-02-15 at 00:37 +0100, Benoit Papillault wrote:
>   
>> The same code is also used for probe request generation
>>     
>
> Still NACK. You really need to make this depend on channel
> configuration, which needs new nl80211 infrastructure.
>
> johannes
>   
You are referring to "we need nl80211/cfg80211 changes" I guess. Could 
you elaborate a bit on this? I'm still new to mac80211. Could you tell 
me what needs to be changed and where I can start ? iw source code ? is 
cfg80211 the source code under net/wireless ?

Regards,
Benoit

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Benoit PAPILLAULT]

Johannes Berg a écrit :
> On Mon, 2010-02-15 at 08:45 +0100, Benoit PAPILLAULT wrote:
>
>   
>>> I'm not familiar with this part of the code; but have you tested if
>>> this doesn't break monitor-while-operating mode (i.e. doesn't remove
>>> other-STA frames from monitor interfaces)?
>>>
>>>   
>>>       
>> Yes, it has been tested in this case. In fact, this patch changes RX 
>> path only in ieee80211_rx_h_data / ieee80211_rx_h_action and 
>> ieee80211_rx_h_mgmt. In all 3 cases, it returns RX_DROP_MONITOR.
>>     
>
> ???
>
>         if (ieee80211_drop_unencrypted(rx, mgmt->frame_control))
>                 return RX_DROP_UNUSABLE;
>   
This code was not in wireless-testing/master yesterday when I sent the 
patches.
> However. GUYS!!! Read the code! Real monitor mode bypasses _ALL_ the RX
> handlers, frames just short-circuit up right after they are received
> from the driver.
>   
Just for my own understanding. It seems the implementation has 2 
different code path :
- "real" monitor mode which is handled right after ieee80211_rx() so it 
is not affected
- "cook" monitor mode which is handled as part of the RX handlers.

BTW, why do we have 2 different code path? I'm sure I missed something 
obvious here.
> johannes
>   

Regards,
Benoit

Re: [PATCH 2/2] mac80211: Add HT IE to IBSS beacons and probe responses.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]

On Mon, 2010-02-15 at 23:32 +0100, Benoit PAPILLAULT wrote:

> You are referring to "we need nl80211/cfg80211 changes" I guess. Could
> 
> you elaborate a bit on this? I'm still new to mac80211. Could you tell
> 
> me what needs to be changed and where I can start ? iw source code ?
> is 
> cfg80211 the source code under net/wireless ?

Yes cfg80211 is most of net/wireless/.

Is it not obvious to you that in order to use HT with IBSS you need to
have a way to configure the IBSS to use HT?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Johannes Berg]

On Mon, 2010-02-15 at 23:36 +0100, Benoit PAPILLAULT wrote:

> Just for my own understanding. It seems the implementation has 2 
> different code path :
> - "real" monitor mode which is handled right after ieee80211_rx() so
> it 
> is not affected
> - "cook" monitor mode which is handled as part of the RX handlers.
> 
> BTW, why do we have 2 different code path? I'm sure I missed something
> 
> obvious here.

cooked monitor reports only usable, decrypted frames that the kernel
didn't know how to handle.

johannes

Re: [PATCH 1/2] mac80211: Drop protected data frames that have not been decrypted

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 650 bytes --]

On Mon, 2010-02-15 at 00:37 +0100, Benoit Papillault wrote:
> Fix for the following issue : a STA connected to a WPA2 AP was showing
> frames from others STA in tcpdump on wlan0 (promiscuous mode). In
> fact,
> those frames are not decrypted and appears as 802.3 junk. This patch
> just drops any protected data frames that have not been decrypted.

Those frames are only passed up when you set IFF_PROMISC on the
interface, so that's not really a bug, in a sense you wanted to see
them. Now, they may not actually be useful to you, and I'd argue that
checking IFF_PROMISC has no place in mac80211, but it's not really a
bug.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]