[PATCH] mac80211: Validate michael MIC before attempting packet decode.

[PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Michael Skeffington]

In order to allow wpa_supplicant to correctly identify a perceived WPA TKIP key
recovery attack the michael MIC must be checked before the packet decode is
attempted.  A packet with an invalid MIC will always fail a decrypt check which
previously was being checked first.  Therefore the MIC failure bit of
status flags
describing the error would remain unset.

Signed-off-by: Michael Skeffington <mike@hellotwist.com>

---

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index bc08185..71f1a56 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3176,9 +3176,10 @@ static void ieee80211_rx_handlers(struct
ieee80211_rx_data *rx,
                CALL_RXH(ieee80211_rx_h_check_more_data)
                CALL_RXH(ieee80211_rx_h_uapsd_and_pspoll)
                CALL_RXH(ieee80211_rx_h_sta_process)
+               /* must be before decrypt so MIC failures are reported
to netlink */
+               CALL_RXH(ieee80211_rx_h_michael_mic_verify)
                CALL_RXH(ieee80211_rx_h_decrypt)
                CALL_RXH(ieee80211_rx_h_defragment)
-               CALL_RXH(ieee80211_rx_h_michael_mic_verify)
                /* must be after MMIC verify so header is counted in MPDU mic */
 #ifdef CONFIG_MAC80211_MESH
                if (ieee80211_vif_is_mesh(&rx->sdata->vif))

Re: [PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Johannes Berg]

On Tue, 2017-05-09 at 14:16 -0400, Michael Skeffington wrote:
> In order to allow wpa_supplicant to correctly identify a perceived
> WPA TKIP key
> recovery attack the michael MIC must be checked before the packet
> decode is
> attempted.  A packet with an invalid MIC will always fail a decrypt
> check which
> previously was being checked first.  Therefore the MIC failure bit of
> status flags
> describing the error would remain unset.

This isn't how the Michael MIC works. I have no idea what problem
you're trying to solve, but this is not the solution.

johannes

Re: [PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Jouni Malinen]

On Tue, May 09, 2017 at 02:16:31PM -0400, Michael Skeffington wrote:
> In order to allow wpa_supplicant to correctly identify a perceived WPA TKIP key
> recovery attack the michael MIC must be checked before the packet decode is
> attempted.  A packet with an invalid MIC will always fail a decrypt check which
> previously was being checked first.  Therefore the MIC failure bit of
> status flags
> describing the error would remain unset.

Which driver and WLAN hardware are you using? Michael MIC is encrypted,
so to be able to check that, the frame will obviously need to be
decrypted first. If that WEP decryption fails, this frame needs to be
dropped without indicating Michael MIC failure. WEP part here is
completely independent of Michael MIC.

It is possible that there is a driver that handles these steps in
hardware/firmware and if so, that driver may have a bug if you do not
see Michael MIC failures reported correctly. Anyway, as Johannes pointed
out, this part in mac80211 is in the correct sequence and that cannot be
changed since it would completely break TKIP for more or less all
software-based cases.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Michael Skeffington]

I am using an rt5350 SoC using the rt2x00 driver.  We were doing
WiFi-alliance certification testing on our device and the it wasn't
issuing countermeasures appropriately.

Your assumption is correct.  I had overlooked that devices using this
driver have hardware decoding and the driver sets RX_FLAG_MMIC_ERROR.
In retrospect, the change I proposed is totally broken.

I'm running through the failure case again so I can identify where in
the rx_decrypt function it falls through.  It seems odd that it would
drop the packet in rx_decrypt given that it doesn't actually do any
decryption.  I suspect thats related to the underlying bug.

On Wed, May 10, 2017 at 8:24 AM, Jouni Malinen <j@w1.fi> wrote:
> On Tue, May 09, 2017 at 02:16:31PM -0400, Michael Skeffington wrote:
>> In order to allow wpa_supplicant to correctly identify a perceived WPA TKIP key
>> recovery attack the michael MIC must be checked before the packet decode is
>> attempted.  A packet with an invalid MIC will always fail a decrypt check which
>> previously was being checked first.  Therefore the MIC failure bit of
>> status flags
>> describing the error would remain unset.
>
> Which driver and WLAN hardware are you using? Michael MIC is encrypted,
> so to be able to check that, the frame will obviously need to be
> decrypted first. If that WEP decryption fails, this frame needs to be
> dropped without indicating Michael MIC failure. WEP part here is
> completely independent of Michael MIC.
>
> It is possible that there is a driver that handles these steps in
> hardware/firmware and if so, that driver may have a bug if you do not
> see Michael MIC failures reported correctly. Anyway, as Johannes pointed
> out, this part in mac80211 is in the correct sequence and that cannot be
> changed since it would completely break TKIP for more or less all
> software-based cases.
>
> --
> Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Johannes Berg]

On Thu, 2017-05-11 at 16:22 -0400, Michael Skeffington wrote:
> I am using an rt5350 SoC using the rt2x00 driver.  We were doing
> WiFi-alliance certification testing on our device and the it wasn't
> issuing countermeasures appropriately.
> 
> Your assumption is correct.  I had overlooked that devices using this
> driver have hardware decoding and the driver sets RX_FLAG_MMIC_ERROR.
> In retrospect, the change I proposed is totally broken.
> 
> I'm running through the failure case again so I can identify where in
> the rx_decrypt function it falls through.  It seems odd that it would
> drop the packet in rx_decrypt given that it doesn't actually do any
> decryption.  I suspect thats related to the underlying bug.

Here's the driver code from rt2500usb (but it's similar in the others):

                rxdesc->flags |= RX_FLAG_MMIC_STRIPPED;
                if (rxdesc->cipher_status == RX_CRYPTO_SUCCESS)
                        rxdesc->flags |= RX_FLAG_DECRYPTED;
                else if (rxdesc->cipher_status == RX_CRYPTO_FAIL_MIC)
                        rxdesc->flags |= RX_FLAG_MMIC_ERROR;

I think if you just change it to be

[...]
                else if (rxdesc->cipher_status == RX_CRYPTO_FAIL_MIC)
                        rxdesc->flags |= RX_FLAG_MMIC_ERROR |
					 RX_FLAG_DECRYPTED;

things will start working. This is arguably correct since to be able to
check the MMIC, the frame has to have been decrypted (properly) before.

johannes

Re: [PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Michael Skeffington]

Johannes,

Thank you for that.  I need to make a quick hack to send an invalid
MIC packet from another device to test the countermeasures.  Should I
submit a new patch with this change when I've completed testing or are
you already prepared to do so?

Michael

On Fri, May 12, 2017 at 4:52 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> Here's the driver code from rt2500usb (but it's similar in the others):
>
>                 rxdesc->flags |= RX_FLAG_MMIC_STRIPPED;
>                 if (rxdesc->cipher_status == RX_CRYPTO_SUCCESS)
>                         rxdesc->flags |= RX_FLAG_DECRYPTED;
>                 else if (rxdesc->cipher_status == RX_CRYPTO_FAIL_MIC)
>                         rxdesc->flags |= RX_FLAG_MMIC_ERROR;
>
> I think if you just change it to be
>
> [...]
>                 else if (rxdesc->cipher_status == RX_CRYPTO_FAIL_MIC)
>                         rxdesc->flags |= RX_FLAG_MMIC_ERROR |
>                                          RX_FLAG_DECRYPTED;
>
> things will start working. This is arguably correct since to be able to
> check the MMIC, the frame has to have been decrypted (properly) before.
>
> johannes

Re: [PATCH] mac80211: Validate michael MIC before attempting packet decode.

[Johannes Berg]

On Tue, 2017-05-16 at 15:57 -0400, Michael Skeffington wrote:
> Johannes,
> 
> Thank you for that.  I need to make a quick hack to send an invalid
> MIC packet from another device to test the countermeasures.  Should I
> submit a new patch with this change when I've completed testing or
> are you already prepared to do so?

Please do, you're able to test it, and I'm not really all that familiar
with that particular driver either, even if I maintain mac80211 :)

johannes