Skip to contentSkip to conversation

ath9k truncated management packets from TKIP connected stations

ath9k-devel New Bill Jordan 5 participants 9 messages Aug 16, 2011 – Aug 24, 2011 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore

Bill Jordan

author
#0 lore raw

I'm not quite sure what the correct fix is for this.

Ath9k in AP mode with a TKIP security: If a connected station sends a management packet, the packet is truncated by 8 bytes before being delivered to hostapd. This prevents the station from reauthenticating or connecting to a different SSID on the same radio.

In ath9k_rx_accept, for management packets, strip_mic will be true, and RX_FLAG_MMIC_STRIPPED will be set in rxs->flag. In ath9k_rx_skb_postprocess, if ah->sw_mgmt_crypto is set, RX_FLAG_DECRYPTED will be cleared. However, RX_FLAG_MMIC_STRIPPED will still be set, so, in ath_rx_tasklet, 8 bytes will be trimmed off the end of the skb.

I'm thinking that in ath9k_rx_accept, is_valid_tkip ?should also consider ieee80211_is_mgmt(fc). But this wouldn't take into consideration ah->sw_mgmt_crypto.

Alternatively, RX_FLAG_MMIC_STRIPPED could be cleared in ath9k_rx_skb_postprocess when RX_FLAG_DECRYPTED is cleared.

I'm looking for input from someone who understands this code better.

Thanks,
Bill Jordan

#1 lore raw
Quoted 14 lines · Bill Jordan

I'm not quite sure what the correct fix is for this.

Ath9k in AP mode with a TKIP security: If a connected station sends a management packet, the packet is truncated by 8 bytes before being delivered to hostapd. This prevents the station from reauthenticating or connecting to a different SSID on the same radio.

In ath9k_rx_accept, for management packets, strip_mic will be true, and RX_FLAG_MMIC_STRIPPED will be set in rxs->flag. In ath9k_rx_skb_postprocess, if ah->sw_mgmt_crypto is set, RX_FLAG_DECRYPTED will be cleared. However, RX_FLAG_MMIC_STRIPPED will still be set, so, in ath_rx_tasklet, 8 bytes will be trimmed off the end of the skb.

Quoted 8 lines

I'm thinking that in ath9k_rx_accept, is_valid_tkip should also consider ieee80211_is_mgmt(fc). But this wouldn't take into consideration ah->sw_mgmt_crypto.

Alternatively, RX_FLAG_MMIC_STRIPPED could be cleared in ath9k_rx_skb_postprocess when RX_FLAG_DECRYPTED is cleared.

I'm looking for input from someone who understands this code better.

We should probably just keep strip_mic set to false for mgmt frames.

- Felix

author
#2 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

Signed-off-by Bill Jordan <[email protected]>
---
 drivers/net/wireless/ath/ath9k/recv.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/net/wireless/ath/ath9k/recv.c +2 -1
index 7409402..d8737f2 100644
@@ -824,7 +824,8 @@ static bool ath9k_rx_accept(struct ath_common *common,
 	is_mc = !!is_multicast_ether_addr(hdr->addr1);
 	is_valid_tkip = rx_stats->rs_keyix != ATH9K_RXKEYIX_INVALID &&
 		test_bit(rx_stats->rs_keyix, common->tkip_keymap);
-	strip_mic = is_valid_tkip && !(rx_stats->rs_status &
+	strip_mic = is_valid_tkip && !ieee80211_is_mgmt(fc) &&
+		!(rx_stats->rs_status &
 		(ATH9K_RXERR_DECRYPT | ATH9K_RXERR_CRC | ATH9K_RXERR_MIC));
 
 	if (!rx_stats->rs_datalen)
Signature · 1 line
1.7.6
#3 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Quoted 13 lines · Bill Jordan

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

Signed-off-by Bill Jordan<[email protected]>
---
  drivers/net/wireless/ath/ath9k/recv.c |    3 ++-
  1 files changed, 2 insertions(+), 1 deletions(-)
drivers/net/wireless/ath/ath9k/recv.c +0 -0
index 7409402..d8737f2 100644

drivers/net/wireless/ath/ath9k/recv.c static bool ath9k_rx_accept(struct ath_common *common, · line 831

Quoted 9 lines
drivers/net/wireless/ath/ath9k/recv.c +2 -1
@@ -824,7 +824,8 @@ static bool ath9k_rx_accept(struct ath_common *common,
  	is_mc = !!is_multicast_ether_addr(hdr->addr1);
  	is_valid_tkip = rx_stats->rs_keyix != ATH9K_RXKEYIX_INVALID&&
  		test_bit(rx_stats->rs_keyix, common->tkip_keymap);
-	strip_mic = is_valid_tkip&&  !(rx_stats->rs_status&
+	strip_mic = is_valid_tkip&&  !ieee80211_is_mgmt(fc)&&
+		!(rx_stats->rs_status&
  		(ATH9K_RXERR_DECRYPT | ATH9K_RXERR_CRC | ATH9K_RXERR_MIC));
 
  	if (!rx_stats->rs_datalen)

Maybe it would be a better idea to check for ieee80211_is_data(fc) instead of !ieee80211_is_mgmt(fc) - just to avoid more potential corner cases.

- Felix

author
#4 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

Signed-off-by Bill Jordan <[email protected]>
---
 drivers/net/wireless/ath/ath9k/recv.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/net/wireless/ath/ath9k/recv.c +2 -1
index 7409402..9adddd9 100644
@@ -824,7 +824,8 @@ static bool ath9k_rx_accept(struct ath_common *common,
 	is_mc = !!is_multicast_ether_addr(hdr->addr1);
 	is_valid_tkip = rx_stats->rs_keyix != ATH9K_RXKEYIX_INVALID &&
 		test_bit(rx_stats->rs_keyix, common->tkip_keymap);
-	strip_mic = is_valid_tkip && !(rx_stats->rs_status &
+	strip_mic = is_valid_tkip && ieee80211_is_data(fc) &&
+		!(rx_stats->rs_status &
 		(ATH9K_RXERR_DECRYPT | ATH9K_RXERR_CRC | ATH9K_RXERR_MIC));
 
 	if (!rx_stats->rs_datalen)
Signature · 1 line
1.7.6
#5 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Quoted 2 lines · Bill Jordan

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

A bit more information in the commit log would be nice. Is this is a regression or an old bug? Maybe a stable candidate?

Signature · 1 line
Kalle Valo
author
#6 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Quoted 7 lines · Kalle Valo
Quoted 2 lines · Bill Jordan

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

A bit more information in the commit log would be nice. Is this is a regression or an old bug? Maybe a stable candidate?

I believe it is an old bug.

Bill Jordan

#7 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Quoted 10 lines · Bill Jordan
Quoted 7 lines · Kalle Valo
Quoted 2 lines · Bill Jordan

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

A bit more information in the commit log would be nice. Is this is a regression or an old bug? Maybe a stable candidate?

I believe it is an old bug.

No it's not. It was introduced by:

commit 66760eac005d569393bac34136bcbb8af55d8a5a

Author: Felix Fietkau <[email protected]>
Date:   Wed Jul 13 23:35:05 2011 +0800

ath9k: improve reliability of MIC error detection

#8 lore raw

PATCH 1/1 ath9k: fix MGMT packets when using TKIP

Quoted 20 lines · Christian Lamparter
Quoted 10 lines · Bill Jordan
Quoted 7 lines · Kalle Valo
Quoted 2 lines · Bill Jordan

Prevent 8 bytes from being truncated from MGMT packets when using TKIP.

A bit more information in the commit log would be nice. Is this is a regression or an old bug? Maybe a stable candidate?

I believe it is an old bug.

No it's not. It was introduced by:

Yes it is. Stripping mic was recently added.

commit 66760eac005d569393bac34136bcbb8af55d8a5a

Author: Felix Fietkau <[email protected]>
Date:   Wed Jul 13 23:35:05 2011 +0800
Quoted 7 lines

ath9k: improve reliability of MIC error detection

_______________________________________________
ath9k-devel mailing list
ath9k-devel at lists.ath9k.org
https://lists.ath9k.org/mailman/listinfo/ath9k-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20110824/ad8e19cf/attachment.htm