[RFC] mac80211: validate key before MIC verify

[RFC] mac80211: validate key before MIC verify

[Stanislaw Gruszka]

I have strange crash on rt61pci hardware when switching off radio
by rfkill switch:
https://bugzilla.redhat.com/attachment.cgi?id=615362   

After debugging the issue, I figured out problem happens because
key->u.ccmp.tfm of group key get corrupted. Corruption happen in

ieee80211_rx_h_michael_mic_verify():

        /* update IV in key information to be able to detect replays */
        rx->key->u.tkip.rx[rx->security_idx].iv32 = rx->tkip_iv32;
        rx->key->u.tkip.rx[rx->security_idx].iv16 = rx->tkip_iv16;

because rt61pci always set RX_FLAG_MMIC_STRIPPED and RX_FLAG_IV_STRIPPED
flags.

This problem was introduces in:

816c04f mac80211: consolidate MIC failure report handling

which already has fixes of invalid usage of rx->key pointer:

1140afa mac80211: fix rx->key NULL pointer dereference in promiscuous mode
a66b98d mac80211: fix rx->key NULL dereference during mic failure

This patch fix the problem by checking for key pointer is valid and if
key type is TKIP, before doing any other MIC verification.

Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

---
I did not test patch ...

diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index bdb53ab..6f800f7 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -97,6 +97,14 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
 		return RX_CONTINUE;
 
 	/*
+	 * Some hardware seems to generate Michael MIC failure reports; even
+	 * though, the frame was not encrypted with TKIP and therefore has no
+	 * MIC. Ignore the flag them to avoid triggering countermeasures.
+	 */
+	if (!rx->key || rx->key->conf.cipher != WLAN_CIPHER_SUITE_TKIP)
+		return RX_CONTINUE;
+
+	/*
 	 * 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
@@ -106,19 +114,13 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
 		if (status->flag & RX_FLAG_MMIC_ERROR)
 			goto mic_fail;
 
-		if (!(status->flag & RX_FLAG_IV_STRIPPED) && rx->key)
+		if (!(status->flag & RX_FLAG_IV_STRIPPED))
 			goto update_iv;
 
 		return RX_CONTINUE;
 	}
 
-	/*
-	 * Some hardware seems to generate Michael MIC failure reports; even
-	 * though, the frame was not encrypted with TKIP and therefore has no
-	 * MIC. Ignore the flag them to avoid triggering countermeasures.
-	 */
-	if (!rx->key || rx->key->conf.cipher != WLAN_CIPHER_SUITE_TKIP ||
-	    !(status->flag & RX_FLAG_DECRYPTED))
+	if (!(status->flag & RX_FLAG_DECRYPTED))
 		return RX_CONTINUE;
 
 	if (rx->sdata->vif.type == NL80211_IFTYPE_AP && rx->key->conf.keyidx) {
@@ -165,8 +167,7 @@ mic_fail:
 	 * a driver that supports HW encryption. Send up the key idx only if
 	 * the key is set.
 	 */
-	mac80211_ev_michael_mic_failure(rx->sdata,
-					rx->key ? rx->key->conf.keyidx : -1,
+	mac80211_ev_michael_mic_failure(rx->sdata, rx->key->conf.keyidx,
 					(void *) skb->data, NULL, GFP_ATOMIC);
 	return RX_DROP_UNUSABLE;
 }

Re: [RFC] mac80211: validate key before MIC verify

[Johannes Berg]

On Fri, 2012-09-21 at 14:41 +0200, Stanislaw Gruszka wrote:

> --- a/net/mac80211/wpa.c
> +++ b/net/mac80211/wpa.c
> @@ -97,6 +97,14 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
>  		return RX_CONTINUE;
>  
>  	/*
> +	 * Some hardware seems to generate Michael MIC failure reports; even
> +	 * though, the frame was not encrypted with TKIP and therefore has no
> +	 * MIC. Ignore the flag them to avoid triggering countermeasures.
> +	 */
> +	if (!rx->key || rx->key->conf.cipher != WLAN_CIPHER_SUITE_TKIP)
> +		return RX_CONTINUE;
> +
> +	/*
>  	 * 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

Hm, this doesn't seem _quite_ right, but I'm not sure: it seems that
previously it was possible that we don't have a key pointer but the
driver set all of RX_FLAG_MMIC_STRIPPED, RX_FLAG_IV_STRIPPED and
RX_FLAG_MMIC_ERROR, in which case after your change the frame will be
accepted rather than rejected.

johannes

Re: [RFC] mac80211: validate key before MIC verify

[Stanislaw Gruszka]

On Fri, Sep 21, 2012 at 02:59:40PM +0200, Johannes Berg wrote:
> On Fri, 2012-09-21 at 14:41 +0200, Stanislaw Gruszka wrote:
> 
> > --- a/net/mac80211/wpa.c
> > +++ b/net/mac80211/wpa.c
> > @@ -97,6 +97,14 @@ ieee80211_rx_h_michael_mic_verify(struct ieee80211_rx_data *rx)
> >  		return RX_CONTINUE;
> >  
> >  	/*
> > +	 * Some hardware seems to generate Michael MIC failure reports; even
> > +	 * though, the frame was not encrypted with TKIP and therefore has no
> > +	 * MIC. Ignore the flag them to avoid triggering countermeasures.
> > +	 */
> > +	if (!rx->key || rx->key->conf.cipher != WLAN_CIPHER_SUITE_TKIP)
> > +		return RX_CONTINUE;
> > +
> > +	/*
> >  	 * 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
> 
> Hm, this doesn't seem _quite_ right, but I'm not sure: it seems that
> previously it was possible that we don't have a key pointer but the
> driver set all of RX_FLAG_MMIC_STRIPPED, RX_FLAG_IV_STRIPPED and
> RX_FLAG_MMIC_ERROR, in which case after your change the frame will be
> accepted rather than rejected.

I wanted to cleanup stuff, but yeah, that seem to be wrong. I guess
I can just add check before rx->key->u.tkip.rx usage to fix the
problem. Eventually fix flags setting in driver.

Stanislaw