2011-04-26 20:27:30

by Arik Nemtsov

[permalink] [raw]
Subject: [PATCH] mac80211: report MIC failure for truncated packets in AP mode

MIC failure notifications for packets too short to contain a key index
are currently ignored in AP-mode. Fix the check to only ignore packets
with an existing non-zero key index.

The wl12xx chip always truncates packets with a failed MIC and requires
this change to operate correctly in AP-mode.

No such check is made in STA mode. Therefore its relatively safe to assume
there's no other HW that relies on the current code to avoid spurious
MIC failures with correct yet truncated packets.

Signed-off-by: Arik Nemtsov <[email protected]>
---
net/mac80211/rx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index a864890..875fc3c 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2391,7 +2391,7 @@ static void ieee80211_rx_michael_mic_report(struct ieee80211_hdr *hdr,
if (!ieee80211_has_protected(hdr->frame_control))
return;

- if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx) {
+ if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx > 0) {
/*
* APs with pairwise keys should never receive Michael MIC
* errors for non-zero keyidx because these are reserved for
--
1.7.1



2011-04-26 22:03:23

by Arik Nemtsov

[permalink] [raw]
Subject: Re: [PATCH] mac80211: report MIC failure for truncated packets in AP mode

On Tue, Apr 26, 2011 at 23:55, Christian Lamparter
<[email protected]> wrote:
> On Tue, Apr 26, 2011 at 10:27 PM, Arik Nemtsov <[email protected]> wrote:
>> MIC failure notifications for packets too short to contain a key index
>> are currently ignored in AP-mode. Fix the check to only ignore packets
>> with an existing non-zero key index.
>>
>> The wl12xx chip always truncates packets with a failed MIC and requires
>> this change to operate correctly in AP-mode.
>>
>> No such check is made in STA mode. Therefore its relatively safe to assume
>> there's no other HW that relies on the current code to avoid spurious
>> MIC failures with correct yet truncated packets.
>>
>> Signed-off-by: Arik Nemtsov <[email protected]>
>> ---
>> ?net/mac80211/rx.c | ? ?2 +-
>> ?1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
>> index a864890..875fc3c 100644
>> --- a/net/mac80211/rx.c
>> +++ b/net/mac80211/rx.c
>> @@ -2391,7 +2391,7 @@ static void ieee80211_rx_michael_mic_report(struct ieee80211_hdr *hdr,
>> ? ? ? ?if (!ieee80211_has_protected(hdr->frame_control))
>> ? ? ? ? ? ? ? ?return;
>>
>> - ? ? ? if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx) {
>> + ? ? ? if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx > 0) {
>> ? ? ? ? ? ? ? ?/*
>> ? ? ? ? ? ? ? ? * APs with pairwise keys should never receive Michael MIC
>> ? ? ? ? ? ? ? ? * errors for non-zero keyidx because these are reserved for
>> --
> wait! Since you seem able to trigger MIC events frequently, could you
> please test if the following patch:
>
> <http://www.spinics.net/lists/linux-wireless/msg67571.html>
>
> <a little more info:http://www.spinics.net/lists/linux-wireless/msg67461.html>
>
> would help in your case as well?
>

I seem to have missed this thread entirely :)
The patch you mentioned does indeed help. I tested in STA and AP mode.

This bit is important for wl12xx:

+ /*
+ * No way to verify the MIC if the hardware stripped it or
+ * the IV with the key index. In this case we have solely rely
+ * on the driver to set RX_FLAG_MMIC_ERROR in the event of a
+ * MIC failure report.
+ */
+ if (status->flag & (RX_FLAG_MMIC_STRIPPED | RX_FLAG_IV_STRIPPED)) {
+ if (status->flag & RX_FLAG_MMIC_ERROR)
+ goto mic_fail;

This prevents us from getting to the problematic check that I tried to
remove with my patch.

Just for the record - generating a MIC failure is pretty easy. I'm
using the (very cool) mac80211 debugfs feature that allows simulating
a MIC failure (see ieee80211_if_parse_tkip_mic_test()).
It works well with a rt2x00 based card and the latest compat. I'm
simulating it from AP as well as STA.

To summarize - either patch will work for us.

Regards,
Arik

2011-04-27 06:16:32

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH] mac80211: report MIC failure for truncated packets in AP mode

On Wed, 2011-04-27 at 01:03 +0300, Arik Nemtsov wrote:
> On Tue, Apr 26, 2011 at 23:55, Christian Lamparter
> <[email protected]> wrote:
> > On Tue, Apr 26, 2011 at 10:27 PM, Arik Nemtsov <[email protected]> wrote:
> >> MIC failure notifications for packets too short to contain a key index
> >> are currently ignored in AP-mode. Fix the check to only ignore packets
> >> with an existing non-zero key index.
> >>
> >> The wl12xx chip always truncates packets with a failed MIC and requires
> >> this change to operate correctly in AP-mode.
> >>
> >> No such check is made in STA mode. Therefore its relatively safe to assume
> >> there's no other HW that relies on the current code to avoid spurious
> >> MIC failures with correct yet truncated packets.
> >>
> >> Signed-off-by: Arik Nemtsov <[email protected]>
> >> ---
> >> net/mac80211/rx.c | 2 +-
> >> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> >> index a864890..875fc3c 100644
> >> --- a/net/mac80211/rx.c
> >> +++ b/net/mac80211/rx.c
> >> @@ -2391,7 +2391,7 @@ static void ieee80211_rx_michael_mic_report(struct ieee80211_hdr *hdr,
> >> if (!ieee80211_has_protected(hdr->frame_control))
> >> return;
> >>
> >> - if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx) {
> >> + if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx > 0) {
> >> /*
> >> * APs with pairwise keys should never receive Michael MIC
> >> * errors for non-zero keyidx because these are reserved for
> >> --
> > wait! Since you seem able to trigger MIC events frequently, could you
> > please test if the following patch:
> >
> > <http://www.spinics.net/lists/linux-wireless/msg67571.html>
> >
> > <a little more info:http://www.spinics.net/lists/linux-wireless/msg67461.html>
> >
> > would help in your case as well?
> >
>
> I seem to have missed this thread entirely :)
> The patch you mentioned does indeed help. I tested in STA and AP mode.
>
> This bit is important for wl12xx:
>
> + /*
> + * No way to verify the MIC if the hardware stripped it or
> + * the IV with the key index. In this case we have solely rely
> + * on the driver to set RX_FLAG_MMIC_ERROR in the event of a
> + * MIC failure report.
> + */
> + if (status->flag & (RX_FLAG_MMIC_STRIPPED | RX_FLAG_IV_STRIPPED)) {
> + if (status->flag & RX_FLAG_MMIC_ERROR)
> + goto mic_fail;
>
> This prevents us from getting to the problematic check that I tried to
> remove with my patch.
>
> Just for the record - generating a MIC failure is pretty easy. I'm
> using the (very cool) mac80211 debugfs feature that allows simulating
> a MIC failure (see ieee80211_if_parse_tkip_mic_test()).
> It works well with a rt2x00 based card and the latest compat. I'm
> simulating it from AP as well as STA.
>
> To summarize - either patch will work for us.

Great! If this can be solved in a generic way in mac80211, I'd prefer if
that one is used.

Christian, are you planning to submit this patch again any time soon? If
not, we could include the wl12xx patch for now and revert it later when
the proper fix in mac80211 is applied.

What do you guys think?

--
Cheers,
Luca.


2011-04-26 20:55:33

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH] mac80211: report MIC failure for truncated packets in AP mode

On Tue, Apr 26, 2011 at 10:27 PM, Arik Nemtsov <[email protected]> wrote:
> MIC failure notifications for packets too short to contain a key index
> are currently ignored in AP-mode. Fix the check to only ignore packets
> with an existing non-zero key index.
>
> The wl12xx chip always truncates packets with a failed MIC and requires
> this change to operate correctly in AP-mode.
>
> No such check is made in STA mode. Therefore its relatively safe to assume
> there's no other HW that relies on the current code to avoid spurious
> MIC failures with correct yet truncated packets.
>
> Signed-off-by: Arik Nemtsov <[email protected]>
> ---
> ?net/mac80211/rx.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index a864890..875fc3c 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -2391,7 +2391,7 @@ static void ieee80211_rx_michael_mic_report(struct ieee80211_hdr *hdr,
> ? ? ? ?if (!ieee80211_has_protected(hdr->frame_control))
> ? ? ? ? ? ? ? ?return;
>
> - ? ? ? if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx) {
> + ? ? ? if (rx->sdata->vif.type == NL80211_IFTYPE_AP && keyidx > 0) {
> ? ? ? ? ? ? ? ?/*
> ? ? ? ? ? ? ? ? * APs with pairwise keys should never receive Michael MIC
> ? ? ? ? ? ? ? ? * errors for non-zero keyidx because these are reserved for
> --
wait! Since you seem able to trigger MIC events frequently, could you
please test if the following patch:

<http://www.spinics.net/lists/linux-wireless/msg67571.html>

<a little more info:http://www.spinics.net/lists/linux-wireless/msg67461.html>

would help in your case as well?

Regards,
Christian

2011-04-27 08:46:59

by Christian Lamparter

[permalink] [raw]
Subject: Re: [PATCH] mac80211: report MIC failure for truncated packets in AP mode

On Wed, Apr 27, 2011 at 8:16 AM, Luciano Coelho <[email protected]> wrote:
> On Wed, 2011-04-27 at 01:03 +0300, Arik Nemtsov wrote:
>> On Tue, Apr 26, 2011 at 23:55, Christian Lamparter
>> <[email protected]> wrote:
>> > On Tue, Apr 26, 2011 at 10:27 PM, Arik Nemtsov <[email protected]> wrote:
>> >> MIC failure notifications for packets too short to contain a key index
>> >> are currently ignored in AP-mode.

>> > wait! Since you seem able to trigger MIC events frequently, could you
>> > please test if the following patch:
>> >
>> > <http://www.spinics.net/lists/linux-wireless/msg67571.html>
>> >
>> > <a little more info:http://www.spinics.net/lists/linux-wireless/msg67461.html>
>> >
>> > would help in your case as well?
>> >
>>
>> I seem to have missed this thread entirely :)
>> The patch you mentioned does indeed help. I tested in STA and AP mode.
>>
>> This bit is important for wl12xx:
>>
>> + ? ? ? /*
>> + ? ? ? ?* No way to verify the MIC if the hardware stripped it or
>> + ? ? ? ?* the IV with the key index. In this case we have solely rely
>> + ? ? ? ?* on the driver to set RX_FLAG_MMIC_ERROR in the event of a
>> + ? ? ? ?* MIC failure report.
>> + ? ? ? ?*/
>> + ? ? ? if (status->flag & (RX_FLAG_MMIC_STRIPPED | RX_FLAG_IV_STRIPPED)) {
>> + ? ? ? ? ? ? ? if (status->flag & RX_FLAG_MMIC_ERROR)
>> + ? ? ? ? ? ? ? ? ? ? ? goto mic_fail;
>>
>> This prevents us from getting to the problematic check that I tried to
>> remove with my patch.
>>
>> Just for the record - generating a MIC failure is pretty easy. I'm
>> using the (very cool) mac80211 debugfs feature that allows simulating
>> a MIC failure (see ieee80211_if_parse_tkip_mic_test()).
>> It works well with a rt2x00 based card and the latest compat. I'm
>> simulating it from AP as well as STA.
>>
>> To summarize - either patch will work for us.
>
> Great! If this can be solved in a generic way in mac80211, I'd prefer if
> that one is used.
>
> Christian, are you planning to submit this patch again any time soon? If
> not, we could include the wl12xx patch for now and revert it later when
> the proper fix in mac80211 is applied.
Well, I didn't know about the if_parse_tkip_mic_test() and tried to get
aircrack-ng's tkip attack working, this was such a waste of time...

And yes I plan to resubmit the patch [Friday?!], because carl9170
(and to some degree ath9k) have similar problems with spurious
MIC failures.

Thanks,
Chr