mac80211: drop data frames without key on encrypted links
authorDavid Bauer <mail@david-bauer.net>
Thu, 16 Apr 2020 19:30:27 +0000 (21:30 +0200)
committerDavid Bauer <mail@david-bauer.net>
Fri, 17 Apr 2020 11:27:40 +0000 (13:27 +0200)
If we know that we have an encrypted link (based on having had
a key configured for TX in the past) then drop all data frames
in the key selection handler if there's no key anymore.

This fixes an issue with mac80211 internal TXQs - there we can
buffer frames for an encrypted link, but then if the key is no
longer there when they're dequeued, the frames are sent without
encryption. This happens if a station is disconnected while the
frames are still on the TXQ.

Detecting that a link should be encrypted based on a first key
having been configured for TX is fine as there are no use cases
for a connection going from with encryption to no encryption.
With extended key IDs, however, there is a case of having a key
configured for only decryption, so we can't just trigger this
behaviour on a key being configured.

Cc: stable@vger.kernel.org
Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
Signed-off-by: David Bauer <mail@david-bauer.net>
package/kernel/mac80211/patches/subsys/314-mac80211-drop-data-frames-without-key-on-encrypted-l.patch [new file with mode: 0644]

diff --git a/package/kernel/mac80211/patches/subsys/314-mac80211-drop-data-frames-without-key-on-encrypted-l.patch b/package/kernel/mac80211/patches/subsys/314-mac80211-drop-data-frames-without-key-on-encrypted-l.patch
new file mode 100644 (file)
index 0000000..54e09af
--- /dev/null
@@ -0,0 +1,148 @@
+From a0761a301746ec2d92d7fcb82af69c0a6a4339aa Mon Sep 17 00:00:00 2001
+From: Johannes Berg <johannes.berg@intel.com>
+Date: Thu, 26 Mar 2020 15:09:42 +0200
+Subject: mac80211: drop data frames without key on encrypted links
+
+If we know that we have an encrypted link (based on having had
+a key configured for TX in the past) then drop all data frames
+in the key selection handler if there's no key anymore.
+
+This fixes an issue with mac80211 internal TXQs - there we can
+buffer frames for an encrypted link, but then if the key is no
+longer there when they're dequeued, the frames are sent without
+encryption. This happens if a station is disconnected while the
+frames are still on the TXQ.
+
+Detecting that a link should be encrypted based on a first key
+having been configured for TX is fine as there are no use cases
+for a connection going from with encryption to no encryption.
+With extended key IDs, however, there is a case of having a key
+configured for only decryption, so we can't just trigger this
+behaviour on a key being configured.
+
+Cc: stable@vger.kernel.org
+Reported-by: Jouni Malinen <j@w1.fi>
+Signed-off-by: Johannes Berg <johannes.berg@intel.com>
+Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
+---
+ net/mac80211/debugfs_sta.c |  3 ++-
+ net/mac80211/key.c         | 20 ++++++++++++--------
+ net/mac80211/sta_info.h    |  1 +
+ net/mac80211/tx.c          | 12 +++++++++---
+ 4 files changed, 24 insertions(+), 12 deletions(-)
+
+--- a/net/mac80211/debugfs_sta.c
++++ b/net/mac80211/debugfs_sta.c
+@@ -5,7 +5,7 @@
+  * Copyright 2007     Johannes Berg <johannes@sipsolutions.net>
+  * Copyright 2013-2014  Intel Mobile Communications GmbH
+  * Copyright(c) 2016 Intel Deutschland GmbH
+- * Copyright (C) 2018 - 2019 Intel Corporation
++ * Copyright (C) 2018 - 2020 Intel Corporation
+  */
+ #include <linux/debugfs.h>
+@@ -78,6 +78,7 @@ static const char * const sta_flag_names
+       FLAG(MPSP_OWNER),
+       FLAG(MPSP_RECIPIENT),
+       FLAG(PS_DELIVER),
++      FLAG(USES_ENCRYPTION),
+ #undef FLAG
+ };
+--- a/net/mac80211/key.c
++++ b/net/mac80211/key.c
+@@ -6,7 +6,7 @@
+  * Copyright 2007-2008        Johannes Berg <johannes@sipsolutions.net>
+  * Copyright 2013-2014  Intel Mobile Communications GmbH
+  * Copyright 2015-2017        Intel Deutschland GmbH
+- * Copyright 2018-2019  Intel Corporation
++ * Copyright 2018-2020  Intel Corporation
+  */
+ #include <linux/if_ether.h>
+@@ -262,22 +262,29 @@ static void ieee80211_key_disable_hw_acc
+                         sta ? sta->sta.addr : bcast_addr, ret);
+ }
+-int ieee80211_set_tx_key(struct ieee80211_key *key)
++static int _ieee80211_set_tx_key(struct ieee80211_key *key, bool force)
+ {
+       struct sta_info *sta = key->sta;
+       struct ieee80211_local *local = key->local;
+       assert_key_lock(local);
++      set_sta_flag(sta, WLAN_STA_USES_ENCRYPTION);
++
+       sta->ptk_idx = key->conf.keyidx;
+-      if (!ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT))
++      if (force || !ieee80211_hw_check(&local->hw, AMPDU_KEYBORDER_SUPPORT))
+               clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
+       ieee80211_check_fast_xmit(sta);
+       return 0;
+ }
++int ieee80211_set_tx_key(struct ieee80211_key *key)
++{
++      return _ieee80211_set_tx_key(key, false);
++}
++
+ static void ieee80211_pairwise_rekey(struct ieee80211_key *old,
+                                    struct ieee80211_key *new)
+ {
+@@ -441,11 +448,8 @@ static int ieee80211_key_replace(struct
+               if (pairwise) {
+                       rcu_assign_pointer(sta->ptk[idx], new);
+                       if (new &&
+-                          !(new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX)) {
+-                              sta->ptk_idx = idx;
+-                              clear_sta_flag(sta, WLAN_STA_BLOCK_BA);
+-                              ieee80211_check_fast_xmit(sta);
+-                      }
++                          !(new->conf.flags & IEEE80211_KEY_FLAG_NO_AUTO_TX))
++                              _ieee80211_set_tx_key(new, true);
+               } else {
+                       rcu_assign_pointer(sta->gtk[idx], new);
+               }
+--- a/net/mac80211/sta_info.h
++++ b/net/mac80211/sta_info.h
+@@ -98,6 +98,7 @@ enum ieee80211_sta_info_flags {
+       WLAN_STA_MPSP_OWNER,
+       WLAN_STA_MPSP_RECIPIENT,
+       WLAN_STA_PS_DELIVER,
++      WLAN_STA_USES_ENCRYPTION,
+       NUM_WLAN_STA_FLAGS,
+ };
+--- a/net/mac80211/tx.c
++++ b/net/mac80211/tx.c
+@@ -590,10 +590,13 @@ ieee80211_tx_h_select_key(struct ieee802
+       struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
+       struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
+-      if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT))
++      if (unlikely(info->flags & IEEE80211_TX_INTFL_DONT_ENCRYPT)) {
+               tx->key = NULL;
+-      else if (tx->sta &&
+-               (key = rcu_dereference(tx->sta->ptk[tx->sta->ptk_idx])))
++              return TX_CONTINUE;
++      }
++
++      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)))
+@@ -654,6 +657,9 @@ ieee80211_tx_h_select_key(struct ieee802
+               if (!skip_hw && tx->key &&
+                   tx->key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE)
+                       info->control.hw_key = &tx->key->conf;
++      } else if (!ieee80211_is_mgmt(hdr->frame_control) && tx->sta &&
++                 test_sta_flag(tx->sta, WLAN_STA_USES_ENCRYPTION)) {
++              return TX_DROP;
+       }
+       return TX_CONTINUE;