Re: [PATCH v2] mac80211: Encrypt "Group addressed privacy" action frames

2016-06-22 Thread Masashi Honma 

On 2016年06月22日 01:42, Jouni Malinen wrote:


This is somewhat problematic since no indication of the frame length is
passed in here and we are reading beyond the frame header below.. Maybe
this should use the same style as ieee80211_is_robust_mgmt_frame()
instead, i.e., use skb as the argument and define
_ieee80211_is_group_privacy_action() to take in struct ieee80211_hdr *.

Thank you. I will modify it.




These read the buffer at offset 24, i.e., just after the header. This
should do same as ieee80211_is_robust_mgmt_frame(), i.e., return false
if skb->len < 25.

Thank you. I will modify it also.



This looks completely separate item and I don't see why we would even
delete that empty line.. In any case, it should probably not be in this
patch.
   


Sorry. This is just a miss of removal of printk().

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

Re: [PATCH v2] mac80211: Encrypt "Group addressed privacy" action frames

2016-06-21 Thread Jouni Malinen 
On Tue, Jun 21, 2016 at 03:23:39PM +0900, Masashi Honma wrote:

> diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
> +static inline bool ieee80211_is_group_privacy_action(struct ieee80211_hdr 
> *hdr)

This is somewhat problematic since no indication of the frame length is
passed in here and we are reading beyond the frame header below.. Maybe
this should use the same style as ieee80211_is_robust_mgmt_frame()
instead, i.e., use skb as the argument and define
_ieee80211_is_group_privacy_action() to take in struct ieee80211_hdr *.

> + return mgmt->u.action.category == WLAN_CATEGORY_MESH_ACTION ||
> + mgmt->u.action.category == WLAN_CATEGORY_MULTIHOP_ACTION;

These read the buffer at offset 24, i.e., just after the header. This
should do same as ieee80211_is_robust_mgmt_frame(), i.e., return false
if skb->len < 25.

> diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> @@ -608,7 +611,6 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
>   bool skip_hw = false;
>  
>   /* TODO: add threshold stuff again */
> -
>   switch (tx->key->conf.cipher) {
>   case WLAN_CIPHER_SUITE_WEP40:
>   case WLAN_CIPHER_SUITE_WEP104:

This looks completely separate item and I don't see why we would even
delete that empty line.. In any case, it should probably not be in this
patch.
 
-- 
Jouni MalinenPGP id EFC895FA
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] mac80211: Encrypt "Group addressed privacy" action frames

2016-06-21 Thread Masashi Honma 
Previously, the action frames to group address was not encrypted. But
[1] "Table 8-38 Category values" indicates "Mesh" and "Multihop" category
action frames should be encrypted (Group addressed privacy == yes). And the
encyption key should be MGTK ([1] 10.13 Group addressed robust management frame
procedures). So this patch modifies the code to make it suitable for spec.

[1] IEEE Std 802.11-2012

Signed-off-by: Masashi Honma 
---
 include/linux/ieee80211.h | 20 
 net/mac80211/tx.c |  7 +--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index b118744..3ff7d3f 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -19,6 +19,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -2487,6 +2488,25 @@ static inline bool ieee80211_is_public_action(struct 
ieee80211_hdr *hdr,
 }
 
 /**
+ * ieee80211_is_group_privacy_action - check if frame is a group addressed
+ * privacy action frame
+ * @hdr: the frame
+ */
+static inline bool ieee80211_is_group_privacy_action(struct ieee80211_hdr *hdr)
+{
+   struct ieee80211_mgmt *mgmt;
+
+   if (!ieee80211_is_action(hdr->frame_control) ||
+   !is_multicast_ether_addr(hdr->addr1))
+   return false;
+
+   mgmt = (struct ieee80211_mgmt *)hdr;
+
+   return mgmt->u.action.category == WLAN_CATEGORY_MESH_ACTION ||
+   mgmt->u.action.category == WLAN_CATEGORY_MULTIHOP_ACTION;
+}
+
+/**
  * ieee80211_tu_to_usec - convert time units (TU) to microseconds
  * @tu: the TUs
  */
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2030443..5ad205e 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -590,6 +590,9 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
else if (tx->sta &&
 (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
tx->key = key;
+   else if (ieee80211_is_group_privacy_action(hdr) &&
+(key = rcu_dereference(tx->sdata->default_multicast_key)))
+   tx->key = key;
else if (ieee80211_is_mgmt(hdr->frame_control) &&
 is_multicast_ether_addr(hdr->addr1) &&
 ieee80211_is_robust_mgmt_frame(tx->skb) &&
@@ -608,7 +611,6 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
bool skip_hw = false;
 
/* TODO: add threshold stuff again */
-
switch (tx->key->conf.cipher) {
case WLAN_CIPHER_SUITE_WEP40:
case WLAN_CIPHER_SUITE_WEP104:
@@ -622,7 +624,8 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx)
case WLAN_CIPHER_SUITE_GCMP_256:
if (!ieee80211_is_data_present(hdr->frame_control) &&
!ieee80211_use_mfp(hdr->frame_control, tx->sta,
-  tx->skb))
+  tx->skb) &&
+   !ieee80211_is_group_privacy_action(hdr))
tx->key = NULL;
else
skip_hw = (tx->key->conf.flags &
-- 
2.5.0

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