2010-02-14 23:37:42

by Benoit Papillault

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

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 <[email protected]>
---
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



2010-02-15 09:35:39

by Johannes Berg

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

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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-15 07:45:44

by Benoit Papillault

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

G?bor Stefanik a ?crit :
> On Mon, Feb 15, 2010 at 12:37 AM, Benoit Papillault
> <[email protected]> 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 <[email protected]>
>> ---
>> 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 [email protected]
>> 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


2010-02-15 09:36:40

by Johannes Berg

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

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 <[email protected]>
> ---
> 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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-15 22:32:21

by Benoit Papillault

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

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



2010-02-15 09:34:46

by Johannes Berg

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

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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-14 23:37:43

by Benoit Papillault

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

The same code is also used for probe request generation

Signed-off-by: Benoit Papillault <[email protected]>
---
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


2010-02-15 00:10:33

by Gábor Stefanik

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

On Mon, Feb 15, 2010 at 12:37 AM, Benoit Papillault
<[email protected]> 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 <[email protected]>
> ---
> ?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 [email protected]
> 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. :-)

2010-02-16 07:17:29

by Johannes Berg

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

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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2010-02-16 07:18:21

by Johannes Berg

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

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


2010-02-15 22:36:30

by Benoit Papillault

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

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


2010-02-16 09:58:43

by Johannes Berg

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

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


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part