* [PATCH] mac80211: Encrypt "Group addressed privacy" action frames @ 2016-06-15 5:38 Masashi Honma 2016-06-18 9:11 ` Jouni Malinen 0 siblings, 1 reply; 15+ messages in thread From: Masashi Honma @ 2016-06-15 5:38 UTC (permalink / raw) To: linux-wireless; +Cc: 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 <masashi.honma@gmail.com> --- net/mac80211/tx.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 2030443..80afc47 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -578,6 +578,21 @@ ieee80211_tx_h_check_control_port_protocol(struct ieee80211_tx_data *tx) return TX_CONTINUE; } +static bool debug_noinline +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; +} + static ieee80211_tx_result debug_noinline ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx) { @@ -590,6 +605,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) && @@ -620,6 +638,8 @@ ieee80211_tx_h_select_key(struct ieee80211_tx_data *tx) case WLAN_CIPHER_SUITE_CCMP_256: case WLAN_CIPHER_SUITE_GCMP: case WLAN_CIPHER_SUITE_GCMP_256: + if (ieee80211_is_group_privacy_action(hdr)) + break; if (!ieee80211_is_data_present(hdr->frame_control) && !ieee80211_use_mfp(hdr->frame_control, tx->sta, tx->skb)) -- 2.5.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-15 5:38 [PATCH] mac80211: Encrypt "Group addressed privacy" action frames Masashi Honma @ 2016-06-18 9:11 ` Jouni Malinen 2016-06-20 0:51 ` Masashi Honma 0 siblings, 1 reply; 15+ messages in thread From: Jouni Malinen @ 2016-06-18 9:11 UTC (permalink / raw) To: Masashi Honma; +Cc: linux-wireless On Wed, Jun 15, 2016 at 02:38:32PM +0900, Masashi Honma wrote: > 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. > net/mac80211/tx.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) What about RX side? Shouldn't there be a matching change there to enforce use of group addressed privacy for the specific Action categories? This will make devices using fixed implementation not interoperate with devices using older version, I'd assume, but it looks like the current use of mesh with RSN is pretty hopelessly broken as far as no PMF case is concerned at least when using the wpa_supplicant implementation (sets IGTK incorrectly and ends up using BIP even when PMF was not enabled), so there does not seem to be any convenient way of addressing this apart from requiring all devices in the MBSS to get updated to the fixed versions. > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c > +static bool debug_noinline > +ieee80211_is_group_privacy_action(struct ieee80211_hdr *hdr) And this helper should likely be in some more generic location so that it could be shared for TX and RX.. -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-18 9:11 ` Jouni Malinen @ 2016-06-20 0:51 ` Masashi Honma 2016-06-20 21:25 ` Jouni Malinen 0 siblings, 1 reply; 15+ messages in thread From: Masashi Honma @ 2016-06-20 0:51 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless On 2016年06月18日 18:11, Jouni Malinen wrote: > What about RX side? Shouldn't there be a matching change there to > enforce use of group addressed privacy for the specific Action > categories? Thank you. Yes, RX side modification is needed. I was not aware of it because ping test was OK. Now I recognize it is because MGTK and IGTK is same as you say. > This will make devices using fixed implementation not > interoperate with devices using older version, I'd assume, but it looks > like the current use of mesh with RSN is pretty hopelessly broken as far > as no PMF case is concerned at least when using the wpa_supplicant > implementation (sets IGTK incorrectly and ends up using BIP even when > PMF was not enabled), so there does not seem to be any convenient way of > addressing this apart from requiring all devices in the MBSS to get > updated to the fixed versions. Yes. This patch breaks backward compatibility. I do not have smart idea to avoid also. I will create new define like this. CONFIG_MAC80211_MESH_GROUP_ADDRESSED_PRIVACY > And this helper should likely be in some more generic location so that > it could be shared for TX and RX.. Sure. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-20 0:51 ` Masashi Honma @ 2016-06-20 21:25 ` Jouni Malinen 2016-06-21 6:16 ` Masashi Honma 2016-06-21 6:23 ` [PATCH v2] " Masashi Honma 0 siblings, 2 replies; 15+ messages in thread From: Jouni Malinen @ 2016-06-20 21:25 UTC (permalink / raw) To: Masashi Honma; +Cc: linux-wireless On Mon, Jun 20, 2016 at 09:51:28AM +0900, Masashi Honma wrote: > On 2016年06月18日 18:11, Jouni Malinen wrote: > Yes. This patch breaks backward compatibility. > I do not have smart idea to avoid also. > I will create new define like this. > CONFIG_MAC80211_MESH_GROUP_ADDRESSED_PRIVACY Do we really want that? Group addressed privacy is what the standard requires to be used with mesh and if we make it build time configurable, we'll just end up with two different implementation that will never interoperate with each other.. I don't really see any better option for this apart from fixing this and requiring all STAs in a secure mesh to be updated in synchronized manner. This way it will be a one time issue, but that won't be there forever. If something is needed to for temporary backwards compatibility support, that should be something that can be enabled at runtime (and be disabled by default). That said, I'm not sure I'd go with that extra complexity taken into account how badly (i.e., completely incorrectly) the PMF case was implemented in wpa_supplicant. I'm not planning on adding any backwards compatibility mode there for due to the previous behavior not really being good from security view point either (using the same key with two different algorithms). -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-20 21:25 ` Jouni Malinen @ 2016-06-21 6:16 ` Masashi Honma 2016-06-21 17:01 ` Jouni Malinen 2016-06-21 6:23 ` [PATCH v2] " Masashi Honma 1 sibling, 1 reply; 15+ messages in thread From: Masashi Honma @ 2016-06-21 6:16 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless On 2016年06月21日 06:25, Jouni Malinen wrote: > Do we really want that? Sorry, I mis-understood your previous massage. I have thought you required backward compatibility. Ok, I will remove backward compatibility code. > What about RX side? Previously, MGTK and IGTK was identical key. Now new wpa_supplicant can provide correct IGTK. (Because of your great works !) I have tested with new IGTK, RX side can work without modification. I will send new patch. The patch move ieee80211_is_group_privacy_action() to appropriate file and fix a bug of skip_hw flag. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-21 6:16 ` Masashi Honma @ 2016-06-21 17:01 ` Jouni Malinen 2016-06-21 19:40 ` Johannes Berg ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: Jouni Malinen @ 2016-06-21 17:01 UTC (permalink / raw) To: Masashi Honma; +Cc: linux-wireless On Tue, Jun 21, 2016 at 03:16:22PM +0900, Masashi Honma wrote: > On 2016年06月21日 06:25, Jouni Malinen wrote: > > What about RX side? > > Previously, MGTK and IGTK was identical key. > Now new wpa_supplicant can provide correct IGTK. > I have tested with new IGTK, RX side can work without > modification. Please keep in mind that "working" here means two things: (1) being able decrypt the frame, (2) being able to reject the frame if it was not properly protected. It is that (2) that is unlikely to be covered here.. We actually cover (2) for some cases by "accident" since ieee80211_rx_h_decrypt() assigns rx->key to rx->sta->gtk[i] if one is available. I'm not completely sure this is correct since it applies to management frame as well, but that's the way commit 897bed8b4320774e56f282cdc1cceb4d77442797 ('mac80211: clean up RX key checks') implemented it (Johannes: Could you please take a look whether that gtk[] case was really supposed to apply for non-Data frames?). Interestingly, even on the TX side, we had code that picked tx->key for these group addressed Action frames, but that got then cleared later.. That said, if rx->sta->gtk[i] is not set for any value of i, we would not enforce encryption of "group addressed privacy" Action frames as far as I can tell. This may be a pretty small window since RX MGTK is supposed to get set immediately for each peer. However, I would not be surprised if there were indeed a window between adding the STA entry and marking it authorized and configuring the RX MGTK. And even if this is not possible, this should really be commented somewhere so that there is less of a change of accidentally optimizing or cleaning up something that is needed for this to be protected.. And when operating with PMF enabled, this is clearly broken, i.e., the RX path accepts BIP protected version of the broadcast Mesh Action frame while that frame needs to be rejected since it was not encrypted with CCMP/GCMP. To cover all these RX cases properly, I'd expect there to be RX path changes that use ieee80211_is_group_privacy_action() and reject some cases.. This should like be there in the !ieee80211_has_protected(fc) case in ieee80211_rx_h_decrypt() before selecting the key and if ieee80211_is_group_privacy_action() returns true, return RX_DROP_MONITOR would be needed. -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-21 17:01 ` Jouni Malinen @ 2016-06-21 19:40 ` Johannes Berg 2016-06-22 10:54 ` Masashi Honma 2016-06-22 10:55 ` [PATCH v3] " Masashi Honma 2 siblings, 0 replies; 15+ messages in thread From: Johannes Berg @ 2016-06-21 19:40 UTC (permalink / raw) To: Jouni Malinen, Masashi Honma; +Cc: linux-wireless > We actually cover (2) for some cases by "accident" since > ieee80211_rx_h_decrypt() assigns rx->key to rx->sta->gtk[i] if one is > available. I'm not completely sure this is correct since it applies > to management frame as well, but that's the way commit > 897bed8b4320774e56f282cdc1cceb4d77442797 ('mac80211: clean up RX key > checks') implemented it (Johannes: Could you please take a look > whether that gtk[] case was really supposed to apply for non-Data > frames?). > Hm, yeah, that's kinda questionable. AFAICT we still do the right thing since ieee80211_drop_unencrypted() contains a check for ieee80211_is_data() and we return in this if branch, so we never get to the TAINTED check or the actual decrypt. We could try to just drop unencrypted data frames (that aren't control port protocol) right here, but it might wreak havoc with the reorder buffer in case it (erroneously) happens. johannes ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-21 17:01 ` Jouni Malinen 2016-06-21 19:40 ` Johannes Berg @ 2016-06-22 10:54 ` Masashi Honma 2016-06-22 10:55 ` [PATCH v3] " Masashi Honma 2 siblings, 0 replies; 15+ messages in thread From: Masashi Honma @ 2016-06-22 10:54 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless On 2016年06月22日 02:01, Jouni Malinen wrote: > Please keep in mind that "working" here means two things: > (1) being able decrypt the frame, > (2) being able to reject the frame if it was not properly protected. It > is that (2) that is unlikely to be covered here.. > > We actually cover (2) for some cases by "accident" since > ieee80211_rx_h_decrypt() assigns rx->key to rx->sta->gtk[i] if one is > available. I'm not completely sure this is correct since it applies to > management frame as well, but that's the way commit > 897bed8b4320774e56f282cdc1cceb4d77442797 ('mac80211: clean up RX key > checks') implemented it (Johannes: Could you please take a look whether > that gtk[] case was really supposed to apply for non-Data frames?). > Interestingly, even on the TX side, we had code that picked tx->key for > these group addressed Action frames, but that got then cleared later.. > > That said, if rx->sta->gtk[i] is not set for any value of i, we would > not enforce encryption of "group addressed privacy" Action frames as far > as I can tell. This may be a pretty small window since RX MGTK is > supposed to get set immediately for each peer. However, I would not be > surprised if there were indeed a window between adding the STA entry and > marking it authorized and configuring the RX MGTK. And even if this is > not possible, this should really be commented somewhere so that there is > less of a change of accidentally optimizing or cleaning up something > that is needed for this to be protected.. > > And when operating with PMF enabled, this is clearly broken, i.e., the > RX path accepts BIP protected version of the broadcast Mesh Action frame > while that frame needs to be rejected since it was not encrypted with > CCMP/GCMP. > > To cover all these RX cases properly, I'd expect there to be RX path > changes that use ieee80211_is_group_privacy_action() and reject some > cases.. This should like be there in the !ieee80211_has_protected(fc) > case in ieee80211_rx_h_decrypt() before selecting the key and if > ieee80211_is_group_privacy_action() returns true, return RX_DROP_MONITOR > would be needed. > Thank you Jouni and Johannes. Indeed, received unencrypted Group Addressed Privacy action frame is dropped at below if condition in ieee80211_drop_unencrypted_mgmt(). /* BIP does not use Protected field, so need to check MMIE */ if (unlikely(ieee80211_is_multicast_robust_mgmt_frame(rx->skb) && ieee80211_get_mmie_keyidx(rx->skb) < 0)) { if (ieee80211_is_deauth(fc) || ieee80211_is_disassoc(fc)) cfg80211_rx_unprot_mlme_mgmt(rx->sdata->dev, rx->skb->data, rx->skb->len); return -EACCES; } Because the frame was not encrypted and does not have MMIE. And there could be one more case. Group Addressed Privacy action frame could have robustness by MMIC because of previous wrong implementation. The frame could not be cought by ieee80211_drop_unencrypted_mgmt(). Because the frame has MMIE. So I have added new condition to ieee80211_rx_h_decrypt() by follwing patch. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-21 17:01 ` Jouni Malinen 2016-06-21 19:40 ` Johannes Berg 2016-06-22 10:54 ` Masashi Honma @ 2016-06-22 10:55 ` Masashi Honma 2016-06-29 15:08 ` Masashi Honma 2 siblings, 1 reply; 15+ messages in thread From: Masashi Honma @ 2016-06-22 10:55 UTC (permalink / raw) To: j, johannes; +Cc: linux-wireless, 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 <masashi.honma@gmail.com> --- include/linux/ieee80211.h | 32 ++++++++++++++++++++++++++++++++ net/mac80211/rx.c | 7 ++++++- net/mac80211/tx.c | 6 +++++- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h index b118744..7c0771a 100644 --- a/include/linux/ieee80211.h +++ b/include/linux/ieee80211.h @@ -19,6 +19,7 @@ #include <linux/types.h> #include <linux/if_ether.h> +#include <linux/etherdevice.h> #include <asm/byteorder.h> #include <asm/unaligned.h> @@ -2487,6 +2488,37 @@ 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_is_group_privacy_action - check if frame is a group addressed + * privacy action frame + * @skb: the skb containing the frame, length will be checked + */ +static inline bool ieee80211_is_group_privacy_action(struct sk_buff *skb) +{ + if (skb->len < IEEE80211_MIN_ACTION_SIZE) + return false; + return _ieee80211_is_group_privacy_action((void *)skb->data); +} + +/** * ieee80211_tu_to_usec - convert time units (TU) to microseconds * @tu: the TUs */ diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 5e65e83..2300c0f 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -1624,8 +1624,13 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx) if (mmie_keyidx < NUM_DEFAULT_KEYS || mmie_keyidx >= NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS) return RX_DROP_MONITOR; /* unexpected BIP keyidx */ - if (rx->sta) + if (rx->sta) { + if (ieee80211_is_group_privacy_action(skb) && + test_sta_flag(rx->sta, WLAN_STA_MFP)) + return RX_DROP_MONITOR; + rx->key = rcu_dereference(rx->sta->gtk[mmie_keyidx]); + } if (!rx->key) rx->key = rcu_dereference(rx->sdata->keys[mmie_keyidx]); } else if (!ieee80211_has_protected(fc)) { diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 2030443..e194df6 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(tx->skb) && + (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) && @@ -622,7 +625,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(tx->skb)) tx->key = NULL; else skip_hw = (tx->key->conf.flags & -- 2.5.0 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-22 10:55 ` [PATCH v3] " Masashi Honma @ 2016-06-29 15:08 ` Masashi Honma 2016-06-29 16:25 ` Johannes Berg 0 siblings, 1 reply; 15+ messages in thread From: Masashi Honma @ 2016-06-29 15:08 UTC (permalink / raw) To: j, johannes; +Cc: linux-wireless On 2016年06月22日 19:55, Masashi Honma wrote: > 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 <masashi.honma@gmail.com> > --- > include/linux/ieee80211.h | 32 ++++++++++++++++++++++++++++++++ > net/mac80211/rx.c | 7 ++++++- > net/mac80211/tx.c | 6 +++++- > 3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h > index b118744..7c0771a 100644 > --- a/include/linux/ieee80211.h > +++ b/include/linux/ieee80211.h > @@ -19,6 +19,7 @@ > > #include <linux/types.h> > #include <linux/if_ether.h> > +#include <linux/etherdevice.h> > #include <asm/byteorder.h> > #include <asm/unaligned.h> > > @@ -2487,6 +2488,37 @@ 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_is_group_privacy_action - check if frame is a group addressed > + * privacy action frame > + * @skb: the skb containing the frame, length will be checked > + */ > +static inline bool ieee80211_is_group_privacy_action(struct sk_buff *skb) > +{ > + if (skb->len < IEEE80211_MIN_ACTION_SIZE) > + return false; > + return _ieee80211_is_group_privacy_action((void *)skb->data); > +} > + > +/** > * ieee80211_tu_to_usec - convert time units (TU) to microseconds > * @tu: the TUs > */ > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > index 5e65e83..2300c0f 100644 > --- a/net/mac80211/rx.c > +++ b/net/mac80211/rx.c > @@ -1624,8 +1624,13 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx) > if (mmie_keyidx < NUM_DEFAULT_KEYS || > mmie_keyidx >= NUM_DEFAULT_KEYS + NUM_DEFAULT_MGMT_KEYS) > return RX_DROP_MONITOR; /* unexpected BIP keyidx */ > - if (rx->sta) > + if (rx->sta) { > + if (ieee80211_is_group_privacy_action(skb) && > + test_sta_flag(rx->sta, WLAN_STA_MFP)) > + return RX_DROP_MONITOR; > + > rx->key = rcu_dereference(rx->sta->gtk[mmie_keyidx]); > + } > if (!rx->key) > rx->key = rcu_dereference(rx->sdata->keys[mmie_keyidx]); > } else if (!ieee80211_has_protected(fc)) { > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c > index 2030443..e194df6 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(tx->skb) && > + (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) && > @@ -622,7 +625,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(tx->skb)) > tx->key = NULL; > else > skip_hw = (tx->key->conf.flags & Is there any comment about this ? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-29 15:08 ` Masashi Honma @ 2016-06-29 16:25 ` Johannes Berg 2016-06-29 23:20 ` Masashi Honma 0 siblings, 1 reply; 15+ messages in thread From: Johannes Berg @ 2016-06-29 16:25 UTC (permalink / raw) To: Masashi Honma, j; +Cc: linux-wireless On Thu, 2016-06-30 at 00:08 +0900, Masashi Honma wrote: [please don't quote everything to add a single line] > Is there any comment about this ? > I applied it today, but forgot to say so. Sorry about that. johannes ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-29 16:25 ` Johannes Berg @ 2016-06-29 23:20 ` Masashi Honma 0 siblings, 0 replies; 15+ messages in thread From: Masashi Honma @ 2016-06-29 23:20 UTC (permalink / raw) To: Johannes Berg, j; +Cc: linux-wireless On 2016年06月30日 01:25, Johannes Berg wrote: > [please don't quote everything to add a single line] > I applied it today, but forgot to say so. Thanks, and sorry for wrong quotation. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-20 21:25 ` Jouni Malinen 2016-06-21 6:16 ` Masashi Honma @ 2016-06-21 6:23 ` Masashi Honma 2016-06-21 16:42 ` Jouni Malinen 1 sibling, 1 reply; 15+ messages in thread From: Masashi Honma @ 2016-06-21 6:23 UTC (permalink / raw) To: j; +Cc: linux-wireless, 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 <masashi.honma@gmail.com> --- 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 <linux/types.h> #include <linux/if_ether.h> +#include <linux/etherdevice.h> #include <asm/byteorder.h> #include <asm/unaligned.h> @@ -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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-21 6:23 ` [PATCH v2] " Masashi Honma @ 2016-06-21 16:42 ` Jouni Malinen 2016-06-22 10:53 ` Masashi Honma 0 siblings, 1 reply; 15+ messages in thread From: Jouni Malinen @ 2016-06-21 16:42 UTC (permalink / raw) To: Masashi Honma; +Cc: linux-wireless 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 Malinen PGP id EFC895FA ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2] mac80211: Encrypt "Group addressed privacy" action frames 2016-06-21 16:42 ` Jouni Malinen @ 2016-06-22 10:53 ` Masashi Honma 0 siblings, 0 replies; 15+ messages in thread From: Masashi Honma @ 2016-06-22 10:53 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless 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(). ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-06-29 23:21 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-15 5:38 [PATCH] mac80211: Encrypt "Group addressed privacy" action frames Masashi Honma 2016-06-18 9:11 ` Jouni Malinen 2016-06-20 0:51 ` Masashi Honma 2016-06-20 21:25 ` Jouni Malinen 2016-06-21 6:16 ` Masashi Honma 2016-06-21 17:01 ` Jouni Malinen 2016-06-21 19:40 ` Johannes Berg 2016-06-22 10:54 ` Masashi Honma 2016-06-22 10:55 ` [PATCH v3] " Masashi Honma 2016-06-29 15:08 ` Masashi Honma 2016-06-29 16:25 ` Johannes Berg 2016-06-29 23:20 ` Masashi Honma 2016-06-21 6:23 ` [PATCH v2] " Masashi Honma 2016-06-21 16:42 ` Jouni Malinen 2016-06-22 10:53 ` Masashi Honma
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.