mac80211 drops packet with old IV after rekeying

Commit Message

Alexander Wetzel May 18, 2015, 7:47 p.m. UTC

Hello,

I'm the one banging my head against this issue for quite some time, so
if I can do anything to help here contact me. I'll check the mailing
list from time to time but if you have something I should reply please
add/keep me on CC.

I can now reproduce the issue reliable within minutes on demand and can
also patch the kernels at both ends. (Just started looking at openwrt
and got my first openwrt kernel patch crashing the wlan driver:-)

But now to the topic:

> Right. I think the "new key with old PN" part is probably not really
> happening, although it seems possible. I'd think we should look at the
> receiver first and only then move on to the transmitter if issues
> persist. Having a sniffer capture of the problem with known keys (!)
> would be useful though.

For my understanding that has already be done. And at least for me it
looks like we have hard evidence for that fact.

In the linux bug report you can find an capture extract to verify that -
taken with a remote monitor station - and the PSK's needed to decrypt
the traffic:
https://bugzilla.kernel.org/show_bug.cgi?id=92451
You are probably interested in comment 14.

Maybe a short warning here:
The wireshark patch needed to make sense of these captures was written
by me and on my still sketchy understanding how this all works. But I
see no way how it could mix up keys and all it all only a minor
modification was needed. (It's taking a short cut to decide if it will
add the PSK to the packet by only looking at the key index and not at
the appropriate flags, but hardly relevant here.)

The Key information used to decrypt the packets is added in the same
section as the key index, if you have problems finding it.

This is an older capture and I'll verify that with a new one soon.
I have quite many retransmissions in it and the monitoring station also
missed quite some frames for some incomprehensible reason.

If you wanted the full capture and not the sipped down one from the
kernel bugzilla you can download it here:
https://cal.a.wdyn.eu:65443/index.php/s/UdMpcULG16Lz1Ah

I'll hope I can provide a better one at the weekend.

I have also still some output of my poking around with printk's in the
kernel and attached you an sample from them, see debug-out.txt.gz.
I have not preserved the exact kernel patch for those printk messages,
but attached a version close of it.
(The additional MIC check was a failed experiment of many to get it
working without removing the replay detection mechanism.)

The XXX debug lines are not in the patch, these are just some printk's
at the start of the function with the name printed out to see when the
key was installed.

Comments

Johannes Berg May 18, 2015, 9:55 p.m. UTC | #1

On Mon, 2015-05-18 at 21:47 +0200, Alexander Wetzel wrote:

> For my understanding that has already be done. And at least for me it
> looks like we have hard evidence for that fact.
[...]
> The Key information used to decrypt the packets is added in the same
> section as the key index, if you have problems finding it.

[building a new wireshark was awkward ... between their git being really
slow and the build needing to be completely deleted first ...]

I agree with you - what you can see in the capture, assuming the TK/PMK
display is correct, is that

packet 11: PN 0x11F2B, old key
packet 15: PN 0x11F40, old key 
packet 19: PN 0x11F2C, new key

Note how packet 15, since it's VO priority, goes out far before packet
19, although packet 19 got the sequence number immediately after packet
11.

So... I guess we can, for now, go back to my earlier email and look at
the transmitter problem after all. I still think the receiver has a
similar issue though.

To be honest though, I'm not sure how to really solve this. Without
multi index capability, the spec doesn't really support PTK rekeying
well. With it, this is clearly no problem, but that would depend on more
code and driver support etc. and perhaps can't even be done with all
drivers/devices.

The first idea here would be to stop using HW crypto for TX while
changing the key, but I think at least ath10k wouldn't support that, not
sure what would happen though? Either way, it'd need a TX path flush, so
I guess it doesn't really make a difference.

So really, I guess what we need to do - and this will suck for
performance - is to stop queues and flush the TX path while the old key
is still programmed into the device, reinstall the key, and only then
restart transmission...

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

--- ../wpa.c	2015-04-13 00:12:50.000000000 +0200
+++ net/mac80211/wpa.c	2015-05-14 21:01:44.030860184 +0200
@@ -1,5 +1,4 @@ 
 /*
- * Copyright 2002-2004, Instant802 Networks, Inc.
  * Copyright 2008, Jouni Malinen <[e-mail]>
  *
  * This program is free software; you can redistribute it and/or modify
@@ -502,29 +501,48 @@ 
 
 	if (!ieee80211_is_data(hdr->frame_control) &&
 	    !ieee80211_is_robust_mgmt_frame(skb))
+	        {printk(KERN_DEBUG "DDD - 1");
 		return RX_CONTINUE;
+		}
 
 	data_len = skb->len - hdrlen - IEEE80211_CCMP_HDR_LEN - mic_len;
 	if (!rx->sta || data_len < 0)
+		{printk(KERN_DEBUG "DDD - 2");
 		return RX_DROP_UNUSABLE;
+		}
 
 	if (status->flag & RX_FLAG_DECRYPTED) {
 		if (!pskb_may_pull(rx->skb, hdrlen + IEEE80211_CCMP_HDR_LEN))
+			{printk(KERN_DEBUG "DDD - 3");
 			return RX_DROP_UNUSABLE;
+			}
 	} else {
 		if (skb_linearize(rx->skb))
+			{printk(KERN_DEBUG "DDD - 4");
 			return RX_DROP_UNUSABLE;
+			}
 	}
 
 	ccmp_hdr2pn(pn, skb->data + hdrlen);
 
 	queue = rx->security_idx;
+/*
+	temp = ieee80211_rx_h_michael_mic_verify(rx);
+	printk(KERN_DEBUG "DDD - 5 queue:%i Mutex=%i MIC=%i",queue, rx->local->key_mtx.count, temp);
+
+	if(temp == RX_DROP_UNUSABLE) {
+		printk(KERN_DEBUG "DDD - MIC verify failed");
+		return RX_DROP_UNUSABLE;
+	}
 
+	print_hex_dump_debug("cnt: ", DUMP_PREFIX_OFFSET, IEEE80211_CCMP_PN_LEN, 1, key->u.ccmp.rx_pn[queue], IEEE80211_CCMP_PN_LEN, false);
+	print_hex_dump_debug("pn : ", DUMP_PREFIX_OFFSET, IEEE80211_CCMP_PN_LEN, 1, pn, IEEE80211_CCMP_PN_LEN, false);
 	if (memcmp(pn, key->u.ccmp.rx_pn[queue], IEEE80211_CCMP_PN_LEN) <= 0) {
 		key->u.ccmp.replays++;
+		//print_hex_dump_debug("skb->data: ", DUMP_PREFIX_OFFSET, 16, 1, skb->data, skb->len, true);
 		return RX_DROP_UNUSABLE;
 	}
-
+*/
 	if (!(status->flag & RX_FLAG_DECRYPTED)) {
 		u8 aad[2 * AES_BLOCK_SIZE];
 		u8 b_0[AES_BLOCK_SIZE];
@@ -536,17 +554,21 @@ 
 			    skb->data + hdrlen + IEEE80211_CCMP_HDR_LEN,
 			    data_len,
 			    skb->data + skb->len - mic_len, mic_len))
+			{printk(KERN_DEBUG "DDD - 6");
 			return RX_DROP_UNUSABLE;
+			}
 	}
 
 	memcpy(key->u.ccmp.rx_pn[queue], pn, IEEE80211_CCMP_PN_LEN);
 
 	/* Remove CCMP header and MIC */
 	if (pskb_trim(skb, skb->len - mic_len))
+		{printk(KERN_DEBUG "DDD - 7");
 		return RX_DROP_UNUSABLE;
+		}
 	memmove(skb->data + IEEE80211_CCMP_HDR_LEN, skb->data, hdrlen);
 	skb_pull(skb, IEEE80211_CCMP_HDR_LEN);
-
+	printk(KERN_DEBUG "DDD - 8");
 	return RX_CONTINUE;
 }