Skip to contentSkip to conversation

ath: Fix hardware decryption of WEP

PATCH New Bill Jordan 3 participants 5 messages Aug 8, 2011 – Aug 9, 2011 Atheros ath generic utilities

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ath: Fix hardware decryption of WEP

The first 4 hardware key indexes are reserved for WEP, but never programmed. The result was that WEP always used software decryption.

---
 drivers/net/wireless/ath/key.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/net/wireless/ath/key.c +5 -3
index 17b0efd..73177f1 100644
@@ -501,7 +501,11 @@ int ath_key_config(struct ath_common *common,
 	if (key->keylen)
 		memcpy(hk.kv_val, key->key, key->keylen);
 
-	if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
+	if (hk.kv_type == ATH_CIPHER_WEP) {
+		if (key->keyidx >= IEEE80211_WEP_NKID)
+			return -EOPNOTSUPP;
+		idx = key->keyidx;
+	} else if (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
 		switch (vif->type) {
 		case NL80211_IFTYPE_AP:
 			memcpy(gmac, vif->addr, ETH_ALEN);
@@ -577,8 +581,6 @@ EXPORT_SYMBOL(ath_key_config);
 void ath_key_delete(struct ath_common *common, struct ieee80211_key_conf *key)
 {
 	ath_hw_keyreset(common, key->hw_key_idx);
-	if (key->hw_key_idx < IEEE80211_WEP_NKID)
-		return;
 
 	clear_bit(key->hw_key_idx, common->keymap);
 	if (key->cipher != WLAN_CIPHER_SUITE_TKIP)
Signature · 1 line
1.7.4.4
#1 lore raw

PATCH 1/1 ath: Fix hardware decryption of WEP

Quoted 14 lines · Bill Jordan

The first 4 hardware key indexes are reserved for WEP, but never programmed. The result was that WEP always used software decryption.

---
  drivers/net/wireless/ath/key.c |    8 +++++---
  1 files changed, 5 insertions(+), 3 deletions(-)
drivers/net/wireless/ath/key.c +0 -0
index 17b0efd..73177f1 100644

drivers/net/wireless/ath/key.c int ath_key_config(struct ath_common *common, · line 508

Quoted 9 lines
drivers/net/wireless/ath/key.c +5 -1
@@ -501,7 +501,11 @@ int ath_key_config(struct ath_common *common,
  	if (key->keylen)
  		memcpy(hk.kv_val, key->key, key->keylen);
 
-	if (!(key->flags&  IEEE80211_KEY_FLAG_PAIRWISE)) {
+	if (hk.kv_type == ATH_CIPHER_WEP) {
+		if (key->keyidx>= IEEE80211_WEP_NKID)
+			return -EOPNOTSUPP;
+		idx = key->keyidx;
+	} else if (!(key->flags&  IEEE80211_KEY_FLAG_PAIRWISE)) {

I'm not sure this is a good idea, this would break adding pairwise WEP keys. Also, maybe WEP keys could use the BSSID in the lookup to avoid the first 4 keycache slots, that way it'd work properly in multi-BSSID AP setups.

- Felix

#2 lore raw

PATCH 1/1 ath: Fix hardware decryption of WEP

Quoted 4 lines · Felix Fietkau
Quoted 3 lines · Bill Jordan

The first 4 hardware key indexes are reserved for WEP, but never programmed. The result was that WEP always used software decryption.

Not really.. They have a special role receiving group addressed frames and may be used with static WEP configuration, but there is no strict reservation of them being for WEP or other key indexes not being available for WEP.

Quoted 5 lines
Quoted 5 lines
drivers/net/wireless/ath/key.c +0 -0
@@ -501,7 +501,11 @@ int ath_key_config(struct ath_common *common,
  	if (key->keylen)
  		memcpy(hk.kv_val, key->key, key->keylen);
 
Quoted 10 lines

drivers/net/wireless/ath/key.c int ath_key_config(struct ath_common *common,

Quoted 6 lines
drivers/net/wireless/ath/key.c +5 -1
-	if (!(key->flags&  IEEE80211_KEY_FLAG_PAIRWISE)) {
+	if (hk.kv_type == ATH_CIPHER_WEP) {
+		if (key->keyidx>= IEEE80211_WEP_NKID)
+			return -EOPNOTSUPP;
+		idx = key->keyidx;
+	} else if (!(key->flags&  IEEE80211_KEY_FLAG_PAIRWISE)) {

I'm not sure this is a good idea, this would break adding pairwise WEP keys. Also, maybe WEP keys could use the BSSID in the lookup to avoid the first 4 keycache slots, that way it'd work properly in multi-BSSID AP setups.

Agreed, this should not be applied.

As far as AP mode is concerned, there is no need to actually receive group addressed frames, so other key slots can be used regardless of the key index. Multi-BSSID AP setup with multiple BSSes that use static WEP keys is not going to work well, but then again, no one really should be using WEP anymore and I have no problems forcing that special use case to use software encryption/decryption. While dynamic WEP key setup with IEEE 802.1X is not really that much better, doing changes to improve some static WEP cases at the cost of reduced dynamic WEP functionality does not sound like a good direction.

If the goal here was to improve station mode functionality in the case where a single WEP network is being used, it should be done more carefully to avoid causing problems to other cases.

Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA
#3 lore raw

PATCH 1/1 ath: Fix hardware decryption of WEP

Quoted 39 lines · Jouni Malinen
Quoted 4 lines · Felix Fietkau
Quoted 3 lines · Bill Jordan

The first 4 hardware key indexes are reserved for WEP, but never programmed. The result was that WEP always used software decryption.

Not really.. They have a special role receiving group addressed frames and may be used with static WEP configuration, but there is no strict reservation of them being for WEP or other key indexes not being available for WEP.

Quoted 5 lines
Quoted 5 lines
drivers/net/wireless/ath/key.c +0 -0
@@ -501,7 +501,11 @@ int ath_key_config(struct ath_common *common,
 ? ? if (key->keylen)
 ? ? ? ? ? ? memcpy(hk.kv_val, key->key, key->keylen);
 
Quoted 10 lines

drivers/net/wireless/ath/key.c int ath_key_config(struct ath_common *common, · line 508

Quoted 6 lines
drivers/net/wireless/ath/key.c +5 -1
- ? if (!(key->flags& ?IEEE80211_KEY_FLAG_PAIRWISE)) {
+ ? if (hk.kv_type == ATH_CIPHER_WEP) {
+ ? ? ? ? ? if (key->keyidx>= IEEE80211_WEP_NKID)
+ ? ? ? ? ? ? ? ? ? return -EOPNOTSUPP;
+ ? ? ? ? ? idx = key->keyidx;
+ ? } else if (!(key->flags& ?IEEE80211_KEY_FLAG_PAIRWISE)) {

I'm not sure this is a good idea, this would break adding pairwise WEP keys. Also, maybe WEP keys could use the BSSID in the lookup to avoid the first 4 keycache slots, that way it'd work properly in multi-BSSID AP setups.

Agreed, this should not be applied.

As far as AP mode is concerned, there is no need to actually receive group addressed frames, so other key slots can be used regardless of the key index. Multi-BSSID AP setup with multiple BSSes that use static WEP keys is not going to work well, but then again, no one really should be using WEP anymore and I have no problems forcing that special use case to use software encryption/decryption. While dynamic WEP key setup with IEEE 802.1X is not really that much better, doing changes to improve some static WEP cases at the cost of reduced dynamic WEP functionality does not sound like a good direction.

Quoted 3 lines

If the goal here was to improve station mode functionality in the case where a single WEP network is being used, it should be done more carefully to avoid causing problems to other cases.

I was attempting to fix WEP AP mode. I was surprised to find that received data packets were decrypted in software.

I thought WEP should be programming the first 4 hardware keys based on code in ath9k_rx_skb_postprocess:

	} else if (ieee80211_has_protected(fc)
		   && !decrypt_error && skb->len >= hdrlen + 4) {
		keyix = skb->data[hdrlen + 3] >> 6;

		if (test_bit(keyix, common->keymap))
			rxs->flag |= RX_FLAG_DECRYPTED;
	}

This code only works if the first 4 hardware keys are used.

In order to support dynamic WEP keys, all WEP decryption must be done in software? Am I understanding that correctly?

Bill Jordan

#4 lore raw

PATCH 1/1 ath: Fix hardware decryption of WEP

Quoted 2 lines · Bill Jordan

I was attempting to fix WEP AP mode. I was surprised to find that received data packets were decrypted in software.

Ah, I see. This is bit problematic when keeping in mind that there could be multiple BSSes in use with different set of WEP configuration. If you put one of them in the key indexes 0..3, you may not be able to use the other BSSes correctly (in theory, it could be possible with some hacks when only a single WEP key is configured per BSS, but I don't think it is worth the effort to do that for WEP). As such, there would need to be code that is capable of dynamically kicking out the key cache entries 0..3 should another BSS be set up. I don't think mac80211 has support for this at the moment, so that may require considerable amount of extra work. And again, it is questionable whether there is really any good justification for doing this with WEP that is not even allowed to be used with HT and as such, is limited in throughput anyway.

Quoted 11 lines

I thought WEP should be programming the first 4 hardware keys based on code in ath9k_rx_skb_postprocess:

	} else if (ieee80211_has_protected(fc)
		   && !decrypt_error && skb->len >= hdrlen + 4) {
		keyix = skb->data[hdrlen + 3] >> 6;

		if (test_bit(keyix, common->keymap))
			rxs->flag |= RX_FLAG_DECRYPTED;
	}

This code only works if the first 4 hardware keys are used.

There are some semi-odd hacks needed for figuring out whether the frame was really decrypted in some cases (i.e., whether the flag in RX descriptor is valid). This is one of those corner cases..

Quoted 2 lines

In order to support dynamic WEP keys, all WEP decryption must be done in software? Am I understanding that correctly?

Not with dynamic WEP keys. With those, the AP does not really receive any broadcast/multicast frames and only the per-station individual key is used for RX at the AP. As such, hardware can pick the correct key based on the transmitter address.

The most problematic case with key cache and support for multiple BSSes is use of static WEP where the different BSSes have conflicting needs for finding the key on RX processing if the different BSSes share the same key index (and especially if they use multiple key indexes). While it would be theoretically possible to improve this, it is way too easy to break some other corner cases unless you are very careful with the key cache use in the driver. As such, I don't really want to spend the time reviewing this type of changes without a good justification for the improvement (and anything with WEP in it is unlikely to be considered good justification in 2011 ;-).

Signature · 1 line
Jouni Malinen                                            PGP id EFC895FA